Aman-Mittal opened a new issue, #210:
URL: https://github.com/apache/fineract-backoffice-ui/issues/210

   ## Problem
   
   Four third-party surfaces are named directly by application code, measured 
across the 2,109 hand-written TS files (excluding the generated OpenAPI client):
   
   | Dependency | Reached directly from |
   | --- | --- |
   | `@ngx-translate/core` | 256 files |
   | `@ionic/angular` — `<ion-*>` components | 250 files |
   | `@ionic/angular` — imperative controllers | 12 files |
   | Web Storage (`localStorage` / `sessionStorage`) | 12 call sites across 4 
services |
   | Object URLs + download anchors | 18 call sites across 4 features |
   
   Three separate problems follow from this, and they are not the same problem.
   
   ### 1. Replaceability
   
   A library reaching a quarter of the codebase cannot be upgraded across a 
breaking major, evaluated against an alternative, or removed, without editing 
every file that names it. This is the ordinary argument for a boundary, and on 
its own it would not justify the work.
   
   ### 2. A trust boundary nobody can enumerate
   
   `security.md` §4 names web storage as a trust boundary, but nothing lists 
what is in it. The consequence is concrete and currently on `main`:
   
   `AuthService.logout()` removes the session key and leaves 
`fineract_runtime_config` — the API endpoint that every subsequent request, and 
every subsequent set of credentials, is sent to — in place across sign-outs. 
Not because anyone decided that; because no single place knew the key existed.
   
   A second one in the same area: `AuthService.getStoredSession()` calls 
`JSON.parse` unguarded. One bad character in `fineract_session` throws during 
bootstrap and leaves the app on a blank screen with no route to the login page.
   
   ### 3. Correctness that repetition erodes
   
   Four features each hand-rolled the same object-URL-and-anchor download, and 
they have drifted into five behaviours:
   
   - all five revoke the object URL on the **success path only**, so a throw in 
between leaks the blob for the life of the document;
   - two append the anchor to `document.body` and two do not — a difference 
that matters in Firefox and was clearly discovered rather than decided;
   - one sets `a.download` straight from a Fineract-supplied filename, which is 
to say from whatever another user uploaded.
   
   ## Proposal
   
   Introduce `src/app/core/adapters/` — a contract (interface + 
`InjectionToken`) per capability, with one implementation naming the library it 
wraps. Enforce it in ESLint, recording the existing violations in the 
shrink-only suppressions baseline this repo already uses.
   
   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 — 
the part services reach for, and the part with lifecycle semantics worth 
testing — goes behind the boundary.
   - **A facade over the generated OpenAPI client.** ADR 0001 considered 
exactly that and rejected it on maintenance cost; that decision stands.
   
   ## Related: verifying the generated client
   
   ADR 0001 stabilised generated *method names* against generator churn. It 
says nothing about change originating upstream: when Fineract removes an 
endpoint, the generated client loses the method and the app fails to compile 
across every feature that called it, with no single diagnostic naming the 
endpoint that went away.
   
   A manifest of the operations the application depends on, plus a check, is 
the complement to ADR 0001 rather than a reversal of it — and it costs no 
indirection layer.
   
   ## Also proposed: a GA readiness gate
   
   `security.md` opens with "This project is currently **not release-ready**." 
Nothing in the repo says what would have to change for that line to come out. A 
small script encoding the machine-checkable part of it would make the answer a 
command rather than a conversation.
   
   ## Acceptance criteria
   
   - [ ] Adapters for i18n, overlays, storage and downloads, each with a 
contract, one implementation and a test double
   - [ ] Core services and feature call sites migrated off the direct 
dependency for those four
   - [ ] `logout()` clears every session-scoped key; storage reads do not throw 
on a corrupt value
   - [ ] Download filenames sanitised in one place; object URLs revoked on the 
failure path too
   - [ ] ESLint enforces the boundary, with the existing backlog recorded so it 
can only shrink
   - [ ] A check that verifies the generated API operations the app depends on 
still exist
   - [ ] A GA readiness gate, runnable locally, honest about what it cannot 
determine
   - [ ] Documentation: an ADR for the decision, a working guide for day-to-day 
use, CI docs updated
   


-- 
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]

Reply via email to