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

   ## What is wrong
   
   Eight destructive actions across **working capital**, **accounting**, 
**calendars**, **meetings** and **organization** ask for confirmation with 
`window.confirm` and a hardcoded English string:
   
   ```ts
   if (!row.id || !window.confirm('Delete this calendar?')) return;
   ```
   
   That produces the browser's own grey dialog: unstyled, untranslated, unable 
to signal that the action is destructive, and it freezes the page while it is 
open. Everywhere else the application uses its own `DialogService.confirm`, 
which is themed, translated and marks destructive actions in red. Across the 
app there are 29 of these native calls left; this issue covers the remaining 8.
   
   ## Business value
   
   The consequences here are mostly indirect, which is what makes them easy to 
get wrong. Deleting a provisioning category or criterion changes how loan-loss 
provisioning is calculated, and the effect shows up later in the accounts 
rather than immediately on screen. Deleting an account-number format changes 
how identifiers are generated from that point on, while everything already 
issued keeps the old shape. Deleting a calendar or meeting removes the schedule 
a group's collections are organised around.
   
   None of these announce themselves as significant, and the browser's dialog 
does nothing to help — it shows one English sentence with no context, whatever 
locale the user picked.
   
   ## Files
   
   | File | Line |
   |---|---|
   | `src/app/features/working-capital/breach/wc-breach-list.component.ts` | 
120 |
   | 
`src/app/features/working-capital/loan-products/wc-loan-products-list.component.ts`
 | 125 |
   | 
`src/app/features/working-capital/near-breach/wc-near-breach-list.component.ts` 
| 116 |
   | 
`src/app/features/accounting/provisioning-categories/provisioning-categories-list.component.ts`
 | 114 |
   | 
`src/app/features/accounting/provisioning-criteria/provisioning-criteria-list.component.ts`
 | 114 |
   | `src/app/features/calendars/calendars-list.component.ts` | 128 |
   | `src/app/features/meetings/meetings-list.component.ts` | 135 |
   | 
`src/app/features/organization/account-number-formats/account-number-formats-list.component.ts`
 | 118 |
   
   ## How to fix
   
   There is a worked example in the tree — 
`src/app/features/system/codes/codes-list.component.ts` was converted this way 
and is the pattern to copy.
   
   ```ts
   private readonly dialogService = inject(DialogService);
   private readonly translate = inject(TranslateService);
   
   void this.dialogService
     .confirm({
       title: this.translate.instant('CALENDARS.DELETE'),
       message: this.translate.instant('CALENDARS.CONFIRM_DELETE', { name: 
row.title }),
       destructive: true,
     })
     .then((confirmed) => {
       if (!confirmed) return;
       // ...the existing delete call, unchanged
     });
   ```
   
   Notes:
   
   - `confirm()` returns a `Promise<boolean>`, so the early-return guard 
becomes a `.then()` block. That is the only structural change.
   - Add the new keys to `src/assets/i18n/en.json`. Only `en.json` is required 
— `npm run i18n:check` fails on keys referenced in code but missing there, and 
does not require `hi.json`/`ko.json`.
   - Where the consequence is not obvious from the row, say it. The 
provisioning and account-number-format ones benefit most from a sentence 
explaining what changes afterwards.
   - Set `destructive: true` on every one of these.
   
   ## Verifying
   
   ```
   npm run lint
   npm run i18n:check
   npm test
   npm run build
   ```
   
   If the screen has a spec, add a case asserting the delete is not called when 
the dialog resolves `false`. That is the behaviour most likely to break in the 
rewrite, since the guard moves from a synchronous `return` into a callback.
   
   ## Picking this up
   
   No need to be assigned — assignment here is limited to committers. Comment 
that you are starting, then open a PR; the comment is enough to stop two people 
duplicating the work.
   
   This issue touches files under `working-capital/`, `accounting/`, 
`calendars/`, `meetings/` and `organization/` only, so it does not overlap with 
the other `window.confirm` issues and several can be in flight at the same time.
   


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