From faeca92f30eef658dbd1346e4228db7adce7ced6 Mon Sep 17 00:00:00 2001 From: Megan Schott Date: Fri, 3 Jul 2026 07:54:47 -0500 Subject: [PATCH] =?UTF-8?q?feat(web+poller):=20self-serve=20"get=20alerts?= =?UTF-8?q?=20for=20this=20trip"=20toggle=20on=20/trips=20=E2=80=94=20PR?= =?UTF-8?q?=202/2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Family members can now opt into a trip's alerts from the dashboard, no Claude needed: - /trips gets a per-trip toggle (client component + server action behind the same signed-in + family-allowlist gates). The member id written is ALWAYS the session sub; opt-IN is guarded on the member's profile having a Pushover key (clear error pointing at /me instead of a silent no-op); applies to the trip's un-recorded days. - dynamodb-writes.setPlanAlertSubscription: the web's ONLY write into the shared trip partition — deliberately narrow (UpdateItem on exactly one attribute via ATOMIC set ADD/DELETE, so it can't race with or clobber MCP planner edits; boundary documented in the module). IAM already covers it (LeadingKeys USER#*), no CDK change. - Reader surfaces alert_subscribers per day (SS → sorted array; a JS Set can't cross the Server Component prop boundary). - Poller: dedupe dispatch by Pushover KEY (alert_routing. dedupe_resolved_by_key + weather-loop equivalent). The owner is an implicit recipient under "megan" and can now ALSO opt in under their sub via the toggle — same key must mean one push, not two. Applied to all three resolver dispatch sites + the weather fanout. Tests: atomic ADD/DELETE command shape, reader SS normalization, dedupe (first-id-wins, missing-key drop). Co-Authored-By: Claude Opus 4.8 --- infra/lambda/poller/alert_routing.py | 30 +++++++++ infra/lambda/poller/index.py | 48 +++++++------- .../lambda/poller/tests/test_alert_routing.py | 25 ++++++++ web/src/app/trips/TripAlertToggle.tsx | 60 +++++++++++++++++ web/src/app/trips/actions.ts | 64 +++++++++++++++++++ web/src/app/trips/page.tsx | 39 ++++++++--- web/src/lib/dynamodb-writes.test.ts | 31 +++++++++ web/src/lib/dynamodb-writes.ts | 46 +++++++++++++ web/src/lib/dynamodb.test.ts | 20 ++++++ web/src/lib/dynamodb.ts | 10 +++ 10 files changed, 341 insertions(+), 32 deletions(-) create mode 100644 web/src/app/trips/TripAlertToggle.tsx create mode 100644 web/src/app/trips/actions.ts diff --git a/infra/lambda/poller/alert_routing.py b/infra/lambda/poller/alert_routing.py index 5986181..03a806c 100644 --- a/infra/lambda/poller/alert_routing.py +++ b/infra/lambda/poller/alert_routing.py @@ -83,3 +83,33 @@ def resolve_alert_recipients( if existing is None or c.priority > existing.priority: best[c.user_id] = c return best + + +def dedupe_resolved_by_key( + resolved: dict[str, AlertCandidate], + get_user_key, +) -> list[tuple[str, str, AlertCandidate]]: + """Resolve each recipient's Pushover key and collapse duplicates. + + The same human can appear under two ids: the shared-partition owner + is an IMPLICIT plan-alert recipient ("megan") and may also opt in via + the /trips web toggle under their Cognito sub — both profiles carry + the same Pushover key, so without this one event would push twice. + First id wins (dict order — plan/owner candidates are appended before + favoriter ones at every call site). Ids with no resolvable key are + dropped with a log line. + + Returns [(user_id, user_key, candidate)] ready to dispatch. + """ + out: list[tuple[str, str, AlertCandidate]] = [] + seen_keys: set[str] = set() + for user_id, candidate in resolved.items(): + user_key = get_user_key(user_id) + if not user_key: + print(f"[poller] No pushover_user_key for user {user_id} — skipping") + continue + if user_key in seen_keys: + continue + seen_keys.add(user_key) + out.append((user_id, user_key, candidate)) + return out diff --git a/infra/lambda/poller/index.py b/infra/lambda/poller/index.py index 3a5e180..949677f 100644 --- a/infra/lambda/poller/index.py +++ b/infra/lambda/poller/index.py @@ -227,6 +227,7 @@ def filter_to_favoriters( f"Fanning out to {len(active_plans)} plan(s)." ) seen_users: set[str] = set() + seen_user_keys: set[str] = set() weather_alerts_sent = 0 for plan in active_plans: user_id = plan["user_id"] @@ -244,6 +245,12 @@ def filter_to_favoriters( user_key = get_user_key(user_id) if not user_key: continue + # ...and by Pushover key: the owner is implicit AND may + # be opted in under their sub via the web toggle — same + # key, one push (mirrors dedupe_resolved_by_key). + if user_key in seen_user_keys: + continue + seen_user_keys.add(user_key) # Mark BEFORE send: same pattern as the other # cooldowns. If the Pushover send fails the # cooldown still prevents re-fire spam within the @@ -464,14 +471,13 @@ def filter_to_favoriters( f"{len(plan_targets)} plan targets → " f"{len(resolved)} unique recipients" ) - for target_user, candidate in resolved.items(): - user_key = get_user_key(target_user) - if not user_key: - print( - f"[poller] No pushover_user_key for " - f"user {target_user} — skipping" - ) - continue + # Dedupe by Pushover key (owner implicit + web + # opt-in under their sub = same key, one push). + for target_user, user_key, candidate in ( + alert_routing.dedupe_resolved_by_key( + resolved, get_user_key + ) + ): if candidate.notifier_fn(user_key, **candidate.kwargs): total_alerts += 1 @@ -561,13 +567,12 @@ def filter_to_favoriters( f"{len(favoriters)} favoriters, {len(plan_targets)} plan " f"targets → {len(resolved)} unique recipients" ) - for target_user, candidate in resolved.items(): - user_key = get_user_key(target_user) - if not user_key: - print( - f"[poller] No pushover_user_key for user {target_user} — skipping" - ) - continue + # Dedupe by Pushover key: the owner is implicit AND may + # also be opted in under their sub via the web toggle — + # same key, one push (see alert_routing docstring). + for target_user, user_key, candidate in ( + alert_routing.dedupe_resolved_by_key(resolved, get_user_key) + ): if candidate.notifier_fn(user_key, **candidate.kwargs): total_alerts += 1 @@ -657,13 +662,12 @@ def filter_to_favoriters( f"{len(favoriters)} favoriters, {len(plan_targets)} plan " f"targets → {len(resolved)} unique recipients" ) - for target_user, candidate in resolved.items(): - user_key = get_user_key(target_user) - if not user_key: - print( - f"[poller] No pushover_user_key for user {target_user} — skipping" - ) - continue + # Dedupe by Pushover key: the owner is implicit AND may + # also be opted in under their sub via the web toggle — + # same key, one push (see alert_routing docstring). + for target_user, user_key, candidate in ( + alert_routing.dedupe_resolved_by_key(resolved, get_user_key) + ): if candidate.notifier_fn(user_key, **candidate.kwargs): total_alerts += 1 diff --git a/infra/lambda/poller/tests/test_alert_routing.py b/infra/lambda/poller/tests/test_alert_routing.py index c38bf94..34bb055 100644 --- a/infra/lambda/poller/tests/test_alert_routing.py +++ b/infra/lambda/poller/tests/test_alert_routing.py @@ -303,3 +303,28 @@ def test_plan_framing_beats_favorite_for_low_wait(self): assert set(resolved.keys()) == {"alice", "bob"} assert resolved["alice"].kwargs["plan_id"] == "PLAN#p1" # plan wins assert "plan_id" not in resolved["bob"].kwargs + + +class TestDedupeResolvedByKey: + """One human, two ids (implicit owner "megan" + web opt-in under + their sub) must get ONE push, not two — collapse by Pushover key.""" + + def test_same_key_collapses_first_id_wins(self): + resolved = resolve_alert_recipients([ + AlertCandidate("megan", PRIORITY_PLAN, _fake_notifier, {"a": 1}), + AlertCandidate("sub-megan-uuid", PRIORITY_PLAN, _fake_notifier, {"a": 2}), + AlertCandidate("sub-jim", PRIORITY_PLAN, _fake_notifier, {"a": 3}), + ]) + keys = {"megan": "pk-megan", "sub-megan-uuid": "pk-megan", "sub-jim": "pk-jim"} + out = alert_routing.dedupe_resolved_by_key(resolved, keys.get) + assert [(u, k) for u, k, _ in out] == [ + ("megan", "pk-megan"), ("sub-jim", "pk-jim"), + ] + + def test_missing_key_dropped(self): + resolved = resolve_alert_recipients([ + AlertCandidate("ghost", PRIORITY_PLAN, _fake_notifier, {}), + AlertCandidate("sub-jim", PRIORITY_FAVORITE, _fake_notifier, {}), + ]) + out = alert_routing.dedupe_resolved_by_key(resolved, {"sub-jim": "pk-jim"}.get) + assert [(u, k) for u, k, _ in out] == [("sub-jim", "pk-jim")] diff --git a/web/src/app/trips/TripAlertToggle.tsx b/web/src/app/trips/TripAlertToggle.tsx new file mode 100644 index 0000000..13b73a7 --- /dev/null +++ b/web/src/app/trips/TripAlertToggle.tsx @@ -0,0 +1,60 @@ +"use client"; + +/** + * "Get alerts for this trip" toggle — the /trips self-serve opt-in. + * + * Renders the signed-in member's subscription state for one trip (any + * un-recorded day subscribed = on) and flips it via the setTripAlerts + * server action, which writes the atomic set ADD/DELETE. The page passes + * `subscribed` from the freshly-read rows, so state survives reloads and + * reflects MCP-side changes too. + */ + +import { useState, useTransition } from "react"; + +import { setTripAlerts } from "./actions"; + +export default function TripAlertToggle({ + planIds, + subscribed, +}: { + planIds: string[]; + subscribed: boolean; +}) { + const [pending, startTransition] = useTransition(); + const [error, setError] = useState(null); + + const flip = () => { + setError(null); + startTransition(async () => { + const res = await setTripAlerts(planIds, !subscribed); + if (!res.ok) setError(res.error ?? "Couldn't update."); + // On success the action revalidates /trips; the re-render brings + // the new `subscribed` down from the server rows. + }); + }; + + return ( +
+ + {error &&

{error}

} +
+ ); +} diff --git a/web/src/app/trips/actions.ts b/web/src/app/trips/actions.ts new file mode 100644 index 0000000..1db5ff6 --- /dev/null +++ b/web/src/app/trips/actions.ts @@ -0,0 +1,64 @@ +"use server"; + +/** + * Server actions for /trips — the "get alerts for this trip" toggle. + * + * Same gates as the page: signed in + family allowlist. The member id + * written is ALWAYS the session sub (never from the form), and the write + * itself is the atomic set ADD/DELETE in dynamodb-writes.ts — see the + * boundary note there (the web's only write into the shared partition). + */ + +import { revalidatePath } from "next/cache"; + +import { auth } from "@/auth"; +import { getUserProfile, setPlanAlertSubscription } from "@/lib/dynamodb-writes"; +import { isTripsAllowed } from "@/lib/trips-access"; + +export interface ToggleResult { + ok: boolean; + error?: string; +} + +export async function setTripAlerts( + planIds: string[], + subscribed: boolean, +): Promise { + const session = await auth(); + const sub = session?.user?.id; + if (!sub) return { ok: false, error: "Not signed in." }; + if (!isTripsAllowed(session.user?.email)) { + return { ok: false, error: "Family accounts only." }; + } + + // Sanity-bound the input (comes from the client component): plan ids + // are ISO-timestamp SK suffixes. Reject junk rather than write it. + const ids = (planIds ?? []).filter( + (p) => typeof p === "string" && p.length > 0 && p.length < 100, + ); + if (ids.length === 0 || ids.length > 30) { + return { ok: false, error: "No plan days to update." }; + } + + // Receiving pushes needs a Pushover key on the member's profile — the + // poller reads it from USER#/PROFILE. Guard opt-IN so the toggle + // can't silently succeed at doing nothing; opt-OUT is always allowed. + if (subscribed) { + const profile = await getUserProfile(sub); + if (!profile?.pushoverUserKey) { + return { + ok: false, + error: + "Add your Pushover key on the My alerts page first — that's where trip pushes get sent.", + }; + } + } + + try { + await setPlanAlertSubscription(sub, ids, subscribed); + } catch { + return { ok: false, error: "Couldn't update — try again." }; + } + revalidatePath("/trips"); + return { ok: true }; +} diff --git a/web/src/app/trips/page.tsx b/web/src/app/trips/page.tsx index 8d780f0..fa12987 100644 --- a/web/src/app/trips/page.tsx +++ b/web/src/app/trips/page.tsx @@ -18,6 +18,8 @@ import { getUpcomingTrips, type Trip, type TripDay } from "@/lib/dynamodb"; import { findPark } from "@/lib/parks"; import { isTripsAllowed } from "@/lib/trips-access"; +import TripAlertToggle from "./TripAlertToggle"; + // Shared, family-scoped, low-traffic — always render fresh. export const dynamic = "force-dynamic"; @@ -60,6 +62,7 @@ export default async function TripsPage() { } const trips = await getUpcomingTrips(); + const viewerSub = session?.user?.id ?? ""; return (
@@ -84,7 +87,7 @@ export default async function TripsPage() { ) : (
{trips.map((trip) => ( - + ))}
)} @@ -92,17 +95,33 @@ export default async function TripsPage() { ); } -function TripSection({ trip }: { trip: Trip }) { +function TripSection({ trip, viewerSub }: { trip: Trip; viewerSub: string }) { + // The toggle applies to days still in play (recorded days are history — + // their alerts can't fire again). Subscribed = opted in on ANY such day; + // the owner is implicitly subscribed server-side, so for them this + // toggle just adds a redundant (deduped) entry — harmless. + const openDays = trip.days.filter((d) => !d.outcome_recorded); + const subscribed = openDays.some((d) => + d.alert_subscribers.includes(viewerSub), + ); return (
-
-

- {trip.name ?? "Trip"} -

-

- {formatDay(trip.start_date)} – {formatDay(trip.end_date)} ·{" "} - {trip.days.length} {trip.days.length === 1 ? "day" : "days"} -

+
+
+

+ {trip.name ?? "Trip"} +

+

+ {formatDay(trip.start_date)} – {formatDay(trip.end_date)} ·{" "} + {trip.days.length} {trip.days.length === 1 ? "day" : "days"} +

+
+ {openDays.length > 0 && ( + d.plan_id)} + subscribed={subscribed} + /> + )}
{trip.days.map((day) => ( diff --git a/web/src/lib/dynamodb-writes.test.ts b/web/src/lib/dynamodb-writes.test.ts index 7fa786a..b2db504 100644 --- a/web/src/lib/dynamodb-writes.test.ts +++ b/web/src/lib/dynamodb-writes.test.ts @@ -117,3 +117,34 @@ describe("getUserParkSubscriptions", () => { expect(await getUserParkSubscriptions("sub-123")).toEqual(new Set()); }); }); + +describe("setPlanAlertSubscription", () => { + it("issues one atomic ADD per plan row with the session sub as a Set", async () => { + sendMock.mockResolvedValue({}); + const { setPlanAlertSubscription } = await import("./dynamodb-writes"); + await setPlanAlertSubscription("sub-sis", ["p1", "p2"], true); + + expect(sendMock).toHaveBeenCalledTimes(2); + const inputs = sendMock.mock.calls.map((c) => c[0].input); + expect(inputs.map((i) => i.Key)).toEqual([ + { PK: "USER#megan", SK: "PLAN#p1" }, + { PK: "USER#megan", SK: "PLAN#p2" }, + ]); + for (const i of inputs) { + // Atomic set op — NOT a read-modify-write SET (that's what makes + // concurrent MCP/web edits safe). + expect(i.UpdateExpression).toBe("ADD alert_subscribers :m"); + expect(i.ExpressionAttributeValues[":m"]).toEqual(new Set(["sub-sis"])); + expect(i.ConditionExpression).toBe("attribute_exists(PK)"); + } + }); + + it("uses atomic DELETE on unsubscribe", async () => { + sendMock.mockResolvedValue({}); + const { setPlanAlertSubscription } = await import("./dynamodb-writes"); + await setPlanAlertSubscription("sub-sis", ["p1"], false); + expect(sendMock.mock.calls[0][0].input.UpdateExpression).toBe( + "DELETE alert_subscribers :m", + ); + }); +}); diff --git a/web/src/lib/dynamodb-writes.ts b/web/src/lib/dynamodb-writes.ts index 0f0b5de..eac4b45 100644 --- a/web/src/lib/dynamodb-writes.ts +++ b/web/src/lib/dynamodb-writes.ts @@ -188,6 +188,52 @@ export async function getUserParkSubscriptions( return new Set(found.filter((k): k is ParkKey => k !== null)); } +// ─── Plan alert opt-in (2026-07-03) ────────────────────────────────── +// +// ⚠️ BOUNDARY NOTE: this is the web's ONLY write into the SHARED trip +// partition (USER#megan) — everywhere else the web writes strictly +// per-user rows. Deliberately narrow: an UpdateItem that touches exactly +// one attribute (alert_subscribers) via ATOMIC set ADD/DELETE, so it can +// never race with (or clobber) the MCP planner's edits to the same rows. +// The IAM LeadingKeys grant (USER#*) already covers it — no CDK change. + +/** Shared trip partition owner — must match SHARED_TRIP_USER in + * dynamodb.ts and the MCP planner's _SHARED_USER_ID. */ +const SHARED_TRIP_USER = "megan"; + +/** + * Opt the signed-in member in/out of a set of plan days' alerts. + * + * `sub` MUST come from the session (auth().user.id) — never the request + * body (same rule as every write in this module). Adds/removes it in each + * row's alert_subscribers String Set; the poller then includes the + * member's USER#/PROFILE Pushover key in that plan's fanout. DDB + * semantics: ADD creates the set, DELETE of the last member removes the + * attribute (= back to owner-only). + */ +export async function setPlanAlertSubscription( + sub: string, + planIds: string[], + subscribed: boolean, +): Promise { + await Promise.all( + planIds.map((planId) => + client.send( + new UpdateCommand({ + TableName: tableName, + Key: { PK: `USER#${SHARED_TRIP_USER}`, SK: `PLAN#${planId}` }, + UpdateExpression: subscribed + ? "ADD alert_subscribers :m" + : "DELETE alert_subscribers :m", + ExpressionAttributeValues: { ":m": new Set([sub]) }, + // Only mutate rows the planner actually wrote — never create. + ConditionExpression: "attribute_exists(PK)", + }), + ), + ), + ); +} + // ─── Favorite rides (M3 Phase 2) ───────────────────────────────────── // // Schema: USER# / FAV_RIDE# with denormalized park_key. diff --git a/web/src/lib/dynamodb.test.ts b/web/src/lib/dynamodb.test.ts index 98cb29c..20080ac 100644 --- a/web/src/lib/dynamodb.test.ts +++ b/web/src/lib/dynamodb.test.ts @@ -390,3 +390,23 @@ describe("getUpcomingTrips", () => { expect(trips[1].name).toBe("June trip"); }); }); + +describe("getUpcomingTrips alert_subscribers", () => { + it("normalizes the DDB String Set to a sorted array (absent → [])", async () => { + mockTripsAndPlans( + [makeTripRow()], + [{ + Items: [ + makePlanRow({ SK: "PLAN#p1", planned_for_date: "2099-09-01", + alert_subscribers: new Set(["sub-b", "sub-a"]) }), + makePlanRow({ SK: "PLAN#p2", planned_for_date: "2099-09-02" }), + ], + LastEvaluatedKey: undefined, + }], + ); + const { getUpcomingTrips } = await import("./dynamodb"); + const trips = await getUpcomingTrips(); + expect(trips[0].days[0].alert_subscribers).toEqual(["sub-a", "sub-b"]); + expect(trips[0].days[1].alert_subscribers).toEqual([]); + }); +}); diff --git a/web/src/lib/dynamodb.ts b/web/src/lib/dynamodb.ts index 6c0aa82..a8a0f37 100644 --- a/web/src/lib/dynamodb.ts +++ b/web/src/lib/dynamodb.ts @@ -117,6 +117,10 @@ export interface TripDay { ride_count: number; outcome_recorded: boolean; rides: { ride_name: string; ride_id?: string }[]; + /** ADDITIONAL alert recipients opted in on this day (Cognito subs). + * The plan owner is implicit and never stored. Powers the /trips + * "get alerts" toggle state. */ + alert_subscribers: string[]; } export interface Trip { @@ -135,6 +139,8 @@ interface PlanRow { active?: boolean; outcome_recorded?: boolean; ride_sequence?: { ride_name?: string; ride_id?: string }[]; + // DDB String Set — the DocumentClient unmarshalls SS to a JS Set. + alert_subscribers?: Set | string[]; } interface TripRow { @@ -233,6 +239,9 @@ export async function getUpcomingTrips(): Promise { ride_name: rd.ride_name ?? "(unnamed)", ride_id: rd.ride_id, })), + // Set (from DDB SS) or array — normalize to a serializable array + // (a JS Set can't cross the Server Component boundary as a prop). + alert_subscribers: [...(r.alert_subscribers ?? [])].sort(), })), }); } @@ -278,6 +287,7 @@ export async function getUpcomingTrips(): Promise { ride_name: rd.ride_name ?? "(unnamed)", ride_id: rd.ride_id, })), + alert_subscribers: [...(r.alert_subscribers ?? [])].sort(), }, ], });