From 5ce6b29d4a232c7efa517aca261a7351dfd5c3c6 Mon Sep 17 00:00:00 2001 From: Moheeb Zara Date: Tue, 19 May 2026 20:44:08 -0700 Subject: [PATCH] test: integration coverage for dues_payment_methods RLS and dispatcher skip - dues_payment_methods RLS: member read, admin-only write, cross-space denial, 046 unverified-admin status gate (4 cases). - notification dispatcher: a muted category becomes 'skipped' while billing always sends; no pref row sends (default on). First real exercise of the skip path (cron is gated off in prod). - Fix integration/db.ts rowsAsUser: the SELECT must be the last psql -c statement or its output is swallowed (it was never used before). - Dues card: hide "No active dues subscription" for spaces without Stripe, where it read as confusing. --- components/billing/dues-card.tsx | 4 +- docs/HANDOFF.md | 14 ++- integration/db.ts | 19 +++- integration/dues-payment-methods.test.ts | 90 +++++++++++++++ .../notification-prefs-dispatch.test.ts | 104 ++++++++++++++++++ 5 files changed, 220 insertions(+), 11 deletions(-) create mode 100644 integration/dues-payment-methods.test.ts create mode 100644 integration/notification-prefs-dispatch.test.ts diff --git a/components/billing/dues-card.tsx b/components/billing/dues-card.tsx index f88fa4e..293df90 100644 --- a/components/billing/dues-card.tsx +++ b/components/billing/dues-card.tsx @@ -53,9 +53,9 @@ export function DuesCard({ billing, methods = [] }: { billing: Billing; methods? ? ` · renews ${new Date(billing.currentPeriodEnd).toLocaleDateString()}` : ''}

- ) : ( + ) : stripeReady ? (

No active dues subscription.

- )} + ) : null} {billing?.status && ( {paid ? 'Current' : 'Action needed'} diff --git a/docs/HANDOFF.md b/docs/HANDOFF.md index d922e8b..97ccc7e 100644 --- a/docs/HANDOFF.md +++ b/docs/HANDOFF.md @@ -30,11 +30,15 @@ Branch `main`. Same session as pass-57; addressed three follow-up requests the u ### Audit (one pass) - Made `isSafeDuesUrl` load-bearing (the Zod url field now refines on it, removing the duplicate inline https check). Verified ChipInput limits match the profile Zod schema (skills/interests 40×60, willing_to 20×60, affiliations 50×200). Confirmed `getMyBilling`'s only real caller is `/me` (shape change from `null` to always-object is safe). External links render `target=_blank rel=noopener noreferrer`; url validated absolute-https so no javascript:/data: scheme. -### NOT done / open -- **NOT deployed, NOT committed.** Migrations 048 (notification_preferences, pass-57) + 049 (dues_payment_methods) both apply on deploy. No new env var; DEPLOYMENT unchanged. -- **Browser click-through not done.** Verified type-check + build + 593 unit tests only; `/me` and `/settings` are auth-gated and need a live Supabase session + a space (and Stripe/links data) to render. The /me visual pass is explicitly staged for the user to review in a browser. Flagging per the working agreement (do not claim UI success without rendering it). -- **No integration test** for dues_payment_methods RLS or the dispatcher skip (pass-57). Pure logic for both is unit-covered. Recommended follow-up (harness exists). -- **Deploy bundling decision pending:** pass-57 (notification prefs) + pass-58 can ship together (one deploy, migrations 048+049) or separately. Awaiting user go. +### Post-deploy audit follow-up (LOCAL, NOT yet deployed) +Second ultrathink audit after the `ab8c22a` deploy found NO deploy-blocking bug. Re-read the dispatcher, dues actions, and dues card with fresh eyes: URL is Zod-validated absolute-https; the `delete` platform flows through a parameterized `.eq` filter (double-gated by the space pin + RLS); writes/reads are scoped to the authenticated member; billing is never muted (logic + test). Then hardened + polished (all local, post-`ab8c22a`): +- **Integration coverage added (now 37, was 31).** `integration/dues-payment-methods.test.ts` (4: admin insert + member read + non-admin write denied + cross-space read denied + cross-space write denied + 046 unverified-admin gate) and `integration/notification-prefs-dispatch.test.ts` (2: muted category -> `skipped` while billing always sends; no-pref-row -> sends). The dispatcher skip was unexercised in prod (cron gated off), so this is its first real exercise. All 37 pass vs local Postgres. +- **Harness bug fixed.** `integration/db.ts` `rowsAsUser` ran `begin; set; select; commit;` in one `psql -c`, which prints only the LAST statement's result, so the SELECT output was swallowed (returned `[]`). It was never used before; now the SELECT is the last statement (session `set`, no trailing `commit`). +- **Dues-card copy polish.** Suppressed the "No active dues subscription" line for spaces with no Stripe (it read as confusing alongside the contact-admin note for external-only / unconfigured spaces). + +### Still open +- **Browser click-through not done.** Verified type-check + build + 593 unit + 37 integration only; `/me` and `/settings` are auth-gated and need a live Supabase session to render. The /me visual pass is staged for the owner to review in a browser. Flagging per the working agreement (do not claim UI success without rendering it). +- The audit follow-up above (integration tests + harness fix + dues-card copy) is committed locally but NOT yet deployed; the integration tests are non-runtime (CI/dev only). The dues-card copy is the only prod-affecting line and rides the next deploy unless deployed sooner. --- diff --git a/integration/db.ts b/integration/db.ts index 0070403..22fbc28 100644 --- a/integration/db.ts +++ b/integration/db.ts @@ -58,14 +58,25 @@ export function rows>(selectSql: string): T[] { export function rowsAsUser>(uid: string, selectSql: string): T[] { const claims = JSON.stringify({ sub: uid }).replace(/'/g, "''") + // The SELECT must be the LAST statement: psql -c with several statements + // only prints the final command's result, so a trailing `commit` would + // swallow the SELECT output. Use session-level `set` (the one-shot psql + // connection exits afterward) and end on the SELECT. const r = run([ '-tAc', - `begin; set local role authenticated; set local "request.jwt.claims" = '${claims}';` + - `select coalesce(json_agg(t),'[]'::json)::text from (${selectSql}) t; commit;`, + `set role authenticated; set "request.jwt.claims" = '${claims}'; ` + + `select coalesce(json_agg(t),'[]'::json)::text from (${selectSql}) t;`, ]) if (!r.ok) throw new Error(r.err) - // last non-empty line is the json (BEGIN/SET/COMMIT print nothing with -tA) - const line = r.out.trim().split('\n').filter(Boolean).pop() ?? '[]' + // json_agg(coalesce(...,'[]')) yields a line starting with '['; pick it and + // ignore any psql command tags (SET) that may print around it. + const line = + r.out + .trim() + .split('\n') + .map(l => l.trim()) + .filter(l => l.startsWith('[')) + .pop() ?? '[]' return JSON.parse(line) as T[] } diff --git a/integration/dues-payment-methods.test.ts b/integration/dues-payment-methods.test.ts new file mode 100644 index 0000000..5763caa --- /dev/null +++ b/integration/dues-payment-methods.test.ts @@ -0,0 +1,90 @@ +// RLS for dues_payment_methods (migration 049): admin-configured external dues +// links are member-readable but admin/board-writable, and scoped per space. +// Proven against real Postgres via the role-simulation harness (set role +// authenticated + request.jwt.claims sub), the same technique as +// privilege-status-gate.test.ts. +import { describe, it, expect, afterAll } from 'vitest' +import { + dbReady, + execService, + execAsUser, + rowsAsUser, + seedSpace, + seedMember, + dropSpace, +} from './db' + +const d = describe.skipIf(!dbReady) + +d('dues_payment_methods RLS', () => { + const spaces: string[] = [] + function space(): string { + const { spaceId } = seedSpace() + spaces.push(spaceId) + return spaceId + } + afterAll(() => spaces.forEach(dropSpace)) + + it('admin can insert, any member can read, a non-admin member cannot write', () => { + const sid = space() + const admin = seedMember(sid, { role: 'admin' }) + const member = seedMember(sid, { role: 'member' }) + + const ins = execAsUser( + admin.userId, + `insert into dues_payment_methods (space_id, platform, url) values ('${sid}','paypal','https://paypal.me/itest');`, + ) + expect(ins.ok).toBe(true) + + const seen = rowsAsUser<{ platform: string; url: string }>( + member.userId, + `select platform, url from dues_payment_methods where space_id='${sid}'`, + ) + expect(seen).toHaveLength(1) + expect(seen[0].platform).toBe('paypal') + + const memberWrite = execAsUser( + member.userId, + `insert into dues_payment_methods (space_id, platform, url) values ('${sid}','venmo','https://venmo.com/itest');`, + ) + expect(memberWrite.ok).toBe(false) + }) + + it('a member of another space cannot read these methods', () => { + const sidA = space() + seedMember(sidA, { role: 'admin' }) + execService( + `insert into dues_payment_methods (space_id, platform, url) values ('${sidA}','zeffy','https://www.zeffy.com/itest');`, + ) + + const sidB = space() + const memberB = seedMember(sidB, { role: 'member' }) + const seen = rowsAsUser( + memberB.userId, + `select platform from dues_payment_methods where space_id='${sidA}'`, + ) + expect(seen).toHaveLength(0) + }) + + it('an admin cannot write a method into another space (WITH CHECK pins space)', () => { + const sidA = space() + const adminA = seedMember(sidA, { role: 'admin' }) + const sidB = space() + + const r = execAsUser( + adminA.userId, + `insert into dues_payment_methods (space_id, platform, url) values ('${sidB}','paypal','https://paypal.me/cross');`, + ) + expect(r.ok).toBe(false) + }) + + it('an unverified member cannot write even with admin role (046 status gate)', () => { + const sid = space() + const pending = seedMember(sid, { role: 'admin', status: 'unverified' }) + const r = execAsUser( + pending.userId, + `insert into dues_payment_methods (space_id, platform, url) values ('${sid}','paypal','https://paypal.me/x');`, + ) + expect(r.ok).toBe(false) + }) +}) diff --git a/integration/notification-prefs-dispatch.test.ts b/integration/notification-prefs-dispatch.test.ts new file mode 100644 index 0000000..8ee549e --- /dev/null +++ b/integration/notification-prefs-dispatch.test.ts @@ -0,0 +1,104 @@ +// End-to-end integration of the notification dispatcher's preference check +// (migration 048): the ROUTE HANDLER reads notification_preferences and marks +// a muted row 'skipped' instead of sending, while billing types always send. +// This path is gated off in prod until CRON_SECRET is provisioned, so the +// integration harness is its first real exercise. Mirrors stripe-webhook's +// route-handler invocation (load supabase env, lazy-import POST). With no +// RESEND_API_KEY, sendEmail is a non-retryable no-op so an UNmuted row lands +// on 'failed' (NOT 'skipped'), which is exactly the distinction we assert. +import { describe, it, expect, afterAll } from 'vitest' +import { execFileSync } from 'node:child_process' +import { NextRequest } from 'next/server' +import { dbReady, execService, rows, seedSpace, seedMember, dropSpace } from './db' + +const CRON_SECRET = 'itest_cron_secret' + +function loadSupabaseEnv(): boolean { + try { + const out = execFileSync('supabase', ['status', '-o', 'env'], { encoding: 'utf8' }) + const get = (k: string) => + out.split('\n').find(l => l.startsWith(k + '='))?.slice(k.length + 1).replace(/^"|"$/g, '') + const url = get('API_URL') + const key = get('SERVICE_ROLE_KEY') + if (!url || !key) return false + process.env.NEXT_PUBLIC_SUPABASE_URL = url + process.env.SUPABASE_SERVICE_ROLE_KEY = key + process.env.CRON_SECRET = CRON_SECRET + delete process.env.RESEND_API_KEY + return true + } catch { + return false + } +} +const envOk = loadSupabaseEnv() +const d = describe.skipIf(!dbReady || !envOk) + +async function runCron(): Promise { + const { POST } = await import('@/app/api/cron/notifications/route') + const req = new NextRequest('http://localhost/api/cron/notifications', { + method: 'POST', + headers: { authorization: `Bearer ${CRON_SECRET}` }, + }) + await POST(req) +} + +function seedNotif(spaceId: string, memberId: string, type: string, dedupe: string): void { + execService( + `insert into notifications (space_id, member_id, type, channel, recipient, subject, body_html, body_text, status, dedupe_key) + values ('${spaceId}','${memberId}','${type}','email','x@itest.local','s','h','t','pending','${dedupe}');`, + ) +} + +function statusOf(spaceId: string, dedupe: string): string | undefined { + const r = rows<{ status: string }>( + `select status from notifications where space_id='${spaceId}' and dedupe_key='${dedupe}'`, + ) + return r[0]?.status +} + +// Drain until the given row leaves 'pending' (the global cron may need more +// than one pass if other test files left pending rows in the batch window). +async function drainUntilResolved(spaceId: string, dedupe: string): Promise { + for (let i = 0; i < 6; i++) { + if (statusOf(spaceId, dedupe) !== 'pending') return + await runCron() + } +} + +d('dispatcher honors notification preferences', () => { + const spaces: string[] = [] + afterAll(() => spaces.forEach(dropSpace)) + + it('skips a muted muteable category but always sends billing', async () => { + const { spaceId } = seedSpace() + spaces.push(spaceId) + const { memberId } = seedMember(spaceId, { role: 'member' }) + execService( + `insert into notification_preferences (space_id, member_id, category, enabled) + values ('${spaceId}','${memberId}','bookings', false);`, + ) + const bk = `bk-${spaceId}` + const du = `du-${spaceId}` + seedNotif(spaceId, memberId, 'booking_confirmed', bk) + seedNotif(spaceId, memberId, 'dues_lapsed', du) + + await drainUntilResolved(spaceId, bk) + await drainUntilResolved(spaceId, du) + + expect(statusOf(spaceId, bk)).toBe('skipped') + // billing is never muteable: it must NOT be skipped (no RESEND -> 'failed') + expect(statusOf(spaceId, du)).not.toBe('skipped') + }) + + it('sends a muteable category when the member has no pref row (default on)', async () => { + const { spaceId } = seedSpace() + spaces.push(spaceId) + const { memberId } = seedMember(spaceId, { role: 'member' }) + const bk = `bk2-${spaceId}` + seedNotif(spaceId, memberId, 'booking_confirmed', bk) + + await drainUntilResolved(spaceId, bk) + + expect(statusOf(spaceId, bk)).not.toBe('skipped') + }) +})