Panel P5.3.2: Playwright e2e for the check-in scan→verdict flow - #98
Conversation
…e migration 000002's silent no-op
…t selector flakiness
The seeded admin@test.com account's real bcrypt-verified password is "password" (matches backend/migrations/seed.sql's own comment), but scripts/seed.sh, scripts/start-all.sh, and .github/SECURITY.md all printed/documented "password123" as the hint. Corrected all three. Also wires a new e2e-panel CI job (Playwright, backend+Postgres up, npm run e2e -w panel) after docker-build-image. It's a soft check — deliberately excluded from ci-success's needs: until proven stable.
…a fresh CI database
github-actions
Bot
added
documentation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds Playwright coverage for the panel check-in scan-to-verdict flow, repairs missing ChangesPanel check-in E2E
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Playwright
participant BackendAPI
participant Postgres
participant Panel
Playwright->>BackendAPI: Seed event and check-in fixtures
BackendAPI->>Postgres: Store seeded data
Playwright->>Panel: Inject JWT and open station page
Panel->>BackendAPI: Submit badge scan
BackendAPI-->>Panel: Return verdict
Panel-->>Playwright: Render verdict card
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
backend/migrations/000025_fix_event_staff_columns.up.sql (1)
14-16: 🩺 Stability & Availability | 🔵 TrivialConsider the locking impact of adding columns with defaults and foreign keys.
Adding a
DEFAULTwith a volatile function likegen_random_uuid()requires a table rewrite and anACCESS EXCLUSIVElock, which blocks all reads and writes. Additionally, adding a foreign key constraint requires a table scan and aSHARE ROW EXCLUSIVElock on both tables.If
event_staffis large in production, consider breaking this into safer, non-blocking steps:
- Add the
idcolumn as nullable.- Backfill the IDs in batches.
- Set the column to
NOT NULLand add the default.- Add the foreign key constraint with
NOT VALID, then runVALIDATE CONSTRAINTin a separate transaction.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/migrations/000025_fix_event_staff_columns.up.sql` around lines 14 - 16, Refactor the event_staff migration to avoid a table rewrite and long blocking locks: add id as nullable without a default, backfill UUIDs in batches, then enforce NOT NULL and add the gen_random_uuid() default. Add the assigned_by foreign key as NOT VALID and validate it separately, preserving the existing users(id) relationship.Source: Linters/SAST tools
.github/workflows/ci.yml (1)
216-216: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDisable credential persistence on checkout for this job.
This job runs several supply-chain-exposed commands (
npm ci,npx playwright install --with-deps,go run main.go) with the ephemeralGITHUB_TOKENstill persisted in git config for the whole job. Impact is bounded by the job's already-minimalcontents: readpermission, but explicitly dropping it is a one-line hardening step with no downside.🔒 Proposed fix
- uses: actions/checkout@v5 + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml at line 216, Update the actions/checkout@v5 step to disable credential persistence by configuring persist-credentials as false, ensuring the ephemeral GITHUB_TOKEN is not retained in git configuration for the remainder of the job.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/ci.yml:
- Line 216: Update the actions/checkout@v5 step to disable credential
persistence by configuring persist-credentials as false, ensuring the ephemeral
GITHUB_TOKEN is not retained in git configuration for the remainder of the job.
In `@backend/migrations/000025_fix_event_staff_columns.up.sql`:
- Around line 14-16: Refactor the event_staff migration to avoid a table rewrite
and long blocking locks: add id as nullable without a default, backfill UUIDs in
batches, then enforce NOT NULL and add the gen_random_uuid() default. Add the
assigned_by foreign key as NOT VALID and validate it separately, preserving the
existing users(id) relationship.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f114e740-9f2a-484f-a9e0-e9c0f302509f
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
.github/SECURITY.md.github/workflows/ci.yml.superpowers/sdd/progress.mdbackend/migrations/000025_fix_event_staff_columns.down.sqlbackend/migrations/000025_fix_event_staff_columns.up.sqldocs/superpowers/plans/2026-07-21-panel-p5.3.2-e2e-checkin.mddocs/superpowers/specs/2026-07-21-panel-p5.3.2-e2e-checkin-design.mdpanel/e2e/checkin.spec.tspanel/e2e/fixtures/seedCheckinEvent.tspanel/package.jsonpanel/playwright.config.tspanel/vitest.config.tsscripts/seed.shscripts/start-all.sh
|
Thanks for the review — both nitpicks addressed:
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
Panel P5.3.2 — Playwright e2e for the check-in scan→verdict flow
Second of five P5.3 sub-cycles. Ships the first Playwright e2e suite for
panel/— none existed before (only an unrelated, CI-unwired local harness inlanding/). Covers the check-in station's scan→verdict flow against a real backend + Postgres, no mocks.Spec:
docs/superpowers/specs/2026-07-21-panel-p5.3.2-e2e-checkin-design.md· Plan:docs/superpowers/plans/2026-07-21-panel-p5.3.2-e2e-checkin.md(amended mid-execution — see below)What changed
panel/e2e/— a self-contained seed helper (fixtures/seedCheckinEvent.ts) drives the real backend API (login, create event, attendee, badge template, staff, checkin-settings with printing off, station) to reachreadiness.ready === true, thencheckin.spec.tsinjects the JWT intolocalStorage(bypassing the login UI — the route guard only checks token presence) and drives the real station UI. One test covers all three reachable verdicts from one setup:checked_in→already_checked_in→not_found.@playwright/test(chromium only, matchinglanding/'s precedent),panel/playwright.config.ts(locale pinned toen-USto avoid host-OS-locale-dependent selector flakiness).panel/vitest.config.ts— excludese2e/**from Vitest's own discovery (its default glob matches*.spec.tstoo — without this, the CI-blockingtest-paneljob would break the moment the e2e spec existed).e2e-paneljob (dev-mode: real Postgres +go runbackend + panel dev server), deliberately soft/non-blocking (not inci-success'sneeds:) until proven stable over real PRs.scripts/seed.sh,scripts/start-all.sh,.github/SECURITY.md(saidpassword123; the real bcrypt-verified plaintext ispassword).A real production bug, found and fixed along the way
Standing up a live stack to seed the e2e event surfaced that
POST /api/events/{event_id}/staffhas always 500'd in every real deployment — migration000002'sCREATE TABLE IF NOT EXISTS event_staff (id, ..., assigned_by, ...)silently no-op'd because the table already existed (created narrower by migration000001), so the live schema never gained the columnsAssignStaffToEvent's INSERT writes to. Fixed with a new migration (000025, schema-only, no Go changes) — user-approved as an inline task in this cycle. Proven end-to-end with a real201response before and after.Verification
Final whole-branch review (Opus) caught 1 Critical: the new CI job would have failed on a genuinely fresh database (backend's onprem-mode bootstrap requires
IDENTO_ADMIN_EMAIL/IDENTO_ADMIN_PASSWORDon an empty DB, which the job didn't set) — the local sweep had passed only because the local dev Postgres had leftover state from earlier testing, masking it. Fixed, then empirically verified: destroyed the local DB volume, recreated it genuinely empty, and re-ran the full suite end to end against a from-scratch-bootstrapped backend — passed. Everything else (theevent_stafffix's safety, seed helper API contracts, readiness math, auth bypass, selectors, soft-gating) was independently verified sound with zero other findings.Deferred
blocked(zone-rule) verdict coverage, print/agent-integration coverage, cross-browser coverage. Promotinge2e-panelfrom soft to required once proven stable over real PRs. P5.3.3 (WCAG-AA a11y audit) follows.🤖 Generated with Claude Code
Summary by CodeRabbit