Repository navigation
web: social activity feed (pings + sightings) - #75
Conversation
Evolves /notifications into a unified, chronological activity feed (pings + favorited-food sightings), with "ping a friend" as a first-class action there instead of buried in /friends' accepted- friends list. /friends stays scoped to search/request/accept. - No schema changes: `pings` has no read_at column, so ping unread state is derived from a device-local "last viewed the feed" marker (web/src/lib/feedLastSeen.ts, localStorage, same shape as preferences.ts) rather than a server flag. Sightings keep their existing server-persisted read_at. - Closes #62: sighting mark-as-read is now a real <button> (keyboard- reachable via focus+Enter), alongside the pre-existing onmouseenter for mouse users, which is unchanged. - Signed-out: feed shell + sign-in value prop pointing at the header's existing "Sign in with Google" control (no duplicate button). - Carry-over from PR #69's review (issue #66 comment): replaced home-dashboard.spec.ts's `**/api/**`-only residency guard with a page.on("request") listener flagging any same-origin /api/ path or disallowed cross-origin request (fires for no-cors fetches too, which the old glob-based route() interception missed). Explicit allow-list for the Google Fonts cross-origin request app.css (#42) already makes on every page. Proven red-first via mutation (temporary no-cors fetch in +page.svelte, reverted -- diff to that file is empty). - Fixed friends-notifications.spec.ts's two known nits: cookie name now derived from PUBLIC_SUPABASE_URL instead of hardcoded "your-project-ref", and the unawaited-PATCH race (both the friendships Accept instance and its unfixed twin on the notifications toggle) replaced with expect.poll. Red-green TDD throughout; see issue #66 for the full red/green log.
…race - friends-notifications.spec.ts: #62's keyboard test now asserts toBeFocused() before pressing Enter (proves the button is actually in the tab order, matching #62's "keyboard-only walkthrough" wording, not just that it's clickable-when-focused). The alert toggle's badge locator is now scoped to its own section instead of ".badge".first(), which would have silently started matching the wrong badge if the feed's own "New" badges ever preceded it in DOM order. - notifications/+page.svelte: refresh() now resolves friends before assigning pings, so a ping can no longer render with the "A friend" fallback for one tick before its sender's name loads. Full suite (45/45), check, and build all still green after these.
…read coverage (#75 review) F1 (blocking): refresh().then(() => saveFeedLastSeen(...)) ran unconditionally, even when refresh() early-returned with no session -- a signed-out visit silently stamped the device-local unread watermark. Now only stamps when a session exists, and stamps from the newest rendered item's own created_at (Postgres clock) instead of the browser clock. F2 (blocking): feedLastSeen.ts had no test that would fail if isUnread()'s ping branch were hardcoded to `return true`, or if the saveFeedLastSeen() call were deleted outright. Added two tests to friends-notifications.spec.ts: a seeded watermark correctly splits an older/newer ping pair into read/unread, and a signed-out visit leaves the watermark key unset. Both were shown red against pristine pre-fix code, and against each of the two named mutations independently (verified locally, reverted after). Nits: fixed the sort comparator (previously returned -1 for both orderings of two equal timestamps, an invalid comparator); sendPing() now checks the insert's error and shows a failure state instead of an unconditional "Ping sent" (also made pingSent/pingError mutually exclusive so a fast retry can't show both at once). Added a red-first test for the failure state. Optional: feedLastSeen's localStorage key is now scoped by user id, so a second account on the same browser doesn't inherit the first account's watermark.
|
Pushed a follow-up commit fixing the two blocking findings + nits from this PR's review. Note on process: at the time I picked this up, PR #75 had zero GitHub review comments and zero issue comments ( F1 (blocking) — fixed. F2 (blocking) — fixed. Added two tests to Nits — both fixed. The feed sort comparator returned Optional — taken. Verification: Not merging — leaving for review per dispatch instructions. |
Closes #66. Closes #62. Part of epic #63.
What changed
Evolves
/notificationsinto a unified, chronological social activity feed(pings + favorited-food sightings), matching the epic's design decision
("social is a feed, not a settings page"). "Ping a friend" becomes a
first-class action on the feed instead of being buried in
/friends'accepted-friends list;
/friendsstays scoped to search/request/accept.web/src/routes/notifications/+page.svelte— rewritten: keeps theexisting favorited-dish alert toggle, adds a "Ping a friend" composer
(friend picker + hall + optional message, sourced from accepted
friendships), and renders
pings(as receiver) +food_sightingsmerged into one
<ul aria-label="Activity feed">, newest first, eachitem badged "New" when unread. Realtime subscription for new pings
moved here from
/friends.web/src/routes/friends/+page.svelte— ping composer and "Pingsyou've received" inbox removed (moved to the feed); accepted friends
now render as a plain
Friendbadge + name, with a pointer to theNotifications feed for pinging. Search/request/accept unchanged.
web/src/lib/feedLastSeen.ts— new.pingshas noread_atcolumnand this issue is explicitly no-schema-changes, so ping unread state
is derived from a device-local "last viewed the feed" timestamp
(localStorage, same shape as the existing
preferences.ts) insteadof a server flag. Sightings keep their existing server-persisted
read_at.Closes #62
Sighting mark-as-read is now a real
<button>Mark as read</button>(keyboard-reachable: focus + Enter), shown whenever a sighting is
unread, alongside the pre-existing
onmouseenterfor mouse users,which is unchanged (per #62's "existing mouse behavior unchanged"
acceptance). Mutation-verified: temporarily removed the button, the
red-first spec (
friends-notifications.spec.ts— "unread sightingmark-as-read is reachable by keyboard...") failed as expected; restored,
green again.
Carry-over from PR #69's review (required by issue #66's comment)
home-dashboard.spec.ts's residency guard only intercepted**/api/**, so a direct cross-origin fetch (e.g. to*.supabase.co)passed it green even in
no-corsmode. Replaced with apage.on("request")listener flagging any same-origin/api/path ordisallowed cross-origin request — this fires for
no-corsrequests too,since it's request-level, not route-interception-level.
Explicit allow-list for
fonts.googleapis.com/fonts.gstatic.com:app.css(#42)@imports Google Fonts, so the home page legitimatelymakes a cross-origin request already — a blanket cross-origin guard
would false-positive on every run without this, not just a mutated one.
Proven red-first via mutation: temporarily added a
fetch(..., { mode: "no-cors" })call to+page.svelte'sonMount,confirmed the new guard fails (
unexpected /api/ or cross-origin request(s): https://your-project-ref.supabase.co/rest/v1/whatever).Then, with that same mutation still in place, swapped in the old
**/api/**-only guard (checked out from the pre-#66 commit) andre-ran it — it passed (
1 passed), confirming the old guard reallywould have missed this leak, not just that the new one catches it.
Reverted both the mutation and the swapped-in old spec afterward;
git diffon+page.svelteis empty — not owned by this PR, onlyused to prove the guard red-first.
Nits fixed in friends-notifications.spec.ts (per issue #66)
sb-<project-ref>-auth-token) is now derived fromPUBLIC_SUPABASE_URL(read fromweb/.env, falling back to.env.example) instead of hardcoded"your-project-ref"— matches@supabase/supabase-js's own default storage-key derivation.requests.find(...)read synchronouslyright after
.click(), before the async handler's request hasnecessarily round-tripped through the mock router) — fixed with
expect.pollat both the previously-flagged instance (friendshipsAccept) and its unfixed twin (notifications toggle PATCH).
Tests
web/e2e/friends-notifications.spec.ts— 9 tests total (up from 6before this PR): friends signed-out, friendships accept-flow PATCH,
notifications toggle PATCH, and read-sighting-stays-de-emphasized are
carried over (matched pre-existing behavior in the red run below);
signed-out value-prop with no duplicate sign-in button,
accepted-friends-have-no-ping-controls, feed chronological ordering +
unread badges across both item types, ping-composer POST body
assertion, and keyboard mark-as-read (closes #62) are new.
web/e2e/home-dashboard.spec.ts— residency guard replaced (seeabove); the other 3 tests in this file are untouched.
Red evidence
Ran the full new spec against the pre-#66 route files (
git stashonjust the two
+page.sveltefiles): 5 failed (all the new/changedbehavior), 4 passed (pre-existing behavior, confirming no regression
in the rewrite of the spec's mechanics). Full log posted on issue #66.
Green evidence
PORT=4307 npx playwright test— 45/45 passed (full suite).pnpm --filter web check—COMPLETED 399 FILES 0 ERRORS 0 WARNINGS.pnpm --filter web build— clean (adapter-auto's "could not detect aproduction environment" notice is pre-existing/benign).
Data residency
No schema changes. Pings/sightings continue to require a signed-in
session (server-backed by definition, per CLAUDE.md's residency
table). The new
feedLastSeenmarker is device-local UI state, nothealth/food data. Signed-out anonymous path (
/,/today, halls,menus, ranking, filters) is untouched by this PR.
Follow-up commit (self-review)
A second commit tightens three things found on re-check before reporting done:
toBeFocused()before pressing Enter,proving the button is actually in the tab order (matches web a11y: notifications markRead is mouse-only (onmouseenter) #62's
"keyboard-only walkthrough" wording), not just that it activates once
focused.
<section>instead of
.badge.first(), which would have silently matched thewrong badge if the feed's own "New" badges ever preceded it in DOM
order.
refresh()innotifications/+page.sveltenow resolvesfriendsbefore assigning
pings, so a ping can no longer render with the"A friend" fallback for one tick before its sender's name loads.
Full suite (45/45), check, and build all re-verified green after this
commit.
Follow-up commit: two blocking review findings + nits
The original author agent was gone by the time this PR got reviewed; a
new agent (dispatched separately, not the original author) picked up
the two blocking findings and nits below. Note on sourcing: at the
time of this fix, PR #75 itself has zero review comments and zero
issue comments on GitHub (
reviews: [],review_comments: 0) — thefindings summarized below were relayed by the dispatching agent's own
text, not lifted verbatim from a GitHub review thread on this PR. Said
so explicitly here so this isn't mistaken for a review artifact that
exists in the repo.
F1 (blocking).
notifications/+page.svelte'sonMounthad:refresh()early-returns with no session, but the.then()still ranunconditionally — a signed-out visit that rendered zero feed items
silently stamped the watermark, marking every older ping as seen
before the user ever signed in. Fixed to only stamp when a session
exists, and to stamp from the newest rendered item's own
created_at(server/Postgres clock) instead ofnew Date()(browserclock) — removes a class of drift where a just-inserted ping could
look already-seen if the two clocks disagreed.
One documented side effect of also taking the optional item below:
because the watermark key is now scoped by user id
(
udine-feed-last-seen:<uuid>instead of a single globaludine-feed-last-seen), an already-signed-in user's very first visitafter this ships reads an empty watermark under the new key and sees
every still-unread ping as "New" once, even ones they'd already seen
under the old global key. The old global key is simply never read
again (not actively migrated/cleaned up) — acceptable pre-release,
flagging so it doesn't read as an oversight.
Also intentional: a signed-in user whose feed has zero items gets no
stamp at all (
newestCreatedAtguard) — nothing to mark as seen, sothis is a no-op rather than a bug.
F2 (blocking).
feedLastSeen.tshad no test that would fail ifisUnread()'s ping branch were hardcoded toreturn true, or if thesaveFeedLastSeen()call were deleted outright — both mutations stayedgreen under the pre-existing suite. Added two tests to
friends-notifications.spec.ts:a newer ping stays badged" — seeds the scoped localStorage key via
page.addInitScript, mocks an older + a newer ping, asserts only thenewer one is badged "New". Also asserts the app itself advances the
stored watermark to the newest item's
created_atafter the visit —this second assertion is what catches the "delete the save call"
mutation (the seed-and-check half alone doesn't, since it never
depends on the app's own save happening).
F1's direct regression test.
Both were run red-first against the pristine pre-fix commit
(
git stashon justfeedLastSeen.ts+notifications/+page.svelte,new spec code kept): both failed as expected. Then, with the real fix
restored, each of the two named mutations was reproduced independently
(
isUnread's ping branch hardcoded toreturn true; thesaveFeedLastSeen()call deleted fromonMount) and each was shown toturn exactly one of the two new assertions red, then reverted. Full
transcript of this red/green/red/green cycle is in the branch history
via local test runs (not committed — the mutations were never
committed, only run locally and reverted).
Nit 1. The feed's sort comparator,
(a, b) => (a.createdAt < b.createdAt ? 1 : -1), returned-1for both orderings of two equal-timestampitems (
cmp(a,b)andcmp(b,a)both said "I go first") — an invalidcomparator. Replaced with
(a, b) => b.createdAt.localeCompare(a.createdAt).Nit 2.
sendPing()setpingSent = trueunconditionally, withoutchecking the insert's result — an RLS rejection (no longer friends,
blocked, etc.) still showed "Ping sent". Now checks
{ error }fromthe insert and shows a
role="alert"failure message instead; alsomade
pingSent/pingErrormutually exclusive (a fast retry thatsucceeds while an earlier failure's 3s badge is still showing no longer
renders both messages at once — caught in self-review before this went
out, not part of the original finding). Added a red-first test
mocking a 403 RLS-style PostgREST error response and asserting no
"Ping sent" text appears.
Optional (taken).
feedLastSeen.ts's localStorage key is nowscoped by user id (see the F1 side-effect note above) so a second
account signing in on the same browser doesn't inherit the first
account's watermark and silently mark its own unread pings as seen.
Red/green evidence for this commit
friends-notifications.spec.tsnew/changed tests run in isolation(
-g "watermark",-g "RLS rejection") red against pristine pre-fixcode and against each of the three mutations above (isUnread
hardcoded, save call deleted, error-check removed) — see per-finding
notes above.
CI=1 PORT=4314 npx playwright test— 53/53 passed (full suite;up from 45 because of onboarding/first-run: anonymous-first value prop, web + mobile #74's onboarding suite merged in from
main,plus the 3 new tests this commit adds — 12 tests now in
friends-notifications.spec.ts, not the 9 previously reported here).pnpm --filter web check—COMPLETED 400 FILES 0 ERRORS 0 WARNINGS.pnpm --filter web build— clean (same pre-existing adapter-autonotice as before).
origin/maininto this branch first (onboarding/first-run: anonymous-first value prop, web + mobile #74 onboarding + Add README #77README landed since this PR opened) — no conflicts, full suite
re-run after the merge.
Not merging
Per dispatch instructions — leaving for review.