Web 1039 playwright client lifecycle happy paths e 2 e specs#3725
Conversation
|
Note
|
| Layer / File(s) | Summary |
|---|---|
Pending client lifecycle actions playwright/tests/clients/lifecycle/activate.spec.ts, playwright/tests/clients/lifecycle/reject.spec.ts, playwright/tests/clients/lifecycle/withdraw.spec.ts |
Adds E2E coverage for Pending clients transitioning to Active, Rejected, or Withdrawn, including snackbar, status, and timeline assertions. |
Closed and rejected client reversals playwright/tests/clients/lifecycle/reactivate-after-close.spec.ts, playwright/tests/clients/lifecycle/undo-rejection.spec.ts |
Adds E2E coverage for returning Closed or Rejected clients to Pending and validates cleared or preserved timeline fields. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Possibly related PRs
- openMF/web-app#3686: Adds or changes the
createTestClient()factory and related test fixtures used by these specs. - openMF/web-app#3699: Introduces Playwright infrastructure used by the lifecycle tests, including client setup and authentication initialization.
- openMF/web-app#3722: Adds client action page objects, selectors, and seeded state helpers for these lifecycle flows.
Suggested reviewers: iohacker
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title is clearly related to the PR’s main change: adding Playwright client lifecycle happy-path e2e specs. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
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 help to get the list of available commands.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
playwright/utils/api-setup-manager.ts (1)
184-184: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid using
anyfor API response types.The
getClientTemplatemethod returnsPromise<any>. Based on learnings, you should introduce specific interfaces or types for API response shapes rather than usingany. If typing the entire API response layer is too broad for this PR, please track it as a separate enhancement backlog item.🤖 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 `@playwright/utils/api-setup-manager.ts` at line 184, Replace the Promise<any> return type on getClientTemplate with a specific interface or response type matching the API payload shape, and update the method implementation as needed to satisfy it. Avoid broad any usage; if the full response layer cannot be typed within this change, record that broader work as a separate enhancement backlog item.Source: Learnings
🤖 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.
Inline comments:
In `@playwright/factories/client.ts`:
- Around line 137-188: The cleanup function is currently defined too late to
remove clients when a post-creation reason lookup or state transition fails.
Move the cleanup definition immediately after resolving clientId, wrap the state
transition switch in a catch that awaits cleanup before rethrowing the original
error, and preserve the existing cleanup behavior and returned cleanup handle.
In `@playwright/tests/clients/lifecycle/illegal-transitions.spec.ts`:
- Line 206: Add the missing non-throwing tryExecuteClientCommand method to
FineractApiClient, accepting clientId, command, and payload, posting to the
client command endpoint, and returning the ok, status, and bodyText fields
consumed by the lifecycle spec. Preserve the raw response status and text while
avoiding exceptions for unsuccessful responses.
- Around line 26-31: Add UI coverage in illegal-transitions.spec.ts that
arranges each predecessor client state, opens the actions menu through
ClientViewPage, and asserts the disallowed action is absent using stable
selectors. Keep the existing API-only transition matrix unchanged as separate
backend enforcement coverage, and structure the new checks with clear
Arrange-Act-Assert phases and minimal timing assumptions.
---
Nitpick comments:
In `@playwright/utils/api-setup-manager.ts`:
- Line 184: Replace the Promise<any> return type on getClientTemplate with a
specific interface or response type matching the API payload shape, and update
the method implementation as needed to satisfy it. Avoid broad any usage; if the
full response layer cannot be typed within this change, record that broader work
as a separate enhancement backlog item.
🪄 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: d85030c8-f30c-4c2f-bac0-5bd067e94dce
📒 Files selected for processing (20)
playwright.config.tsplaywright/config/selectors.tsplaywright/factories/client.spec.tsplaywright/factories/client.tsplaywright/factories/index.tsplaywright/fixtures/fineract-api.tsplaywright/pages/client-actions/activate-client.page.tsplaywright/pages/client-actions/reactivate-client.page.tsplaywright/pages/client-actions/reject-client.page.tsplaywright/pages/client-actions/transfer-client.page.tsplaywright/pages/client-actions/withdraw-client.page.tsplaywright/pages/index.tsplaywright/tests/clients/lifecycle/activate.spec.tsplaywright/tests/clients/lifecycle/illegal-transitions.spec.tsplaywright/tests/clients/lifecycle/reactivate-after-close.spec.tsplaywright/tests/clients/lifecycle/reject.spec.tsplaywright/tests/clients/lifecycle/undo-rejection.spec.tsplaywright/tests/clients/lifecycle/withdraw.spec.tsplaywright/utils/api-setup-manager.spec.tsplaywright/utils/api-setup-manager.ts
588e161 to
4623b9f
Compare
4623b9f to
55facad
Compare
55facad to
c19dba9
Compare
Adds 5 happy-path specs under playwright/tests/clients/lifecycle/:
- activate.spec.ts — Pending → Active
- reject.spec.ts — Pending → Rejected
- undo-rejection.spec.ts — Rejected → Pending (Undo Rejection)
- withdraw.spec.ts — Pending → Withdrawn
- reactivate-after-close.spec.ts — Closed → Pending (Reactivate)
Each spec: API arrange → UI act → snackbar assert + GET /clients/{id}
status assert + timeline entry check. All 5 pass locally.
c19dba9 to
647e4a4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
playwright/tests/clients/lifecycle/undo-rejection.spec.ts (1)
60-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the Undo Rejection date field out of the spec.
input[formcontrolname="reopenedDate"]locks this E2E test to an Angular form control, andwaitFor({ state: 'visible', timeout: 30000 })adds a brittle 30-second delay instead of asserting readiness normally. Add a stabledata-testidfor the Undo RejectionreopenedDatefield and use an assertion-based locator, ideally through the client-action page object.🤖 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 `@playwright/tests/clients/lifecycle/undo-rejection.spec.ts` around lines 60 - 63, The Undo Rejection date interaction is coupled to an Angular form-control selector and an explicit wait. Add a stable data-testid to the reopenedDate field, expose or reuse it through the client-action page object, and update the test to locate it there and use an assertion-based readiness check before filling and blurring, removing the 30-second waitFor call.Source: Path instructions
🤖 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.
Inline comments:
In `@playwright/tests/clients/lifecycle/reject.spec.ts`:
- Around line 20-21: Update the lifecycle test documentation near the listed
assertions to reference preservation of submittedOnDate rather than an assertion
of rejectedOnDate, accurately reflecting the API response and existing test
behavior.
---
Nitpick comments:
In `@playwright/tests/clients/lifecycle/undo-rejection.spec.ts`:
- Around line 60-63: The Undo Rejection date interaction is coupled to an
Angular form-control selector and an explicit wait. Add a stable data-testid to
the reopenedDate field, expose or reuse it through the client-action page
object, and update the test to locate it there and use an assertion-based
readiness check before filling and blurring, removing the 30-second waitFor
call.
🪄 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 Plus
Run ID: 02519824-879a-4975-a985-b58d1c3d126c
📒 Files selected for processing (5)
playwright/tests/clients/lifecycle/activate.spec.tsplaywright/tests/clients/lifecycle/reactivate-after-close.spec.tsplaywright/tests/clients/lifecycle/reject.spec.tsplaywright/tests/clients/lifecycle/undo-rejection.spec.tsplaywright/tests/clients/lifecycle/withdraw.spec.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- playwright/tests/clients/lifecycle/withdraw.spec.ts
- playwright/tests/clients/lifecycle/reactivate-after-close.spec.ts
Description
Added Playwright end-to-end specs covering all five client lifecycle state transitions. Previously there was zero automated coverage of the client lifecycle
Each spec follows the pattern: API sets up the predecessor state → Playwright drives the UI transition → asserts both the snackbar message (UI layer) and
GET /clients/{id}status + timeline (API layer). The dual assertion ensures the backend state actually changed, not just the UI.Transitions covered:
activate.spec.ts)reject.spec.ts)undo-rejection.spec.ts)withdraw.spec.ts)reactivate-after-close.spec.ts)Dependencies: Requires WEB-1037(#{WEB-1037 PR number}) to be merged first — this PR depends on the client action page objects, seeded factory, and
ApiSetupManagerintroduced there.Related issues and discussion
#WEB-1039 https://mifosforge.jira.com/browse/WEB-1039
Screenshots, if any
Screen.Recording.2026-07-25.at.12.20.25.AM.mov
Checklist
web-app/.github/CONTRIBUTING.md.Summary by CodeRabbit
Summary by CodeRabbit