fix(client): cache a visit note saved from the place dialog

The place dialog stopped reloading the days after a save, so the new
visit note only reached the Zustand store. Nothing wrote it to Dexie,
the tab never sees its own assignment:updated echo, and the time save
before it cached the server row with the old note. A trip reopened
offline then showed the old note until the next full sync.

The note now goes through assignmentRepo.setNotes, which caches the
saved row on its day online and queues the write behind any other
write of the same visit offline, like setTimes does. A new store
action applies it, so the page hook no longer patches the store by
hand. usePlaceEdits drops its direct API import and leaves the
offline baseline.
This commit is contained in:
Maurice
2026-10-08 13:44:24 +02:00
parent 9d30a33e78
commit 72c284467b
8 changed files with 129 additions and 20 deletions
-1
View File
@@ -206,7 +206,6 @@
"src/pages/settings/useSettings.ts",
"src/pages/sharedTrip/useSharedTrip.ts",
"src/pages/tripPlanner/useBookingEdits.ts",
"src/pages/tripPlanner/usePlaceEdits.ts",
"src/pages/tripPlanner/usePlaceFormOpeners.ts",
"src/pages/tripPlanner/usePlannerDialogs.ts",
"src/pages/tripPlanner/useRoadtripStopEdits.ts",
@@ -41,6 +41,7 @@ function makeActions() {
assignPlaceToDay: vi.fn(async () => ({ id: 555 })),
refreshDays: vi.fn(async () => undefined),
setAssignmentTimes: vi.fn(async () => undefined),
setAssignmentNotes: vi.fn(async () => undefined),
addFile: vi.fn(async () => undefined),
deletePlace: vi.fn(async (): Promise<{ tourPlaceIds?: number[] } | undefined> => undefined),
deletePlacesMany: vi.fn(async (): Promise<{ tourPlaceIds?: number[] } | undefined> => undefined),
@@ -191,9 +192,10 @@ describe('usePlaceEdits', () => {
// The times go through the store and the visit's repo, never straight to the API.
expect(actions.setAssignmentTimes).toHaveBeenCalledWith(42, 10, 101, { place_time: '09:00', end_time: null })
expect(updateTime).not.toHaveBeenCalled()
expect(updateNotes).toHaveBeenCalledWith(42, 101, { notes: null })
// The note lands on the visit without reloading the days over a queued time.
expect(useTripStore.getState().assignments[10][0].notes).toBeNull()
// The note goes through the store and the visit's repo too, which caches it on the day,
// and the days are not reloaded over a queued time.
expect(actions.setAssignmentNotes).toHaveBeenCalledWith(42, 10, 101, null)
expect(updateNotes).not.toHaveBeenCalled()
expect(actions.refreshDays).not.toHaveBeenCalled()
expect(actions.addFile).toHaveBeenCalledTimes(2)
expect(toast.error).toHaveBeenCalledWith('files.uploadError')
@@ -208,6 +210,7 @@ describe('usePlaceEdits', () => {
expect(actions.updatePlace).toHaveBeenCalledWith(42, 2, { name: 'Castle' })
expect(actions.setAssignmentTimes).not.toHaveBeenCalled()
expect(actions.setAssignmentNotes).not.toHaveBeenCalled()
expect(updateTime).not.toHaveBeenCalled()
expect(actions.refreshDays).not.toHaveBeenCalled()
})
+6 -14
View File
@@ -1,5 +1,4 @@
import { useCallback, useMemo, useState } from 'react'
import { assignmentsApi } from '../../api/client'
import { roadtripInsertion } from '../../components/Roadtrip/dayWindow'
import { reanchorAfterInsert } from '../../components/Roadtrip/roadtripModel'
import type { usePlaceSelection } from '../../hooks/usePlaceSelection'
@@ -110,19 +109,12 @@ export function usePlaceEdits(options: PlaceEditsOptions) {
await tripActions.setAssignmentTimes(tripId, Number(dayKey), editingAssignmentId, {
place_time: place_time || null, end_time: end_time || null,
})
}
// The form only includes assignment_notes when the user changed it, so
// an untouched note never produces a PUT (#2163). '' clears like null.
// The note is set on the visit in place: reloading the days would show
// the server's time while the new one still waits in the queue.
if (assignment_notes !== undefined) {
const notes = assignment_notes || null
await assignmentsApi.updateNotes(tripId, editingAssignmentId, { notes })
useTripStore.setState(state => ({
assignments: Object.fromEntries(Object.entries(state.assignments).map(([key, items]) => [
key, items.map(a => (a.id === editingAssignmentId ? { ...a, notes } : a)),
])),
}))
// The form only includes assignment_notes when the user changed it, so
// an untouched note never produces a PUT (#2163). '' clears like null.
// Through the visit's repo as well, so the cached day keeps the new note.
if (assignment_notes !== undefined) {
await tripActions.setAssignmentNotes(tripId, Number(dayKey), editingAssignmentId, assignment_notes || null)
}
}
}
// Upload pending files with place_id
@@ -203,7 +203,10 @@ beforeEach(() => {
vi.spyOn(tripsApi, 'getMembers').mockResolvedValue({ owner: null, members: [] })
vi.spyOn(accommodationsApi, 'list').mockResolvedValue({ accommodations: [] })
vi.spyOn(assignmentsApi, 'updateTime').mockResolvedValue({})
vi.spyOn(assignmentsApi, 'updateNotes').mockResolvedValue({})
// The note route answers with the saved visit, which the repo caches on the day.
vi.spyOn(assignmentsApi, 'updateNotes').mockImplementation(async (_tripId, id, { notes }) => ({
assignment: buildAssignment({ id, day_id: 7, place: buildPlace({ id: 1, lat: 1, lng: 2 }), notes }),
}))
vi.spyOn(airtrailApi, 'sync').mockResolvedValue({ changed: 0 })
vi.spyOn(mapsApi, 'reverse').mockResolvedValue({ name: '', address: '' } as never)
vi.spyOn(mapsApi, 'search').mockResolvedValue({ places: [] } as never)
+29 -1
View File
@@ -11,7 +11,7 @@ import type { Assignment, Day } from '../types'
vi.mock('../api/assignmentEndDay', () => ({ saveAssignmentEndDay: vi.fn() }))
vi.mock('../sync/networkMode', () => ({ isEffectivelyOffline: vi.fn(() => true) }))
vi.mock('../sync/authGate', () => ({ isAuthed: () => true }))
vi.mock('../api/client', () => ({ apiClient: { request: vi.fn() }, assignmentsApi: { updateTime: vi.fn(), clearDay: vi.fn() } }))
vi.mock('../api/client', () => ({ apiClient: { request: vi.fn() }, assignmentsApi: { updateTime: vi.fn(), updateNotes: vi.fn(), clearDay: vi.fn() } }))
const assignment = { id: 7, day_id: 1, place_id: 2, order_index: 0, assignment_time: '07:00', place: { id: 2, name: 'Berlin' } } as Assignment
beforeEach(async () => {
@@ -80,6 +80,34 @@ describe('assignment time persistence', () => {
})
})
describe('assignment note persistence', () => {
it('saves through the note route online and caches the saved row on the day', async () => {
vi.mocked(isEffectivelyOffline).mockReturnValue(false)
vi.mocked(assignmentsApi.updateNotes).mockResolvedValue({ assignment: { ...assignment, notes: 'Bring cash' } })
const saved = await assignmentRepo.setNotes(9, assignment, 'Bring cash')
expect(assignmentsApi.updateNotes).toHaveBeenCalledWith(9, 7, { notes: 'Bring cash' })
expect(saved.notes).toBe('Bring cash')
expect((await offlineDb.days.get(1))?.assignments?.[0]).toMatchObject({ notes: 'Bring cash', assignment_time: '07:00' })
expect(await offlineDb.mutationQueue.count()).toBe(0)
})
it('stores the offline note with a replayable write', async () => {
const saved = await assignmentRepo.setNotes(9, assignment, null)
expect(saved.notes).toBeNull()
expect((await offlineDb.days.get(1))?.assignments?.[0].notes).toBeNull()
expect(await offlineDb.mutationQueue.toArray()).toEqual([expect.objectContaining({
url: '/trips/9/assignments/7/notes', method: 'PUT', body: { notes: null }, resource: 'assignments', entityId: 7,
})])
})
it('keeps the cached visit unchanged after a refused save', async () => {
vi.mocked(isEffectivelyOffline).mockReturnValue(false)
vi.mocked(assignmentsApi.updateNotes).mockRejectedValue(new Error('Denied'))
await expect(assignmentRepo.setNotes(9, assignment, 'x')).rejects.toThrow('Denied')
expect((await offlineDb.days.get(1))?.assignments?.[0]).toEqual(assignment)
})
})
describe('an online edit of a visit with an older write still queued', () => {
const first = { place_time: '07:00', end_time: null }
const second = { place_time: '09:00', end_time: null }
+24
View File
@@ -98,4 +98,28 @@ export const assignmentRepo = {
mutationQueue.sendSoon()
return updated
},
/**
* The note a visit carries on its day (#2163). Saved through here rather than straight
* to the API so the cached day holds the new note too: the tab never sees its own
* `assignment:updated` echo, and a trip reopened offline reads its days from the cache.
* A write of the same visit still waiting in the queue keeps this one behind it.
*/
async setNotes(tripId: number | string, assignment: Assignment, notes: string | null): Promise<Assignment> {
if (!(await mutationQueue.mustQueue('assignments', assignment.id))) {
const saved = assignmentSchema.parse((await assignmentsApi.updateNotes(tripId, assignment.id, { notes })).assignment)
await cacheAssignment(saved)
return saved
}
const updated = { ...assignment, notes }
await offlineDb.transaction('rw', offlineDb.days, offlineDb.mutationQueue, async () => {
await mutationQueue.enqueue({
id: generateUUID(), tripId: Number(tripId), method: 'PUT',
url: `/trips/${tripId}/assignments/${assignment.id}/notes`,
body: { notes }, resource: 'assignments', entityId: assignment.id,
})
await cacheAssignment(updated)
})
mutationQueue.sendSoon()
return updated
},
}
@@ -13,6 +13,7 @@ export interface AssignmentsSlice {
setAssignmentEndDay: (tripId: number | string, dayId: number, assignmentId: number, endDay: boolean) => Promise<void>
setAssignmentRouteExcluded: (tripId: number | string, dayId: number, assignmentId: number, excluded: boolean) => Promise<void>
setAssignmentTimes: (tripId: number | string, dayId: number, assignmentId: number, times: AssignmentTimes) => Promise<void>
setAssignmentNotes: (tripId: number | string, dayId: number, assignmentId: number, notes: string | null) => Promise<void>
assignPlaceToDay: (tripId: number | string, dayId: number | string, placeId: number | string, position?: number | null) => Promise<Assignment | undefined>
removeAssignment: (tripId: number | string, dayId: number | string, assignmentId: number) => Promise<void>
clearDayAssignments: (tripId: number | string, dayId: number) => Promise<void>
@@ -77,6 +78,25 @@ export const createAssignmentsSlice = (set: SetState, get: GetState): Assignment
throw err
}
},
setAssignmentNotes: async (tripId, dayId, assignmentId, notes) => {
const assignment = get().assignments[String(dayId)]?.find(a => a.id === assignmentId)
if (!assignment || assignmentId < 0) return
// Only the note is patched: the saved row carries the place as the server has it,
// and the visit's times may still wait in the queue ahead of this write.
const apply = (value: string | null) => set(state => ({
assignments: Object.fromEntries(Object.entries(state.assignments).map(([key, items]) => [
key, items.map(a => a.id === assignmentId ? { ...a, notes: value } : a),
])),
}))
apply(notes)
try {
const saved = await assignmentRepo.setNotes(tripId, assignment, notes)
apply(saved.notes ?? null)
} catch (err: unknown) {
apply(assignment.notes ?? null)
throw err
}
},
assignPlaceToDay: async (tripId, dayId, placeId, position) => {
const state = get()
const place = state.places.find(p => p.id === Number.parseInt(String(placeId)))
@@ -427,6 +427,46 @@ describe('assignmentsSlice', () => {
});
});
describe('setAssignmentNotes', () => {
const visit = () => buildAssignment({ id: 5, day_id: 1, place_id: 10, notes: 'old', assignment_time: '09:00' });
it('FE-ASSIGN-030: shows the note at once and keeps the one the server saved', async () => {
seedStore(useTripStore, { assignments: { '1': [visit()] } });
let seen: unknown;
server.use(
http.put('/api/trips/1/assignments/5/notes', async ({ request }) => {
seen = await request.json();
expect(useTripStore.getState().assignments['1'][0].notes).toBe('Bring cash');
return HttpResponse.json({ assignment: { ...visit(), notes: 'Bring cash' } });
}),
);
await useTripStore.getState().setAssignmentNotes(1, 1, 5, 'Bring cash');
expect(seen).toEqual({ notes: 'Bring cash' });
expect(useTripStore.getState().assignments['1'][0]).toMatchObject({ notes: 'Bring cash', assignment_time: '09:00' });
});
it('FE-ASSIGN-031: a refused write puts the old note back', async () => {
seedStore(useTripStore, { assignments: { '1': [visit()] } });
server.use(http.put('/api/trips/1/assignments/5/notes', () => HttpResponse.json({ error: 'no' }, { status: 403 })));
await expect(useTripStore.getState().setAssignmentNotes(1, 1, 5, null)).rejects.toBeTruthy();
expect(useTripStore.getState().assignments['1'][0].notes).toBe('old');
});
it('FE-ASSIGN-032: a visit it cannot find, or one not yet saved, is left alone', async () => {
const unsaved = { ...visit(), id: -3 };
seedStore(useTripStore, { assignments: { '1': [unsaved] } });
await useTripStore.getState().setAssignmentNotes(1, 1, 5, 'x');
await useTripStore.getState().setAssignmentNotes(1, 1, -3, 'x');
expect(useTripStore.getState().assignments['1'][0]).toEqual(unsaved);
});
});
describe('setAssignmentTimes', () => {
// The pool place still carries an End of its own, which the visit shows once its
// own override is gone, exactly as a fresh read of the day would.