WEB-1065: [Playwright] Active-client factory + sequential LIFO cleanup

CleanupGuard.flush() now awaits each deleter in turn instead of
dispatching all of them concurrently via Promise.allSettled. LIFO
ordering was previously only apparent: a child resource's DELETE could
still reach Fineract after its parent's, producing an FK violation.

Adds createActiveTestClient() with a monotonic date chain
(submitted -> activated -> account opening) and a local pre-flight guard,
so an inverted date fails with a message naming both fields rather than
an opaque Fineract validation error.

Fixes the factories barrel: client.ts and client.factory.ts both exported
createTestClient with incompatible signatures and only the pure builder
was reachable through the barrel. It is now buildTestClientPayload.

Author: devvaansh

playwright/factories/client.factory.spec.ts

@@ -7,7 +7,14 @@
  */
 
 import { test, expect } from '../fixtures/test-fixtures';
-import { createTestClient, DEFAULT_TEST_CLIENT_LASTNAME } from './client.factory';
+import {
+  createTestClient,
+  createActiveTestClient,
+  DEFAULT_TEST_CLIENT_LASTNAME,
+  DEFAULT_TEST_CLIENT_SUBMITTED_ON_DATE,
+  DEFAULT_TEST_CLIENT_ACTIVATION_DATE,
+  DEFAULT_ACCOUNT_OPENING_DATE
+} from './client.factory';
 import { E2E_NAME_PATTERN } from '../utils/naming';
 
 // Live-backend specs — run under the `integration` Playwright project
@@ -71,3 +78,83 @@ test.describe('createTestClient() against live Fineract', () => {
     await expect(apiSetup.api.getClient(client.resourceId)).rejects.toThrow(/404|not found/i);
   });
 });
+
+test.describe('createActiveTestClient() against live Fineract', () => {
+  test('creates a client already in Active status', async ({ apiSetup, cleanupGuard }) => {
+    const client = await createActiveTestClient(apiSetup, cleanupGuard);
+
+    const fetched = await apiSetup.api.getClient(client.resourceId);
+    expect(fetched.id).toBe(client.resourceId);
+    expect(fetched.active).toBe(true);
+    expect(fetched.status?.value).toBe('Active');
+    // The activation date must land one day after submission — this is
+    // the ordering every downstream account/charge factory relies on.
+    expect(fetched.timeline?.submittedOnDate).toEqual([
+      2024,
+      1,
+      1
+    ]);
+    expect(fetched.timeline?.activatedOnDate).toEqual([
+      2024,
+      1,
+      2
+    ]);
+  });
+
+  test('rejects an activation date that precedes submission', async ({ apiSetup, cleanupGuard }) => {
+    // Fails locally, before any HTTP request, so the error names the
+    // offending fields instead of surfacing as an opaque Fineract
+    // validation message.
+    await expect(
+      createActiveTestClient(apiSetup, cleanupGuard, {
+        submittedOnDate: '10 January 2024',
+        activationDate: '05 January 2024'
+      })
+    ).rejects.toThrow(/activationDate .* must not precede submittedOnDate/);
+    expect(cleanupGuard.size()).toBe(0);
+  });
+
+  test('rejects an unparseable date literal', async ({ apiSetup, cleanupGuard }) => {
+    await expect(createActiveTestClient(apiSetup, cleanupGuard, { activationDate: 'not-a-date' })).rejects.toThrow(
+      /unable to parse dates/
+    );
+    expect(cleanupGuard.size()).toBe(0);
+  });
+
+  test('exposes an account-opening date after the activation date', () => {
+    // Guards the constant chain itself: a future edit that bumps the
+    // activation date past the account-opening date would silently
+    // break every Track B/C factory that defaults to it.
+    expect(Date.parse(DEFAULT_ACCOUNT_OPENING_DATE)).toBeGreaterThan(Date.parse(DEFAULT_TEST_CLIENT_ACTIVATION_DATE));
+    expect(Date.parse(DEFAULT_TEST_CLIENT_ACTIVATION_DATE)).toBeGreaterThan(
+      Date.parse(DEFAULT_TEST_CLIENT_SUBMITTED_ON_DATE)
+    );
+  });
+
+  test('extra cannot override the validated active-client invariants', async ({ apiSetup, cleanupGuard }) => {
+    // `createTestClient` spreads `extra` after its own defaults, so an
+    // earlier implementation let `extra` replace `submittedOnDate` —
+    // meaning the date-order guard validated one pair while Fineract
+    // received another. The invariants must win.
+    const client = await createActiveTestClient(apiSetup, cleanupGuard, {
+      extra: {
+        active: false,
+        activationDate: '31 December 2023',
+        submittedOnDate: '20 January 2024'
+      }
+    });
+
+    const fetched = await apiSetup.api.getClient(client.resourceId);
+    expect(fetched.active).toBe(true);
+    expect(fetched.timeline?.submittedOnDate).toEqual([
+      2024,
+      1,
+      1
+    ]);
+    expect(fetched.timeline?.activatedOnDate).toEqual([
+      2024,
+      1,
+      2
+    ]);
+  });
+});

playwright/factories/client.factory.ts

@@ -45,6 +45,28 @@ export const DEFAULT_TEST_CLIENT_LASTNAME = 'E2E';
 /** Default submitted-on date applied to every pending test client. */
 export const DEFAULT_TEST_CLIENT_SUBMITTED_ON_DATE = '01 January 2024';
 
+/**
+ * Default activation date for {@link createActiveTestClient}.
+ *
+ * Deliberately one day AFTER {@link DEFAULT_TEST_CLIENT_SUBMITTED_ON_DATE}:
+ * Fineract rejects an activation that predates submission, and the
+ * resulting error message names neither field, so an accidental
+ * inversion surfaces as an opaque validation failure.
+ */
+export const DEFAULT_TEST_CLIENT_ACTIVATION_DATE = '02 January 2024';
+
+/**
+ * Earliest date a downstream account or charge may safely use.
+ *
+ * Loan applications, savings applications, and client charges are all
+ * rejected by Fineract when their own date precedes the owning
+ * client's activation date. Factories in Tracks B/C default their
+ * `submittedOnDate` to this constant so the whole chain
+ * (submitted -> activated -> account/charge) is monotonic by
+ * construction rather than by each spec author remembering the rule.
+ */
+export const DEFAULT_ACCOUNT_OPENING_DATE = '03 January 2024';
+
 /** Fineract `legalFormId` for an individual person. */
 const LEGAL_FORM_PERSON = 1;
 
@@ -135,3 +157,117 @@ export async function createTestClient(
     officeId
   };
 }
+
+/** Caller-supplied tweaks to the default active-client payload. */
+export interface CreateActiveTestClientOverrides extends Omit<CreateTestClientOverrides, 'extra'> {
+  /**
+   * Override the default activation date. MUST NOT precede
+   * `submittedOnDate` — Fineract rejects the create outright.
+   */
+  activationDate?: string;
+  /**
+   * Extra payload fields merged BEFORE this factory's own invariants.
+   *
+   * `active`, `activationDate`, and `submittedOnDate` are always
+   * applied last and cannot be overridden here — they are the fields
+   * this factory validates, and letting `extra` replace them would
+   * mean validating one value while sending another to Fineract. Use
+   * the dedicated `submittedOnDate` / `activationDate` options
+   * instead, which go through {@link assertDateNotBefore}.
+   */
+  extra?: Record<string, unknown>;
+}
+
+/**
+ * Create an ACTIVE client owned by the current test and queue its
+ * deletion on the supplied {@link CleanupGuard}.
+ *
+ * Why this exists as a separate factory rather than an
+ * `{ active: true }` flag on {@link createTestClient}: every flow that
+ * opens a loan account, opens a savings account, or applies a client
+ * charge requires an active client, and each of those flows carries a
+ * date-ordering constraint relative to the activation date. Encoding
+ * the chain once here keeps ~20 downstream specs from re-deriving it.
+ *
+ * Cleanup caveat: Fineract only hard-deletes clients that are pending
+ * with no attached accounts. The deleter registered here will
+ * therefore FAIL for an active client, and the {@link CleanupGuard}
+ * will report it in `summary.failed` without throwing. That is
+ * deliberate and accepted — E2E databases are ephemeral, and the
+ * `E2E_` name prefix keeps orphans greppable. The registration is
+ * kept (rather than skipped) so that a test which happens to leave the
+ * client account-free still gets it removed.
+ *
+ * @param setup     The per-test {@link ApiSetupManager}.
+ * @param guard     The per-test {@link CleanupGuard}.
+ * @param overrides See {@link CreateActiveTestClientOverrides}.
+ * @returns A {@link TestClient} projection. No follow-up GET is
+ *          issued; callers needing the activation timeline should
+ *          call `setup.api.getClient(id)` themselves.
+ * @throws When `activationDate` precedes `submittedOnDate`, so the
+ *         failure names the offending fields instead of surfacing as
+ *         an opaque Fineract validation error.
+ */
+export async function createActiveTestClient(
+  setup: ApiSetupManager,
+  guard: CleanupGuard,
+  overrides: CreateActiveTestClientOverrides = {}
+): Promise<TestClient> {
+  const submittedOnDate = overrides.submittedOnDate ?? DEFAULT_TEST_CLIENT_SUBMITTED_ON_DATE;
+  const activationDate = overrides.activationDate ?? DEFAULT_TEST_CLIENT_ACTIVATION_DATE;
+
+  assertDateNotBefore(activationDate, submittedOnDate, 'createActiveTestClient', 'activationDate', 'submittedOnDate');
+
+  return createTestClient(setup, guard, {
+    firstname: overrides.firstname,
+    lastname: overrides.lastname,
+    officeId: overrides.officeId,
+    submittedOnDate,
+    extra: {
+      // Caller extras first, invariants last. `createTestClient`
+      // spreads this object after its own defaults, so anything left
+      // in here wins — including `submittedOnDate`. Ordering the
+      // spread this way keeps the three validated fields
+      // authoritative: otherwise the guard above could pass on one
+      // date pair while Fineract received another.
+      ...overrides.extra,
+      active: true,
+      activationDate,
+      submittedOnDate
+    }
+  });
+}
+
+/**
+ * Guard that `later` is not chronologically before `earlier`.
+ *
+ * Both inputs use Fineract's `dd MMMM yyyy` wire format, which
+ * `Date.parse` handles natively. An unparseable input is treated as a
+ * programming error rather than silently skipped, because a typo in a
+ * date literal would otherwise disable the very check that catches
+ * date-ordering bugs.
+ */
+function assertDateNotBefore(
+  later: string,
+  earlier: string,
+  caller: string,
+  laterLabel: string,
+  earlierLabel: string
+): void {
+  const laterMs = Date.parse(later);
+  const earlierMs = Date.parse(earlier);
+
+  if (Number.isNaN(laterMs) || Number.isNaN(earlierMs)) {
+    throw new Error(
+      `${caller}: unable to parse dates — ${laterLabel}='${later}', ${earlierLabel}='${earlier}'. ` +
+        `Expected Fineract's 'dd MMMM yyyy' format, e.g. '01 January 2024'.`
+    );
+  }
+
+  if (laterMs < earlierMs) {
+    throw new Error(
+      `${caller}: ${laterLabel} ('${later}') must not precede ${earlierLabel} ('${earlier}') — ` +
+        `Fineract rejects the create with a validation error that names neither field.`
+    );
+  }
+}

playwright/factories/index.ts

@@ -17,13 +17,50 @@
  * The barrel keeps spec files insulated from factory file moves and lets
  * the React counterpart export an identically-shaped module — making the
  * cross-framework portability swap a path-only change.
+ *
+ * Two flavours of factory live behind this barrel, and the distinction
+ * matters:
+ *
+ *  - **API factories** (`client.factory.ts`) hit Fineract REST, return
+ *    a real `resourceId`, and register their own teardown on the
+ *    per-test `CleanupGuard`. Use these to arrange preconditions.
+ *  - **Payload builders** (`client.ts`) are pure and never touch the
+ *    network. They produce form-shaped data for driving a page object
+ *    through the UI.
+ *
+ * Both modules previously exported a function called `createTestClient`
+ * with incompatible signatures, and only the pure builder was reachable
+ * through this barrel — so `import { createTestClient } from '../../factories'`
+ * and `import { createTestClient } from '../../factories/client.factory'`
+ * silently resolved to different functions. The pure builder is now
+ * re-exported as `buildTestClientPayload` to make the distinction
+ * explicit at the import site.
  */
 
+// ── API factories (network + cleanup registration) ─────────────────
+
 export {
   createTestClient,
+  createActiveTestClient,
+  DEFAULT_TEST_CLIENT_LASTNAME,
+  DEFAULT_TEST_CLIENT_SUBMITTED_ON_DATE,
+  DEFAULT_TEST_CLIENT_ACTIVATION_DATE,
+  DEFAULT_ACCOUNT_OPENING_DATE,
+  type CreateTestClientOverrides,
+  type CreateActiveTestClientOverrides
+} from './client.factory';
+
+// ── Pure payload builders (no network) ─────────────────────────────
+
+export {
+  createTestClient as buildTestClientPayload,
   createSeededClient,
   type TestClientPayload,
   type ClientState,
-  type CreateTestClientOverrides,
+  type CreateTestClientOverrides as BuildTestClientPayloadOverrides,
   type SeededTestClient
 } from './client';
+
+// ── Shared resolvers ───────────────────────────────────────────────
+
+export { resolveDefaultOfficeId, FIRST_OFFICE_CACHE_KEY } from './_shared';

playwright/tests/clients/create-client.spec.ts

@@ -14,7 +14,7 @@ import {
   type GeneralStepData,
   type FamilyMemberData
 } from '../../pages';
-import { createTestClient } from '../../factories';
+import { buildTestClientPayload } from '../../factories';
 import { BEHAVIOR } from '../../config/behavior';
 
 /**
@@ -28,7 +28,7 @@ import { BEHAVIOR } from '../../config/behavior';
  *      single test exercises every PR-1 page object surface
  *      (`ClientsListPage`, `CreateClientPage`, `ClientViewPage`).
  *
- *   2. Negative path — uses the {@link createTestClient} payload factory
+ *   2. Negative path — uses the {@link buildTestClientPayload} payload factory
  *      to build a baseline that is one required field short, then
  *      asserts the wizard refuses to expose the Preview step until the
  *      missing field is supplied. The factory is intentionally scoped
@@ -216,7 +216,7 @@ test.describe('Create Client — CRUD', () => {
       );
     }
 
-    const invalidPayload = createTestClient({
+    const invalidPayload = buildTestClientPayload({
       office: officeName,
       lastname: ''
     });

playwright/utils/cleanup-guard.spec.ts

@@ -73,6 +73,41 @@ test.describe('CleanupGuard.flush() ordering', () => {
     expect(summary.failed).toEqual([]);
   });
 
+  /**
+   * Regression guard: an earlier implementation snapshotted the stack
+   * in LIFO order but then dispatched every deleter in the same tick
+   * via `Promise.allSettled`. Ordering was therefore only apparent —
+   * a slow child deleter could still have its DELETE reach Fineract
+   * after its parent's, producing an FK violation. Staggered delays
+   * make a concurrent drain observably interleave.
+   */
+  test('awaits each deleter before starting the next', async () => {
+    const guard = new CleanupGuard();
+    const events: string[] = [];
+
+    const slowDeleter = (label: string, delayMs: number) => async () => {
+      events.push(`start:${label}`);
+      await new Promise((resolve) => setTimeout(resolve, delayMs));
+      events.push(`end:${label}`);
+    };
+
+    // Registered parent-first, so LIFO must run 'child' to completion
+    // before 'parent' even starts. The child is deliberately the
+    // slower of the two — under a concurrent drain 'parent' would
+    // finish first and the interleaving would be visible.
+    guard.register('parent', slowDeleter('parent', 1));
+    guard.register('child', slowDeleter('child', 25));
+
+    await guard.flush();
+
+    expect(events).toEqual([
+      'start:child',
+      'end:child',
+      'start:parent',
+      'end:parent'
+    ]);
+  });
+
   test('drains the stack to zero after a flush', async () => {
     const guard = new CleanupGuard();
     guard.register('a', async () => undefined);