AI audit improvements #7

Merged
robin merged 3 commits from dev into main 2026-08-22 23:17:40 +02:00
12 changed files with 296 additions and 120 deletions
Showing only changes of commit 9b7c83532b - Show all commits
+3 -3
View File
@@ -1,6 +1,6 @@
import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'
import { setActivePinia, createPinia } from 'pinia'
import { fetchWithAuth, apiClient } from '../client'
import { fetchWithAuth, apiClient, SessionExpiredError } from '../client'
import { useAuthStore } from '@/stores/auth'
import * as authApi from '@/api/auth'
import router from '@/router'
@@ -122,7 +122,7 @@ describe('api client (fetchWithAuth)', () => {
} as unknown as Response)
global.fetch = fetchMock
await fetchWithAuth('/lists')
await expect(fetchWithAuth('/lists')).rejects.toThrow(SessionExpiredError)
expect(fetchMock).toHaveBeenCalledTimes(1)
expect(router.currentRoute.value.name).toBe('login')
@@ -144,7 +144,7 @@ describe('api client (fetchWithAuth)', () => {
} as unknown as Response)
global.fetch = fetchMock
await fetchWithAuth('/lists')
await expect(fetchWithAuth('/lists')).rejects.toThrow(SessionExpiredError)
expect(fetchMock).toHaveBeenCalledTimes(1)
expect(authStore.accessToken).toBeNull()
+13 -2
View File
@@ -6,6 +6,17 @@ export { API_BASE_URL, extractErrorMessage }
let refreshPromise: Promise<unknown> | null = null
// Thrown instead of returning the stale 401 Response when redirecting to
// login, so callers show an accurate message rather than reading `!response.ok`
// and flashing an unrelated "Failed to ..." banner in the instant before
// navigation away completes.
export class SessionExpiredError extends Error {
constructor() {
super('Your session has expired. Please log in again.')
this.name = 'SessionExpiredError'
}
}
async function redirectToLogin(): Promise<void> {
const currentRoute = router.currentRoute.value
if (currentRoute.name === 'login') {
@@ -57,7 +68,7 @@ export async function fetchWithAuth(
if (response.status === 401 && !skipRefresh) {
if (!authStore.refreshToken) {
await redirectToLogin()
return response
throw new SessionExpiredError()
}
try {
@@ -74,7 +85,7 @@ export async function fetchWithAuth(
})
} catch {
await redirectToLogin()
return response
throw new SessionExpiredError()
}
}
+2 -1
View File
@@ -38,7 +38,8 @@ function handleConfirm() {
</div>
<p class="modal-description">
Are you sure you want to delete this list? This action cannot be undone and all items in this list will be deleted.
Are you sure you want to delete this list? This action cannot be undone and all items in
this list will be deleted.
</p>
<div class="modal-footer">
+10 -20
View File
@@ -1,10 +1,9 @@
<script setup lang="ts">
import { computed, ref } from 'vue'
import { computed } from 'vue'
import { RouterLink } from 'vue-router'
import type { LocalList } from '@/database/db'
import { useListsStore } from '@/stores/lists'
import { useClickOutside } from '@/composables/useClickOutside'
import { useEscapeKey } from '@/composables/useEscapeKey'
import { useDismissableMenu } from '@/composables/useDismissableMenu'
const props = defineProps<{ list: LocalList }>()
const emit = defineEmits<{
@@ -13,8 +12,11 @@ const emit = defineEmits<{
}>()
const listsStore = useListsStore()
const isMenuOpen = ref(false)
const menuContainerRef = ref<HTMLElement | null>(null)
const {
isOpen: isMenuOpen,
containerRef: menuContainerRef,
toggle: toggleMenu,
} = useDismissableMenu()
// The server always reports total_items/completed_items once a list has
// synced at least once, so the common case never touches the store's full
@@ -31,19 +33,6 @@ const completedCount = computed(() =>
: listsStore.itemsForList(props.list.id).filter((item) => item.is_completed).length,
)
useClickOutside(menuContainerRef, () => {
isMenuOpen.value = false
})
useEscapeKey(() => {
if (isMenuOpen.value) isMenuOpen.value = false
})
function toggleMenu(event: Event) {
event.preventDefault()
event.stopPropagation()
isMenuOpen.value = !isMenuOpen.value
}
function handleShare(event: Event) {
event.preventDefault()
event.stopPropagation()
@@ -57,7 +46,6 @@ function handleDelete(event: Event) {
isMenuOpen.value = false
emit('delete', props.list)
}
</script>
<template>
@@ -125,7 +113,9 @@ function handleDelete(event: Event) {
stroke-linejoin="round"
>
<polyline points="3 6 5 6 21 6" />
<path d="M19 6v14a2 2 0 0 1-2 2H7a2 2 0 0 1-2-2V6m3 0V4a2 2 0 0 1 2-2h4a2 2 0 0 1 2 2v2" />
<path
d="M19 6v14a2 2 0 0 1-2 2H7a2 2 0 0 1-2-2V6m3 0V4a2 2 0 0 1 2-2h4a2 2 0 0 1 2 2v2"
/>
<line x1="10" y1="11" x2="10" y2="17" />
<line x1="14" y1="11" x2="14" y2="17" />
</svg>
+18 -24
View File
@@ -2,26 +2,25 @@
import { ref } from 'vue'
import type { LocalListItem } from '@/database/db'
import { useListsStore } from '@/stores/lists'
import { useClickOutside } from '@/composables/useClickOutside'
import { useEscapeKey } from '@/composables/useEscapeKey'
import { useDismissableMenu } from '@/composables/useDismissableMenu'
const props = defineProps<{ item: LocalListItem }>()
const emit = defineEmits<{
delete: [item: LocalListItem]
}>()
const listsStore = useListsStore()
const isEditing = ref(false)
const editedTitle = ref(props.item.title)
const isMenuOpen = ref(false)
const menuContainerRef = ref<HTMLElement | null>(null)
useClickOutside(menuContainerRef, () => {
isMenuOpen.value = false
})
useEscapeKey(() => {
if (isMenuOpen.value) isMenuOpen.value = false
})
// Captured separately from `editedTitle` at the moment editing starts: if the
// item is updated remotely (another device) while the field is open,
// `props.item.title` moves but this doesn't, so saveTitle() can tell "user
// didn't touch it" apart from "server changed underneath us" instead of
// diffing against the live (possibly just-changed) prop and overwriting the
// remote edit with the untouched original text.
const originalTitle = ref(props.item.title)
const {
isOpen: isMenuOpen,
containerRef: menuContainerRef,
toggle: toggleMenu,
} = useDismissableMenu()
function toggleCompleted() {
listsStore.setListItemCompleted(props.item.id, !props.item.is_completed)
@@ -29,31 +28,24 @@ function toggleCompleted() {
function startEditing() {
editedTitle.value = props.item.title
originalTitle.value = props.item.title
isEditing.value = true
}
function saveTitle() {
const title = editedTitle.value.trim()
if (title && title !== props.item.title) {
if (title && title !== originalTitle.value) {
listsStore.updateListItem(props.item.id, { title })
}
isEditing.value = false
}
function toggleMenu(event: Event) {
event.preventDefault()
event.stopPropagation()
isMenuOpen.value = !isMenuOpen.value
}
function handleDelete(event: Event) {
event.preventDefault()
event.stopPropagation()
isMenuOpen.value = false
emit('delete', props.item)
listsStore.deleteListItem(props.item.id)
}
</script>
<template>
@@ -114,7 +106,9 @@ function handleDelete(event: Event) {
stroke-linejoin="round"
>
<polyline points="3 6 5 6 21 6" />
<path d="M19 6v14a2 2 0 0 1-2 2H7a2 2 0 0 1-2-2V6m3 0V4a2 2 0 0 1 2-2h4a2 2 0 0 1 2 2v2" />
<path
d="M19 6v14a2 2 0 0 1-2 2H7a2 2 0 0 1-2-2V6m3 0V4a2 2 0 0 1 2-2h4a2 2 0 0 1 2 2v2"
/>
<line x1="10" y1="11" x2="10" y2="17" />
<line x1="14" y1="11" x2="14" y2="17" />
</svg>
@@ -65,8 +65,6 @@ describe('ListItemRow', () => {
await wrapper.find('.submenu-item-danger').trigger('click')
expect(deleteSpy).toHaveBeenCalledWith('item-1')
expect(wrapper.emitted('delete')).toBeTruthy()
expect(wrapper.emitted('delete')?.[0]).toEqual([sampleItem])
expect(wrapper.find('.submenu-dropdown').exists()).toBe(false)
})
})
+28
View File
@@ -0,0 +1,28 @@
import { ref } from 'vue'
import { useClickOutside } from '@/composables/useClickOutside'
import { useEscapeKey } from '@/composables/useEscapeKey'
// Bundles the isOpen/containerRef/toggle wiring shared by every dropdown menu
// (list card, list item row, list detail header) so it isn't hand-rolled per
// component on top of useClickOutside/useEscapeKey.
export function useDismissableMenu() {
const isOpen = ref(false)
const containerRef = ref<HTMLElement | null>(null)
function close() {
isOpen.value = false
}
function toggle(event: Event) {
event.preventDefault()
event.stopPropagation()
isOpen.value = !isOpen.value
}
useClickOutside(containerRef, close)
useEscapeKey(() => {
if (isOpen.value) close()
})
return { isOpen, containerRef, toggle, close }
}
+39 -11
View File
@@ -1,5 +1,12 @@
import Dexie, { type Table } from 'dexie'
import type { List, ListItem } from '@/types/list'
import type {
List,
ListItem,
CreateListPayload,
CreateListItemPayload,
UpdateListItemPayload,
SetListItemCompletedPayload,
} from '@/types/list'
export interface LocalList extends List {
pendingSync?: boolean
@@ -9,18 +16,16 @@ export interface LocalListItem extends ListItem {
pendingSync?: boolean
}
export type SyncOperationType =
| 'createList'
| 'createListItem'
| 'updateListItem'
| 'setListItemCompleted'
| 'deleteList'
| 'deleteListItem'
export interface DeleteListPayload {
id: string
}
export interface SyncQueueEntry {
export interface DeleteListItemPayload {
id: string
}
interface SyncQueueEntryBase {
id?: number
type: SyncOperationType
payload: unknown
localListId?: string
localListItemId?: string
createdAt: number
@@ -28,6 +33,29 @@ export interface SyncQueueEntry {
lastError?: string
}
type SyncOperationPayloads = {
createList: CreateListPayload
createListItem: CreateListItemPayload
updateListItem: UpdateListItemPayload
setListItemCompleted: SetListItemCompletedPayload
deleteList: DeleteListPayload
deleteListItem: DeleteListItemPayload
}
export type SyncOperationType = keyof SyncOperationPayloads
// A discriminated union keyed on `type` instead of a single `payload: unknown`
// shape, so `processSyncEntry`'s switch narrows `entry.payload` to the right
// type per case without a manual cast - and adding/changing an operation type
// here is a compile error everywhere it's handled inconsistently.
export type SyncQueueEntry = {
[K in SyncOperationType]: SyncQueueEntryBase & { type: K; payload: SyncOperationPayloads[K] }
}[SyncOperationType]
type DistributiveOmit<T, K extends keyof T> = T extends unknown ? Omit<T, K> : never
export type NewSyncQueueEntry = DistributiveOmit<SyncQueueEntry, 'id' | 'createdAt' | 'attempts'>
class AppDatabase extends Dexie {
lists!: Table<LocalList, string>
listItems!: Table<LocalListItem, string>
+115
View File
@@ -436,4 +436,119 @@ describe('useListsStore', () => {
expect(store.pendingCount).toBe(0)
})
it('removes user from list on the server when online without adding to sync queue', async () => {
listsApiMocks.removeUserFromListApi.mockResolvedValueOnce(undefined)
const store = useListsStore()
await store.removeUserFromList('list-1', 'friend@example.com')
expect(listsApiMocks.removeUserFromListApi).toHaveBeenCalledWith({
list_id: 'list-1',
email: 'friend@example.com',
})
expect(store.pendingCount).toBe(0)
})
it('throws error when removing user from list while offline without calling API', async () => {
Object.defineProperty(navigator, 'onLine', { value: false, configurable: true })
const store = useListsStore()
await expect(store.removeUserFromList('list-1', 'friend@example.com')).rejects.toThrow(
'Cannot remove user from list while offline',
)
expect(listsApiMocks.removeUserFromListApi).not.toHaveBeenCalled()
expect(store.pendingCount).toBe(0)
})
it('propagates error when removing user from list fails on server', async () => {
listsApiMocks.removeUserFromListApi.mockRejectedValueOnce(new Error('User not found'))
const store = useListsStore()
await expect(store.removeUserFromList('list-1', 'unknown@example.com')).rejects.toThrow(
'User not found',
)
expect(store.pendingCount).toBe(0)
})
it('does not sync before the debounce delay elapses, then syncs once it does', async () => {
listsApiMocks.createListItemApi.mockResolvedValueOnce({
id: 'server-item-1',
list_id: 'list-1',
title: 'Milk',
is_completed: false,
})
const store = useListsStore()
await store.createListItem('list-1', 'Milk')
await vi.advanceTimersByTimeAsync(399)
expect(listsApiMocks.createListItemApi).not.toHaveBeenCalled()
await vi.advanceTimersByTimeAsync(1)
expect(listsApiMocks.createListItemApi).toHaveBeenCalledTimes(1)
})
it('collapses a burst of mutations within the debounce window into a single sync pass', async () => {
listsApiMocks.createListItemApi
.mockResolvedValueOnce({
id: 'server-item-a',
list_id: 'list-1',
title: 'A',
is_completed: false,
})
.mockResolvedValueOnce({
id: 'server-item-b',
list_id: 'list-1',
title: 'B',
is_completed: false,
})
const store = useListsStore()
await store.createListItem('list-1', 'A')
await store.createListItem('list-1', 'B')
expect(listsApiMocks.createListItemApi).not.toHaveBeenCalled()
await vi.advanceTimersByTimeAsync(400)
expect(listsApiMocks.createListItemApi).toHaveBeenCalledTimes(2)
expect(store.pendingCount).toBe(0)
})
it('serializes pullListItems() behind an in-flight sync() so they never race on the same rows', async () => {
let resolveGetLists!: (value: unknown[]) => void
listsApiMocks.getListsApi.mockImplementationOnce(
() =>
new Promise((resolve) => {
resolveGetLists = resolve
}),
)
listsApiMocks.getListItemsApi.mockResolvedValueOnce([
{ id: 'server-item-1', list_id: 'list-1', title: 'Milk', is_completed: false },
])
const store = useListsStore()
const syncPromise = store.sync()
const pullPromise = store.pullListItems('list-1')
await Promise.resolve()
await Promise.resolve()
await Promise.resolve()
// pullListItems is chained behind sync() via the shared operation queue,
// so its own (already-mocked, instantly resolvable) API call must not
// fire while sync()'s getListsApi call is still pending.
expect(listsApiMocks.getListItemsApi).not.toHaveBeenCalled()
resolveGetLists([])
await syncPromise
await pullPromise
expect(listsApiMocks.getListItemsApi).toHaveBeenCalledWith('list-1')
expect(store.listItems.find((item) => item.id === 'server-item-1')).toBeDefined()
})
})
+47 -33
View File
@@ -1,6 +1,12 @@
import { ref, computed } from 'vue'
import { defineStore } from 'pinia'
import { db, type LocalList, type LocalListItem, type SyncQueueEntry } from '@/database/db'
import {
db,
type LocalList,
type LocalListItem,
type SyncQueueEntry,
type NewSyncQueueEntry,
} from '@/database/db'
import {
getListsApi,
createListApi,
@@ -86,14 +92,27 @@ export const useListsStore = defineStore('lists', () => {
isLoaded.value = true
}
async function loadLists() {
if (!isLoaded.value) {
await refresh()
// Dedupes concurrent first-load refreshes: loadLists() and loadListItems()
// are both called from ListDetailView's onMounted and would otherwise each
// see isLoaded === false and kick off their own full-table Dexie scan.
let loadPromise: Promise<void> | null = null
async function ensureLoaded() {
if (isLoaded.value) return
if (!loadPromise) {
loadPromise = refresh().finally(() => {
loadPromise = null
})
}
await loadPromise
}
async function loadLists() {
await ensureLoaded()
void sync()
}
async function enqueue(entry: Omit<SyncQueueEntry, 'id' | 'createdAt' | 'attempts'>) {
async function enqueue(entry: NewSyncQueueEntry) {
await db.syncQueue.add({
...entry,
createdAt: Date.now(),
@@ -263,14 +282,16 @@ export const useListsStore = defineStore('lists', () => {
.filter((entry) => entry.localListId === oldId)
.toArray()
for (const entry of affectedQueueEntries) {
const payload = entry.payload as { list_id?: string; id?: string }
const payload = entry.payload
const updatedPayload =
'list_id' in payload
? { ...payload, list_id: newId }
: 'id' in payload
? { ...payload, id: newId }
: payload
await db.syncQueue.update(entry.id!, {
localListId: newId,
payload: payload?.list_id
? { ...payload, list_id: newId }
: payload?.id
? { ...payload, id: newId }
: entry.payload,
payload: updatedPayload,
})
}
}
@@ -295,14 +316,16 @@ export const useListsStore = defineStore('lists', () => {
.filter((queueEntry) => queueEntry.localListItemId === oldId)
.toArray()
for (const queueEntry of affectedQueueEntries) {
const payload = queueEntry.payload as { list_item_id?: string; id?: string }
const payload = queueEntry.payload
const updatedPayload =
'list_item_id' in payload
? { ...payload, list_item_id: newId }
: 'id' in payload
? { ...payload, id: newId }
: payload
await db.syncQueue.update(queueEntry.id!, {
localListItemId: newId,
payload: payload?.list_item_id
? { ...payload, list_item_id: newId }
: payload?.id
? { ...payload, id: newId }
: queueEntry.payload,
payload: updatedPayload,
})
}
}
@@ -316,7 +339,7 @@ export const useListsStore = defineStore('lists', () => {
async function processSyncEntry(entry: SyncQueueEntry) {
switch (entry.type) {
case 'createList': {
const created = await createListApi(entry.payload as { name: string; user_ids?: string[] })
const created = await createListApi(entry.payload)
if (entry.localListId) {
await remapListId(entry.localListId, created.id)
}
@@ -327,16 +350,14 @@ export const useListsStore = defineStore('lists', () => {
// created item (with its own real id) in the response body, so we
// remap our client-generated placeholder id to it instead of keeping
// the made-up one around.
const created = await createListItemApi(entry.payload as { list_id: string; title: string })
const created = await createListItemApi(entry.payload)
if (entry.localListItemId) {
await remapListItemId(entry.localListItemId, created.id)
}
break
}
case 'updateListItem': {
await updateListItemApi(
entry.payload as { list_item_id: string; title?: string; is_completed?: boolean },
)
await updateListItemApi(entry.payload)
if (entry.localListItemId) {
await markListItemSynced(entry.localListItemId)
}
@@ -344,21 +365,16 @@ export const useListsStore = defineStore('lists', () => {
}
case 'setListItemCompleted': {
if (!entry.localListItemId) break
await setListItemCompletedApi(
entry.localListItemId,
entry.payload as { is_completed: boolean },
)
await setListItemCompletedApi(entry.localListItemId, entry.payload)
await markListItemSynced(entry.localListItemId)
break
}
case 'deleteList': {
const payload = entry.payload as { id: string }
await deleteListApi(payload.id)
await deleteListApi(entry.payload.id)
break
}
case 'deleteListItem': {
const payload = entry.payload as { id: string }
await deleteListItemApi(payload.id)
await deleteListItemApi(entry.payload.id)
break
}
}
@@ -533,9 +549,7 @@ export const useListsStore = defineStore('lists', () => {
}
async function loadListItems(listId: string) {
if (!isLoaded.value) {
await refresh()
}
await ensureLoaded()
void pullListItems(listId)
}
+20 -19
View File
@@ -1,12 +1,11 @@
<script setup lang="ts">
import { ref, computed, onMounted } from 'vue'
import { ref, computed, onMounted, watch } from 'vue'
import { useRouter } from 'vue-router'
import { useListsStore } from '@/stores/lists'
import type { LocalListItem } from '@/database/db'
import ListItemRow from '@/components/ListItemRow.vue'
import DeleteListModal from '@/components/DeleteListModal.vue'
import { useClickOutside } from '@/composables/useClickOutside'
import { useEscapeKey } from '@/composables/useEscapeKey'
import { useDismissableMenu } from '@/composables/useDismissableMenu'
const props = defineProps<{ id: string }>()
@@ -16,22 +15,28 @@ const listsStore = useListsStore()
const newItemTitle = ref('')
const isAddingItem = ref(false)
const itemError = ref('')
const isMenuOpen = ref(false)
const showDeleteModal = ref(false)
const menuContainerRef = ref<HTMLElement | null>(null)
useClickOutside(menuContainerRef, () => {
isMenuOpen.value = false
})
useEscapeKey(() => {
if (isMenuOpen.value) isMenuOpen.value = false
})
const {
isOpen: isMenuOpen,
containerRef: menuContainerRef,
toggle: toggleMenu,
} = useDismissableMenu()
onMounted(() => {
listsStore.loadLists()
listsStore.loadListItems(props.id)
})
// Vue Router reuses this component instance when navigating between two
// list-detail routes, so the initial onMounted load alone would leave a
// newly-navigated-to list's items unfetched.
watch(
() => props.id,
(newId) => {
listsStore.loadListItems(newId)
},
)
const list = computed(() => listsStore.lists.find((entry) => entry.id === props.id))
const items = computed(() => listsStore.itemsForList(props.id))
@@ -46,12 +51,6 @@ const completedItems = computed(() =>
items.value.filter((item) => item.is_completed).sort(byModifiedDesc),
)
function toggleMenu(event: Event) {
event.preventDefault()
event.stopPropagation()
isMenuOpen.value = !isMenuOpen.value
}
function handleOpenDelete() {
isMenuOpen.value = false
showDeleteModal.value = true
@@ -131,7 +130,9 @@ async function handleAddItem() {
stroke-linejoin="round"
>
<polyline points="3 6 5 6 21 6" />
<path d="M19 6v14a2 2 0 0 1-2 2H7a2 2 0 0 1-2-2V6m3 0V4a2 2 0 0 1 2-2h4a2 2 0 0 1 2 2v2" />
<path
d="M19 6v14a2 2 0 0 1-2 2H7a2 2 0 0 1-2-2V6m3 0V4a2 2 0 0 1 2-2h4a2 2 0 0 1 2 2v2"
/>
<line x1="10" y1="11" x2="10" y2="17" />
<line x1="14" y1="11" x2="14" y2="17" />
</svg>
+1 -5
View File
@@ -88,11 +88,7 @@ async function handleCreateList() {
<ul v-if="listsStore.sortedLists.length > 0" class="lists">
<li v-for="list in listsStore.sortedLists" :key="list.id">
<ListCard
:list="list"
@share="handleOpenShare(list)"
@delete="handleOpenDelete(list)"
/>
<ListCard :list="list" @share="handleOpenShare(list)" @delete="handleOpenDelete(list)" />
</li>
</ul>