WEB-1040: [Playwright] Client lifecycle — illegal state-transition ma…

[devvaansh]

Description

This PR adds a data-table-driven negative-path E2E spec that locks down Fineract's client state machine against illegal state transitions.

The existing lifecycle suite (activate.spec.ts, reject.spec.ts, reactivate-after-close.spec.ts etc.) covers only happy paths. Without negative-path coverage, a future Fineract command addition or backend regression could silently accept an illegal transition — corrupting client financial history — and CI would not catch it.

Three illegal transitions are asserted as a MATRIX data table:

From state	Command	Why illegal
Active	activate	Client already active — would silently reset activation date
Pending	reactivate	Only closed clients may be reactivated — skips approval workflow
Active	reject	Only pending-activation clients may be rejected — leaves active loans ownerless

Each row asserts two invariants:

1. Fineract returns HTTP ≥ 400 with the expected InvalidClientStateTransitionException error-code in the body (the same JSON the UI snackbar renders).
2. A follow-up GET /clients/{id} confirms status.value is unchanged — proving no partial state mutation occurred.

Adding a new illegal transition to guard in future requires only a new row in MATRIX — no structural changes to the spec.

fineract-api.ts gains tryExecuteClientCommand() — a non-throwing sibling of executeClientCommand that returns { ok, status, bodyText } needed for negative-path assertions where the error body is the assertion target.

Dependencies: None beyond the existing Playwright infrastructure already in this repo.

---

Related issues and discussion

WEB-1040 https://mifosforge.jira.com/browse/WEB-1040

---

Screenshots, if any

No UI changes — API-layer tests only (see note below).

---

Checklist

• If you have multiple commits please combine them into one commit by squashing them.
• Read and understood the contribution guidelines at CONTRIBUTING.md.

---

[coderabbitai]

Note

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)

Validation error: Unrecognized key: "pre_merge_checks"

⚙️ Configuration instructions

• Please see the configuration documentation for more information.
• You can also validate your configuration using the online YAML validator.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Walkthrough

Adds Fineract API fixture helpers for non-throwing client commands and reason-code setup, plus a data-driven Playwright suite that verifies illegal client lifecycle commands return expected errors without changing client status.

Changes

Client lifecycle negative paths

Layer / File(s)	Summary
Client command and reason-code fixture support playwright/fixtures/fineract-api.ts	Adds client rejection and withdrawal reason identifiers, reason-code provisioning helpers, and tryExecuteClientCommand() returning HTTP status and response text without throwing.
Illegal transition matrix and assertions playwright/tests/clients/lifecycle/illegal-transitions.spec.ts	Defines predecessor-state setup, illegal command payloads, expected error patterns, and assertions that failed commands leave status.value unchanged.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant IllegalTransitionMatrix
  participant FineractApiClient
  participant FineractAPI
  IllegalTransitionMatrix->>FineractApiClient: Execute illegal client command
  FineractApiClient->>FineractAPI: POST client command
  FineractAPI-->>FineractApiClient: Error status and response body
  FineractApiClient-->>IllegalTransitionMatrix: Return ok, status, bodyText
  IllegalTransitionMatrix->>FineractApiClient: Fetch client state
  FineractApiClient->>FineractAPI: GET client
  FineractAPI-->>IllegalTransitionMatrix: Return unchanged status.value

Loading

Possibly related PRs

• openMF/web-app#3722: Adds overlapping Fineract API fixture helpers for client rejection and withdrawal reason codes.
• openMF/web-app#3725: Adds overlapping client lifecycle action and reason-seeding helpers used by negative-path tests.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 clearly matches the main change: a Playwright client lifecycle illegal state-transition test.

✨ 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

playwright/fixtures/fineract-api.ts (1)

132-135: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Remove the duplicate deleteClient method
playwright/fixtures/fineract-api.ts defines deleteClient twice (132-135 and 712-715). Keep only one implementation; TypeScript rejects duplicate method bodies, and the later one would override the earlier behavior in emitted JS.

🤖 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/fixtures/fineract-api.ts` around lines 132 - 135, Remove the
duplicate deleteClient method from the Fineract API fixture, keeping the
existing implementation at the other definition site. Ensure the class retains
exactly one deleteClient(clientId: number) method with its current response
validation behavior.

🤖 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.

Outside diff comments:
In `@playwright/fixtures/fineract-api.ts`:
- Around line 132-135: Remove the duplicate deleteClient method from the
Fineract API fixture, keeping the existing implementation at the other
definition site. Ensure the class retains exactly one deleteClient(clientId:
number) method with its current response validation behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 22848769-e5e2-4b21-8a13-485c20858496

📥 Commits

Reviewing files that changed from the base of the PR and between a2cbe85 and 45e606d.

📒 Files selected for processing (2)

• playwright/fixtures/fineract-api.ts
• playwright/tests/clients/lifecycle/illegal-transitions.spec.ts