Read Recent updates afresh per opening and guard their URLs by origin
CI / ci (push) Successful in 8m51s
CI / ci (pull_request) Successful in 8m26s

Address the review of PR #176 (Feature 21, task 96e2dc3f):

- MQ-176-01: useUpdates drops its list as soon as the sheet closes
  (gcTime 0), so each opening reads GET /api/v1/updates afresh and only
  marks seen through the newest of that read, never a cached list.
- MQ-176-02: parseUpdate checks each url with appPath from
  push/notification.ts, by parsed origin, instead of a prefix check that
  tabs and newlines got past; the url kept is the path as parsed.
- MQ-176-03: TestMarkUpdatesSeen asserts updated_at is kept and settings
  subscribers are not told.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-10 12:35:58 -04:00
co-authored by Claude Opus 5.5
parent 74e33a6236
commit 9978e200e6
4 changed files with 128 additions and 20 deletions
+20 -3
View File
@@ -45,6 +45,7 @@ var weekDates = []string{"2026-10-12", "2026-10-13", "2026-10-14", "2026-10-15",
type env struct {
svc *home.Service
st *store.Store
settings *household.SettingsService
notifier *notify.Service
loc *time.Location
}
@@ -116,7 +117,8 @@ func newEnv(t *testing.T) *env {
if err != nil {
t.Fatal(err)
}
svc, err := home.NewService(household.NewSettingsService(st), plans, suggestSvc, shop, notifier)
settings := household.NewSettingsService(st)
svc, err := home.NewService(settings, plans, suggestSvc, shop, notifier)
if err != nil {
t.Fatal(err)
}
@@ -124,7 +126,7 @@ func newEnv(t *testing.T) *env {
if err != nil {
t.Fatal(err)
}
return &env{svc: svc, st: st, notifier: notifier, loc: loc}
return &env{svc: svc, st: st, settings: settings, notifier: notifier, loc: loc}
}
// at reads the Home screen with the clock at the household-local wall
@@ -480,6 +482,15 @@ func TestMarkUpdatesSeen(t *testing.T) {
if !unread() {
t.Fatal("not unread with an event never seen")
}
// The settings were last edited well before: marking updates seen must
// keep that time and tell no settings subscriber.
edited := (store.Timestamp{Time: time.Date(2026, 10, 1, 9, 0, 0, 0, time.UTC)}).String()
if _, err := e.st.Writer.ExecContext(ctx, `UPDATE household_settings SET updated_at = ?`, edited); err != nil {
t.Fatal(err)
}
var notified int
unsubscribe := e.settings.Subscribe(func(household.Settings) { notified++ })
defer unsubscribe()
if err := e.svc.MarkUpdatesSeen(ctx, first); err != nil {
t.Fatal(err)
}
@@ -509,7 +520,7 @@ func TestMarkUpdatesSeen(t *testing.T) {
if err := e.svc.MarkUpdatesSeen(ctx, first); err != nil {
t.Fatal(err)
}
settings, err := household.NewSettingsService(e.st).Get(ctx)
settings, err := e.settings.Get(ctx)
if err != nil {
t.Fatal(err)
}
@@ -519,6 +530,12 @@ func TestMarkUpdatesSeen(t *testing.T) {
if settings.UpdatedBy != nil {
t.Errorf("updated_by = %q, want unset: marking updates seen is not a settings edit", *settings.UpdatedBy)
}
if settings.UpdatedAt != edited {
t.Errorf("updated_at = %s, want %s: marking updates seen is not a settings edit", settings.UpdatedAt, edited)
}
if notified != 0 {
t.Errorf("settings subscribers told %d times, want none: marking updates seen is not a settings edit", notified)
}
// The clock reads 17:00 UTC.
if err := e.svc.MarkUpdatesSeen(ctx, first.Add(2*time.Hour)); !errors.Is(err, home.ErrSeenInFuture) {
+51 -3
View File
@@ -6,7 +6,7 @@ import { ApiProvider, createApiClient, createQueryClient, createVerifiedSession
import { createNetworkStatus } from '../pwa/networkStatus.ts'
import { HOME } from '../test/ApiProviders.tsx'
import { errorResponse, fakeLocation, jsonResponse, ME } from '../test/fetchStub.ts'
import { HOME_PATH, UPDATES_PATH, UPDATES_SEEN_PATH } from './api.ts'
import { HOME_KEY, HOME_PATH, UPDATES_PATH, UPDATES_SEEN_PATH } from './api.ts'
import { HomePage } from './HomePage.tsx'
afterEach(() => {
@@ -90,7 +90,8 @@ function Where() {
return <p data-testid="where">{location.pathname + location.search}</p>
}
// renderHome loads /me (for the CSRF token the POST needs) and renders Home.
// renderHome loads /me (for the CSRF token the POST needs) and renders Home,
// answering its query client.
async function renderHome() {
const api = createApiClient(fakeLocation('/'), createNetworkStatus(window), createVerifiedSession(localStorage))
await api.loadMe()
@@ -101,13 +102,15 @@ async function renderHome() {
],
{ initialEntries: ['/'] },
)
const queryClient = createQueryClient()
render(
<ApiProvider client={api}>
<QueryClientProvider client={createQueryClient()}>
<QueryClientProvider client={queryClient}>
<RouterProvider router={router} />
</QueryClientProvider>
</ApiProvider>,
)
return queryClient
}
// openSheet taps the unread bell and answers the open sheet.
@@ -153,6 +156,51 @@ describe('Recent updates sheet', () => {
expect(screen.queryByTestId('unread-dot')).not.toBeInTheDocument()
})
it('reads the updates afresh on each opening, so one stored since the last opening is listed and marked seen', async () => {
// The server holds the stored events, newest first, and says Home is
// unread while the newest is after the time marked seen.
const stored = [...UPDATES.updates]
let seenThrough = ''
const fetchMock = stubServer({
[`GET ${HOME_PATH}`]: () => jsonResponse(200, { ...HOME, recentUpdates: { unread: stored[0]!.created_at > seenThrough } }),
[`GET ${UPDATES_PATH}`]: () => jsonResponse(200, { updates: stored }),
[`POST ${UPDATES_SEEN_PATH}`]: (init) => {
seenThrough = (JSON.parse(String(init?.body)) as { through: string }).through
return new Response(null, { status: 204 })
},
})
const queryClient = await renderHome()
await rows(await openSheet())
await waitFor(() => expect(posts(fetchMock)).toHaveLength(1))
await waitFor(() => expect(homeReads(fetchMock)).toBe(2))
fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Escape' })
// A new event is stored a moment later, well within the query client's
// staleTime, and Home is read again, as on coming back to it.
const reminder = {
type: 'day_of_reminder',
title: 'Tacos tonight',
body: 'Dinner at 6:00.',
url: '/plan',
created_at: new Date().toISOString(),
}
stored.unshift(reminder)
await queryClient.invalidateQueries({ queryKey: HOME_KEY })
expect(await screen.findByTestId('unread-dot')).toBeInTheDocument()
const listed = await rows(await openSheet())
expect(listed).toHaveLength(4)
expect(listed[0]).toHaveTextContent('Tacos tonight')
await waitFor(() =>
expect(posts(fetchMock)).toEqual([
JSON.stringify({ through: UPDATES.updates[0]!.created_at }),
JSON.stringify({ through: reminder.created_at }),
]),
)
await waitFor(() => expect(homeReads(fetchMock)).toBe(4))
expect(screen.queryByTestId('unread-dot')).not.toBeInTheDocument()
})
it.each([
[0, '/plan'],
[1, '/discover'],
+39
View File
@@ -0,0 +1,39 @@
import { describe, expect, it } from 'vitest'
import { parseUpdates } from './api.ts'
const origin = 'https://mealquest.example'
const where = 'GET /api/v1/updates'
// body is a GET /api/v1/updates body holding one update that opens url.
function body(url: string) {
return {
updates: [{ type: 'plan_ready', title: 'Your week is drafted', body: '', url, created_at: '2026-10-10T16:00:00.000Z' }],
}
}
describe('parseUpdates', () => {
it('keeps a same-origin path with its query and fragment, as parsed', () => {
expect(parseUpdates(where, body('/plan/shopping?week=2026-10-05#tuesday'), origin)[0]!.url).toBe(
'/plan/shopping?week=2026-10-05#tuesday',
)
expect(parseUpdates(where, body('/plan/../discover'), origin)[0]!.url).toBe('/discover')
})
// Each of these would open another site, or run script, once parsed: URL
// parsing strips tabs and newlines and reads a backslash as a slash.
it.each([
['a tab', '/\t/evil.example/x'],
['a newline', '/\n/evil.example/x'],
['a carriage return', '/\r/evil.example/x'],
['a backslash', '/\\evil.example/x'],
['dot segments', '/..//evil.example/x'],
['a protocol-relative URL', '//evil.example/x'],
['an absolute URL', 'https://evil.example/x'],
['javascript:', 'javascript:alert(1)'],
['a relative path', 'plan'],
])('refuses an update url with %s', (_, url) => {
expect(() => parseUpdates(where, body(url), origin)).toThrow(
`${where}: update url ${JSON.stringify(url)} is not a path on this app`,
)
})
})
+18 -14
View File
@@ -3,6 +3,7 @@ import { useApi } from '../api/index.ts'
import { isRecord } from '../api/errors.ts'
import { oneOf, photoId } from '../dinners/api.ts'
import { weekLabel } from '../plan/week.ts'
import { appPath } from '../push/notification.ts'
export const HOME_PATH = '/api/v1/home'
export const HOME_KEY = ['home'] as const
@@ -108,13 +109,10 @@ export type UpdateType = (typeof UPDATE_TYPES)[number]
// takes back.
export type Update = { type: UpdateType; title: string; body: string; url: string; created_at: string }
// isAppPath says whether url is a path on the app's own origin, as the
// server only ever stores.
function isAppPath(url: string): boolean {
return url.startsWith('/') && !url.startsWith('//') && !url.includes('\\')
}
function parseUpdate(where: string, body: unknown): Update {
// parseUpdate checks one update. Its url must stay on origin once parsed, as
// the server only ever stores a path on the app's own origin; url is that
// path as parsed.
function parseUpdate(where: string, body: unknown, origin: string): Update {
if (
!isRecord(body) ||
typeof body.title !== 'string' ||
@@ -124,7 +122,8 @@ function parseUpdate(where: string, body: unknown): Update {
) {
throw new Error(`${where}: an update is not a notification event`)
}
if (!isAppPath(body.url)) throw new Error(`${where}: update url "${body.url}" is not a path on this app`)
const url = appPath(body.url, origin)
if (url === null) throw new Error(`${where}: update url ${JSON.stringify(body.url)} is not a path on this app`)
if (Number.isNaN(Date.parse(body.created_at))) {
throw new Error(`${where}: update created_at "${body.created_at}" is not a time`)
}
@@ -132,24 +131,29 @@ function parseUpdate(where: string, body: unknown): Update {
type: oneOf(where, 'type', UPDATE_TYPES, body.type),
title: body.title,
body: body.body,
url: body.url,
url,
created_at: body.created_at,
}
}
// parseUpdates checks a GET /api/v1/updates body: the updates, newest first.
export function parseUpdates(where: string, body: unknown): Update[] {
// parseUpdates checks a GET /api/v1/updates body, from the app at origin:
// the updates, newest first.
export function parseUpdates(where: string, body: unknown, origin: string): Update[] {
if (!isRecord(body) || !Array.isArray(body.updates)) throw new Error(`${where}: response is not the updates`)
return body.updates.map((update) => parseUpdate(where, update))
return body.updates.map((update) => parseUpdate(where, update, origin))
}
// useUpdates is the Recent updates (GET /api/v1/updates): the last ten
// notification events, newest first.
// notification events, newest first. The list is dropped as soon as the
// sheet showing it closes (gcTime 0), so each opening reads it afresh and
// never shows, or marks seen, a list read before an event stored since.
export function useUpdates() {
const { apiFetch } = useApi()
return useQuery({
queryKey: UPDATES_KEY,
queryFn: async () => parseUpdates(`GET ${UPDATES_PATH}`, await apiFetch<unknown>(UPDATES_PATH)),
queryFn: async () =>
parseUpdates(`GET ${UPDATES_PATH}`, await apiFetch<unknown>(UPDATES_PATH), window.location.origin),
gcTime: 0,
})
}