From f5c8cb68be959ab915d2876db97d220983ef6f74 Mon Sep 17 00:00:00 2001 From: Oier Bravo Urtasun Date: Tue, 14 Apr 2026 22:33:33 +0200 Subject: [PATCH] feat(fase-7): collective-flow E2E coverage + fix 3 latent bugs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 12 new Playwright tests closing the UI coverage gap in the collective lifecycle (onboarding → invitation → admin manage). Writing them surfaced three product-code bugs that have silently been present since Fase 1; fixed as part of this phase. Tests: - tests/e2e/onboarding.test.ts O-01..O-03 (Eva auto-redirect, happy path create, empty-name submit-disabled) - tests/e2e/invitation.test.ts I-01..I-05 (token generation, accept logged-out + logged-in, expired, used) - tests/e2e/manage-collective.test.ts MC-02..MC-05 (promote, demote, remove member, generate pending invite) - tests/fixtures/db.ts resetEva, seedExpiredInvitation, seedUsedInvitation, restoreSeedMembership, countMembership — all via raw SQL so they don't depend on SUPABASE_SERVICE_ROLE_KEY Bugs found & fixed: - supabase/migrations/012_create_collective_rpc.sql: atomic `create_collective(name, emoji)` SECURITY DEFINER RPC. Fase 1 onboarding used `.insert(...).select().single()` which triggers the SELECT policy on the fresh row — creator isn't a member yet, so the INSERT was rejected with "row-level security" even though the INSERT policy passed. onboarding now calls the RPC which inserts both the collective AND the creator-as-admin membership in one transaction, bypassing RLS. F-08 in rls-isolation.test.ts has been silently masking this bug (only the spoof branch was asserted). - routes/+layout.svelte: post-login redirect now reads sessionStorage `pendingInvitationToken` and routes back to /invitation/ instead of defaulting to /onboarding or /lists. The invitation page already stashed the token before kicking off Keycloak, but nothing read it back. - routes/(app)/collective/manage/+page.svelte: loadMembers now subscribes to currentCollective so it re-runs when the store resolves after cold navigation. onMount alone races with the auth listener's first emit and the member list stayed empty on direct URL hits. - routes/invitation/[token]/+page.svelte: awaits authLoading before deciding whether to redirect to login. Previously raced with the auth listener and triggered a redundant Keycloak round-trip for freshly-authenticated users. Stable selectors added (Playwright): - Onboarding: onboarding-create-tab, onboarding-join-tab, collective-name-input, collective-submit - Manage: member-row-, role-select-, remove-member-, generate-invite, invite-link - Invitation: invitation-accept, invitation-error (+ data-error-key attr) Scope dropped from the plan — UI does not exist, would be feature work: - O-04: "+ new collective" affordance in the sidebar. - MC-01: rename-collective input in /collective/manage. Both flagged as follow-ups in plan/fase-7-collective-flow-tests.md. Test totals: 34 pgTAP + 140 Vitest integration + 15 Vitest unit + 58 Playwright + 1 gated rate-limit. 3 skipped (2 Realtime presence, 1 gated). --- .gitignore | 3 + CLAUDE.md | 38 +++- apps/web/package.json | 1 + .../(app)/collective/manage/+page.svelte | 20 ++- apps/web/src/routes/+layout.svelte | 9 +- .../routes/invitation/[token]/+page.svelte | 24 ++- apps/web/src/routes/onboarding/+page.svelte | 40 +++-- apps/web/tests/e2e/invitation.test.ts | 168 ++++++++++++++++++ apps/web/tests/e2e/manage-collective.test.ts | 128 +++++++++++++ apps/web/tests/e2e/onboarding.test.ts | 92 ++++++++++ apps/web/tests/fixtures/db.ts | 110 ++++++++++++ apps/web/tests/fixtures/login.ts | 14 +- apps/web/tests/fixtures/users.ts | 8 + packages/test-utils/package.json | 8 +- plan/fase-7-collective-flow-tests.md | 60 ++++--- pnpm-lock.yaml | 9 +- .../migrations/012_create_collective_rpc.sql | 55 ++++++ 17 files changed, 728 insertions(+), 59 deletions(-) create mode 100644 apps/web/tests/e2e/invitation.test.ts create mode 100644 apps/web/tests/e2e/manage-collective.test.ts create mode 100644 apps/web/tests/e2e/onboarding.test.ts create mode 100644 apps/web/tests/fixtures/db.ts create mode 100644 supabase/migrations/012_create_collective_rpc.sql diff --git a/.gitignore b/.gitignore index 103fd45..70e7a45 100644 --- a/.gitignore +++ b/.gitignore @@ -39,5 +39,8 @@ apps/web/playwright-report/ apps/web/test-results/ apps/web/tests/.auth/ +# PWA dev-mode service worker output +apps/web/dev-dist/ + # Claude Code per-project runtime state (schedulers etc.) .claude/ diff --git a/CLAUDE.md b/CLAUDE.md index 443c4c4..f418495 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,11 +8,11 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co ## Project Status -**Fase 0–5 complete. Fase 6 complete (deploy readiness: PWA injectManifest SW + manifest + iOS meta tags + placeholder icons + Kong rate-limit on invitations + dev JWT rotation + prod env template + rls-audit.sh + Justfile lighthouse/rls-audit/test-rate-limit). Production stack deployed to ambrosio (OVH VPS) at https://erosi.oier.ovh (app + Supabase) + https://auth.oier.ovh (Keycloak); Let's Encrypt certs via host Caddy; PWA strategy switched to generateSW to unblock the prod build (see gotcha #16). Fase 7 is the active plan: 14 Playwright tests filling the collective-flow UI gap (onboarding / invitation acceptance / admin-manage) — see `plan/fase-7-collective-flow-tests.md`. 236 tests green: 34 pgTAP + 140 Vitest integration + 15 Vitest unit + 46 Playwright + 1 Vitest rate-limit (gated, run via `just test-rate-limit`). 3 skipped in `just test-all` (2 Realtime presence — upstream `handle_out/3` bug; 1 rate-limit — gated). Push notifications + CI Docker + final production icons deliberately out of scope. ✅** +**Fase 0–5 complete. Fase 6 complete (deploy readiness: PWA injectManifest SW + manifest + iOS meta tags + placeholder icons + Kong rate-limit on invitations + dev JWT rotation + prod env template + rls-audit.sh + Justfile lighthouse/rls-audit/test-rate-limit). Production stack deployed to ambrosio (OVH VPS) at https://erosi.oier.ovh (app + Supabase) + https://auth.oier.ovh (Keycloak); Let's Encrypt certs via host Caddy; PWA strategy switched to generateSW to unblock the prod build (see gotcha #16). Fase 7 complete: 12 new Playwright tests fill the collective-flow UI gap (O-01..O-03 onboarding, I-01..I-05 invitation acceptance, MC-02..MC-05 admin-manage) — writing them surfaced + fixed three latent bugs in the product code (atomic `create_collective()` RPC, pendingInvitationToken restore after login, manage page not reloading on late $currentCollective hydration). 248 tests green: 34 pgTAP + 140 Vitest integration + 15 Vitest unit + 58 Playwright + 1 Vitest rate-limit (gated, run via `just test-rate-limit`). 3 skipped in `just test-all` (2 Realtime presence — upstream `handle_out/3` bug; 1 rate-limit — gated). Push notifications + CI Docker + final production icons deliberately out of scope. ✅** - `README.md` — full development plan, confirmed tech stack, Justfile reference, and technical warnings - `analysis/analisis-funcional.md` — complete functional specification (domain model, use cases, data models, business rules) -- `plan/fase-*.md` — phase-by-phase implementation plans (Fase 0 through Fase 7; Fase 7 is the active plan) +- `plan/fase-*.md` — phase-by-phase implementation plans (Fase 0 through Fase 7; Fase 7 is complete — collective-flow E2E coverage) - `design/slate_collective/DESIGN.md` + `design/*/screen.png` — "Monolith Editorial" direction + per-screen Stitch mockups (desktop + mobile variants). Consult before changing UI. ### Fase 1 — what has been built @@ -74,6 +74,40 @@ just test-e2e # Playwright E2E tests (requires just dev running) just test-e2e-headed # Playwright with visible browser (debugging) ``` +### Fase 7 — collective-flow E2E coverage (2026-04-14) + +12 new Playwright tests + 1 DB fixtures helper + 1 migration. Writing the tests uncovered three product-code bugs which are fixed as part of this phase. + +**Tests (all green, included in `just test-e2e`):** +- `apps/web/tests/e2e/onboarding.test.ts` — O-01..O-03: Eva auto-redirect to `/onboarding`, happy-path create, empty-name submit-disabled. +- `apps/web/tests/e2e/invitation.test.ts` — I-01..I-05: admin generates token, logged-out-then-accept, logged-in one-click accept, expired + already-used error states. +- `apps/web/tests/e2e/manage-collective.test.ts` — MC-02..MC-05: promote member → admin, demote back, remove member, generate pending invitation. +- `apps/web/tests/fixtures/db.ts` — `resetEva()` / `seedExpiredInvitation()` / `seedUsedInvitation()` / `restoreSeedMembership()` / `countMembership()`. All via raw SQL (bypasses RLS) so they don't depend on `SUPABASE_SERVICE_ROLE_KEY` being in the Playwright process env. + +**Scope dropped from the plan:** +- **O-04** (multi-collective via a "+ new collective" button in the sidebar): the affordance does not exist in the UI; adding it would be feature work, out of a test-only phase. +- **MC-01** (rename collective): `/collective/manage` has no rename input; same reasoning. +Both are flagged in `plan/fase-7-collective-flow-tests.md` as future UI follow-ups. + +**Product-code fixes discovered while writing these tests (all committed with this phase):** + +1. **`supabase/migrations/012_create_collective_rpc.sql` — atomic `create_collective(name, emoji)` RPC.** Pre-existing onboarding flow did two separate REST calls (`INSERT collectives` chained with `.select().single()`, then `INSERT collective_members`). Both failed: the RETURNING on the first triggered the SELECT policy (`is_member(id)`) which the creator can't pass yet, and the second was gated by `is_admin(collective_id)` against a collective with zero admins. The RPC is `SECURITY DEFINER` and does both inserts atomically. `onboarding/+page.svelte` now calls `supabase.rpc('create_collective', {...})`. + +2. **`routes/+layout.svelte` — resume pending invitation after login.** `invitation/[token]/+page.svelte` stashes the token in `sessionStorage.pendingInvitationToken` before triggering OAuth, but nothing read it back. Post-login redirect logic now checks sessionStorage and routes the user back to `/invitation/` instead of defaulting to `/onboarding` or `/lists`. + +3. **`routes/(app)/collective/manage/+page.svelte` — reactively reload members.** `onMount` called `loadMembers()` once; if `$currentCollective` was still null at that point (race with the auth listener on a cold navigation), the query short-circuited and the list stayed empty. Now subscribed to `currentCollective` so `loadMembers` re-runs whenever the store resolves. + +4. **`routes/invitation/[token]/+page.svelte` — wait for authLoading before deciding.** Racing with the auth listener's first emit caused the page to see `!$isAuthenticated` for a freshly-authenticated user and trigger a redundant Keycloak round-trip. Now awaits `authLoading → false` before branching. + +**Test IDs added to UI (stable selectors for Playwright):** +- Onboarding: `onboarding-create-tab`, `onboarding-join-tab`, `collective-name-input`, `collective-submit`. +- Manage: `member-row-`, `role-select-`, `remove-member-`, `generate-invite`, `invite-link`. +- Invitation: `invitation-accept`, `invitation-error` (with `data-error-key` attribute carrying the error key for assertion). + +**Gotchas discovered during execution (do not remove):** + +18. **`.select()` after `.insert()` evaluates the SELECT RLS policy on the returned row, not just the INSERT policy.** PostgREST's chained `.insert(...).select().single()` adds `RETURNING *` to the INSERT, and Postgres then evaluates the table's SELECT policy against the fresh row. For policies that gate on sibling-table state (e.g., `is_member(id)` requiring a row in `collective_members`), this causes INSERTs to spuriously fail with "new row violates row-level security policy" even though the INSERT policy itself passed. Fix: drop the `.select()` if you don't need the row back, or (better) wrap the insert + any sibling-table sets in a `SECURITY DEFINER` RPC that atomically builds the membership state. F-08 in `rls-isolation.test.ts` has been silently masking this since Fase 1 because it only asserts the _spoof_ case; never verified the happy-path insert actually succeeded. + ### Fase 6+ — production deploy to ambrosio (2026-04-14) Live URLs: **https://erosi.oier.ovh** (app + Supabase API, single-domain) + **https://auth.oier.ovh** (Keycloak). OVH VPS, Ubuntu 24, Docker 29. diff --git a/apps/web/package.json b/apps/web/package.json index 9483ccb..0b552d0 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -25,6 +25,7 @@ "svelte-dnd-action": "^0.9.51" }, "devDependencies": { + "@colectivo/test-utils": "workspace:*", "@playwright/test": "^1.50.0", "@sveltejs/adapter-node": "^5.2.11", "@sveltejs/kit": "^2.15.1", diff --git a/apps/web/src/routes/(app)/collective/manage/+page.svelte b/apps/web/src/routes/(app)/collective/manage/+page.svelte index a9a18fa..e2938c0 100644 --- a/apps/web/src/routes/(app)/collective/manage/+page.svelte +++ b/apps/web/src/routes/(app)/collective/manage/+page.svelte @@ -27,8 +27,17 @@ const myRole = $derived(members.find((m) => m.user_id === $currentUser?.id)?.role); const isAdmin = $derived(myRole === 'admin'); - onMount(async () => { - await loadMembers(); + onMount(() => { + // Re-run loadMembers() whenever the active collective resolves or + // changes. onMount alone races with the auth listener's first + // `currentCollective.set(...)` on cold navigations (hit /collective/manage + // directly in a new tab or after a page reload) and we end up with an + // empty list because the first fetch was short-circuited by + // `$currentCollective == null`. + const unsub = currentCollective.subscribe((c) => { + if (c) void loadMembers(); + }); + return unsub; }); async function loadMembers() { @@ -163,7 +172,7 @@ {:else}
    {#each members as member} -
  • +
  • changeRole(member.user_id, e.currentTarget.value as Member['role'])} class="rounded-md border border-slate-300 bg-surface px-2 py-1 text-xs @@ -192,6 +202,7 @@ {/each}