Aman-Mittal opened a new pull request, #211:
URL: https://github.com/apache/fineract-backoffice-ui/pull/211
Closes #210.
## What this does
Adds `src/app/core/adapters/`: one contract (interface + `InjectionToken`)
per capability, one implementation naming the library it wraps.
| Token | Replaces | Sites migrated |
| --- | --- | --- |
| `I18N` | `TranslateService`, `\| translate` | core services + error
interceptor |
| `OVERLAY` | Ionic `Toast`/`ModalController` | all 12 |
| `STORAGE` | `localStorage` / `sessionStorage` | all 12 |
| `DOWNLOAD` | object URLs + download anchors | all 18 |
Three design points worth reviewing rather than skimming:
- **Tokens carry a default binding** (`providedIn: 'root'` + factory).
Ionic's controllers and `TranslateService` are root-provided, so before this a
TestBed rendering a component that shows a toast needed no configuration. A
token with no default would have made every such spec — and every future one —
declare a provider it has no opinion about. Each implementation imports its
contract with `import type`, so naming the class from the token does not close
a runtime cycle.
- **`IonicOverlayAdapter` resolves its controllers per call, not as
fields.** The contract covers toasts and modals together, but `ToastController`
is available everywhere while `ModalController` needs `provideIonicAngular()`.
Injecting both eagerly broke 7 specs of components that only ever toast.
- **The `| appTranslate` pipe is impure**, as ngx-translate's own is. A pure
pipe caches on its inputs and the key does not change when the language does;
reading the signal inside `transform` does not rescue that, because
`ɵɵpipeBind` returns the cached value without calling `transform` again. It
memoises on `lang + key + params`, so the per-change-detection cost is a string
comparison.
### Bugs fixed on the way
These were found by building the boundary, not sought:
- **`logout()` left the endpoint override in place** — it cleared the
session key and left `fineract_runtime_config`, the API URL every subsequent
request and set of credentials goes to, across sign-outs. `STORAGE_KEYS` is now
the reviewable inventory and `logout()` clears a scope rather than a remembered
list.
- **`AuthService.getStoredSession()` parsed unguarded.** One bad character
in `fineract_session` threw during bootstrap and left the app blank with no
route to the login page. Reads now fall back.
- **Object URLs leaked on the failure path.** Now revoked in a `finally`.
- **Server-supplied download filenames went through unmodified.** Now
reduced to a sanitised basename in one place (`../../etc/passwd` → `passwd`),
with spaces kept and the extension preserved when truncating.
### Deliberately out of scope
- **`<ion-*>` components.** They are the UI layer per `AGENTS.md`, 250
files, and migrate one component at a time. Only Ionic's imperative surface is
behind the boundary. The structure allows adding them later without moving what
exists.
- **A facade over the generated OpenAPI client.** ADR 0001 considered
exactly that and rejected it on maintenance cost. That decision stands — see
below for the complement.
## Also in this PR
**`npm run api:surface`** records the 413 operations across 127 generated
services the app calls, and verifies each still exists. ADR 0001 stabilised
generated *method names* against generator churn; this covers what it does not
— change originating upstream. When Fineract removes an endpoint, this reports
one message naming the operation and its callers, instead of compile errors
scattered across every feature that used it.
**`npm run ga:check`** encodes the machine-checkable part of what would have
to be true for `security.md` to stop opening with "not release-ready". Eight
gates; **three pass today**:
```
FAIL NGINX sets HTTP security headers
FAIL API endpoint override is validated against an allow-list
FAIL Authorization header is restricted to the API origin
PASS No sanitizer bypasses or raw HTML sinks
FAIL Login form offers no third-party API hosts
FAIL Adapter boundary migration backlog (advisory)
PASS Generated API surface matches the manifest
PASS No known vulnerabilities in production dependencies
```
**Not wired into `ci.yml`**, deliberately: a check that is red on every PR
from the day it lands is one people learn to scroll past. It is there to be
run, and to be wired in once the blockers close. A gate that cannot be decided
from the repository reports `unknown` rather than `pass` — recording a security
control as satisfied because nothing contradicted it is worse than recording
nothing.
## Enforcement
`eslint.config.js` restricts the wrapped dependencies outside
`src/app/core/adapters/**` via `no-restricted-imports` (ngx-translate; Ionic
controllers by `importNames`, so `<ion-*>` is unaffected),
`no-restricted-globals` (Web Storage) and `no-restricted-properties`
(`URL.createObjectURL`).
The **435 existing violations** — almost all `| translate` in components —
go into `eslint-suppressions.json`, the shrink-only baseline this repo already
uses for its migration ratchets. A new violation fails lint immediately; a
fixed one must be pruned, so it cannot come back.
## Breaking changes
- **`DialogService.open()`'s third parameter** is now a `DialogOptions`
object rather than a bare `cssClass`. No production caller passed it; one spec
did.
- **The theme key moves** from `'theme'` to `'fineract_theme'`, per the
`AGENTS.md` prefix rule. No migration shim: each existing user's theme falls
back to their OS preference once, which `ThemeService` already handles as the
no-value case.
## Verification
- `tsc --noEmit` clean (app and spec configs)
- `npm run lint` clean
- `npm run format:check` clean
- **759 unit tests passing** — baseline on `main` is 744; this adds 15
- `npm run api:surface` passes
## Documentation
- `DOCS/adr/0003-adapter-boundary.md` — the decision, the alternatives
considered, the costs accepted
- `DOCS/ADAPTERS.md` — day-to-day use, testing with the fakes, how to add a
fifth adapter
- `DOCS/CI_CHECKS.md` — the two new checks, the suppressions baseline, the
GA gate table
- `AGENTS.md` — boundary summary for contributors
## Review suggestion
Two commits, and they read independently: `feat(core)` is the boundary and
its migrations, `feat(scripts)` is the checks. The ADR is the best entry point
for the *why*; `src/app/testing/adapters.ts` is the shortest demonstration of
what the boundary buys — a spec that asserts on the request the code made
rather than on the DOM a library built from it.
--
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]
