Aman-Mittal opened a new issue, #252:
URL: https://github.com/apache/fineract-backoffice-ui/issues/252
## Business value
When a write fails, the user is not told. The error handler logs to the
browser console and stops, so from the operator's side the button was pressed
and nothing happened — no message, no change on screen, no indication whether
to try again.
In a banking back office that is worse than an error dialog. The two
outcomes an operator has to distinguish are "the platform rejected this" and
"the platform accepted this and the screen has not caught up", and a silent
failure looks identical to both. The realistic responses are to press the
button again — which risks a duplicate for anything that did in fact succeed —
or to assume it worked and move on, which leaves the record in a state nobody
knows about until it surfaces in a reconciliation.
Fineract's rejections are also the useful kind. It answers with a specific
reason: a mandatory parameter missing, a date outside the allowed range, a
status that forbids the command. All of that is discarded.
The application already has the piece needed to fix it.
`NotificationService` is used correctly in some newer components; these 114
sites predate that habit.
## Reproducing it
```
grep -rn '=> console\.error' src/app/features --include=*.ts | grep -v spec
| wc -l # 114
```
By feature: products 23, loans 22, clients 22, system 21, working-capital
10, accounting 4, transfers 3, security 2, mix 2, and one each in settings,
reporting and organization.
The shape is consistent:
```ts
// src/app/features/clients/tabs/client-notes-list.component.ts
this.noteService.deleteResourceTypeResourceIdNotesNoteId('clients',
this.clientId(), id).subscribe({
next: () => this.loadNotes(),
error: (err) => console.error('Failed to delete note', err),
});
```
To see it: open a client's notes, stop the backend (or use devtools to fail
the request), and delete a note. The row stays, and nothing tells you why.
## Describing the change
Report the failure to the user through `NotificationService`, with a
translated message:
```ts
private readonly notifications = inject(NotificationService);
private readonly i18n = inject(I18N);
// ...
error: () => void
this.notifications.error(this.i18n.translate('CLIENTS.NOTE_DELETE_FAILED')),
```
Three things worth getting right:
- **Keep the message about the action, not the exception.** "The note could
not be deleted." is what the user needs; the raw platform message is often a
validation code that means nothing to them. Whether to surface the platform's
`defaultUserMessage` as well is a fair question to raise on the PR — do not do
it silently in a first pass.
- **Reset any pending state.** Several of these sites set an `isSaving` or
`isLoading` flag before the request and only clear it in `next`, so a failure
leaves the form's submit button disabled for good. Clear it in `error` too.
- **A failed *load* is a different problem** and is already tracked in #223
— that one wants the table's `hasError` retry state, not a toast. This issue is
about writes: save, update, delete, and commands.
**One pull request per feature directory**, please, and a first contribution
should take one feature rather than all twelve. `organization`, `settings` and
`reporting` are one site each and are the gentlest starting point; `clients`,
`loans`, `products` and `system` are the substantial ones.
## Scope
In scope: the write-path handlers listed above — a user-facing error
message, and clearing any pending flag.
Out of scope: failed list loads (#223), a global `ErrorHandler`, retry
behaviour, and replacing `console.error` where it genuinely reports a
programming fault rather than a rejected request.
## Getting started
- `NotificationService` is at
`src/app/core/services/notification.service.ts`; `I18N` is covered in
`DOCS/ADAPTERS.md`.
- `src/app/features/groups/group-note-form.component.ts` and
`src/app/features/groups/tabs/group-notes-list.component.ts` show the intended
shape on both a form and a list.
- Message keys go in `src/assets/i18n/en.json` under the feature's existing
section.
- A unit spec asserting the notification is welcome —
`provideFakeAdapters()` in `src/app/testing/adapters.ts` records every toast
requested, so the assertion is on the request rather than on the DOM.
- `npm run build`, `npm run lint` and `npm test` must pass.
--
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]
