opensource-joe opened a new pull request, #382: URL: https://github.com/apache/fineract-backoffice-ui/pull/382
## What and why PR 2 of the five agreed on #355. All 65 routes in `products.routes.ts` inherited their tab from the section above them, so every page under `products` read `Products · Fineract`. This applies the convention settled in #380: 1. The section root restates the `nav.*` key from its `app.routes.ts` entry. 2. Every other route takes the key its page already uses for its own visible heading. 3. Where the heading is a record's name and cannot be a static key, the route takes a generic key naming the screen type. Refs #355 ## The part worth reviewing: this file needed six new keys `clients` needed none and `system` needs none. `products` needs six, and the reason is a rule about **route shapes** rather than about feature areas, which is the correction I flagged on #380 before it merged. Five of the six are record views. Rule 2 says take the page's own heading key, and for these that key is `COMMON.DETAILS` or `COMMON.OVERVIEW`, because the visible heading is the record's name and the card above it is generically labelled. Following the rule literally would have given four different screens the tab `Details · Fineract`, reproducing this issue's own problem one level down: | Route | Title | | |---|---|---| | `loan/view/:id` | `nav.loanProductDetails` | existed | | `savings-accounts/view/:id` | `SAVINGS.ACCOUNT_DETAILS` | new | | `fixed-deposits/view/:id` | `FIXED_DEPOSITS.ACCOUNT_DETAILS` | new | | `recurring-deposits/view/:id` | `RECURRING_DEPOSITS.ACCOUNT_DETAILS` | new | | `shares/view/:id` | `SHARE_ACCOUNTS.ACCOUNT_DETAILS` | new | The sixth is `ACTIONS.ACCOUNT_ACTION`, for `:accountType/:accountId/action/:command`. That component picks its heading from a command-to-key map at runtime, so no static title can name it. **This is the same shape as the `tellers` `:command` route you asked about on #355**, and `products` has two of them, so the case is now answered in code rather than in prose. All six are added to `en.json` only. `i18n.js` sets `fallbackLng: "en"`, so the other locales fall back rather than break. ## Verification | | | |---|---| | `npx ng test fineract-backoffice-ui` | **1104 of 1104 SUCCESS**, real headless Chromium | | `products.routes.spec.ts` alone | 11 of 11 | | `npm run lint` | exit 0, no new warnings | | `npm run lint:prune` | exit 0 | | `npm run format:check` | exit 0 | | `node scripts/check-translations.mjs` | exit 0, 2206 keys | **The spec was verified in the failing direction as well.** It carries the two guards from #380 plus one that file did not need: | Change | Result | |---|---| | delete the title on `savings-accounts/view/:id` | 3 failed, naming the route | | point two view routes at the same `COMMON.DETAILS` | **1 failed, the new uniqueness test** | | replace a key with the phrase `'Rates'` | 1 failed, the key-shape test | | restore | 11 of 11 | That third guard, **no two routes may share a title**, is the one `clients` did not need. Titling 65 routes is only worth doing if the tabs come out distinguishable, and it is what fails if someone later takes the `COMMON.DETAILS` shortcut. ## Screenshots Not applicable. The change is in the browser tab. ## Two things noticed, neither changed here **`products/recurring` and `products/share` render the wrong labels.** They are the *product* lists but their headings use `nav.recurringDeposits` and `nav.shares`, which are the *account* labels. Their titles use the correct `nav.recurringDepositProducts` and `nav.shareProducts`, so on those two the tab is right and the on-screen heading is wrong. Worth a separate fix if you agree. **Three list components carry hardcoded English headings** rather than translation keys: `fixed-deposits-list`, `recurring-deposits-list` and `share-accounts-list` render `title="Fixed Deposit Accounts"` and similar. `check-translations.mjs` cannot catch this, because it only validates strings that are already keys. A real i18n bug but not a titling one, so those three routes take the correct `nav.*` keys and I have left the headings alone. Happy to file it separately. `products.routes.ts` stays flat, as you asked. ## 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. - [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]
