12 KiB
12 KiB
phase, plan, type, wave, depends_on, files_modified, autonomous, requirements, must_haves
| phase | plan | type | wave | depends_on | files_modified | autonomous | requirements | must_haves | |||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| quick-260607-l6l | 01 | execute | 1 |
|
true |
|
- BLOCKING —
PATCH /:uid/editandDELETE /:uidinevents.tsselectcalendars.url/calendars.userIdfrom.from(calendarEvents)with no join → invalid SQL → 503. Add the missinginnerJoin(calendars, ...)to BOTH lookups and add a regression test that runs the real query builder (the existing tests mockdb.select()and cannot catch this). me.tspasses the (often-missing)emailclaim asdisplayNameand ignoresname/preferred_username→ blank legend name. DerivedisplayNamerobustly from claims.GET /api/eventsreturns ALL users' events (no ownership predicate). Filter tocalendars.userId = currentUserId OR calendars.isShared = true.
Purpose: Unblock event delete/edit (Gate 2), fix the blank calendar-legend name, and make GET /api/events correct for the second household member.
Output: Corrected events.ts, me.ts, and a real-query-builder regression test in events.test.ts.
<execution_context> @$HOME/.claude/get-shit-done/workflows/execute-plan.md </execution_context>
@apps/api/src/routes/events.ts @apps/api/src/routes/me.ts @apps/api/src/db/schema.ts @apps/api/src/auth/user.ts @apps/api/tests/routes/events.test.ts @.planning/phases/03-event-write-back-pwa-install/.continue-here.mdReference idioms already in events.ts:
- Working join: GET / at ~line 142-183 uses
.from(calendarEvents).innerJoin(calendars, eq(calendarEvents.calendarId, calendars.id))
- Working ownership predicate: /writable-calendars at ~line 501-509 uses
.where(or(eq(calendars.userId, currentUserId), eq(calendars.isShared, true)))
- DO NOT modify `.env` or Authelia config — those are operator actions and `.env` is permission-locked. BUG 2 is a code-only fix (read the right claims robustly); the operator handles any Authelia claim-emission config separately. - DO NOT use playwright-cli (broken in this WSL2 env). Running-app verification is a manual operator browser re-test, noted as a follow-up — not a plan task. - Do not touch ROADMAP.md (quick task). - Keep each task an atomic, self-contained commit. - Match existing idioms in `events.ts` (the working GET join and the writable-calendars ownership predicate) rather than inventing new query shapes. Task 1: Add missing calendars join to edit + delete lookups, with a real-query-builder regression test apps/api/src/routes/events.ts, apps/api/tests/routes/events.test.ts - Regression test (RED before fix, GREEN after): build the EXACT lookup query used by PATCH /:uid/edit and DELETE /:uid against the real Drizzle `db` instance and call `.toSQL()` (no DB connection needed — toSQL does not execute). Assert the generated `sql` string contains an inner join referencing `calendar_events` → `calendars` (e.g. matches /inner join .*calendars/i). The current un-joined query selects `calendars.url`/`calendars.userId` with no join, so its SQL omits the join clause and the assertion FAILS; after the fix it PASSES. - Existing events.test.ts cases (POST/PATCH/DELETE 202 paths) must still pass — they mock `db.select()` and use a `.from().where()` chain. Adding `.innerJoin(...)` before `.where()` means the edit/delete mock chain must now route through innerJoin → where. Update the PATCH and DELETE `beforeEach` blocks so the mocked select chain exposes `.innerJoin(...).where(...)` (mirror the GET-suite chain at the top of the file: mockInnerJoin1Fn / mockWhereFn), returning the seeded mockDbRows. Verify the secondary isShared ownership-fallback `.from(calendars).where(...)` chain still resolves. In `apps/api/src/routes/events.ts`, add `.innerJoin(calendars, eq(calendarEvents.calendarId, calendars.id))` between `.from(calendarEvents)` and `.where(eq(calendarEvents.uid, uid))` in BOTH the `PATCH /:uid/edit` lookup (~line 295) and the `DELETE /:uid` lookup (~line 392). Mirror the working `GET /` join idiom at ~line 153 exactly. Do not change the selected columns or the ownership-check logic that follows.In `apps/api/tests/routes/events.test.ts`, add a new `describe` block (e.g. "regression: edit/delete lookups join calendars") that imports the real query builder from `../../src/db/client.js` (NOT the mocked one — use `vi.importActual` or place this test where the db mock does not apply, or construct the query via `drizzle-orm` directly against the real schema with a throwaway driver). Build the edit/delete lookup select with the same columns/from/innerJoin/where as the handler, call `.toSQL()`, and assert `result.sql` matches `/inner join[\s\S]*calendars/i`. The test must FAIL on the un-joined query and PASS after the join is added.
Then update the PATCH `/:uid/edit` and DELETE `/:uid` describe-block `beforeEach` mocks so the mocked select chain routes `.from(calendarEvents).innerJoin(...).where(...)` to the seeded `mockDbRows` (reuse the GET-suite `mockInnerJoin1Fn` → `mockWhereFn` wiring already defined at the top of the file), and keep the secondary `.from(calendars).where(...)` isShared-fallback chain working.
Prefer the lightest approach that catches the class of bug: a `.toSQL()` string assertion on the real query builder. Do NOT introduce a new full DB test harness (no SQLite container, no live MariaDB) — investigate the existing mock structure first and reuse it.
Derive a display name robustly from the OIDC claims object returned by `getAuth(c)`, preferring in order: `name`, then `preferred_username`, then `email`, then a sensible fallback derived from `sub` (e.g. `'Member ' + sub` or the local-part if email exists). Each candidate must be read defensively (`typeof claim === 'string' && claim.trim() !== ''`) since claims may be missing or empty. Pass the resolved display name as the third argument to `upsertUser(iss, sub, displayName)`.
Keep `iss`/`sub` extraction unchanged (identity stays keyed on iss+sub per D-10). Do not change the dev-bypass branch. Do not alter `upsertUser`'s signature or `user.ts`. Add a brief comment noting the claim-preference order and that Authelia-side claim emission is an operator concern (out of scope here).
Add an ownership predicate to the existing `.where(...)` so only events on calendars owned by the current user OR shared calendars are returned. Combine the new ownership filter with the existing date-window `or(...)` block using `and(...)`, i.e. effectively `and(<existing window or-block>, or(eq(calendars.userId, currentUserId), eq(calendars.isShared, true)))`. Mirror the authoritative ownership idiom already used by `/writable-calendars` (~line 509). Do not change the window-span guard, the join chain, or the expansion logic.
<success_criteria>
- BUG 1: Both edit and delete lookups innerJoin calendars; a real-query-builder regression test guards against the missing-join regression.
- BUG 2: me.ts derives a non-blank displayName from OIDC claims (name → preferred_username → email → fallback).
- BUG 3: GET /api/events filters to current-user-owned OR shared calendars.
- Full test suite + typecheck green.
- Each fix landed as a separate atomic commit. </success_criteria>
<manual_follow_up> NOT a plan task — operator action after merge:
- Re-test in a real browser through the tunnel: delete an event (dialog should close, event disappears), edit an event, and confirm the calendar legend shows the member's name (BUG 2 may additionally require Authelia to emit the
name/emailclaim — operator's call; the code now reads whatever claims are present). - playwright-cli is intentionally NOT used (broken in this WSL2 env). </manual_follow_up>