From 74e33a62365739c7d500715da0d437c18e9aaeae Mon Sep 17 00:00:00 2001 From: dev_agent Date: Sat, 10 Oct 2026 09:03:47 -0400 Subject: [PATCH 1/4] Add the Recent updates sheet behind the Home bell (Feature 21) GET /api/v1/updates lists the last 10 distinct notification events from the outbox, newest first and one per event key whatever its channels. POST /api/v1/updates/seen {"through": created_at} moves household_settings.updates_seen_at forward, so /api/v1/home's recentUpdates.unread is true only for an event stored since. The Home bell opens a Sheet listing the updates with relative times; a row opens its URL, and opening the sheet marks the updates seen, clearing the grape dot optimistically and putting it back if the mark fails. Maestro task 96e2dc3f-1f8a-4114-bcb5-e0f93c349165. Co-Authored-By: Claude Opus 5.5 --- docs/ARCHITECTURE.md | 25 ++- internal/db/notifications.sql.go | 47 ++++++ internal/db/querier.go | 11 ++ internal/db/settings.sql.go | 17 ++ internal/home/home.go | 51 ++++++ internal/home/home_test.go | 182 +++++++++++++++++++++ internal/household/settings.go | 16 ++ internal/notify/notify.go | 39 +++++ internal/server/home.go | 63 ++++++++ internal/server/home_test.go | 175 +++++++++++++++++++++ internal/server/server.go | 5 +- internal/server/server_test.go | 16 +- migrations/queries/notifications.sql | 17 ++ migrations/queries/settings.sql | 8 + web/src/home/HomePage.tsx | 14 +- web/src/home/UpdatesSheet.module.css | 49 ++++++ web/src/home/UpdatesSheet.test.tsx | 226 +++++++++++++++++++++++++++ web/src/home/UpdatesSheet.tsx | 114 ++++++++++++++ web/src/home/api.ts | 98 +++++++++++- 19 files changed, 1165 insertions(+), 8 deletions(-) create mode 100644 web/src/home/UpdatesSheet.module.css create mode 100644 web/src/home/UpdatesSheet.test.tsx create mode 100644 web/src/home/UpdatesSheet.tsx diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 07e3b92..27e3e46 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1115,7 +1115,30 @@ Runs are recorded in `schedule_runs` and executed through `jobs`. notification was stored in `notification_outbox` after `household_settings.updates_seen_at`, or at all while that is null (`notify.StoredSince`); an event whose channels are all switched off is - never stored and never counts. Nothing writes `updates_seen_at` yet. + never stored and never counts. +- `GET /api/v1/updates` (`home.Service.Updates`, over `notify.Recent`) + answers the Recent updates sheet: the last 10 distinct events in + `notification_outbox`, newest first, one per event key whatever its + channels or delivery status (an event stored for push and Google Chat is + one update): + + ```json + {"updates": [{"type": "plan_ready", "title": "…", "body": "…", "url": "/plan", "created_at": "2026-10-09T14:00:00.000Z"}]} + ``` + + Every field comes from the stored payload, which never holds a webhook + URL or secret; `url` is the same-origin path the event opens. +- `POST /api/v1/updates/seen` with `{"through": ""}` (CSRF + header required) records that the household has seen its updates + through the newest one it was shown, and answers 204. The last-seen + instant is `household_settings.updates_seen_at` — the one place it is + stored. It only moves forward (`MarkUpdatesSeen`), so a `through` older + than the stored one is accepted and changes nothing, and an event + stored after `through` keeps `unread` true. It is not a settings edit: + `updated_by` and `updated_at` are left as they are and settings + subscribers are not told. 400 `invalid_request` for a body that is not + exactly that object, a `through` that is not RFC 3339, or one after + now. ## Frontend diff --git a/internal/db/notifications.sql.go b/internal/db/notifications.sql.go index 3758e5d..94c1dd6 100644 --- a/internal/db/notifications.sql.go +++ b/internal/db/notifications.sql.go @@ -182,6 +182,53 @@ func (q *Queries) ListNotificationPrefs(ctx context.Context) ([]NotificationPref return items, nil } +const listRecentNotifications = `-- name: ListRecentNotifications :many +SELECT o.event_key, o.payload, o.created_at FROM notification_outbox o +WHERE o.id = ( + SELECT i.id FROM notification_outbox i + WHERE i.event_key = o.event_key + ORDER BY i.created_at DESC, i.id DESC + LIMIT 1 +) +ORDER BY o.created_at DESC, o.event_key DESC +LIMIT ?1 +` + +type ListRecentNotificationsRow struct { + EventKey string + Payload string + CreatedAt string +} + +// Returns the last lim distinct events stored, newest first: one row per +// event key, the last stored for it whatever its channel, so an event +// stored for push and Google Chat is one row. The last, because a channel +// switched on later can store the same event key again, and +// HasNotificationSince counts that row: an event is as new as its newest +// row, so marking it seen also covers that row. +func (q *Queries) ListRecentNotifications(ctx context.Context, lim int64) ([]ListRecentNotificationsRow, error) { + rows, err := q.db.QueryContext(ctx, listRecentNotifications, lim) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListRecentNotificationsRow + for rows.Next() { + var i ListRecentNotificationsRow + if err := rows.Scan(&i.EventKey, &i.Payload, &i.CreatedAt); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const markNotificationSent = `-- name: MarkNotificationSent :execrows UPDATE notification_outbox SET status = 'sent', attempts = attempts + 1, next_attempt_at = NULL, updated_at = ?1 diff --git a/internal/db/querier.go b/internal/db/querier.go index 2213f67..6a11650 100644 --- a/internal/db/querier.go +++ b/internal/db/querier.go @@ -212,6 +212,13 @@ type Querier interface { // Every stored subscription with its keys, for the push sender, oldest // first. ListPushSubscriptions(ctx context.Context) ([]ListPushSubscriptionsRow, error) + // Returns the last lim distinct events stored, newest first: one row per + // event key, the last stored for it whatever its channel, so an event + // stored for push and Google Chat is one row. The last, because a channel + // switched on later can store the same event key again, and + // HasNotificationSince counts that row: an event is as new as its newest + // row, so marking it seen also covers that row. + ListRecentNotifications(ctx context.Context, lim int64) ([]ListRecentNotificationsRow, error) // Reads for the AI household context block (internal/ai/harness). // The most recent names the household turned down, newest first, as // ListPassedNames has them. @@ -254,6 +261,10 @@ type Querier interface { // Records list id sent to Google Chat at sent_to_chat_at, its send // confirmed, unless it is frozen into the Supply archive. MarkShoppingListSentToChat(ctx context.Context, arg MarkShoppingListSentToChatParams) (int64, error) + // Moves updates_seen_at forward to seen_at; affects no row when it is + // already at or after seen_at, so it never moves back. It is not a settings + // edit: updated_by and updated_at are left as they are. + MarkUpdatesSeen(ctx context.Context, seenAt string) (int64, error) // Records an edit of plan id, raising its revision: a synced plan becomes // changed, any other keeps its status. MarkWeekPlanEdited(ctx context.Context, arg MarkWeekPlanEditedParams) error diff --git a/internal/db/settings.sql.go b/internal/db/settings.sql.go index 04e9ec7..65f68dc 100644 --- a/internal/db/settings.sql.go +++ b/internal/db/settings.sql.go @@ -161,6 +161,23 @@ func (q *Queries) GetRecipeSearchSettings(ctx context.Context) (RecipeSearchSett return i, err } +const markUpdatesSeen = `-- name: MarkUpdatesSeen :execrows +UPDATE household_settings +SET updates_seen_at = CAST(?1 AS TEXT) +WHERE id = 1 AND (updates_seen_at IS NULL OR updates_seen_at < CAST(?1 AS TEXT)) +` + +// Moves updates_seen_at forward to seen_at; affects no row when it is +// already at or after seen_at, so it never moves back. It is not a settings +// edit: updated_by and updated_at are left as they are. +func (q *Queries) MarkUpdatesSeen(ctx context.Context, seenAt string) (int64, error) { + result, err := q.db.ExecContext(ctx, markUpdatesSeen, seenAt) + if err != nil { + return 0, err + } + return result.RowsAffected() +} + const recordChatTest = `-- name: RecordChatTest :execrows UPDATE chat_settings SET last_test_at = ?, last_test_status = ?, updated_at = ? diff --git a/internal/home/home.go b/internal/home/home.go index 73f21fb..735ab40 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -195,3 +195,54 @@ func (s *Service) unread(ctx context.Context, seenAt *string) (bool, error) { } return unread, nil } + +// RecentLimit is how many events the Recent updates sheet lists. +const RecentLimit = 10 + +// Update is one event in the Recent updates sheet. +type Update struct { + // Type is the event type, e.g. plan_ready. + Type string `json:"type"` + Title string `json:"title"` + Body string `json:"body"` + // URL is the path-absolute, same-origin path a tap on the update opens. + URL string `json:"url"` + // CreatedAt is when the event was stored, UTC RFC 3339 with + // milliseconds; it is what POST /updates/seen takes back. + CreatedAt string `json:"created_at"` +} + +// Updates returns the last RecentLimit distinct notification events, newest +// first, whatever channels they went to. +func (s *Service) Updates(ctx context.Context) ([]Update, error) { + events, err := s.notifier.Recent(ctx, RecentLimit) + if err != nil { + return nil, fmt.Errorf("home: %w", err) + } + out := make([]Update, len(events)) + for i, ev := range events { + out[i] = Update{ + Type: string(ev.Type), Title: ev.Title, Body: ev.Body, URL: ev.URL, + CreatedAt: store.Timestamp{Time: ev.CreatedAt}.String(), + } + } + return out, nil +} + +// ErrSeenInFuture is returned by MarkUpdatesSeen for a time after now. +var ErrSeenInFuture = errors.New("home: updates cannot be seen through a time after now") + +// MarkUpdatesSeen records that the household has seen its updates through +// through, the created_at of the newest update it was shown, so +// recentUpdates.unread is false until an event is stored after it. A time +// before the one already recorded changes nothing; one after now is +// ErrSeenInFuture, since it would hide events not yet stored. +func (s *Service) MarkUpdatesSeen(ctx context.Context, through time.Time) error { + if through.After(s.now()) { + return ErrSeenInFuture + } + if err := s.settings.MarkUpdatesSeen(ctx, through); err != nil { + return fmt.Errorf("home: %w", err) + } + return nil +} diff --git a/internal/home/home_test.go b/internal/home/home_test.go index 4742350..e0235ca 100644 --- a/internal/home/home_test.go +++ b/internal/home/home_test.go @@ -4,7 +4,9 @@ import ( "context" "database/sql" "encoding/base64" + "encoding/json" "errors" + "fmt" "io" "log/slog" "net/http" @@ -370,3 +372,183 @@ func TestNewServiceRequiresServices(t *testing.T) { t.Fatal("NewService accepted nil services") } } + +// store stores event key of type typ in the outbox at instant at, for each +// of channels, as notify stores an event. +func (e *env) store(t *testing.T, typ notify.EventType, key string, at time.Time, channels ...notify.Channel) { + t.Helper() + payload, err := json.Marshal(notify.Payload{Event: typ, Title: "Title " + key, Body: "Body " + key, URL: "/u/" + key, Tag: key}) + if err != nil { + t.Fatal(err) + } + ts := store.Timestamp{Time: at.UTC().Truncate(time.Millisecond)}.String() + for _, ch := range channels { + if _, err := db.New(e.st.Writer).InsertNotification(t.Context(), db.InsertNotificationParams{ + ID: store.NewID(), EventKey: key, Channel: string(ch), Payload: string(payload), + NextAttemptAt: sql.NullString{String: ts, Valid: true}, Now: ts, + }); err != nil { + t.Fatal(err) + } + } +} + +// Updates are the last ten distinct events, newest first: an event stored +// for push and Google Chat is listed once, whatever became of its delivery. +func TestUpdates(t *testing.T) { + t.Parallel() + e := newEnv(t) + ctx := t.Context() + + got, err := e.svc.Updates(ctx) + if err != nil { + t.Fatal(err) + } + if got == nil || len(got) != 0 { + t.Errorf("updates with no notifications = %#v, want an empty list", got) + } + + base := time.Date(2026, 10, 13, 12, 0, 0, 0, time.UTC) + for i := range 12 { + key := fmt.Sprintf("plan_ready:%02d", i) + e.store(t, notify.PlanReady, key, base.Add(time.Duration(i)*time.Minute), notify.ChannelPush, notify.ChannelChat) + } + // A delivered and a failed row of one event are still one event. + if _, err := e.st.Writer.ExecContext(ctx, `UPDATE notification_outbox SET status = 'sent' WHERE event_key = 'plan_ready:11' AND channel = 'push'`); err != nil { + t.Fatal(err) + } + if _, err := e.st.Writer.ExecContext(ctx, `UPDATE notification_outbox SET status = 'failed' WHERE event_key = 'plan_ready:11' AND channel = 'chat'`); err != nil { + t.Fatal(err) + } + + got, err = e.svc.Updates(ctx) + if err != nil { + t.Fatal(err) + } + if len(got) != home.RecentLimit { + t.Fatalf("updates = %d, want %d: %+v", len(got), home.RecentLimit, got) + } + for i, u := range got { + n := 11 - i + key := fmt.Sprintf("plan_ready:%02d", n) + want := home.Update{ + Type: "plan_ready", Title: "Title " + key, Body: "Body " + key, URL: "/u/" + key, + CreatedAt: store.Timestamp{Time: base.Add(time.Duration(n) * time.Minute)}.String(), + } + if u != want { + t.Errorf("updates[%d] = %+v, want %+v", i, u, want) + } + } +} + +// An event emitted to push and Google Chat is one update. +func TestUpdatesEmittedToTwoChannels(t *testing.T) { + t.Parallel() + e := newEnv(t) + ctx := t.Context() + if err := e.notifier.Emit(ctx, notify.Event{Type: notify.PlanReady, Key: "plan_ready:2026-10-12", Title: "Your week is drafted", Body: "Take a look", URL: "/plan"}); err != nil { + t.Fatal(err) + } + var rows int + if err := e.st.Reader.QueryRowContext(ctx, `SELECT COUNT(*) FROM notification_outbox`).Scan(&rows); err != nil { + t.Fatal(err) + } + if rows != 2 { + t.Fatalf("outbox rows = %d, want push and chat", rows) + } + got, err := e.svc.Updates(ctx) + if err != nil { + t.Fatal(err) + } + if len(got) != 1 || got[0].Type != "plan_ready" || got[0].Title != "Your week is drafted" || got[0].Body != "Take a look" || got[0].URL != "/plan" { + t.Errorf("updates = %+v, want the one plan_ready event", got) + } +} + +// Marking updates seen through the newest clears unread until a newer event +// is stored; marking them seen through an older time does not move the +// mark back, and a time after now is refused. +func TestMarkUpdatesSeen(t *testing.T) { + t.Parallel() + e := newEnv(t) + ctx := t.Context() + unread := func() bool { + t.Helper() + return e.at(t, "2026-10-13 12:00").RecentUpdates.Unread + } + first := time.Date(2026, 10, 13, 16, 0, 0, 0, time.UTC) + e.store(t, notify.NewSuggestions, "new_suggestions:1", first, notify.ChannelPush) + if !unread() { + t.Fatal("not unread with an event never seen") + } + if err := e.svc.MarkUpdatesSeen(ctx, first); err != nil { + t.Fatal(err) + } + if unread() { + t.Error("unread after marking the newest event seen") + } + + second := first.Add(time.Millisecond) + e.store(t, notify.PlanReady, "plan_ready:2", second, notify.ChannelPush, notify.ChannelChat) + if !unread() { + t.Error("not unread with an event stored after the mark") + } + // Seen again through the older event, as a sheet opened before the + // newer one arrived would: nothing moves, so it stays unread. + if err := e.svc.MarkUpdatesSeen(ctx, first); err != nil { + t.Fatal(err) + } + if !unread() { + t.Error("an older mark hid the newer event") + } + if err := e.svc.MarkUpdatesSeen(ctx, second); err != nil { + t.Fatal(err) + } + if unread() { + t.Error("unread after marking the newer event seen") + } + if err := e.svc.MarkUpdatesSeen(ctx, first); err != nil { + t.Fatal(err) + } + settings, err := household.NewSettingsService(e.st).Get(ctx) + if err != nil { + t.Fatal(err) + } + if want := (store.Timestamp{Time: second}).String(); settings.UpdatesSeenAt == nil || *settings.UpdatesSeenAt != want { + t.Errorf("updates_seen_at = %v, want %s: an older mark moved it back", settings.UpdatesSeenAt, want) + } + if settings.UpdatedBy != nil { + t.Errorf("updated_by = %q, want unset: marking updates seen is not a settings edit", *settings.UpdatedBy) + } + + // The clock reads 17:00 UTC. + if err := e.svc.MarkUpdatesSeen(ctx, first.Add(2*time.Hour)); !errors.Is(err, home.ErrSeenInFuture) { + t.Errorf("mark seen through an hour from now = %v, want ErrSeenInFuture", err) + } +} + +// An event stored again for a channel switched on later is as new as that +// row: the update shows its time, and marking it seen clears unread. +func TestUpdateStoredAgainForAnotherChannel(t *testing.T) { + t.Parallel() + e := newEnv(t) + ctx := t.Context() + first := time.Date(2026, 10, 13, 15, 0, 0, 0, time.UTC) + later := first.Add(30 * time.Minute) + e.store(t, notify.PlanReady, "plan_ready:1", first, notify.ChannelPush) + e.store(t, notify.PlanReady, "plan_ready:1", later, notify.ChannelChat) + + got, err := e.svc.Updates(ctx) + if err != nil { + t.Fatal(err) + } + if want := (store.Timestamp{Time: later}).String(); len(got) != 1 || got[0].CreatedAt != want { + t.Fatalf("updates = %+v, want the one event at %s", got, want) + } + e.at(t, "2026-10-13 12:00") + if err := e.svc.MarkUpdatesSeen(ctx, later); err != nil { + t.Fatal(err) + } + if e.at(t, "2026-10-13 12:00").RecentUpdates.Unread { + t.Error("unread after the event was marked seen through its listed time") + } +} diff --git a/internal/household/settings.go b/internal/household/settings.go index 88b8586..f7fc55e 100644 --- a/internal/household/settings.go +++ b/internal/household/settings.go @@ -8,6 +8,7 @@ import ( "regexp" "strings" "sync" + "time" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" @@ -200,6 +201,21 @@ func (s *SettingsService) Update(ctx context.Context, patch SettingsPatch, updat return settings, nil } +// MarkUpdatesSeen records that the household has seen its Recent updates +// through seenAt: updates_seen_at moves forward to seenAt, and is left as it +// is when it is already at or after it. It is not a settings edit, so +// updated_by and updated_at are kept and subscribers are not told. +func (s *SettingsService) MarkUpdatesSeen(ctx context.Context, seenAt time.Time) error { + if seenAt.IsZero() { + return errors.New("household: mark updates seen: zero time") + } + _, err := db.New(s.st.Writer).MarkUpdatesSeen(ctx, store.Timestamp{Time: seenAt.UTC().Truncate(time.Millisecond)}.String()) + if err != nil { + return fmt.Errorf("household: mark updates seen: %w", err) + } + return nil +} + // applyPatch applies a validated patch to the household settings row in q's // transaction, recording updatedBy, and returns the new row. func applyPatch(ctx context.Context, q *db.Queries, patch SettingsPatch, updatedBy string) (db.HouseholdSetting, error) { diff --git a/internal/notify/notify.go b/internal/notify/notify.go index ae73a5a..bc4503b 100644 --- a/internal/notify/notify.go +++ b/internal/notify/notify.go @@ -205,6 +205,45 @@ func (s *Service) StoredSince(ctx context.Context, since *time.Time) (bool, erro return found, nil } +// Stored is one event as it was stored, whatever channels it went to. +type Stored struct { + Type EventType + Title string + Body string + // URL is the path-absolute, same-origin path the event opens. + URL string + // CreatedAt is when the event was last stored, for any channel: the + // instant StoredSince must be given for it not to count. + CreatedAt time.Time +} + +// Recent returns the last n distinct events stored, newest first. An event +// stored for more than one channel is one event, as new as its newest row; +// one whose preference switches every channel off is never stored and +// never listed. +func (s *Service) Recent(ctx context.Context, n int) ([]Stored, error) { + if n <= 0 { + return nil, fmt.Errorf("notify: list %d recent events: want a positive count", n) + } + rows, err := db.New(s.st.Reader).ListRecentNotifications(ctx, int64(n)) + if err != nil { + return nil, fmt.Errorf("notify: list recent events: %w", err) + } + out := make([]Stored, 0, len(rows)) + for _, r := range rows { + var p Payload + if err := json.Unmarshal([]byte(r.Payload), &p); err != nil { + return nil, fmt.Errorf("notify: decode payload of %s: %w", r.EventKey, err) + } + created, err := parseTimestamp(r.CreatedAt) + if err != nil { + return nil, fmt.Errorf("notify: created_at of %s: %w", r.EventKey, err) + } + out = append(out, Stored{Type: p.Event, Title: p.Title, Body: p.Body, URL: p.URL, CreatedAt: created}) + } + return out, nil +} + // emitTx stores ev in tx for each channel that channels chooses from the // preference of ev's type. func (s *Service) emitTx(ctx context.Context, tx *sql.Tx, ev Event, channels func(db.NotificationPref) ([]Channel, error)) error { diff --git a/internal/server/home.go b/internal/server/home.go index f54e7fb..92377fe 100644 --- a/internal/server/home.go +++ b/internal/server/home.go @@ -1,8 +1,10 @@ package server import ( + "errors" "log/slog" "net/http" + "time" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/home" ) @@ -20,3 +22,64 @@ func getHome(logger *slog.Logger, svc *home.Service) http.HandlerFunc { writeJSON(w, r, logger, http.StatusOK, h) } } + +// updatesBody is the GET /updates answer. +type updatesBody struct { + Updates []home.Update `json:"updates"` +} + +// getUpdates answers GET /updates with the Recent updates: the last +// home.RecentLimit distinct notification events, newest first. +func getUpdates(logger *slog.Logger, svc *home.Service) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + updates, err := svc.Updates(r.Context()) + if err != nil { + logger.ErrorContext(r.Context(), "read recent updates", "error", err) + writeError(w, r, logger, http.StatusInternalServerError, "internal", "internal error") + return + } + writeJSON(w, r, logger, http.StatusOK, updatesBody{Updates: updates}) + } +} + +// maxUpdatesSeenBody bounds a POST /updates/seen request body. +const maxUpdatesSeenBody = 1 << 10 + +// updatesSeenRequest is the POST /updates/seen body: through is the +// created_at of the newest update the household was shown. +type updatesSeenRequest struct { + Through *string `json:"through"` +} + +// postUpdatesSeen answers POST /updates/seen by recording that the +// household has seen its updates through the body's through, and answers +// 204. A through before the one already recorded changes nothing. +func postUpdatesSeen(logger *slog.Logger, svc *home.Service) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + req, err := decodePatch[updatesSeenRequest](http.MaxBytesReader(w, r.Body, maxUpdatesSeenBody)) + if err != nil { + writeError(w, r, logger, http.StatusBadRequest, "invalid_request", err.Error()) + return + } + if req.Through == nil { + writeError(w, r, logger, http.StatusBadRequest, "invalid_request", "through: required") + return + } + through, err := time.Parse(time.RFC3339Nano, *req.Through) + if err != nil { + writeError(w, r, logger, http.StatusBadRequest, "invalid_request", "through: want an RFC 3339 timestamp") + return + } + err = svc.MarkUpdatesSeen(r.Context(), through) + if errors.Is(err, home.ErrSeenInFuture) { + writeError(w, r, logger, http.StatusBadRequest, "invalid_request", "through: must not be after now") + return + } + if err != nil { + logger.ErrorContext(r.Context(), "mark recent updates seen", "error", err) + writeError(w, r, logger, http.StatusInternalServerError, "internal", "internal error") + return + } + w.WriteHeader(http.StatusNoContent) + } +} diff --git a/internal/server/home_test.go b/internal/server/home_test.go index f84d5ff..9f98be7 100644 --- a/internal/server/home_test.go +++ b/internal/server/home_test.go @@ -1,13 +1,18 @@ package server import ( + "database/sql" "encoding/json" "net/http" "net/http/httptest" + "strings" "testing" "time" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/notify" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" ) // GET /home answers the Home screen in its JSON shape; newTestHome's @@ -67,3 +72,173 @@ func TestGetHomeNeedsSession(t *testing.T) { t.Errorf("GET /api/v1/home without a session = %d, want 401", rec.Code) } } + +// homeRouter returns a router whose Home reads st, and st. +func homeRouter(t *testing.T) (http.Handler, *store.Store) { + t.Helper() + st := newTestStore(t) + svc := newTestHomeOn(t, st) + return newTestRouterWithHome(t, "dev", newTestSettings(t), newTestRunner(t), svc), st +} + +// storeEvent stores event key in st's outbox at instant at for each of +// channels, as notify stores an event, and returns at as stored. +func storeEvent(t *testing.T, st *store.Store, typ notify.EventType, key, title string, at time.Time, channels ...notify.Channel) string { + t.Helper() + payload, err := json.Marshal(notify.Payload{Event: typ, Title: title, Body: "Body of " + key, URL: "/plan", Tag: key}) + if err != nil { + t.Fatal(err) + } + ts := store.Timestamp{Time: at.UTC().Truncate(time.Millisecond)}.String() + for _, ch := range channels { + if _, err := db.New(st.Writer).InsertNotification(t.Context(), db.InsertNotificationParams{ + ID: store.NewID(), EventKey: key, Channel: string(ch), Payload: string(payload), + NextAttemptAt: sql.NullString{String: ts, Valid: true}, Now: ts, + }); err != nil { + t.Fatal(err) + } + } + return ts +} + +// unread reads recentUpdates.unread from GET /home. +func unread(t *testing.T, h http.Handler) bool { + t.Helper() + rec, body := serve(t, h, signedIn(http.MethodGet, "/api/v1/home", nil)) + if rec.Code != http.StatusOK { + t.Fatalf("GET /api/v1/home = %d: %s", rec.Code, rec.Body) + } + updates, ok := body["recentUpdates"].(map[string]any) + if !ok { + t.Fatalf("GET /api/v1/home has no recentUpdates: %s", rec.Body) + } + return updates["unread"] == true +} + +// markSeen posts body to /updates/seen and returns the answer. +func markSeen(t *testing.T, h http.Handler, body string) *httptest.ResponseRecorder { + t.Helper() + rec := httptest.NewRecorder() + h.ServeHTTP(rec, signedIn(http.MethodPost, "/api/v1/updates/seen", strings.NewReader(body))) + return rec +} + +// GET /updates lists the last distinct events newest first, one per event +// whatever its channels, in their JSON shape. +func TestGetUpdates(t *testing.T) { + t.Parallel() + h, st := homeRouter(t) + + rec := httptest.NewRecorder() + h.ServeHTTP(rec, signedIn(http.MethodGet, "/api/v1/updates", nil)) + if rec.Code != http.StatusOK || rec.Body.String() != `{"updates":[]}` { + t.Errorf("GET /api/v1/updates with none = %d %s, want 200 {\"updates\":[]}", rec.Code, rec.Body) + } + + now := time.Now() + older := storeEvent(t, st, notify.NewSuggestions, "new_suggestions:1", "New Side quests", now.Add(-2*time.Minute), notify.ChannelPush) + newer := storeEvent(t, st, notify.PlanReady, "plan_ready:2026-10-12", "Your week is drafted", now.Add(-time.Minute), notify.ChannelPush, notify.ChannelChat) + + rec = httptest.NewRecorder() + h.ServeHTTP(rec, signedIn(http.MethodGet, "/api/v1/updates", nil)) + want := `{"updates":[` + + `{"type":"plan_ready","title":"Your week is drafted","body":"Body of plan_ready:2026-10-12","url":"/plan","created_at":"` + newer + `"},` + + `{"type":"new_suggestions","title":"New Side quests","body":"Body of new_suggestions:1","url":"/plan","created_at":"` + older + `"}]}` + if rec.Code != http.StatusOK || rec.Body.String() != want { + t.Errorf("GET /api/v1/updates = %d %s, want 200 %s", rec.Code, rec.Body, want) + } +} + +// Unread is true while an event newer than the last seen one exists, +// clears once POST /updates/seen marks the newest seen, and returns when a +// newer event is stored; marking an older one seen again moves nothing. +func TestUpdatesSeen(t *testing.T) { + t.Parallel() + h, st := homeRouter(t) + now := time.Now() + + if unread(t, h) { + t.Error("unread with no events") + } + first := storeEvent(t, st, notify.PlanReady, "plan_ready:1", "Drafted", now.Add(-2*time.Minute), notify.ChannelPush, notify.ChannelChat) + if !unread(t, h) { + t.Error("not unread with an event never seen") + } + if rec := markSeen(t, h, `{"through":"`+first+`"}`); rec.Code != http.StatusNoContent || rec.Body.Len() != 0 { + t.Fatalf("POST /api/v1/updates/seen = %d %s, want 204", rec.Code, rec.Body) + } + if unread(t, h) { + t.Error("unread after the newest event was marked seen") + } + + second := storeEvent(t, st, notify.ListReady, "list_ready:2", "Supply run ready", now.Add(-time.Minute), notify.ChannelPush) + if !unread(t, h) { + t.Error("not unread with an event newer than the last seen") + } + // Already seen: marking the older event seen again is accepted and + // changes nothing. + if rec := markSeen(t, h, `{"through":"`+first+`"}`); rec.Code != http.StatusNoContent { + t.Fatalf("POST /api/v1/updates/seen of an older event = %d %s, want 204", rec.Code, rec.Body) + } + if !unread(t, h) { + t.Error("marking an older event seen hid a newer one") + } + if rec := markSeen(t, h, `{"through":"`+second+`"}`); rec.Code != http.StatusNoContent { + t.Fatalf("POST /api/v1/updates/seen = %d %s, want 204", rec.Code, rec.Body) + } + if unread(t, h) { + t.Error("unread after the newer event was marked seen") + } +} + +// A malformed POST /updates/seen is 400 and records nothing. +func TestUpdatesSeenRejectsMalformed(t *testing.T) { + t.Parallel() + h, st := homeRouter(t) + storeEvent(t, st, notify.PlanReady, "plan_ready:1", "Drafted", time.Now().Add(-time.Minute), notify.ChannelPush) + + for _, body := range []string{ + ``, + `[]`, + `{}`, + `{"through":null}`, + `{"through":42}`, + `{"through":"yesterday"}`, + `{"through":"2026-10-13 12:00:00"}`, + `{"through":"2026-10-13T12:00:00.000Z","extra":true}`, + `{"through":"2026-10-13T12:00:00.000Z"} {}`, + `{"through":"` + time.Now().Add(time.Hour).UTC().Format(time.RFC3339Nano) + `"}`, + } { + rec := markSeen(t, h, body) + if rec.Code != http.StatusBadRequest || !strings.Contains(rec.Body.String(), `"invalid_request"`) { + t.Errorf("POST /api/v1/updates/seen %q = %d %s, want 400 invalid_request", body, rec.Code, rec.Body) + } + } + if !unread(t, h) { + t.Error("a malformed POST /updates/seen marked updates seen") + } +} + +// Both endpoints need a session, and POST /updates/seen the CSRF token. +func TestUpdatesNeedSessionAndCSRF(t *testing.T) { + t.Parallel() + h, _ := homeRouter(t) + body := `{"through":"2026-10-01T12:00:00.000Z"}` + for _, req := range []*http.Request{ + httptest.NewRequest(http.MethodGet, "/api/v1/updates", nil), + httptest.NewRequest(http.MethodPost, "/api/v1/updates/seen", strings.NewReader(body)), + } { + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + if rec.Code != http.StatusUnauthorized { + t.Errorf("%s %s without a session = %d, want 401", req.Method, req.URL.Path, rec.Code) + } + } + req := httptest.NewRequest(http.MethodPost, "/api/v1/updates/seen", strings.NewReader(body)) + req.AddCookie(validSessionCookie) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + if rec.Code != http.StatusForbidden { + t.Errorf("POST /api/v1/updates/seen without the CSRF token = %d, want 403", rec.Code) + } +} diff --git a/internal/server/server.go b/internal/server/server.go index c5f5850..3b0352e 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -67,7 +67,8 @@ type RouteMounter interface { // /api/v1/settings/notifications/test-push; ideasSvc serves POST // /api/v1/ideas/from-ingredients and POST /api/v1/ideas/quick, and with // dinnerSvc POST /api/v1/ideas/{jobId}/items/{i}/quest-log and, with -// planSvc too, /cook-tonight; homeSvc serves GET /api/v1/home; runner serves +// planSvc too, /cook-tonight; homeSvc serves GET /api/v1/home, GET +// /api/v1/updates and POST /api/v1/updates/seen; runner serves // POST /api/v1/jobs and GET /api/v1/jobs/{id}; photos serves // GET /media/photos/{id}-{size}.jpg; assets is the web build // served for every other GET. /health, /auth and the web build are public. @@ -142,6 +143,8 @@ func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, a } apiV1.Get("/me", me(logger)) apiV1.Get("/home", getHome(logger, homeSvc)) + apiV1.Get("/updates", getUpdates(logger, homeSvc)) + apiV1.Post("/updates/seen", postUpdatesSeen(logger, homeSvc)) apiV1.Get("/settings/household", getHouseholdSettings(logger, settings)) apiV1.Patch("/settings/household", patchHouseholdSettings(logger, settings)) apiV1.Get("/settings/notifications", getNotificationSettings(logger, settings)) diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 2ea9637..6a1769d 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -52,13 +52,19 @@ func newTestRouter(t *testing.T, version string) http.Handler { } func newTestRouterWith(t *testing.T, version string, settings *household.SettingsService, runner *jobs.Runner) http.Handler { + t.Helper() + return newTestRouterWithHome(t, version, settings, runner, newTestHome(t)) +} + +// newTestRouterWithHome is newTestRouterWith serving Home from homeSvc. +func newTestRouterWithHome(t *testing.T, version string, settings *household.SettingsService, runner *jobs.Runner, homeSvc *home.Service) http.Handler { t.Helper() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) assets := fstest.MapFS{ "index.html": {Data: []byte(fixtureIndex)}, "assets/index-AbC123.js": {Data: []byte(fixtureAsset)}, } - r, err := NewRouter(logger, version, nil, stubAuth{}, stubSessions{}, settings, newTestSearch(t), newTestAI(t), newTestChat(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestShopping(t), newTestGoogle(t), newTestPush(t), newTestIdeas(t), newTestHome(t), runner, newTestPhotos(t), assets) + r, err := NewRouter(logger, version, nil, stubAuth{}, stubSessions{}, settings, newTestSearch(t), newTestAI(t), newTestChat(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestShopping(t), newTestGoogle(t), newTestPush(t), newTestIdeas(t), homeSvc, runner, newTestPhotos(t), assets) if err != nil { t.Fatal(err) } @@ -265,7 +271,13 @@ func newTestShopping(t *testing.T) *shopping.Service { // connection. One database keeps the routers that never ask for Home cheap. func newTestHome(t *testing.T) *home.Service { t.Helper() - st := newTestStore(t) + return newTestHomeOn(t, newTestStore(t)) +} + +// newTestHomeOn returns a home service over st, a migrated database, which +// it seeds with zone America/New_York and the seed notification preferences. +func newTestHomeOn(t *testing.T, st *store.Store) *home.Service { + t.Helper() if err := household.SeedSettings(context.Background(), st, "America/New_York"); err != nil { t.Fatal(err) } diff --git a/migrations/queries/notifications.sql b/migrations/queries/notifications.sql index 83d0ac9..d952567 100644 --- a/migrations/queries/notifications.sql +++ b/migrations/queries/notifications.sql @@ -74,3 +74,20 @@ SELECT EXISTS ( SELECT 1 FROM notification_outbox WHERE CAST(sqlc.narg(since) AS TEXT) IS NULL OR created_at > CAST(sqlc.narg(since) AS TEXT) ) AS found; + +-- name: ListRecentNotifications :many +-- Returns the last lim distinct events stored, newest first: one row per +-- event key, the last stored for it whatever its channel, so an event +-- stored for push and Google Chat is one row. The last, because a channel +-- switched on later can store the same event key again, and +-- HasNotificationSince counts that row: an event is as new as its newest +-- row, so marking it seen also covers that row. +SELECT o.event_key, o.payload, o.created_at FROM notification_outbox o +WHERE o.id = ( + SELECT i.id FROM notification_outbox i + WHERE i.event_key = o.event_key + ORDER BY i.created_at DESC, i.id DESC + LIMIT 1 +) +ORDER BY o.created_at DESC, o.event_key DESC +LIMIT sqlc.arg(lim); diff --git a/migrations/queries/settings.sql b/migrations/queries/settings.sql index f40a0d7..eb9ec9d 100644 --- a/migrations/queries/settings.sql +++ b/migrations/queries/settings.sql @@ -154,3 +154,11 @@ RETURNING *; UPDATE chat_settings SET last_test_at = ?, last_test_status = ?, updated_at = ? WHERE id = 1 AND webhook_url_ct = ?; + +-- name: MarkUpdatesSeen :execrows +-- Moves updates_seen_at forward to seen_at; affects no row when it is +-- already at or after seen_at, so it never moves back. It is not a settings +-- edit: updated_by and updated_at are left as they are. +UPDATE household_settings +SET updates_seen_at = CAST(sqlc.arg(seen_at) AS TEXT) +WHERE id = 1 AND (updates_seen_at IS NULL OR updates_seen_at < CAST(sqlc.arg(seen_at) AS TEXT)); diff --git a/web/src/home/HomePage.tsx b/web/src/home/HomePage.tsx index fe88894..81c3196 100644 --- a/web/src/home/HomePage.tsx +++ b/web/src/home/HomePage.tsx @@ -1,4 +1,5 @@ import { Bell, ChevronRight, Compass, Search, ShoppingCart, Zap, type LucideIcon } from 'lucide-react' +import { useState } from 'react' import { Link, useNavigate } from 'react-router' import { duration } from '../plan/PlanPage.tsx' import { supplyRunScreen } from '../shopping/api.ts' @@ -11,6 +12,7 @@ import { Photo } from '../ui/Photo.tsx' import { Skeleton } from '../ui/Skeleton.tsx' import { useHome, type Home, type Origin, type Tonight } from './api.ts' import styles from './HomePage.module.css' +import { UpdatesSheet } from './UpdatesSheet.tsx' const ORIGIN_LABEL: Record = { regular: 'Regular', side_quest: 'Side quest' } @@ -108,11 +110,12 @@ function Tiles() { // Quick dinner and Cook from what I have tiles, and the status rows, all // from GET /api/v1/home. // -// The bell only shows whether there are unread updates; the Recent updates -// sheet it opens is still to come. +// The bell carries a grape dot while there are unread updates and opens the +// Recent updates sheet, which marks them seen. export function HomePage() { const home = useHome() const data = home.data + const [updatesOpen, setUpdatesOpen] = useState(false) return (
@@ -124,7 +127,11 @@ export function HomePage() { )}

Tonight's quest

- + setUpdatesOpen(true)} + > {data?.unread && } @@ -142,6 +149,7 @@ export function HomePage() { )} {data !== undefined && } + setUpdatesOpen(false)} /> ) } diff --git a/web/src/home/UpdatesSheet.module.css b/web/src/home/UpdatesSheet.module.css new file mode 100644 index 0000000..14c7eb0 --- /dev/null +++ b/web/src/home/UpdatesSheet.module.css @@ -0,0 +1,49 @@ +/* The Recent updates sheet the Home bell opens. */ +.sheet { + display: flex; + flex-direction: column; + gap: 14px; + max-width: 560px; + margin: 0 auto; +} + +.title { + margin: 0; + font-family: var(--font-heading); + font-size: 22px; + font-weight: 400; +} + +.rows { + margin: 0; + padding: 0; + list-style: none; + display: flex; + flex-direction: column; + gap: var(--space-1); +} + +.icon { + color: var(--color-accent-700); +} + +.time { + font-family: var(--font-body); + font-size: 12px; + font-weight: 700; + color: var(--color-neutral-700); + white-space: nowrap; +} + +.skeletonRow { + height: 56px; + border-radius: var(--radius-md); +} + +.empty { + margin: 0; + font-family: var(--font-body); + font-size: 14px; + line-height: 1.45; + color: var(--color-neutral-700); +} diff --git a/web/src/home/UpdatesSheet.test.tsx b/web/src/home/UpdatesSheet.test.tsx new file mode 100644 index 0000000..b5de4f2 --- /dev/null +++ b/web/src/home/UpdatesSheet.test.tsx @@ -0,0 +1,226 @@ +import { QueryClientProvider } from '@tanstack/react-query' +import { fireEvent, render, screen, waitFor, within } from '@testing-library/react' +import { createMemoryRouter, RouterProvider, useLocation } from 'react-router' +import { afterEach, describe, expect, it, vi, type Mock } from 'vitest' +import { ApiProvider, createApiClient, createQueryClient, createVerifiedSession } from '../api/index.ts' +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 { HomePage } from './HomePage.tsx' + +afterEach(() => { + vi.unstubAllGlobals() +}) + +// minutesAgo is the stored form of the instant n minutes before now. +function minutesAgo(n: number): string { + return new Date(Date.now() - n * 60_000).toISOString() +} + +// UPDATES are a GET /api/v1/updates body, newest first: the week drafted +// five minutes ago, new Side quests three hours ago and the Supply run two +// days ago. +const UPDATES = { + updates: [ + { + type: 'plan_ready', + title: 'Your week is drafted', + body: 'Take a look at the Draft board.', + url: '/plan', + created_at: minutesAgo(5), + }, + { type: 'new_suggestions', title: '5 new Side quests', body: '', url: '/discover', created_at: minutesAgo(3 * 60) }, + { + type: 'list_ready', + title: 'Supply run ready', + body: 'Week 41 · Oct 5 – 11', + url: '/plan/shopping?week=2026-10-05', + created_at: minutesAgo(2 * 24 * 60), + }, + ], +} + +type Responder = (init: RequestInit | undefined) => Response | Promise + +// stubServer answers "METHOD path" requests from responders, with /me; any +// other request fails the test. Home is unread until the updates are marked +// seen, as the server answers. +function stubServer(responders: Record = {}) { + let seen = false + const all: Record = { + 'GET /api/v1/me': () => jsonResponse(200, ME), + [`GET ${HOME_PATH}`]: () => jsonResponse(200, { ...HOME, recentUpdates: { unread: !seen } }), + [`GET ${UPDATES_PATH}`]: () => jsonResponse(200, UPDATES), + [`POST ${UPDATES_SEEN_PATH}`]: () => { + seen = true + return new Response(null, { status: 204 }) + }, + ...responders, + } + const fetchMock = vi.fn((input, init) => { + const key = `${init?.method ?? 'GET'} ${String(input)}` + const respond = Object.hasOwn(all, key) ? all[key] : undefined + if (respond === undefined) throw new Error(`unexpected fetch: ${key}`) + return Promise.resolve(respond(init)) + }) + vi.stubGlobal('fetch', fetchMock) + return fetchMock +} + +// posts lists the bodies of the POST /updates/seen requests sent, each of +// which must carry the CSRF token. +function posts(fetchMock: Mock) { + return fetchMock.mock.calls + .filter(([input, init]) => init?.method === 'POST' && String(input) === UPDATES_SEEN_PATH) + .map(([, init]) => { + expect(new Headers(init?.headers).get('X-MealQuest-CSRF')).toBe(ME.csrfToken) + return String(init?.body) + }) +} + +function homeReads(fetchMock: Mock) { + return fetchMock.mock.calls.filter(([input, init]) => String(input) === HOME_PATH && (init?.method ?? 'GET') === 'GET') + .length +} + +// Where shows the route a row went to. +function Where() { + const location = useLocation() + return

{location.pathname + location.search}

+} + +// renderHome loads /me (for the CSRF token the POST needs) and renders Home. +async function renderHome() { + const api = createApiClient(fakeLocation('/'), createNetworkStatus(window), createVerifiedSession(localStorage)) + await api.loadMe() + const router = createMemoryRouter( + [ + { path: '/', element: }, + { path: '*', element: }, + ], + { initialEntries: ['/'] }, + ) + render( + + + + + , + ) +} + +// openSheet taps the unread bell and answers the open sheet. +async function openSheet() { + fireEvent.click(await screen.findByRole('button', { name: 'Recent updates, unread' })) + return screen.getByRole('dialog', { name: 'Recent updates' }) +} + +async function rows(sheet: HTMLElement) { + return within(await within(sheet).findByRole('list', { name: 'Recent updates' })).getAllByRole('listitem') +} + +describe('Recent updates sheet', () => { + it('lists the updates newest first with how long ago each came', async () => { + stubServer() + await renderHome() + const listed = await rows(await openSheet()) + + expect(listed).toHaveLength(3) + expect(listed[0]).toHaveTextContent('Your week is drafted') + expect(listed[0]).toHaveTextContent('Take a look at the Draft board.') + expect(listed[0]).toHaveTextContent('5 min ago') + expect(listed[1]).toHaveTextContent('5 new Side quests') + expect(listed[1]).toHaveTextContent('3 hr ago') + expect(listed[2]).toHaveTextContent('Supply run ready') + expect(listed[2]).toHaveTextContent('2 days ago') + expect(within(listed[0]!).getByText('5 min ago')).toHaveAttribute('dateTime', UPDATES.updates[0]!.created_at) + }) + + it('marks the updates seen through the newest, and the grape dot clears and stays cleared', async () => { + const fetchMock = stubServer() + await renderHome() + expect(await screen.findByTestId('unread-dot')).toBeInTheDocument() + await openSheet() + + await waitFor(() => expect(posts(fetchMock)).toEqual([JSON.stringify({ through: UPDATES.updates[0]!.created_at })])) + expect(screen.queryByTestId('unread-dot')).not.toBeInTheDocument() + // Home is read again once the mark is saved, and the server says + // nothing is unread: the dot does not come back. + await waitFor(() => expect(homeReads(fetchMock)).toBe(2)) + fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Escape' }) + expect(await screen.findByRole('button', { name: 'Recent updates' })).toBeInTheDocument() + expect(screen.queryByTestId('unread-dot')).not.toBeInTheDocument() + }) + + it.each([ + [0, '/plan'], + [1, '/discover'], + [2, '/plan/shopping?week=2026-10-05'], + ])('row %i goes to %s and closes the sheet', async (i, to) => { + stubServer() + await renderHome() + const listed = await rows(await openSheet()) + + fireEvent.click(within(listed[i]!).getByRole('button')) + expect(await screen.findByTestId('where')).toHaveTextContent(to) + expect(screen.queryByRole('dialog')).not.toBeInTheDocument() + }) + + it('says there are no updates yet, and marks nothing seen', async () => { + const fetchMock = stubServer({ [`GET ${UPDATES_PATH}`]: () => jsonResponse(200, { updates: [] }) }) + await renderHome() + const sheet = await openSheet() + + expect(await within(sheet).findByText(/No updates yet/)).toBeInTheDocument() + expect(within(sheet).queryByRole('list')).not.toBeInTheDocument() + expect(posts(fetchMock)).toEqual([]) + }) + + it('puts the dot back and says so when marking the updates seen fails', async () => { + let refuse!: (response: Response) => void + const fetchMock = stubServer({ + [`POST ${UPDATES_SEEN_PATH}`]: () => new Promise((resolve) => (refuse = resolve)), + }) + await renderHome() + const sheet = await openSheet() + + await waitFor(() => expect(posts(fetchMock)).toHaveLength(1)) + // Cleared at once, while the mark is in flight. + expect(screen.queryByTestId('unread-dot')).not.toBeInTheDocument() + + refuse(errorResponse(500, 'internal')) + expect(await within(sheet).findByRole('alert')).toHaveTextContent( + "Couldn't mark your updates as seen. internal message", + ) + expect(screen.getByTestId('unread-dot')).toBeInTheDocument() + // The updates are still listed. + expect(within(sheet).getByRole('list', { name: 'Recent updates' })).toBeInTheDocument() + }) + + it('reports updates it cannot load, with Retry', async () => { + let fail = true + stubServer({ [`GET ${UPDATES_PATH}`]: () => (fail ? errorResponse(500, 'internal') : jsonResponse(200, UPDATES)) }) + await renderHome() + const sheet = await openSheet() + + expect(await within(sheet).findByRole('alert')).toHaveTextContent("Couldn't load your updates. internal message") + fail = false + fireEvent.click(within(sheet).getByRole('button', { name: 'Retry' })) + expect(await within(sheet).findByRole('list', { name: 'Recent updates' })).toBeInTheDocument() + }) + + it('reports an update that would open another site rather than listing it', async () => { + const fetchMock = stubServer({ + [`GET ${UPDATES_PATH}`]: () => jsonResponse(200, { updates: [{ ...UPDATES.updates[0], url: '//evil.example/plan' }] }), + }) + await renderHome() + const sheet = await openSheet() + + expect(await within(sheet).findByRole('alert')).toHaveTextContent( + 'update url "//evil.example/plan" is not a path on this app', + ) + expect(within(sheet).queryByRole('list')).not.toBeInTheDocument() + expect(posts(fetchMock)).toEqual([]) + }) +}) diff --git a/web/src/home/UpdatesSheet.tsx b/web/src/home/UpdatesSheet.tsx new file mode 100644 index 0000000..cb31bb9 --- /dev/null +++ b/web/src/home/UpdatesSheet.tsx @@ -0,0 +1,114 @@ +import { CalendarDays, ChefHat, Compass, Repeat, ShoppingCart, type LucideIcon } from 'lucide-react' +import { useEffect, useId, useState } from 'react' +import { useNavigate } from 'react-router' +import { ago } from '../plan/PlanPage.tsx' +import { ErrorBanner } from '../ui/ErrorBanner.tsx' +import { Icon } from '../ui/Icon.tsx' +import { ListRow } from '../ui/ListRow.tsx' +import { Sheet } from '../ui/Sheet.tsx' +import { Skeleton } from '../ui/Skeleton.tsx' +import { useMarkUpdatesSeen, useUpdates, type Update, type UpdateType } from './api.ts' +import styles from './UpdatesSheet.module.css' + +const ICONS: Record = { + plan_ready: CalendarDays, + list_ready: ShoppingCart, + day_of_reminder: ChefHat, + new_suggestions: Compass, + rotation_review: Repeat, +} + +// UpdateRow is one update: its icon, title and body, and how long ago it +// came. A tap opens its url. +function UpdateRow({ update, now, onOpen }: { update: Update; now: Date; onOpen: (url: string) => void }) { + return ( +
  • + } + title={update.title} + meta={update.body === '' ? undefined : update.body} + trailing={ + + } + onClick={() => onOpen(update.url)} + /> +
    • + ) +} + +// Updates lists the Recent updates, newest first, and marks them seen +// through the newest once they are shown, so the bell's dot clears. +function Updates({ onOpen }: { onOpen: (url: string) => void }) { + const updates = useUpdates() + const markSeen = useMarkUpdatesSeen() + const { mutate } = markSeen + const newest = updates.data?.[0]?.created_at + // The instant the relative times count from, fixed while the sheet is + // open. + const [now] = useState(() => new Date()) + + useEffect(() => { + if (newest !== undefined) mutate(newest) + }, [newest, mutate]) + + if (updates.isError) { + return ( + void updates.refetch()}>Couldn't load your updates. {updates.error.message} + ) + } + if (updates.isPending) { + return ( +
    + {Array.from({ length: 3 }, (_, i) => ( + + ))} +
    + ) + } + if (updates.data.length === 0) { + return

    No updates yet. Plans, Supply runs and new Side quests show up here.

    + } + return ( + <> + {markSeen.isError && newest !== undefined && ( + mutate(newest)}> + Couldn't mark your updates as seen. {markSeen.error.message} + + )} +
      + {updates.data.map((update, i) => ( + + ))} +
    + + ) +} + +export type UpdatesSheetProps = { + open: boolean + onClose: () => void +} + +// UpdatesSheet is the Recent updates sheet the Home bell opens: the last +// ten notification events with how long ago each came, each opening the +// screen it is about. Opening it marks them seen. +export function UpdatesSheet({ open, onClose }: UpdatesSheetProps) { + const titleId = useId() + const navigate = useNavigate() + const openUpdate = (url: string) => { + onClose() + void navigate(url) + } + return ( + +
    +

    + Recent updates +

    + +
    +     + ) +} diff --git a/web/src/home/api.ts b/web/src/home/api.ts index fb836cc..c9936ac 100644 --- a/web/src/home/api.ts +++ b/web/src/home/api.ts @@ -1,4 +1,4 @@ -import { useQuery } from '@tanstack/react-query' +import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query' import { useApi } from '../api/index.ts' import { isRecord } from '../api/errors.ts' import { oneOf, photoId } from '../dinners/api.ts' @@ -6,6 +6,9 @@ import { weekLabel } from '../plan/week.ts' export const HOME_PATH = '/api/v1/home' export const HOME_KEY = ['home'] as const +export const UPDATES_PATH = '/api/v1/updates' +export const UPDATES_SEEN_PATH = '/api/v1/updates/seen' +export const UPDATES_KEY = ['updates'] as const export const ORIGINS = ['regular', 'side_quest'] as const export type Origin = (typeof ORIGINS)[number] @@ -95,3 +98,96 @@ export function useHome() { queryFn: async () => parseHome(`GET ${HOME_PATH}`, await apiFetch(HOME_PATH)), }) } + +export const UPDATE_TYPES = ['plan_ready', 'list_ready', 'day_of_reminder', 'new_suggestions', 'rotation_review'] as const +export type UpdateType = (typeof UPDATE_TYPES)[number] + +// Update is one of the Recent updates: a notification event, whatever +// channels it went to. url is the same-origin path it opens; created_at is +// when it was stored, the UTC RFC 3339 instant POST /api/v1/updates/seen +// 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 { + if ( + !isRecord(body) || + typeof body.title !== 'string' || + typeof body.body !== 'string' || + typeof body.url !== 'string' || + typeof body.created_at !== 'string' + ) { + 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`) + if (Number.isNaN(Date.parse(body.created_at))) { + throw new Error(`${where}: update created_at "${body.created_at}" is not a time`) + } + return { + type: oneOf(where, 'type', UPDATE_TYPES, body.type), + title: body.title, + body: body.body, + url: body.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[] { + if (!isRecord(body) || !Array.isArray(body.updates)) throw new Error(`${where}: response is not the updates`) + return body.updates.map((update) => parseUpdate(where, update)) +} + +// useUpdates is the Recent updates (GET /api/v1/updates): the last ten +// notification events, newest first. +export function useUpdates() { + const { apiFetch } = useApi() + return useQuery({ + queryKey: UPDATES_KEY, + queryFn: async () => parseUpdates(`GET ${UPDATES_PATH}`, await apiFetch(UPDATES_PATH)), + }) +} + +const MARK_SEEN_KEY = ['updates', 'seen'] as const + +// useMarkUpdatesSeen records that the household has seen its updates +// through the created_at of the newest one shown (POST +// /api/v1/updates/seen). The bell's unread dot clears at once, and a Home +// read in flight is stopped so it cannot bring it back; if the server +// refuses, the dot is put back as it was before the first of the marks in +// flight. Once the last mark in flight settles Home is read again, so the +// dot shows only for an event stored since. +export function useMarkUpdatesSeen() { + const { apiFetch } = useApi() + const queryClient = useQueryClient() + // alone says whether this is the only mark in flight; each counts itself + // until it has settled. + const alone = () => queryClient.isMutating({ mutationKey: MARK_SEEN_KEY }) === 1 + return useMutation({ + mutationKey: MARK_SEEN_KEY, + mutationFn: async (through: string) => { + await apiFetch(UPDATES_SEEN_PATH, { method: 'POST', body: { through } }) + }, + onMutate: async () => { + // A mark sent while another is in flight would snapshot that one's + // optimistic answer, so only the first snapshots. + const unread = alone() ? queryClient.getQueryData(HOME_KEY)?.unread : undefined + await queryClient.cancelQueries({ queryKey: HOME_KEY }) + queryClient.setQueryData(HOME_KEY, (home) => home && { ...home, unread: false }) + return { unread } + }, + onError: (_error, _through, context) => { + if (context?.unread === undefined || !alone()) return + const { unread } = context + queryClient.setQueryData(HOME_KEY, (home) => home && { ...home, unread }) + }, + onSettled: () => { + if (alone()) void queryClient.invalidateQueries({ queryKey: HOME_KEY }) + }, + }) +} -- 2.55.0 From 9978e200e662c1f6b0deb1a69733a481767d5c85 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Sat, 10 Oct 2026 12:35:58 -0400 Subject: [PATCH 2/4] Read Recent updates afresh per opening and guard their URLs by origin 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 --- internal/home/home_test.go | 23 +++++++++++-- web/src/home/UpdatesSheet.test.tsx | 54 ++++++++++++++++++++++++++++-- web/src/home/api.test.ts | 39 +++++++++++++++++++++ web/src/home/api.ts | 32 ++++++++++-------- 4 files changed, 128 insertions(+), 20 deletions(-) create mode 100644 web/src/home/api.test.ts diff --git a/internal/home/home_test.go b/internal/home/home_test.go index e0235ca..fb53d7e 100644 --- a/internal/home/home_test.go +++ b/internal/home/home_test.go @@ -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) { diff --git a/web/src/home/UpdatesSheet.test.tsx b/web/src/home/UpdatesSheet.test.tsx index b5de4f2..e825ca9 100644 --- a/web/src/home/UpdatesSheet.test.tsx +++ b/web/src/home/UpdatesSheet.test.tsx @@ -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

    {location.pathname + location.search}

    } -// 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( - + , ) + 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'], diff --git a/web/src/home/api.test.ts b/web/src/home/api.test.ts new file mode 100644 index 0000000..d4321c7 --- /dev/null +++ b/web/src/home/api.test.ts @@ -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`, + ) + }) +}) diff --git a/web/src/home/api.ts b/web/src/home/api.ts index c9936ac..19f60a1 100644 --- a/web/src/home/api.ts +++ b/web/src/home/api.ts @@ -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(UPDATES_PATH)), + queryFn: async () => + parseUpdates(`GET ${UPDATES_PATH}`, await apiFetch(UPDATES_PATH), window.location.origin), + gcTime: 0, }) } -- 2.55.0 From 6956f6f049229173ae2ba0ad03a2ea495d22b627 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Sat, 10 Oct 2026 14:17:07 -0400 Subject: [PATCH 3/4] Read Recent updates afresh on every opening before marking them seen gcTime 0 only schedules removal of the cached list, so an immediate reopen within the query client's staleTime reused it and marked the old newest time seen. useUpdates now refetches on every mount, and the sheet shows and marks seen only a list read after it opened. Closes MQ-176-01. Co-Authored-By: Claude Opus 5.5 --- web/src/home/UpdatesSheet.test.tsx | 56 ++++++++++++++++++++++++++++++ web/src/home/UpdatesSheet.tsx | 7 ++-- web/src/home/api.ts | 8 ++--- 3 files changed, 65 insertions(+), 6 deletions(-) diff --git a/web/src/home/UpdatesSheet.test.tsx b/web/src/home/UpdatesSheet.test.tsx index e825ca9..cf06a14 100644 --- a/web/src/home/UpdatesSheet.test.tsx +++ b/web/src/home/UpdatesSheet.test.tsx @@ -84,6 +84,12 @@ function homeReads(fetchMock: Mock) { .length } +function updatesReads(fetchMock: Mock) { + return fetchMock.mock.calls.filter( + ([input, init]) => String(input) === UPDATES_PATH && (init?.method ?? 'GET') === 'GET', + ).length +} + // Where shows the route a row went to. function Where() { const location = useLocation() @@ -201,6 +207,56 @@ describe('Recent updates sheet', () => { expect(screen.queryByTestId('unread-dot')).not.toBeInTheDocument() }) + it('on an immediate reopen waits for that opening’s read, and marks the newer event seen, not the list held from before', async () => { + const stored = [...UPDATES.updates] + let reads = 0 + let answer: (() => void) | undefined + const fetchMock = stubServer({ + [`GET ${UPDATES_PATH}`]: () => { + const body = { updates: [...stored] } + reads++ + // Every reading after the first is held until the test answers it. + if (reads === 1) return jsonResponse(200, body) + return new Promise((resolve) => (answer = () => resolve(jsonResponse(200, body)))) + }, + }) + await renderHome() + await rows(await openSheet()) + await waitFor(() => expect(posts(fetchMock)).toHaveLength(1)) + + // Closed and opened again at once, before any cache-collection timer + // could run, with a new event stored in between. + fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Escape' }) + const reminder = { + type: 'day_of_reminder', + title: 'Tacos tonight', + body: 'Dinner at 6:00.', + url: '/plan', + created_at: new Date().toISOString(), + } + stored.unshift(reminder) + fireEvent.click(screen.getByRole('button', { name: /^Recent updates/ })) + const sheet = screen.getByRole('dialog', { name: 'Recent updates' }) + + // The list held from the first opening is neither shown nor marked + // seen while this opening's read is in flight. + await waitFor(() => expect(updatesReads(fetchMock)).toBe(2)) + expect(within(sheet).getByLabelText('Updates loading')).toBeInTheDocument() + expect(within(sheet).queryByRole('list')).not.toBeInTheDocument() + expect(posts(fetchMock)).toHaveLength(1) + + answer!() + const listed = await rows(sheet) + 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 }), + ]), + ) + }) + it.each([ [0, '/plan'], [1, '/discover'], diff --git a/web/src/home/UpdatesSheet.tsx b/web/src/home/UpdatesSheet.tsx index cb31bb9..2c000c2 100644 --- a/web/src/home/UpdatesSheet.tsx +++ b/web/src/home/UpdatesSheet.tsx @@ -44,7 +44,10 @@ function Updates({ onOpen }: { onOpen: (url: string) => void }) { const updates = useUpdates() const markSeen = useMarkUpdatesSeen() const { mutate } = markSeen - const newest = updates.data?.[0]?.created_at + // fresh says this opening's read has come back; until then a list still + // held from an earlier opening is neither shown nor marked seen. + const fresh = updates.isSuccess && updates.isFetchedAfterMount + const newest = fresh ? updates.data[0]?.created_at : undefined // The instant the relative times count from, fixed while the sheet is // open. const [now] = useState(() => new Date()) @@ -58,7 +61,7 @@ function Updates({ onOpen }: { onOpen: (url: string) => void }) { void updates.refetch()}>Couldn't load your updates. {updates.error.message} ) } - if (updates.isPending) { + if (!fresh) { return (
    {Array.from({ length: 3 }, (_, i) => ( diff --git a/web/src/home/api.ts b/web/src/home/api.ts index 19f60a1..5e6f2f9 100644 --- a/web/src/home/api.ts +++ b/web/src/home/api.ts @@ -144,16 +144,16 @@ export function parseUpdates(where: string, body: unknown, origin: string): Upda } // useUpdates is the Recent updates (GET /api/v1/updates): the last ten -// 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. +// notification events, newest first. Each opening of the sheet reads it +// afresh (refetchOnMount 'always'), whatever the query client still holds; +// the sheet shows, and marks seen, only the list that opening read. export function useUpdates() { const { apiFetch } = useApi() return useQuery({ queryKey: UPDATES_KEY, queryFn: async () => parseUpdates(`GET ${UPDATES_PATH}`, await apiFetch(UPDATES_PATH), window.location.origin), - gcTime: 0, + refetchOnMount: 'always', }) } -- 2.55.0 From d9b63bd08c7882f37ffa9fbd53d2b0aff45254d3 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Sat, 10 Oct 2026 15:07:37 -0400 Subject: [PATCH 4/4] Cancel an in-flight Recent updates read when the sheet closes The updates query now passes TanStack Query's abort signal to apiFetch, so closing the sheet while its read is pending aborts that GET. An immediate reopen then sends its own read instead of adopting the one sent before a newer event was stored, and marks seen only what that read returned. Closes MQ-176-01 (task 96e2dc3f-1f8a-4114-bcb5-e0f93c349165). Co-Authored-By: Claude Opus 5.5 --- web/src/home/UpdatesSheet.test.tsx | 51 ++++++++++++++++++++++++++++++ web/src/home/api.ts | 8 +++-- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/web/src/home/UpdatesSheet.test.tsx b/web/src/home/UpdatesSheet.test.tsx index cf06a14..aba8a02 100644 --- a/web/src/home/UpdatesSheet.test.tsx +++ b/web/src/home/UpdatesSheet.test.tsx @@ -257,6 +257,57 @@ describe('Recent updates sheet', () => { ) }) + it('closing the sheet while its first read is in flight cancels that read, and a reopen reads afresh', async () => { + const stored = [...UPDATES.updates] + let firstAborted = false + let answerFirst: (() => void) | undefined + let reads = 0 + const fetchMock = stubServer({ + [`GET ${UPDATES_PATH}`]: (init) => { + const body = { updates: [...stored] } + reads++ + if (reads > 1) return jsonResponse(200, body) + // The first reading is held open, and fails as fetch does if it is + // aborted. + return new Promise((resolve, reject) => { + answerFirst = () => resolve(jsonResponse(200, body)) + init?.signal?.addEventListener('abort', () => { + firstAborted = true + reject(init.signal!.reason as Error) + }) + }) + }, + }) + await renderHome() + await openSheet() + await waitFor(() => expect(updatesReads(fetchMock)).toBe(1)) + + // Closed before the first read answers, a new event stored, and opened + // again at once. + fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Escape' }) + const reminder = { + type: 'day_of_reminder', + title: 'Tacos tonight', + body: 'Dinner at 6:00.', + url: '/plan', + created_at: new Date().toISOString(), + } + stored.unshift(reminder) + fireEvent.click(screen.getByRole('button', { name: /^Recent updates/ })) + const sheet = screen.getByRole('dialog', { name: 'Recent updates' }) + + // The first read was cancelled, and the reopen sent its own. + await waitFor(() => expect(updatesReads(fetchMock)).toBe(2)) + expect(firstAborted).toBe(true) + // Answering the cancelled read changes nothing. + answerFirst!() + + const listed = await rows(sheet) + expect(listed).toHaveLength(4) + expect(listed[0]).toHaveTextContent('Tacos tonight') + await waitFor(() => expect(posts(fetchMock)).toEqual([JSON.stringify({ through: reminder.created_at })])) + }) + it.each([ [0, '/plan'], [1, '/discover'], diff --git a/web/src/home/api.ts b/web/src/home/api.ts index 5e6f2f9..71625f5 100644 --- a/web/src/home/api.ts +++ b/web/src/home/api.ts @@ -146,13 +146,15 @@ export function parseUpdates(where: string, body: unknown, origin: string): Upda // useUpdates is the Recent updates (GET /api/v1/updates): the last ten // notification events, newest first. Each opening of the sheet reads it // afresh (refetchOnMount 'always'), whatever the query client still holds; -// the sheet shows, and marks seen, only the list that opening read. +// the sheet shows, and marks seen, only the list that opening read. Closing +// the sheet aborts a read still in flight, so a reopen sends its own rather +// than adopt one sent before it. export function useUpdates() { const { apiFetch } = useApi() return useQuery({ queryKey: UPDATES_KEY, - queryFn: async () => - parseUpdates(`GET ${UPDATES_PATH}`, await apiFetch(UPDATES_PATH), window.location.origin), + queryFn: async ({ signal }) => + parseUpdates(`GET ${UPDATES_PATH}`, await apiFetch(UPDATES_PATH, { signal }), window.location.origin), refetchOnMount: 'always', }) } -- 2.55.0