Files
qwen3-6-lora/data/raw/sanitized/plans/en-la-funci-n-de-cheeky-dewdrop.md

8.1 KiB

Fix: attendance "Present" not saved automatically on login

Context

GFIBER-649 shipped "auto-mark attendance Present on login" (merged to dev 2026-06-23, confirmed live in both main and dev branches — they differ by only 2 unrelated commits). The implementation, markSelfPresentToday(), has been silently failing for every non-admin user since it shipped. Since a missing attendance row degrades to the harmless-looking "Pending" state (see utils/dashboard/attendance.ts — no record = assumed present in aggregate counts), nobody saw an error; the bug likely surfaced through the admin "pending attendance" bell/digest email never clearing for otherwise-active agents, which matches what was reported: attendance never gets marked Present at login.

The confirmed root cause: RLS blocks every regular user's self-insert. markSelfPresentToday() (app/(protected)/upsell-evaluator/attendance-actions.ts) runs through the anon-key Supabase client (utils/supabase/server.ts), so Row-Level Security applies. The only policy on public.attendance (supabase/migrations/20260605130000_create_attendance.sql) is FOR ALL ... USING/WITH CHECK (role IN ('admin','owner')) — it was written for the admin-only dashboard grid and restricts every operation, including INSERT, to admins/owners. Regular agents default to role = 'user' (20260603000000_add_role_to_profiles.sql), so their self-upsert is rejected by RLS. The calling code never reads the {error} from .upsert(...) — it's discarded entirely — so the failure has been completely invisible. An existing test (__tests__/dashboard-attendance-actions.test.ts:82-85) even asserts this silence as current behavior. This can only be fixed with a DB-level policy change (a migration) — no amount of application code can grant a permission Postgres RLS denies.

Ruled out: a CHECK-constraint drift between prod/dev on attendance_status_valid (migration history shows a prod rollback + dev-only re-narrowing that looked unreconciled). Confirmed with the team that admins can already set status manually without error, which proves the constraint already accepts 'P'/'A' in both environments — no constraint migration needed. This plan sticks to the confirmed RLS fix only.

Fixing the RLS policy, plus moving the call site and adding error visibility, closes the bug and prevents this exact silent-failure pattern from recurring.

Update (post-deploy verification, 2026-07-01): the first INSERT-only policy (below) was necessary but not sufficient. Live testing in DEV showed markSelfPresentToday() still failed to write for a real role='user' account even after the migration was applied. Root cause, found by reproducing the exact INSERT ... ON CONFLICT (agent_id, date) DO NOTHING statement directly against the DEV database: Postgres RLS requires SELECT-level visibility into a potentially-conflicting row to resolve an ON CONFLICT ... DO NOTHING clause, even though DO NOTHING never reads that row. With only an INSERT policy and no SELECT policy, Postgres rejects the whole statement with the same generic "violates row-level security policy" error — indistinguishable from the original bug without direct reproduction. A second migration (20260701000001_attendance_self_checkin_select.sql) adds a FOR SELECT policy scoped to agent_id = auth.uid() (a user may see their own attendance rows). Verified end-to-end against the live DEV database: the exact upsert now succeeds and the row persists.

Approach

1. New migration — self check-in RLS policy

supabase/migrations/20260701000000_attendance_self_checkin_rls.sql:

create policy "Users can self-check-in present today"
  on public.attendance
  for insert
  to authenticated
  with check (
    agent_id = (select auth.uid())
    and status = 'P'
    and date = current_date
  );

Postgres OR's all applicable permissive policies for a given command, so this adds an allowed INSERT path without touching the existing admin FOR ALL policy — admins keep full read/write/delete over every row; regular users still cannot SELECT/UPDATE/DELETE, or insert any row but their own today's 'P'. Only WITH CHECK is needed (no USING) since FOR INSERT policies don't evaluate USING. markSelfPresentToday() calls upsert(..., { ignoreDuplicates: true }), which compiles to INSERT ... ON CONFLICT DO NOTHING — no UPDATE path — so an INSERT-only policy is sufficient and admin-set 'A' rows are never overwritten.

2. attendance-actions.ts — stop swallowing the error

Destructure { error } from the upsert result and console.error("markSelfPresentToday: upsert failed:", error) when present, matching the existing repo convention (app/(protected)/actions.ts). Function stays best-effort — never throws, return type unchanged. This is what turns the next silent RLS/constraint regression into something visible in logs instead of a months-long undetected bug.

3. Move the call site from the page to the shared protected layout

Today markSelfPresentToday() only runs when app/(protected)/upsell-evaluator/page.tsx renders — not literally "on login." app/(protected)/layout.tsx wraps every protected route (dashboard, admin, my-metrics, upsell-evaluator) and already gates on getCurrentUserWithProfile(). Move the call there (after the auth gate resolves, before the admin-only pending-agents fetch), and remove it from page.tsx. This actually matches the intent already documented in the code comment ("login = auto-present per the GFIBER-649 rule") regardless of which page a post-login redirect lands on.

4. Tests (TDD — write these alongside/before the code changes)

  • __tests__/dashboard-attendance-actions.test.ts: add a case asserting console.error is called with the upsert error (using the existing wireAnonDb helper, no changes needed there), and a case asserting no log on success.
  • __tests__/protected-layout.test.ts: must be updated or it breaks — it currently does not mock @/app/(protected)/upsell-evaluator/attendance-actions, confirmed by direct inspection. Add a vi.fn() mock (same pattern as __tests__/upsell-evaluator-page.test.tsx), assert it's called once for any authenticated role and not called when unauthenticated, add mockClear() to the existing beforeEach.
  • __tests__/upsell-evaluator-page.test.tsx: no required changes (it mocks the whole module and never asserts the call happened).

Risks / edge cases (validated)

  • Cross-agent / backdating abuse: impossible — WITH CHECK binds agent_id = auth.uid() (server-derived, unspoofable) and date = current_date (evaluated server-side).
  • UTC timezone assumption: client computes new Date().toISOString() (always UTC); Postgres current_date resolves in the DB session timezone, which is UTC by default and not overridden in supabase/config.toml. A future non-UTC DB timezone change could cause near-midnight false rejects (logged now, not silent) — not a data-safety issue, just worth a code comment.

Verification

No pgTAP/DB-level test harness exists in this repo — RLS behavior must be verified against a live Supabase project:

  1. npm run test — full suite green, including the updated protected-layout and dashboard-attendance-actions tests.
  2. supabase db push to dev (vshcimexazocttjmyrqy); confirm the migration applies cleanly.
  3. Inspect live state: select policyname, cmd, with_check from pg_policies where tablename='attendance'; — confirm both the new self-check-in policy and the existing admin policy are present.
  4. Log in as a real non-admin (role='user') test account in dev, land on any protected page (not just Upsell Evaluator, to confirm the layout move), then confirm a status='P' row exists for that agent/today via the dashboard grid or a direct query.
  5. As an admin, confirm the dashboard's manual attendance marking/clearing still works (the admin FOR ALL policy is untouched).
  6. Reload the same session same-day — confirm no duplicate row or error (exercises ON CONFLICT DO NOTHING).
  7. Push the same migration to production (hwxkegbdnbrzvekrqxbd) and repeat step 4 against a real prod account.