feat(e2e): payment-status write-path E2E (t03)#83
Conversation
extend seed with plano + inadimplente aluno + matricula vencida fixtures add payment-status.spec.ts: gerente registers payment, verifies persistence on reload update CRITICAL-PATHS.md 19 → 20 scenarios Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 29 minutes and 14 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThis PR adds deterministic E2E seed data for a delinquent student enrollment, introduces a new Playwright test validating the payment-registration write-path from the financeiro dashboard, and updates docs to reflect the new scenario coverage and state. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Playwright Test
participant Browser as Browser/UI
participant App as Server/App
participant DB as Database
Test->>Browser: navigate to /dashboard/financeiro (GERENTE)
Browser->>App: GET dashboard/financeiro
App->>DB: query alunos/matriculas
DB-->>App: return seeded INADIMPLENTE aluno + matricula
App-->>Browser: render table with aluno row
Test->>Browser: click "Registrar Pagamento" on aluno row
Browser->>App: POST /financeiro/matricula/:id/pagar
App->>DB: update matricula status -> ATIVA, record payment
DB-->>App: confirmation
App-->>Browser: 200 OK (Pagamento Registrado!)
Browser-->>Test: show success toast
Test->>Browser: reload page
Browser->>App: GET dashboard/financeiro
App->>DB: return updated data (aluno no longer inadimplente)
DB-->>App: updated matricula/aluno
App-->>Browser: render updated state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/e2e/CRITICAL-PATHS.md (1)
44-44:⚠️ Potential issue | 🟡 MinorUpdate local run docs to the new scenario count.
Line 44 still says
run all 19 scenarios, but this document now states 20 scenarios.📝 Suggested doc fix
-npm run e2e # run all 19 scenarios +npm run e2e # run all 20 scenarios🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/CRITICAL-PATHS.md` at line 44, Update the documentation text that reads "run all 19 scenarios" to reflect the new count by changing it to "run all 20 scenarios" where the npm run e2e example is shown (the line containing `npm run e2e # run all 19 scenarios`); ensure only the scenario count in that comment is updated to 20.
🧹 Nitpick comments (1)
tests/e2e/specs/payment-status.spec.ts (1)
10-15: Prefer row-scoped status assertion to reduce false positives.
page.getByText('INADIMPLENTE')is global. Scoping status check to the selected aluno row makes the assertion tighter when multiple inadimplentes exist.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/specs/payment-status.spec.ts` around lines 10 - 15, The status assertion is currently global (page.getByText('INADIMPLENTE')) and can produce false positives; scope that check to the selected aluno row by using the existing row variable (e.g., replace page.getByText('INADIMPLENTE') with row.getByText('INADIMPLENTE') or row.getByRole('cell', { name: 'INADIMPLENTE' })) so the visibility assertion targets the specific row before calling row.getByRole('button', { name: 'Registrar Pagamento' }).click().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@prisma/seed-e2e.ts`:
- Around line 139-142: The upsert calls (e.g., prisma.plano.upsert using
planoE2eId and the other upserts around lines 166-169) currently use update: {}
which preserves any mutated fields from prior E2E runs; change each upsert to
explicitly set the fields that must be deterministic on update (for example
reset enrollment/payment state fields such as matricula to the baseline value
like "VENCIDA", payment/status flags, timestamps, etc.) so the create and update
paths produce the same baseline; update the prisma.plano.upsert and the other
referenced upsert calls to include those explicit update assignments instead of
an empty object.
In `@tests/e2e/CRITICAL-PATHS.md`:
- Line 30: The scenario wording over-claims verification: in
payment-status.spec.ts the test only asserts removal from the inadimplente table
but the CRITICAL-PATHS.md entry says `INADIMPLENTE aluno → ATIVA`; either update
the MD wording to reflect "removed from inadimplente" or strengthen the test to
also assert the student status is set to "ATIVA" (e.g., add an assertion that
the student's status field or API returns "ATIVA" after payment processing).
Locate the spec by the filename payment-status.spec.ts and the existing
assertion against the inadimplente table, then add the status check or adjust
the CRITICAL-PATHS.md line to match the actual assertion scope.
In `@tests/e2e/specs/payment-status.spec.ts`:
- Around line 5-31: The test currently only verifies the aluno disappears from
the INADIMPLENTE list but doesn't assert it became ATIVA; update the test
'GERENTE registers payment: INADIMPLENTE aluno → ATIVA' to explicitly check for
the ATIVA status after confirming payment: after the page.reload() find the same
aluno row (e.g., using page.getByRole('row', { name: /Aluno Inadimplente E2E/
})) or navigate to the aluno detail and assert the status cell or heading
contains 'ATIVA' (or, if you prefer to keep the current assertion only, rename
the test title to reflect that it only removes the aluno from the inadimplente
list).
---
Outside diff comments:
In `@tests/e2e/CRITICAL-PATHS.md`:
- Line 44: Update the documentation text that reads "run all 19 scenarios" to
reflect the new count by changing it to "run all 20 scenarios" where the npm run
e2e example is shown (the line containing `npm run e2e # run all 19
scenarios`); ensure only the scenario count in that comment is updated to 20.
---
Nitpick comments:
In `@tests/e2e/specs/payment-status.spec.ts`:
- Around line 10-15: The status assertion is currently global
(page.getByText('INADIMPLENTE')) and can produce false positives; scope that
check to the selected aluno row by using the existing row variable (e.g.,
replace page.getByText('INADIMPLENTE') with row.getByText('INADIMPLENTE') or
row.getByRole('cell', { name: 'INADIMPLENTE' })) so the visibility assertion
targets the specific row before calling row.getByRole('button', { name:
'Registrar Pagamento' }).click().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 38444f74-fbc3-49ea-bd72-0809c60884a1
📒 Files selected for processing (3)
prisma/seed-e2e.tstests/e2e/CRITICAL-PATHS.mdtests/e2e/specs/payment-status.spec.ts
| | 17 | `enrollment.spec.ts` | GERENTE creates aluno → appears in list | ✅ Covered | | ||
| | 18 | `instrutor-workflow.spec.ts` | INSTRUTOR assigns workout → ALUNO sees "Do Personal" badge | ✅ Covered | | ||
| | 19 | `instrutor-auth-negative.spec.ts` | RECEPCIONISTA/ALUNO blocked from /dashboard/treinos | ✅ Covered | | ||
| | 20 | `payment-status.spec.ts` | GERENTE registers payment: INADIMPLENTE aluno → ATIVA | ✅ Covered | |
There was a problem hiding this comment.
Scenario wording currently over-claims ATIVA verification.
The listed scenario says INADIMPLENTE aluno → ATIVA, but the current spec only asserts removal from the inadimplente table. Please align wording with actual assertion scope (or strengthen the test to verify ATIVA).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/e2e/CRITICAL-PATHS.md` at line 30, The scenario wording over-claims
verification: in payment-status.spec.ts the test only asserts removal from the
inadimplente table but the CRITICAL-PATHS.md entry says `INADIMPLENTE aluno →
ATIVA`; either update the MD wording to reflect "removed from inadimplente" or
strengthen the test to also assert the student status is set to "ATIVA" (e.g.,
add an assertion that the student's status field or API returns "ATIVA" after
payment processing). Locate the spec by the filename payment-status.spec.ts and
the existing assertion against the inadimplente table, then add the status check
or adjust the CRITICAL-PATHS.md line to match the actual assertion scope.
| test('GERENTE registers payment: INADIMPLENTE aluno → ATIVA', async ({ page }) => { | ||
| await loginAs(page, 'GERENTE'); | ||
| await page.goto('/dashboard/financeiro'); | ||
|
|
||
| // Verify INADIMPLENTE aluno is present in the table | ||
| await expect(page.getByText('Aluno Inadimplente E2E')).toBeVisible({ timeout: 15_000 }); | ||
| await expect(page.getByText('INADIMPLENTE')).toBeVisible(); | ||
|
|
||
| // Click "Registrar Pagamento" scoped to the aluno's row | ||
| const row = page.getByRole('row', { name: /Aluno Inadimplente E2E/ }); | ||
| await row.getByRole('button', { name: 'Registrar Pagamento' }).click(); | ||
|
|
||
| // AlertDialog should open | ||
| const dialog = page.getByRole('alertdialog'); | ||
| await expect(dialog).toBeVisible(); | ||
| await expect(dialog.getByRole('heading', { name: 'Confirmar Pagamento?' })).toBeVisible(); | ||
|
|
||
| // Confirm payment | ||
| await dialog.getByRole('button', { name: 'Confirmar e Reativar' }).click(); | ||
|
|
||
| // Assert success toast | ||
| await expect(page.getByText('Pagamento Registrado!')).toBeVisible({ timeout: 10_000 }); | ||
|
|
||
| // Reload and verify persistence — aluno should no longer appear in inadimplente list | ||
| await page.reload(); | ||
| await expect(page.getByText('Aluno Inadimplente E2E')).not.toBeVisible({ timeout: 15_000 }); | ||
| }); |
There was a problem hiding this comment.
The test does not prove the claimed INADIMPLENTE → ATIVA transition.
Right now it only proves the aluno leaves the inadimplente list. It can still pass if the status becomes something other than ATIVA. Either add an explicit ATIVA verification surface or rename the scenario to match what is actually asserted.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/e2e/specs/payment-status.spec.ts` around lines 5 - 31, The test
currently only verifies the aluno disappears from the INADIMPLENTE list but
doesn't assert it became ATIVA; update the test 'GERENTE registers payment:
INADIMPLENTE aluno → ATIVA' to explicitly check for the ATIVA status after
confirming payment: after the page.reload() find the same aluno row (e.g., using
page.getByRole('row', { name: /Aluno Inadimplente E2E/ })) or navigate to the
aluno detail and assert the status cell or heading contains 'ATIVA' (or, if you
prefer to keep the current assertion only, rename the test title to reflect that
it only removes the aluno from the inadimplente list).
reflect 40/40 tests, 20 e2e scenarios, it5 deliverables table t01-t10 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
scope INADIMPLENTE assertion to row locator (strict mode: 5 elements matched) add empty-state check after reload to verify ATIVA persistence reset plano/matricula fields in seed upsert update clause for re-run safety fix CRITICAL-PATHS.md: run comment 19 → 20 scenarios Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/CURRENT-STATE.md`:
- Line 27: The table entry in docs/CURRENT-STATE.md incorrectly states “7
fixtures” for the E2E seed script; update that cell to either correct the
numeric count to match the listed entities or replace the count with a
descriptive phrase (e.g., “deterministic fixtures for
auth/admin/student/payment-status flows”) so it cannot drift—edit the string
"`E2E seed script` | ✅ Done | `prisma/seed-e2e.ts` (7 fixtures: ... )`" to the
corrected text ensuring the file reference `prisma/seed-e2e.ts` remains and the
parenthetical is either an accurate count or the suggested descriptive wording.
- Line 29: The "T03/CI/E2E" status rows (e.g., the "CI E2E job" table row)
contain conflicting state text—mixing "✅ Green", "awaiting CI green", and
"scenario `#20` still needs local run"—so normalize each T03/CI/E2E entry to a
single authoritative status string; pick and use one source-of-truth field (for
example: a single "Status" column value like "✅ Green" or "Awaiting CI") and
remove or consolidate the other phrases in the same cell, and apply the same
normalization to the other occurrences mentioned (lines referenced around 54-55
and 65) so each table row contains only one clear status per T03/CI/E2E entry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 39856649-f548-4580-85df-80bcffe380a0
📒 Files selected for processing (4)
docs/CURRENT-STATE.mdprisma/seed-e2e.tstests/e2e/CRITICAL-PATHS.mdtests/e2e/specs/payment-status.spec.ts
✅ Files skipped from review due to trivial changes (1)
- tests/e2e/CRITICAL-PATHS.md
🚧 Files skipped from review as they are similar to previous changes (2)
- prisma/seed-e2e.ts
- tests/e2e/specs/payment-status.spec.ts
| | Ops documentation | ✅ Done | Runbook, SLOs, threat model | | ||
| | Process documentation | ✅ Done | RFC + Postmortem templates | | ||
| | Local E2E stack | ✅ Done | `supabase start` (Docker) | | ||
| | E2E seed script | ✅ Done | `prisma/seed-e2e.ts` (7 fixtures: 4 staff+aluno + 1 treino + INADIMPLENTE aluno + plano + matrícula) | |
There was a problem hiding this comment.
Fixture count in E2E seed note appears incorrect.
The note says “7 fixtures” but the listed entities add up beyond 7. Please correct the count or simplify wording (e.g., “deterministic fixtures for auth/admin/student/payment-status flows”) to avoid drift.
Suggested wording update
-| E2E seed script | ✅ Done | `prisma/seed-e2e.ts` (7 fixtures: 4 staff+aluno + 1 treino + INADIMPLENTE aluno + plano + matrícula) |
+| E2E seed script | ✅ Done | `prisma/seed-e2e.ts` (deterministic fixtures for auth/admin/student + treino + payment-status flow) |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | E2E seed script | ✅ Done | `prisma/seed-e2e.ts` (7 fixtures: 4 staff+aluno + 1 treino + INADIMPLENTE aluno + plano + matrícula) | | |
| | E2E seed script | ✅ Done | `prisma/seed-e2e.ts` (deterministic fixtures for auth/admin/student + treino + payment-status flow) | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/CURRENT-STATE.md` at line 27, The table entry in docs/CURRENT-STATE.md
incorrectly states “7 fixtures” for the E2E seed script; update that cell to
either correct the numeric count to match the listed entities or replace the
count with a descriptive phrase (e.g., “deterministic fixtures for
auth/admin/student/payment-status flows”) so it cannot drift—edit the string
"`E2E seed script` | ✅ Done | `prisma/seed-e2e.ts` (7 fixtures: ... )`" to the
corrected text ensuring the file reference `prisma/seed-e2e.ts` remains and the
parenthetical is either an accurate count or the suggested descriptive wording.
getByText('Pagamento Registrado!') matched both <li role="status"> and its
child <div> — 2 elements, strict mode throws. Fix: scope to
getByRole('status').filter({ hasText }) to target the toast container only.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Env var was silently defaulting to '' — first auth call would fail with a cryptic Supabase "Invalid API key" instead of a clear missing-env message. Mirrors the existing DATABASE_URL guard pattern. Also corrects a stale CURRENT-STATE.md row that showed CI as green while the companion row correctly stated PR #83 was awaiting CI. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat(e2e): add payment-status write-path scenario (t03)
extend seed with plano + inadimplente aluno + matricula vencida fixtures
add payment-status.spec.ts: gerente registers payment, verifies persistence on reload
update CRITICAL-PATHS.md 19 → 20 scenarios
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* docs: update CURRENT-STATE.md for t01+t02 merged, t03 in review
reflect 40/40 tests, 20 e2e scenarios, it5 deliverables table t01-t10
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(e2e): fix payment-status strict-mode violation + seed idempotency
scope INADIMPLENTE assertion to row locator (strict mode: 5 elements matched)
add empty-state check after reload to verify ATIVA persistence
reset plano/matricula fields in seed upsert update clause for re-run safety
fix CRITICAL-PATHS.md: run comment 19 → 20 scenarios
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(e2e): fix toast strict-mode violation in payment-status test
getByText('Pagamento Registrado!') matched both <li role="status"> and its
child <div> — 2 elements, strict mode throws. Fix: scope to
getByRole('status').filter({ hasText }) to target the toast container only.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(seed): add fail-fast guard for SUPABASE_SERVICE_ROLE_KEY
Env var was silently defaulting to '' — first auth call would fail with
a cryptic Supabase "Invalid API key" instead of a clear missing-env
message. Mirrors the existing DATABASE_URL guard pattern.
Also corrects a stale CURRENT-STATE.md row that showed CI as green
while the companion row correctly stated PR #83 was awaiting CI.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Emiya Kiritsugu <emiyakiritsugu3@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
prisma/seed-e2e.tswith three new deterministic fixtures:Plano(UUID...0020), INADIMPLENTEAluno(UUID...0005), andMatricula VENCIDA(UUID...0030) — no auth user needed since the GERENTE is the actortests/e2e/specs/payment-status.spec.ts: GERENTE navigates to/dashboard/financeiro, triggers "Registrar Pagamento" AlertDialog for the INADIMPLENTE aluno, confirms, asserts toast "Pagamento Registrado!", reloads, and verifies the aluno is no longer in the listtests/e2e/CRITICAL-PATHS.md: 19 → 20 scenarios; removes payment-status from Pending tableTest plan
npm run test→ 40/40 passing (no unit test changes)npm run typecheck→ 0 errorsnpm run lint→ 0 errorsnpm run seed:e2e && npm run e2e -- payment-status→ 1/1 passingnpm run e2e→ 20/20 passing🤖 Generated with Claude Code
Summary by CodeRabbit
Tests
Chores
Documentation