Aman-Mittal opened a new pull request, #414:
URL: https://github.com/apache/fineract-backoffice-ui/pull/414
## What and why
Three related pieces of back-office work, kept on one branch but as three
self-contained commits — each one builds, lints and tests green on its own,
so
they can be reviewed (or reverted) independently.
**1. `fix(dialogs)` — the last of the native `window.confirm` boxes.**
Thirteen
destructive actions still asked for confirmation with the browser's own
dialog:
unstyled, untranslated, unable to signal that the action destroys something,
and
it freezes the page while it is open.
- Products, all 7 sites — **closes #229**.
- System — **closes #230**. Six of that issue's seven were still outstanding
(oidc-config was converted earlier). Four more sites under `system/` that
the
issue's file table predates go with them (delinquency ranges and buckets,
data
tables, templates), so the directory is now clear rather than
clear-except-for-four.
Each message says what is actually lost rather than restating the button —
*"Delete the hook X? Outbound calls stop immediately, and anything listening
on
it goes quiet without further warning."* Where the consequence is not
verifiable
from the UI it is left unstated rather than guessed at, per the guidance in
#232.
This uses the `I18N` adapter rather than `TranslateService`. The issue text
predates ADR-0003 and the two existing worked examples disagree with each
other;
the adapter is what `npm run lint` enforces for new code.
The thirteen specs these touch **move to Vitest in the same commit** (part of
#403). Editing them on the deprecated runner and then converting them would
mean
writing the same assertions twice.
`scripts/codemod-jasmine-to-vitest.mjs` handled every one with nothing left
for
a human. One wrinkle worth knowing for the next batch: **renaming a spec
loses
its entry in `eslint-suppressions.json`**, which turns
`TranslateModule.forRoot()`
into a lint error — the existing `provideTranslateTesting()` helper is the
fix.
Karma baseline falls **191 → 178**.
**2. `feat(roles)` — the role editor previews what a permission change
does.**
The permission matrix answers "which codes does this role hold", which is not
the question an administrator is asking. They are asking what people in this
role will be able to *do*, and `READ_OFFICE` is only legible as
"Organization › Offices" to someone who already knows the answer.
The editor now keeps a running preview above the matrix: codes added and
removed, and the navigation destinations the role **gains or loses** as a
result. It comes from a new
`NavigationConfigService.navDestinationsForPermissions()`, which runs the
same
gates `filteredNavItems` runs — hidden list, developer tools, institution
features, permissions — with the passed-in codes substituted for the
signed-in
user's. Asking the sidebar's own service twice is what stops the preview
drifting from what the sidebar will actually show; a second implementation of
the visibility rule would be worse than no preview at all.
`AuthService.hasPermission`'s superuser handling moves to a pure
`permissionsSatisfy()` for the same reason — `ALL_FUNCTIONS`, the
`ALL_FUNCTIONS_READ` shortcut and its mixed-request fallthrough are subtle
enough that two copies would diverge.
Where a permission gates an action *inside* a screen the role already
reaches,
nothing appears or disappears, and the panel says so rather than going blank:
"no screen changed" and "nothing changed" are different answers.
Also on that screen: a confirmation restating the change before it is written
(revoking a permission affects everyone in the role and tells none of them);
a
filter over the matrix, which is ~700 checkboxes with no way to find one;
translation keys for Check All / Uncheck All, which were hardcoded English;
and
creating a role now lands on its permission matrix instead of the list, since
`POST /roles` takes a name and description only and the matrix is the step
the
administrator was always going to take next.
Relates to **#123**.
**3. `ci` — shard the real-backend e2e suite, and cache what CI
re-downloads.**
The mocked half has been sharded four ways for a while; the slower
real-backend
half was not. It now runs as a three-shard matrix producing blob reports,
with a
`backend-report` job merging them, mirroring the `mocked`/`mocked-report`
pair
exactly. Three rather than four because each backend shard brings up its own
PostgreSQL and Fineract, so it is paid for in bring-up as well as runner
minutes. Each shard gets a **separate stack** — the specs mutate real records
and `backend.setup.ts` seeds reference data, so sharing one would make them
order-dependent across shards.
Caching added, in rough order of what it saves: `.angular/cache` (build,
test and
all three e2e jobs); buildx `type=gha,mode=max` for the container image;
`restore-keys` on the Playwright browser cache; the Apache RAT and
openapi-generator jars.
Two notes for reviewers:
- **`.angular/cache` only works because `angular.json` now sets
`cli.cache.environment: "all"`.** Angular's default is `"local"`, which
disables the build cache whenever `CI` is set — sensible for a fresh
machine
every run, and exactly wrong once `actions/cache` has restored the
directory
first. Without that line the cache step would be theatre.
- **The RAT jar's checksum is still verified on cache hits.** A GitHub
Actions
cache is writable from any branch, so a cached jar is untrusted input;
skipping the check on a hit would convert a saving into a supply-chain
hole.
No new trigger is added to either workflow, so the cache-poisoning reasoning
already recorded in `ci.yml` is unchanged.
> [!IMPORTANT]
> **Branch protection needs updating when this merges.** Require the three
> `E2E (real Fineract, shard n/3)` checks in place of the former single
> `E2E (real Fineract)`. Not `E2E (real Fineract report)` — merging reports
can
> succeed when a shard failed, which is the same trap the existing mocked
note
> warns about.
## Issues
Closed by this PR:
- Closes #229 — product screens use the browser's native confirm box (7
sites).
- Closes #230 — system administration screens use the browser's native
confirm
box. Six of the seven listed were outstanding; the seventh (oidc-config)
was
already done, and four further sites under `system/` postdate the issue's
file
table.
Moved forward but **not** closed — please do not auto-close these on merge:
- #403 (epic) — unit suite off Karma. **191 → 178 remaining.**
- #409 — the 39 product specs. **7 migrated, 32 remain.**
- #404 — the 32 system-administration specs. **6 migrated, 26 remain.**
- #123 (epic) — Permission Feedback UI. The role editor's preview is one
piece
of this; the dashboard card (#140) and the sidebar badges (#138) are
untouched.
Deliberately **not** touched, because they are already claimed and this PR
would
have collided:
- #232 — the working-capital/accounting/scheduling confirm boxes. @AmandaKeay
has calendars and meetings, @HurrairaBaloch has the rest. The
`I18N`-adapter
and message-wording conventions used here are the same ones that issue's
thread settled on, if that is useful reference while it lands.
- #327 — auto-focus the global search input, claimed by @Iyamokuma.
## Verification
Everything below was run locally against the branch tip:
- `npm run lint:prune` — clean. Suppressions **fall by 14** (13 renamed specs
plus role-form's `unicorn/no-array-sort`); none added.
- `npx eslint "src/**/*.html"` — clean.
- `npm run format:check` — clean.
- `npm test -- --watch=false` (Karma) — **868 passing**.
- `npm run test:unit` (Vitest) — **46 files, 360 passing** (31 files before
this
branch: 13 migrated, 2 new).
- `npm run i18n:check`, `check:icons`, `check:a11y-names`,
`check:route-permissions`, `check:internal-endpoints`, `check:version`,
`check:test-runner`, `check-license.sh`, `typecheck:e2e`, `api:surface` —
all clean.
- `npm run build` — production build succeeds.
- `npm run ga:check` — 8/9, **0 blocking**. The one advisory is the adapter
backlog, which *falls* 416 → 403.
- `find src projects -name '*.spec.ts' | wc -l` — 178, matching the
re-recorded `karma-baseline.json`.
- Both workflows parse as YAML and were re-checked against zizmor's rules by
hand (all new `uses:` are SHA-pinned with a version comment;
`persist-credentials: false` on every new checkout; per-job minimum
permissions; no new trigger). The two new Docker action SHAs were verified
against the GitHub API rather than written from memory.
Exercised with mocks and unit tests only. **The e2e sharding change cannot be
verified locally** — it only exercises itself on GitHub Actions, so the
first CI
run on this PR is the real test of commit 3, and the merged
`E2E (real Fineract report)` should be compared against a recent `main` run
to
confirm no tests went missing between the shards.
## Screenshots
Not added — I do not have a running Fineract to capture the role editor
against,
and a mocked screenshot of a permission preview would be misleading about the
thing the preview is for. The behaviour is covered by 18 unit tests in
`role-form.component.test.ts`, including the "permission changed but no
screen
changed" case. Happy to add real ones if a reviewer would rather see them
before
merging.
## Checklist
- [x] I did not hand-edit generated files under `src/app/api/`.
- [x] New component or service code uses the adapter boundary in
`src/app/core/adapters/` instead of direct browser globals or imperative
third-party APIs.
- [x] User-facing strings use translation keys.
- [x] I added or updated tests appropriate to this change, or explained why
tests were not needed.
- [ ] UI workflow changes include suitable e2e coverage, including
real-backend testing where relevant. — **not done.** The role-editor preview
has unit coverage but no e2e spec. It is a natural fit for the `backend`
project (create a role, grant `READ_OFFICE`, assert Organization › Offices
appears in the panel), and I am happy to add one here if you would prefer it in
this PR rather than a follow-up.
- [x] Commits are signed — see [Commit
Signing](CONTRIBUTING.md#commit-signing) in CONTRIBUTING.md.
--
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]
