Web 1039 playwright client lifecycle happy paths e 2 e specs#3725
Web 1039 playwright client lifecycle happy paths e 2 e specs#3725devvaansh wants to merge 2 commits into
Conversation
devvaansh
changed the title
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (19)
Note
|
| Layer / File(s) | Summary |
|---|---|
Client seeding and API transitions playwright/factories/client.ts, playwright/factories/client.spec.ts, playwright/fixtures/fineract-api.ts, playwright/factories/index.ts |
Adds typed seeded-client creation, lifecycle transitions, cleanup handling, API helpers, and unit coverage for call sequences and return shapes. |
Client action page objects playwright/config/selectors.ts, playwright/pages/client-actions/*, playwright/pages/index.ts |
Adds selectors and page objects for client activation, rejection, withdrawal, reactivation, and transfer forms. |
Test execution and template caching playwright.config.ts, playwright/utils/api-setup-manager.ts, playwright/utils/api-setup-manager.spec.ts |
Adjusts Playwright project matching and adds memoized client-template access with concurrency and retry tests. |
Client lifecycle E2E scenarios playwright/tests/clients/lifecycle/* |
Adds valid activation, rejection, withdrawal, reactivation, undo-rejection, and illegal-transition scenarios with UI and API assertions. |
Estimated code review effort: 4 (Complex) | ~45 minutes
Sequence Diagram(s)
sequenceDiagram
participant Test as Lifecycle test
participant Page as Client action page
participant API as Fineract API
participant Client as Client state
Test->>API: seed client state
Test->>Page: open and submit action form
Page->>API: submit lifecycle command
API->>Client: update client state
Test->>API: verify status and timeline
Possibly related PRs
- openMF/web-app#3722: Overlaps in client factory, selector, and Playwright configuration changes.
- openMF/web-app#3699: Modifies the shared client test-data factory.
- openMF/web-app#3684: Provides related
ApiSetupManagercaching infrastructure.
Suggested reviewers: alberto-art3ch
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| 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. |
| 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 E2E specs. |
✨ 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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
6af90ed to
2285ae3
Compare
…verrides, and ApiSetupManager.getClientTemplate - Add 5 Layer-2 selector maps (activate/reject/withdraw/reactivate/transfer) - Add 5 client-action Page Objects under playwright/pages/client-actions/ - Extend client factory with ClientState union, CreateTestClientOverrides, and createSeededClient covering pending/active/rejected/withdrawn/closed with FK-safe cleanup deleters - Extend FineractApiClient with getClientTemplate, rejectClient, withdrawClient, ensureClientRejectionReason, ensureClientWithdrawalReason - Add ApiSetupManager.getClientTemplate domain wrapper with 'none' sentinel - Add 15 factory unit tests + 5 getClientTemplate wrapper tests (95 unit tests total, all passing) - Route factories/client.spec.ts through the 'unit' project; keep .factory.spec.ts under 'integration'
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.
588e161 to
4623b9f
Compare
1 participant
Description
Added Playwright end-to-end specs covering all five client lifecycle state transitions. Previously there was zero automated coverage of the client lifecycle — any regression in the Angular routing, form submission, or Fineract command binding would go undetected until QA or production.
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)illegal-transitions.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
All 6 specs pass (33 total suite passes).
Checklist
web-app/.github/CONTRIBUTING.md.Summary by CodeRabbit
New Features
Tests