Aman-Mittal opened a new pull request, #651: URL: https://github.com/apache/fineract-backoffice-ui/pull/651
## Why Two unguarded dependencies, each found by asking what the existing checks cannot see. **The generated OpenAPI client.** ADR 0001 made generated method *names* deterministic, and `check-api-surface.mjs` reports an endpoint that disappears upstream. Neither covers a change to an emitted *shape* — a field becoming optional, an enum gaining a member, a response nesting one level deeper. Names stay stable, the surface manifest still passes, and the change lands on every file that binds that shape. That is 463 files today, 278 of them feature components, because the generated response type *is* the model those components bind into their templates. That is not hypothetical. The accounting-closures list bound `closure.isClosed` through an untyped `appCellTemplate` context. `GetGlClosureResponse` has no `isClosed`, and neither does the payload Fineract returns — verified against a running instance, which sends `id`, `officeId`, `officeName`, `closingDate`, `deleted`, the audit fields and `allowedOffices`. So the expression was `undefined` on every row, and **every closed period rendered as "Open"**, in the one column whose job is to say otherwise. `strictTemplates` is on and cannot see into an untyped template context; the unit test did not catch it either, because its fixtures were built from the same generated type. **English written straight into a template.** The four `i18n:check` checks and the runtime handler all need something key-shaped to exist first. `<ion-label>Repayments Rescheduling Rule</ion-label>` looks nothing up, so nothing misses and no raw key appears — it renders correctly in English and stays English in Hindi and Korean indefinitely. #627 was three screens of this found by hand. There are 68 more. ## What this changes ### Architectural boundary — ADR 0006 `local/no-generated-api-import` reports an import resolving into `src/app/api` from outside the API adapters and the composition root. Matching is on the resolved path, not the specifier, so `'../../api'` and `'../api/model/getClientsResponse'` are caught alike regardless of the importing file's depth. It gets its own rule id for the reason `no-vendor-ui-import` has one: suppression counts are per rule, and a shared counter lets one boundary's violation pay for another's. The existing 475 are seeded into `eslint-suppressions.json`, which this repository documents as only ever shrinking, so nothing breaks on adoption and the count cannot grow. **This is not the facade ADR 0001 rejected, and that rejection stands.** There is no wrapper over the 155 generated services and none is proposed. The rule renames nothing and adds no indirection on day one; it makes the coupling countable and non-increasing, and gives a mapping a sanctioned place to live when a mapping earns its keep. ### Worked example — one domain, selectively `AccountingClosureApi` states the three operations the two closure screens use, over an `AccountingClosure` model that differs from the generated type in the three ways that matter: - fields the screens need are **non-optional** — the generated type marks everything optional because the spec marks nothing required; - absence is **`null`**, not `undefined`, so a template cannot silently read past it; - `isClosed` is **derived once in a tested mapper**, from the fact that a closure row which is not deleted means the period is closed. `dateFormat` and `locale` moved into the adapter too: the form used to set them on the request object, so a screen knew how Fineract parses dates. Both specs now mock the contract instead of the generated service. That drops the `as unknown as Observable<HttpEvent<…>>` casts the generated overloads forced on every mock, and it matters for a second reason — a fixture built from a generated type inherits the spec's blind spots and will confirm a screen that is broken in a browser. ### New check — `check:template-text` Reports a text node of two or more words starting with a capital, comments stripped first so a JSDoc example is not mistaken for UI. It under-reports on purpose: a single word is too often a unit or an acronym (`USD`, `ID`, `JSON`) to flag without a false-positive rate that would get the check switched off. Per-file baseline, shrinking only — a new occurrence fails, and so does a fix, until `--update` records the lower number, which keeps the fix and the baseline in one commit. Seeded at 71 in 18 files, then the three single-occurrence files fixed to prove the loop works in both directions: **68 in 15**. ### `i18n:check` PHRASES widened Was scoped to files importing `ColumnDef`, because that contract makes `label` a key. #627 showed the defect also lives where no contract reaches. All 590 `label:` literals under `src/app` are now key-shaped, so widening it costs nothing today and stops the 46 outside `ColumnDef` files from regressing. ## Boundaries after this change | Boundary | Mechanism | Remaining | | --- | --- | --- | | i18n (`@ngx-translate`) | `I18N`, `\| appTranslate` | 303 on ratchet | | Ionic components | `src/app/ui/` | 227 on ratchet | | Ionic imperative | `OVERLAY` | 0 — complete | | Web Storage | `STORAGE` | 15 | | Object URLs | `DOWNLOAD` | 0 — complete | | Angular Material | removed | 0 — rule is a wall | | **Generated API client** | **`local/no-generated-api-import`** | **473 on ratchet (was unguarded)** | Deliberately **not** adapted, now written down in `DOCS/ADAPTERS.md`: RxJS (an Angular peer dependency), `@angular/cdk` (retained on purpose when Material went; `src/app/ui/` is built on it), and Angular itself. ## Documentation - `DOCS/adr/0006-generated-api-boundary.md` — the decision, and why it is not ADR 0001's facade. - `DOCS/ADAPTERS.md` — previously said the generated client was deliberately not adapted; now describes the boundary, the three rules the worked example follows, and what is still left alone. - `DOCS/DEPENDENCY_UPGRADES.md` — new. Which checks to run when a dependency moves, in the order that localises a fault fastest, and which boundary each failure implicates. Two rows are admissions rather than reassurance: Angular has no boundary by design, and an upstream spec change still reaches every consumer for the domains not yet migrated. - `DOCS/CI_CHECKS.md`, `AGENTS.md` — the new rule and check where contributors look. ## Checks run | Check | Result | | --- | --- | | `npm run lint` | pass | | `npm run test:eslint-rules` | pass — 28, incl. 10 new resolution cases | | `npm run test:unit` | pass — 267 files, 1771 tests | | `npm run format:check` | pass | | `npm run i18n:check` | pass | | `npm run check:template-text` | pass | | `bash scripts/check-license.sh` | pass | | `npx tsc --noEmit -p tsconfig.app.json` | pass | | the other nine `check:*` scripts | pass | | `--project=mocked`, `mobile`, `backend` | running; result posted as a comment | The ratchet was verified in both directions rather than assumed: a new import of `src/app/api` fails lint with a message naming the ADR, and a new untranslated text node fails `check:template-text`. Backend digest under test: `sha256:c1d4aee9` (created 2026-10-01), on a volume destroyed with `e2e:stack:fresh`. That image is unpinned and moved twice during this work, so I will state the digest rather than imply reproducibility. ## Risk and what is left - **One deliberate UI change.** The closures status column now reads "Closed" for a closed period, where it previously always read "Open". That is the bug being fixed, and it is the only behavioural change here. - **The mapper throws** when Fineract returns a closure with no `id` or `officeId`. Defaulting would send a re-open for the wrong period, so it fails loudly; Fineract always sends both. - **473 files still import the generated client.** Recorded and non-increasing, not fixed. Migration is reviewable one domain at a time — the path Material took from 250 files to zero. - **68 untranslated text nodes in 15 files** remain, listed by `--list`. Worth a follow-up issue; they are translator-visible defects, not regressions. - **Authentication is deliberately untouched.** There is still exactly one implementation: the OIDC button added in #643 raises "not implemented" and the authorization-code flow is #370. An `AuthService` interface over a single implementation would be an abstraction with no second implementer today — it becomes justified when #370 lands, and that is the natural PR to introduce it in. Closes none; follow-up to #624 and #627. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
