opensource-joe opened a new pull request, #383:
URL: https://github.com/apache/fineract-backoffice-ui/pull/383

   ## What and why
   
   PR 3 of the five agreed on #355. All 49 routes in `system.routes.ts` 
inherited their tab from the section above them, so every administrative screen 
read `System ยท Fineract`. That is the worst place in the app for it: 49 
different configuration pages sharing one tab.
   
   Applies the convention settled in #380.
   
   Refs #355
   
   ## No new translation keys, and the reason generalises
   
   `products` needed six. This file needs none across 49 routes, and the 
difference is not luck about which files these are:
   
   | File | Routes | New keys |
   |---|---|---|
   | `clients` | 21 | 0 |
   | **`system`** | **49** | **0** |
   | `products` | 65 | 6 |
   
   **`system` has no `view/:id` route at all.** Every screen here is a list, a 
configuration page or a create/edit form, and each of those already renders its 
own name, so rule 2 always finds a key. A record view is the one shape where 
the visible heading is the *record* rather than the *screen*, which is exactly 
when there is nothing to reuse.
   
   So the accurate statement of the property is: **a file needs new keys if and 
only if it contains routes whose heading is bound at runtime.** That is 
checkable in advance, and it is what I will write back to #355.
   
   ## The one judgement call
   
   `codes/:codeId/values` binds its heading to `[title]="codeName()"`, the 
code's own name at runtime, so no static title can name it. It takes the 
existing generic `CODE_VALUES.TITLE`, the same treatment a record view gets, 
arrived at from a different direction.
   
   Two smaller notes on the mapping:
   
   - `delinquency` renders two data tables with no page-level heading, so it 
takes `nav.delinquency` rather than either table's title.
   - `business-dates` carries its heading in an `<h2>` rather than a card 
title, worth mentioning only because it is the one place in the section where 
the heading is not where the surrounding files put it.
   
   ## Verification
   
   | | |
   |---|---|
   | `npx ng test fineract-backoffice-ui` | **1103 of 1103 SUCCESS**, real 
headless Chromium |
   | `system.routes.spec.ts` alone | 10 of 10 |
   | `npm run lint` | exit 0 |
   | `npm run lint:prune` | exit 0 |
   | `npm run format:check` | exit 0 |
   | `node scripts/check-translations.mjs` | exit 0 |
   
   **Verified in the failing direction too**, since a spec over data that only 
ever passes proves nothing:
   
   | Change | Result |
   |---|---|
   | delete the title on `codes/:codeId/values` | 3 failed, naming the route |
   | point `cache` and `permissions` at the same key | 1 failed, the uniqueness 
test |
   | replace `nav.hooks` with the phrase `'Hooks'` | 1 failed, the key-shape 
test |
   | restore | 10 of 10 |
   
   ## Screenshots
   
   Not applicable. The change is in the browser tab.
   
   ## Checklist
   
   - [x] I did not hand-edit generated files under `src/app/api/`.
   - [x] New component or service code uses the adapter boundary. No new 
component or service code.
   - [x] User-facing strings use translation keys. No new keys were needed.
   - [x] I added or updated tests appropriate to this change.
   - [x] UI workflow changes include suitable e2e coverage. No workflow changes.
   - [x] Commits are signed.
   


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