WEB-895: Add success notification feedback to client action components#3444
Open
Agaba-Ed wants to merge 1 commit intoopenMF:devfrom
Open
WEB-895: Add success notification feedback to client action components#3444Agaba-Ed wants to merge 1 commit intoopenMF:devfrom
Agaba-Ed wants to merge 1 commit intoopenMF:devfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Note
|
| Cohort / File(s) | Summary |
|---|---|
Notifier Service src/app/clients/clients-view/client-actions/client-action-notifier.service.ts, src/app/clients/clients-view/client-actions/client-action-notifier.service.spec.ts |
New root-provided service exposing notify(messageKey) and notifyAndNavigate(messageKey, route, commands?) (uses TranslateService, MatSnackBar, Router). Unit tests verify translation lookups, snackbar params, and relative navigation (default and custom commands). |
Refactored Client Action Components src/app/clients/clients-view/client-actions/.../accept-client-transfer.component.ts, .../activate-client.component.ts, .../add-client-charge.component.ts, .../add-client-collateral.component.ts, .../client-assign-staff.component.ts, .../close-client.component.ts, .../reactivate-client.component.ts, .../reject-client-transfer.component.ts, .../reject-client.component.ts, .../take-survey.component.ts, .../transfer-client.component.ts, .../undo-client-rejection.component.ts, .../undo-client-transfer.component.ts, .../update-client-savings-account.component.ts, .../withdraw-client.component.ts |
Removed Router imports/usage and unused ReactiveFormsModule where applicable; injected ClientActionNotifierService (notifier) and replaced direct router.navigate(...) calls with notifier.notifyAndNavigate(...); converted many injected members to private readonly; added explicit error handling in subscriptions calling notifier.notify(...). |
Unit Tests Added/Updated src/app/clients/clients-view/client-actions/.../*.component.spec.ts (many files) |
Added new or updated Jest specs to mock ClientActionNotifierService and assert notifyAndNavigate on success, notify on failure where applicable, and to cover initialization and submit flows for each affected component. |
Translations (multiple locales) src/assets/translations/en-US.json, src/assets/translations/.../*.json |
Added clients.actions.* translation keys (success and selected failure messages) across many locale files; small punctuation adjustments in some locales to allow new keys. |
New Component Tests (many) src/app/clients/clients-view/client-actions/.../*.component.spec.ts |
Numerous new test files added for individual client-action components validating form init, payload formatting (Dates.formatDate), executeClientCommand calls, and notifier usage for success/failure flows. |
Sequence Diagram(s)
sequenceDiagram
autonumber
actor Component as "Client Action Component"
participant API as "ClientsService / API"
participant Notifier as "ClientActionNotifierService"
participant Translate as "TranslateService"
participant Snackbar as "MatSnackBar"
participant Router as "Router"
Component->>API: executeClientCommand(...) / create... / propose...
API-->>Component: success
Component->>Notifier: notifyAndNavigate(messageKey, route, commands?)
Notifier->>Translate: instant(messageKey)
Notifier->>Translate: instant('labels.buttons.Close')
Translate-->>Notifier: translated message / action label
Notifier->>Snackbar: open(translated message, actionLabel, { duration: 3000 })
Notifier->>Router: navigate(commands, { relativeTo: route })
Router-->>Notifier: navigation completed
API-->>Component: error
Component->>Notifier: notify(failureMessageKey)
Notifier->>Translate: instant(failureMessageKey)
Notifier->>Snackbar: open(translated failure, actionLabel, { duration: 3000 })
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- feat: add comprehensive unit tests for ActivateClientComponent #3041: Overlaps ActivateClientComponent and its tests switching navigation to ClientActionNotifierService.
- WEB-882: Add unit tests for CloseClientComponent #3425: Related changes around CloseClientComponent refactor and tests to use notifyAndNavigate.
- WEB-639: Refactor redundant boolean comparisons #3063: Related edits in AddClientCharge submit/navigation behavior and tests.
Suggested reviewers
- gkbishnoi07
- IOhacker
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly and concisely describes the main change: adding success notification feedback to client action components, which aligns with the primary objective across all modified files. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (6)
src/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.ts (1)
98-103: Consider extracting a shared success-notification helper.This snackbar-then-navigate sequence is now duplicated across multiple client-action components; a small shared helper/service would reduce repetition and future drift.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.ts` around lines 98 - 103, The snackbar+navigate pattern in AcceptClientTransferComponent (within accept-client-transfer.component.ts) — specifically the clientsService.executeClientCommand(...).subscribe(...) callback that calls snackBar.open(this.translateService.instant('clients.actions.acceptTransfer.success'), ...) and router.navigate(['../../'], { relativeTo: this.route }) — is duplicated; extract this into a shared helper or small service (e.g., ClientActionNotifier with a method like notifySuccessAndNavigate(messageKey: string, route: any, relativeTo: ActivatedRoute)) and replace the inline code in AcceptClientTransferComponent (and other components) to call that helper, passing translateService.instant key, snackBar, router and route or inject the helper to perform snackBar.open and router.navigate consistently.src/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.spec.ts (1)
84-88: Consider more specific navigation assertion.The test asserts
router.navigatewas called but doesn't verify the navigation target. This matches the pattern in other tests but could be strengthened.🔧 Optional: Add specific navigation path assertion
it('should navigate after successful submission', () => { component.submit(); - expect(router.navigate).toHaveBeenCalled(); + expect(router.navigate).toHaveBeenCalledWith(['../../'], expect.anything()); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.spec.ts` around lines 84 - 88, The test currently only checks that router.navigate was called after component.submit(); update it to assert the actual navigation target by replacing expect(router.navigate).toHaveBeenCalled() with an assertion that checks the arguments passed to router.navigate (e.g. expect(router.navigate).toHaveBeenCalledWith(['/expected-path', expectedId]) or a matcher like expect(router.navigate).toHaveBeenCalledWith(jasmine.any(Array)) depending on the real route); locate the call in the spec around component.submit() and use the actual route/params that client-assign-staff.component's submit() should navigate to when constructing the assertion.src/app/clients/clients-view/client-actions/take-survey/take-survey.component.ts (1)
155-160: Same error handling recommendation applies here.Similar to
reactivate-client.component.ts, the subscription lacks error handling. Consider using the observer object pattern withnextanderrorcallbacks for consistent error feedback to users.♻️ Suggested error handling pattern
- this.clientsService.createNewSurvey(this.surveyData.id, this.formData).subscribe(() => { - this.snackBar.open(this.translateService.instant('clients.actions.takeSurvey.success'), 'Close', { - duration: 3000 - }); - this.router.navigate(['../../general'], { relativeTo: this.route }); - }); + this.clientsService.createNewSurvey(this.surveyData.id, this.formData).subscribe({ + next: () => { + this.snackBar.open(this.translateService.instant('clients.actions.takeSurvey.success'), 'Close', { + duration: 3000 + }); + this.router.navigate(['../../general'], { relativeTo: this.route }); + }, + error: () => { + this.snackBar.open(this.translateService.instant('labels.error'), 'Close', { + duration: 3000 + }); + } + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/client-actions/take-survey/take-survey.component.ts` around lines 155 - 160, The createNewSurvey subscription in take-survey.component (calling this.clientsService.createNewSurvey with this.surveyData.id and this.formData) currently only handles success; change the subscribe call to use the observer object pattern ({ next: ..., error: ... }) so errors are handled consistently—on success keep the snackBar/translateService success message and router.navigate, and in the error callback show a translated error snackBar (and optionally log or pass the error to a shared error handler) to give user feedback when the request fails.src/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.ts (1)
101-106: Consider adding error handling to the subscription.The
subscribe()callback only handles the success case. IfexecuteClientCommandfails, the error will propagate unhandled, potentially causing silent failures or console errors. This pattern appears consistent across the affected components in this PR.♻️ Suggested error handling pattern
- this.clientsService.executeClientCommand(this.clientId, 'reactivate', data).subscribe(() => { - this.snackBar.open(this.translateService.instant('clients.actions.reactivate.success'), 'Close', { - duration: 3000 - }); - this.router.navigate(['../../'], { relativeTo: this.route }); - }); + this.clientsService.executeClientCommand(this.clientId, 'reactivate', data).subscribe({ + next: () => { + this.snackBar.open(this.translateService.instant('clients.actions.reactivate.success'), 'Close', { + duration: 3000 + }); + this.router.navigate(['../../'], { relativeTo: this.route }); + }, + error: () => { + this.snackBar.open(this.translateService.instant('labels.error'), 'Close', { + duration: 3000 + }); + } + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.ts` around lines 101 - 106, The current subscription in reactivate-client.component.ts calls clientsService.executeClientCommand(this.clientId, 'reactivate', data).subscribe(...) but only handles success; update the subscription to handle errors by adding an error callback to subscribe (or use pipe with catchError) that calls snackBar.open with a translated error message via translateService.instant (e.g., 'clients.actions.reactivate.error') and logs or handles the error, and ensure the success callback still navigates with router.navigate([...], { relativeTo: this.route }); keep references to executeClientCommand, this.clientId, snackBar, translateService, router and route when implementing the error handler.src/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.spec.ts (2)
52-73: Redundant TranslateModule import when mocking TranslateService.Both
TranslateModule.forRoot()(line 55) and a mockTranslateService(line 69) are provided. While the explicit mock takes precedence, importing the module is unnecessary overhead. Consider removingTranslateModule.forRoot()since you're fully mocking the service.♻️ Suggested simplification
await TestBed.configureTestingModule({ imports: [ - ReactivateClientComponent, - TranslateModule.forRoot() + ReactivateClientComponent ], providers: [🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.spec.ts` around lines 52 - 73, Remove the unnecessary TranslateModule.forRoot() import from the TestBed.configureTestingModule call in the ReactivateClientComponent spec: since you already provide a mock TranslateService via the TranslateService provider, delete the TranslateModule.forRoot() entry from the imports array so the test relies only on the mocked TranslateService and avoids extra module setup.
127-133: Strengthen navigation assertion with expected arguments.The assertion only verifies
toHaveBeenCalled(), but per the component code, navigation should be to['../../']withrelativeTo: this.route. A more specific assertion would catch regressions if the navigation path changes.♻️ Suggested improvement
it('should navigate after successful submission', () => { fillValidForm(); component.submit(); - expect(router.navigate).toHaveBeenCalled(); + expect(router.navigate).toHaveBeenCalledWith(['../../'], expect.objectContaining({ relativeTo: expect.anything() })); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.spec.ts` around lines 127 - 133, Update the test to assert the exact navigation arguments instead of just that navigation was called: after calling fillValidForm() and component.submit(), replace expect(router.navigate).toHaveBeenCalled() with expect(router.navigate).toHaveBeenCalledWith(['../../'], { relativeTo: component.route }) so the spec verifies navigation target and the relativeTo route used by the component (references: fillValidForm, component.submit, router.navigate, component.route).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.ts`:
- Around line 99-100: The snackbar action text is hardcoded as 'Close' in the
SnackBar open call inside the AcceptClientTransferComponent; replace the literal
with a translated string by calling this.translateService.instant with a new
i18n key (e.g. 'clients.actions.acceptTransfer.close') so the SnackBar open
becomes
this.snackBar.open(this.translateService.instant('clients.actions.acceptTransfer.success'),
this.translateService.instant('clients.actions.acceptTransfer.close'), {
duration: 3000 }), and add the corresponding translation entries to your i18n
files.
In
`@src/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.ts`:
- Around line 170-172: The snackbar action label is hardcoded as 'Close' in the
this.snackBar.open call inside AddClientChargeComponent; replace the literal
with a translated string using translateService.instant(...) (e.g.
this.translateService.instant('common.close') or the appropriate i18n key used
in your project) so both the message and the action are localized; update the
this.snackBar.open invocation in add-client-charge.component.ts to pass the
translated action label.
In
`@src/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.ts`:
- Around line 83-85: The snackbar action label is hardcoded as 'Close'; change
it to use the translation service so all user-facing strings are i18n'ed by
replacing the literal 'Close' in the snackBar.open call with a translateService
lookup (e.g., translateService.instant with the appropriate translation key) in
the client-assign-staff.component.ts so the call uses
translateService.instant('...') instead of the hardcoded string; keep the
existing success message key (clients.actions.assignStaff.success) and only
update the action label reference to a translation key (e.g., common.close or
clients.actions.close) via translateService.
In
`@src/app/clients/clients-view/client-actions/close-client/close-client.component.ts`:
- Line 107: Replace the hardcoded snack bar action label 'Close' with a
translated string: update the SnackBar call inside CloseClientComponent (the
method that calls this.snackBar.open(...)) to use translateService.instant with
an i18n key (for example
translateService.instant('clients.actions.close.action') or
'clients.actions.close.button') as the second argument so the snackbar action
text is localized. Ensure the chosen i18n key matches your translation files and
keep the existing duration option intact.
In
`@src/app/clients/clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.ts`:
- Around line 95-96: Replace the hardcoded 'Close' action label in the snackbar
call inside RejectClientTransferComponent by using TranslateService;
specifically update the this.snackBar.open(...) call to pass
this.translateService.instant('clients.actions.rejectTransfer.close') (or the
appropriate i18n key) instead of the literal 'Close', and ensure the
corresponding translation key is added to your i18n files; locate the change
where this.snackBar.open is invoked in reject-client-transfer.component.ts and
use translateService.instant to provide the translated action label.
In
`@src/app/clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.ts`:
- Around line 101-102: The snackbar action label is hardcoded as 'Close' in the
undo client rejection component; update the snackBar.open call in
UndoClientRejectionComponent (where this.snackBar.open(...) is invoked) to use
TranslateService.instant with a new i18n key (e.g.
'clients.actions.undoRejection.close') instead of the literal 'Close', and add
the corresponding entry to the translation files so the action label is fully
translatable.
In
`@src/app/clients/clients-view/client-actions/undo-client-transfer/undo-client-transfer.component.ts`:
- Around line 98-99: The snackbar action label is hardcoded as 'Close' in
undo-client-transfer.component.ts; replace it with a translated string via
TranslateService (e.g. call translateService.instant(...) instead of the
literal) so the snackBar.open call uses a translation key (for example
'common.close' or an appropriate i18n key like 'clients.actions.close') to
provide a localized action label; update the snackBar.open invocation that
currently passes 'Close' to use translateService.instant('your.translation.key')
so the UI follows the `@ngx-translate/core` i18n guideline.
In
`@src/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.ts`:
- Around line 80-82: Replace the hardcoded snackbar action label 'Close' in the
this.snackBar.open call inside update-client-savings-account.component (the line
using
this.translateService.instant('clients.actions.updateSavingsAccount.success'))
with a translated string from TranslateService (e.g.
this.translateService.instant('common.close') or an appropriate i18n key such as
'clients.actions.updateSavingsAccount.close'); update the second argument to use
that translateService.instant call so the snackbar action is localized.
In
`@src/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.ts`:
- Around line 107-108: The snackbar action label is hardcoded as 'Close' in
withdraw-client.component.ts; replace it with a translated string using
TranslateService (e.g., this.translateService.instant('common.close') or an
appropriate existing key) so the snackBar.open call uses the localized action
label, and if the key does not exist add a shared i18n key like "common.close"
to the translation files and reference that key from the component.
---
Nitpick comments:
In
`@src/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.ts`:
- Around line 98-103: The snackbar+navigate pattern in
AcceptClientTransferComponent (within accept-client-transfer.component.ts) —
specifically the clientsService.executeClientCommand(...).subscribe(...)
callback that calls
snackBar.open(this.translateService.instant('clients.actions.acceptTransfer.success'),
...) and router.navigate(['../../'], { relativeTo: this.route }) — is
duplicated; extract this into a shared helper or small service (e.g.,
ClientActionNotifier with a method like notifySuccessAndNavigate(messageKey:
string, route: any, relativeTo: ActivatedRoute)) and replace the inline code in
AcceptClientTransferComponent (and other components) to call that helper,
passing translateService.instant key, snackBar, router and route or inject the
helper to perform snackBar.open and router.navigate consistently.
In
`@src/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.spec.ts`:
- Around line 84-88: The test currently only checks that router.navigate was
called after component.submit(); update it to assert the actual navigation
target by replacing expect(router.navigate).toHaveBeenCalled() with an assertion
that checks the arguments passed to router.navigate (e.g.
expect(router.navigate).toHaveBeenCalledWith(['/expected-path', expectedId]) or
a matcher like expect(router.navigate).toHaveBeenCalledWith(jasmine.any(Array))
depending on the real route); locate the call in the spec around
component.submit() and use the actual route/params that
client-assign-staff.component's submit() should navigate to when constructing
the assertion.
In
`@src/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.spec.ts`:
- Around line 52-73: Remove the unnecessary TranslateModule.forRoot() import
from the TestBed.configureTestingModule call in the ReactivateClientComponent
spec: since you already provide a mock TranslateService via the TranslateService
provider, delete the TranslateModule.forRoot() entry from the imports array so
the test relies only on the mocked TranslateService and avoids extra module
setup.
- Around line 127-133: Update the test to assert the exact navigation arguments
instead of just that navigation was called: after calling fillValidForm() and
component.submit(), replace expect(router.navigate).toHaveBeenCalled() with
expect(router.navigate).toHaveBeenCalledWith(['../../'], { relativeTo:
component.route }) so the spec verifies navigation target and the relativeTo
route used by the component (references: fillValidForm, component.submit,
router.navigate, component.route).
In
`@src/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.ts`:
- Around line 101-106: The current subscription in
reactivate-client.component.ts calls
clientsService.executeClientCommand(this.clientId, 'reactivate',
data).subscribe(...) but only handles success; update the subscription to handle
errors by adding an error callback to subscribe (or use pipe with catchError)
that calls snackBar.open with a translated error message via
translateService.instant (e.g., 'clients.actions.reactivate.error') and logs or
handles the error, and ensure the success callback still navigates with
router.navigate([...], { relativeTo: this.route }); keep references to
executeClientCommand, this.clientId, snackBar, translateService, router and
route when implementing the error handler.
In
`@src/app/clients/clients-view/client-actions/take-survey/take-survey.component.ts`:
- Around line 155-160: The createNewSurvey subscription in take-survey.component
(calling this.clientsService.createNewSurvey with this.surveyData.id and
this.formData) currently only handles success; change the subscribe call to use
the observer object pattern ({ next: ..., error: ... }) so errors are handled
consistently—on success keep the snackBar/translateService success message and
router.navigate, and in the error callback show a translated error snackBar (and
optionally log or pass the error to a shared error handler) to give user
feedback when the request fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 120b4142-dbb2-47a9-ac72-81b22d08ee9d
📒 Files selected for processing (31)
src/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.spec.tssrc/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.tssrc/app/clients/clients-view/client-actions/activate-client/activate-client.component.spec.tssrc/app/clients/clients-view/client-actions/activate-client/activate-client.component.tssrc/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.spec.tssrc/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.tssrc/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.spec.tssrc/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.tssrc/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.spec.tssrc/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.tssrc/app/clients/clients-view/client-actions/close-client/close-client.component.spec.tssrc/app/clients/clients-view/client-actions/close-client/close-client.component.tssrc/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.spec.tssrc/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.tssrc/app/clients/clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.spec.tssrc/app/clients/clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.tssrc/app/clients/clients-view/client-actions/reject-client/reject-client.component.spec.tssrc/app/clients/clients-view/client-actions/reject-client/reject-client.component.tssrc/app/clients/clients-view/client-actions/take-survey/take-survey.component.spec.tssrc/app/clients/clients-view/client-actions/take-survey/take-survey.component.tssrc/app/clients/clients-view/client-actions/transfer-client/transfer-client.component.spec.tssrc/app/clients/clients-view/client-actions/transfer-client/transfer-client.component.tssrc/app/clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.spec.tssrc/app/clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.tssrc/app/clients/clients-view/client-actions/undo-client-transfer/undo-client-transfer.component.spec.tssrc/app/clients/clients-view/client-actions/undo-client-transfer/undo-client-transfer.component.tssrc/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.spec.tssrc/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.tssrc/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.spec.tssrc/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.tssrc/assets/translations/en-US.json
Comment on lines
+99
to
+100
| this.snackBar.open(this.translateService.instant('clients.actions.acceptTransfer.success'), 'Close', { | ||
| duration: 3000 |
There was a problem hiding this comment.
Fully localize snackbar actions.
Line 99 still hardcodes 'Close'. Please use a translated key for the action text too.
As per coding guidelines, src/app/**/*.{html,ts} must use proper i18n variables from @ngx-translate/core for all user-facing strings instead of hardcoded text.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.ts`
around lines 99 - 100, The snackbar action text is hardcoded as 'Close' in the
SnackBar open call inside the AcceptClientTransferComponent; replace the literal
with a translated string by calling this.translateService.instant with a new
i18n key (e.g. 'clients.actions.acceptTransfer.close') so the SnackBar open
becomes
this.snackBar.open(this.translateService.instant('clients.actions.acceptTransfer.success'),
this.translateService.instant('clients.actions.acceptTransfer.close'), {
duration: 3000 }), and add the corresponding translation entries to your i18n
files.
Comment on lines
+170
to
+172
| this.snackBar.open(this.translateService.instant('clients.actions.addCharge.success'), 'Close', { | ||
| duration: 3000 | ||
| }); |
There was a problem hiding this comment.
Use i18n for snackbar action text ('Close') as well.
Please localize the action label to stay consistent with the translated message.
💡 Suggested fix
- this.snackBar.open(this.translateService.instant('clients.actions.addCharge.success'), 'Close', {
- duration: 3000
- });
+ this.snackBar.open(
+ this.translateService.instant('clients.actions.addCharge.success'),
+ this.translateService.instant('labels.buttons.Close'),
+ { duration: 3000 }
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| this.snackBar.open(this.translateService.instant('clients.actions.addCharge.success'), 'Close', { | |
| duration: 3000 | |
| }); | |
| this.snackBar.open( | |
| this.translateService.instant('clients.actions.addCharge.success'), | |
| this.translateService.instant('labels.buttons.Close'), | |
| { duration: 3000 } | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.ts`
around lines 170 - 172, The snackbar action label is hardcoded as 'Close' in the
this.snackBar.open call inside AddClientChargeComponent; replace the literal
with a translated string using translateService.instant(...) (e.g.
this.translateService.instant('common.close') or the appropriate i18n key used
in your project) so both the message and the action are localized; update the
this.snackBar.open invocation in add-client-charge.component.ts to pass the
translated action label.
Comment on lines
+83
to
+85
| this.snackBar.open(this.translateService.instant('clients.actions.assignStaff.success'), 'Close', { | ||
| duration: 3000 | ||
| }); |
There was a problem hiding this comment.
Replace hardcoded snackbar action text with a translation key.
The success message is localized, but the action label is still hardcoded.
💡 Suggested fix
- this.snackBar.open(this.translateService.instant('clients.actions.assignStaff.success'), 'Close', {
- duration: 3000
- });
+ this.snackBar.open(
+ this.translateService.instant('clients.actions.assignStaff.success'),
+ this.translateService.instant('labels.buttons.Close'),
+ { duration: 3000 }
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| this.snackBar.open(this.translateService.instant('clients.actions.assignStaff.success'), 'Close', { | |
| duration: 3000 | |
| }); | |
| this.snackBar.open( | |
| this.translateService.instant('clients.actions.assignStaff.success'), | |
| this.translateService.instant('labels.buttons.Close'), | |
| { duration: 3000 } | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.ts`
around lines 83 - 85, The snackbar action label is hardcoded as 'Close'; change
it to use the translation service so all user-facing strings are i18n'ed by
replacing the literal 'Close' in the snackBar.open call with a translateService
lookup (e.g., translateService.instant with the appropriate translation key) in
the client-assign-staff.component.ts so the call uses
translateService.instant('...') instead of the hardcoded string; keep the
existing success message key (clients.actions.assignStaff.success) and only
update the action label reference to a translation key (e.g., common.close or
clients.actions.close) via translateService.
| locale | ||
| }; | ||
| this.clientsService.executeClientCommand(this.clientId, 'close', data).subscribe(() => { | ||
| this.snackBar.open(this.translateService.instant('clients.actions.close.success'), 'Close', { duration: 3000 }); |
There was a problem hiding this comment.
Replace hardcoded 'Close' with a translated label.
Line 107 should use an i18n key for the snackbar action text to keep localization complete.
As per coding guidelines, src/app/**/*.{html,ts} must use proper i18n variables from @ngx-translate/core for all user-facing strings instead of hardcoded text.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/clients/clients-view/client-actions/close-client/close-client.component.ts`
at line 107, Replace the hardcoded snack bar action label 'Close' with a
translated string: update the SnackBar call inside CloseClientComponent (the
method that calls this.snackBar.open(...)) to use translateService.instant with
an i18n key (for example
translateService.instant('clients.actions.close.action') or
'clients.actions.close.button') as the second argument so the snackbar action
text is localized. Ensure the chosen i18n key matches your translation files and
keep the existing duration option intact.
Comment on lines
+95
to
+96
| this.snackBar.open(this.translateService.instant('clients.actions.rejectTransfer.success'), 'Close', { | ||
| duration: 3000 |
There was a problem hiding this comment.
Translate the snackbar action label.
Line 95 uses hardcoded 'Close'. Please switch this to an i18n key via TranslateService.
As per coding guidelines, src/app/**/*.{html,ts} must use proper i18n variables from @ngx-translate/core for all user-facing strings instead of hardcoded text.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/clients/clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.ts`
around lines 95 - 96, Replace the hardcoded 'Close' action label in the snackbar
call inside RejectClientTransferComponent by using TranslateService;
specifically update the this.snackBar.open(...) call to pass
this.translateService.instant('clients.actions.rejectTransfer.close') (or the
appropriate i18n key) instead of the literal 'Close', and ensure the
corresponding translation key is added to your i18n files; locate the change
where this.snackBar.open is invoked in reject-client-transfer.component.ts and
use translateService.instant to provide the translated action label.
Comment on lines
+101
to
+102
| this.snackBar.open(this.translateService.instant('clients.actions.undoRejection.success'), 'Close', { | ||
| duration: 3000 |
There was a problem hiding this comment.
Localize snackbar action text here as well.
Line 101 uses 'Close' directly; please use TranslateService for the action label to keep this message fully translatable.
As per coding guidelines, src/app/**/*.{html,ts} must use proper i18n variables from @ngx-translate/core for all user-facing strings instead of hardcoded text.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.ts`
around lines 101 - 102, The snackbar action label is hardcoded as 'Close' in the
undo client rejection component; update the snackBar.open call in
UndoClientRejectionComponent (where this.snackBar.open(...) is invoked) to use
TranslateService.instant with a new i18n key (e.g.
'clients.actions.undoRejection.close') instead of the literal 'Close', and add
the corresponding entry to the translation files so the action label is fully
translatable.
Comment on lines
+98
to
+99
| this.snackBar.open(this.translateService.instant('clients.actions.undoTransfer.success'), 'Close', { | ||
| duration: 3000 |
There was a problem hiding this comment.
Avoid hardcoded snackbar action text.
On Line 98, localize the 'Close' action label through TranslateService so this UI feedback is fully translatable.
As per coding guidelines, src/app/**/*.{html,ts} must use proper i18n variables from @ngx-translate/core for all user-facing strings instead of hardcoded text.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/clients/clients-view/client-actions/undo-client-transfer/undo-client-transfer.component.ts`
around lines 98 - 99, The snackbar action label is hardcoded as 'Close' in
undo-client-transfer.component.ts; replace it with a translated string via
TranslateService (e.g. call translateService.instant(...) instead of the
literal) so the snackBar.open call uses a translation key (for example
'common.close' or an appropriate i18n key like 'clients.actions.close') to
provide a localized action label; update the snackBar.open invocation that
currently passes 'Close' to use translateService.instant('your.translation.key')
so the UI follows the `@ngx-translate/core` i18n guideline.
Comment on lines
+80
to
+82
| this.snackBar.open(this.translateService.instant('clients.actions.updateSavingsAccount.success'), 'Close', { | ||
| duration: 3000 | ||
| }); |
There was a problem hiding this comment.
Localize the snackbar action label instead of hardcoding 'Close'.
This string is user-facing and should come from translation keys.
💡 Suggested fix
- this.snackBar.open(this.translateService.instant('clients.actions.updateSavingsAccount.success'), 'Close', {
- duration: 3000
- });
+ this.snackBar.open(
+ this.translateService.instant('clients.actions.updateSavingsAccount.success'),
+ this.translateService.instant('labels.buttons.Close'),
+ { duration: 3000 }
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| this.snackBar.open(this.translateService.instant('clients.actions.updateSavingsAccount.success'), 'Close', { | |
| duration: 3000 | |
| }); | |
| this.snackBar.open( | |
| this.translateService.instant('clients.actions.updateSavingsAccount.success'), | |
| this.translateService.instant('labels.buttons.Close'), | |
| { duration: 3000 } | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.ts`
around lines 80 - 82, Replace the hardcoded snackbar action label 'Close' in the
this.snackBar.open call inside update-client-savings-account.component (the line
using
this.translateService.instant('clients.actions.updateSavingsAccount.success'))
with a translated string from TranslateService (e.g.
this.translateService.instant('common.close') or an appropriate i18n key such as
'clients.actions.updateSavingsAccount.close'); update the second argument to use
that translateService.instant call so the snackbar action is localized.
Comment on lines
+107
to
+108
| this.snackBar.open(this.translateService.instant('clients.actions.withdraw.success'), 'Close', { | ||
| duration: 3000 |
There was a problem hiding this comment.
Localize the snackbar action label as well.
Line 107 still uses a hardcoded 'Close' label. Please translate the action text via TranslateService (using an existing shared key, or add one) so the snackbar is fully localized.
As per coding guidelines, src/app/**/*.{html,ts} must use proper i18n variables from @ngx-translate/core for all user-facing strings instead of hardcoded text.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.ts`
around lines 107 - 108, The snackbar action label is hardcoded as 'Close' in
withdraw-client.component.ts; replace it with a translated string using
TranslateService (e.g., this.translateService.instant('common.close') or an
appropriate existing key) so the snackBar.open call uses the localized action
label, and if the key does not exist add a shared i18n key like "common.close"
to the translation files and reference that key from the component.
Agaba-Ed
force-pushed
the
add-client-action-success-notifications
branch
from
af7ef86 to
62cbcbd
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/clients/clients-view/client-actions/take-survey/take-survey.component.ts (1)
115-124:⚠️ Potential issue | 🟡 MinorLogic bug:
isAnyResponse()always returnstrue.The
return falseinsideforEachcallback returns from the callback, not fromisAnyResponse(). The method always returnstrueregardless of whether any answers exist.🐛 Proposed fix
isAnyResponse(): boolean { if (this.surveyData) { - this.surveyData.questionDatas.forEach((element: any) => { - if (element.answer) { - return false; - } - }); + return this.surveyData.questionDatas.some((element: any) => element.answer); } - return true; + return false; }Note: This bug appears to be pre-existing and not introduced by this PR, but worth noting since it affects the component's behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/client-actions/take-survey/take-survey.component.ts` around lines 115 - 124, The isAnyResponse() method incorrectly uses forEach so its inner return doesn't exit the outer function; change it to return the correct boolean by iterating with a for...of loop or using Array.prototype.some on this.surveyData.questionDatas (e.g., if (this.surveyData && this.surveyData.questionDatas.some(q => q.answer)) return false/true as appropriate) so the function returns false immediately when any question has an answer and true only when none do; ensure null/undefined checks for this.surveyData and questionDatas remain in place.
🧹 Nitpick comments (4)
src/app/clients/clients-view/client-actions/client-action-notifier.service.ts (1)
20-26: Add a focused spec aroundClientActionNotifierService.notifyAndNavigate().The provided component specs stub
ClientActionNotifierService, so the actual snackbar translation/open + relative navigation wiring introduced here is currently unexercised. A small service spec would cover the translated message, translated close label, duration, andrelativeTobehavior once for every consumer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/client-actions/client-action-notifier.service.ts` around lines 20 - 26, Add a focused unit test for ClientActionNotifierService.notifyAndNavigate that stubs/mocks the injected MatSnackBar, TranslateService, and Router; call notifyAndNavigate with a sample messageKey and a fake ActivatedRoute and assert that translateService.instant was called for the messageKey and 'labels.buttons.Close', that snackBar.open was called with the translated message, translated close label and duration 3000, and that router.navigate was invoked with the provided commands and { relativeTo: route } (reference methods: ClientActionNotifierService.notifyAndNavigate, snackBar.open, translateService.instant, router.navigate).src/app/clients/clients-view/client-actions/take-survey/take-survey.component.ts (1)
62-68: Stale JSDoc: Router is no longer injected.The
@param {Router} router Routerdocumentation references a dependency that was removed.📝 Proposed fix
/** * Retrieves the survey data from `resolve`. * `@param` {ActivatedRoute} route Activated Route * `@param` {ClientsService} clientsService ClientsService - * `@param` {Router} router Router * `@param` {AuthenticationService} authenticationService AuthenticationService + * `@param` {ClientActionNotifierService} notifier Client Action Notifier Service */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/client-actions/take-survey/take-survey.component.ts` around lines 62 - 68, Update the stale JSDoc on the TakeSurveyComponent constructor: remove the `@param {Router} router Router` entry (or replace it with any newly added dependency if applicable) so the JSDoc matches the actual injected parameters (e.g., ActivatedRoute, ClientsService, AuthenticationService) referenced in the constructor of TakeSurveyComponent; locate the JSDoc block above the constructor in take-survey.component.ts and edit the param list to reflect the current constructor signature.src/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.ts (1)
49-56: Stale JSDoc: Router is no longer injected.The
@param {Router} router Routerdocumentation references a dependency that was removed.📝 Proposed fix
/** * `@param` {FormBuilder} formBuilder Form Builder * `@param` {clientsService} clientsService Clients Service * `@param` {Dates} dateUtils Date Utils * `@param` {ActivatedRoute} route Activated Route - * `@param` {Router} router Router * `@param` {SettingsService} settingsService Setting service + * `@param` {ClientActionNotifierService} notifier Client Action Notifier Service */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.ts` around lines 49 - 56, The JSDoc block above the ReactivateClientComponent constructor is stale: it still lists "@param {Router} router Router" even though Router is no longer injected; open the JSDoc comment in reactivate-client.component.ts (the block that documents the constructor for ReactivateClientComponent or the constructor function itself) and remove the obsolete "@param {Router} router Router" line (or update the param list to exactly match the current constructor parameters such as FormBuilder, clientsService, Dates, ActivatedRoute, SettingsService), ensuring the JSDoc matches the constructor signature.src/app/clients/clients-view/client-actions/transfer-client/transfer-client.component.ts (1)
53-60: Stale JSDoc: Router is no longer injected.The
@param {Router} router Routerdocumentation references a dependency that was removed in this refactor.📝 Proposed fix
/** * `@param` {FormBuilder} formBuilder Form Builder * `@param` {ClientsService} clientsService Clients Service * `@param` {Dates} dateUtils Date Utils * `@param` {ActivatedRoute} route Activated Route - * `@param` {Router} router Router * `@param` {SettingsService} settingsService Setting service + * `@param` {ClientActionNotifierService} notifier Client Action Notifier Service */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/client-actions/transfer-client/transfer-client.component.ts` around lines 53 - 60, The JSDoc above the constructor for TransferClientComponent still lists a removed dependency (`@param {Router} router Router`); update the JSDoc block for the constructor to remove the stale `Router` param entry so the doc matches the current constructor signature (check the constructor in TransferClientComponent and its injected symbols like FormBuilder, ClientsService, Dates, ActivatedRoute, SettingsService and remove the Router line).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.spec.ts`:
- Around line 1-12: Add the standard MPL file header to the new test file
client-assign-staff.component.spec.ts so it matches repository license header
requirements; prepend the official MPL comment block at the very top of the file
(above all imports) for this spec that tests ClientAssignStaffComponent and
related imports (e.g., ClientsService, ClientActionNotifierService,
TranslateModule), then run npm run headers:add (and npm run headers:check) to
ensure the header is formatted and passes CI.
- Around line 83-88: The test triggers an unhandled RxJS error because
clientsService.executeClientCommand is mocked to throwError and
component.submit() subscribes without an error handler; fix by either changing
the spec mock to suppress the error (have
clientsService.executeClientCommand.mockReturnValueOnce return a
throwError(...).pipe(catchError(() => EMPTY)) so the observable completes) or by
adding an error callback to the subscription inside the component's submit()
method (handle/log the error and avoid rethrowing) so notifier.notifyAndNavigate
is not invoked and the test no longer produces an unhandled error; references:
submit(), clientsService.executeClientCommand, notifier.notifyAndNavigate.
In
`@src/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.ts`:
- Around line 98-100: The subscribe on
clientsService.executeClientCommand(this.clientId, 'reactivate', data) only
handles success; add an error callback to the subscription so API failures are
handled: on error call the notifier with a failure message (e.g.,
'clients.actions.reactivate.failure') and keep the user on the same route or
present appropriate UI feedback, and optionally log the error; modify the
subscribe(...) call in reactivate-client.component.ts to include (err) => {
this.notifier.notify('clients.actions.reactivate.failure'); /* optional:
console/error logging */ } as the error handler.
In
`@src/app/clients/clients-view/client-actions/take-survey/take-survey.component.ts`:
- Around line 152-154: The subscribe on clientsService.createNewSurvey only
handles success; add an error callback to the subscription to handle API
failures: in take-survey.component.ts update the call to
clientsService.createNewSurvey(this.surveyData.id, this.formData).subscribe(...)
to include an error handler that logs the error (e.g., console or a logger),
calls notifier.notify with a failure key/message (e.g.,
'clients.actions.takeSurvey.failure') to surface feedback to the user, and
optionally keeps the route/navigation only on success (leave
notifier.notifyAndNavigate for the success callback). Ensure you reference the
existing clientsService.createNewSurvey, notifier.notify/notifyAndNavigate,
route, formData and surveyData.id symbols when making the change.
In
`@src/app/clients/clients-view/client-actions/transfer-client/transfer-client.component.ts`:
- Around line 106-108: The current subscription to
clientsService.executeClientCommand(this.clientId, 'proposeTransfer', data) only
handles success and omits error handling; update the call in
transfer-client.component.ts (the method invoking executeClientCommand) to
provide an error handler (either a second callback to subscribe or a catchError
in the observable chain) that calls notifier.notify (or another notifier method)
to show a failure message and optionally logs or surfaces the error; ensure the
success path still calls
notifier.notifyAndNavigate('clients.actions.transfer.success', this.route) while
the error path uses a clear failure message and preserves any cleanup or state
updates.
---
Outside diff comments:
In
`@src/app/clients/clients-view/client-actions/take-survey/take-survey.component.ts`:
- Around line 115-124: The isAnyResponse() method incorrectly uses forEach so
its inner return doesn't exit the outer function; change it to return the
correct boolean by iterating with a for...of loop or using Array.prototype.some
on this.surveyData.questionDatas (e.g., if (this.surveyData &&
this.surveyData.questionDatas.some(q => q.answer)) return false/true as
appropriate) so the function returns false immediately when any question has an
answer and true only when none do; ensure null/undefined checks for
this.surveyData and questionDatas remain in place.
---
Nitpick comments:
In
`@src/app/clients/clients-view/client-actions/client-action-notifier.service.ts`:
- Around line 20-26: Add a focused unit test for
ClientActionNotifierService.notifyAndNavigate that stubs/mocks the injected
MatSnackBar, TranslateService, and Router; call notifyAndNavigate with a sample
messageKey and a fake ActivatedRoute and assert that translateService.instant
was called for the messageKey and 'labels.buttons.Close', that snackBar.open was
called with the translated message, translated close label and duration 3000,
and that router.navigate was invoked with the provided commands and {
relativeTo: route } (reference methods:
ClientActionNotifierService.notifyAndNavigate, snackBar.open,
translateService.instant, router.navigate).
In
`@src/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.ts`:
- Around line 49-56: The JSDoc block above the ReactivateClientComponent
constructor is stale: it still lists "@param {Router} router Router" even though
Router is no longer injected; open the JSDoc comment in
reactivate-client.component.ts (the block that documents the constructor for
ReactivateClientComponent or the constructor function itself) and remove the
obsolete "@param {Router} router Router" line (or update the param list to
exactly match the current constructor parameters such as FormBuilder,
clientsService, Dates, ActivatedRoute, SettingsService), ensuring the JSDoc
matches the constructor signature.
In
`@src/app/clients/clients-view/client-actions/take-survey/take-survey.component.ts`:
- Around line 62-68: Update the stale JSDoc on the TakeSurveyComponent
constructor: remove the `@param {Router} router Router` entry (or replace it
with any newly added dependency if applicable) so the JSDoc matches the actual
injected parameters (e.g., ActivatedRoute, ClientsService,
AuthenticationService) referenced in the constructor of TakeSurveyComponent;
locate the JSDoc block above the constructor in take-survey.component.ts and
edit the param list to reflect the current constructor signature.
In
`@src/app/clients/clients-view/client-actions/transfer-client/transfer-client.component.ts`:
- Around line 53-60: The JSDoc above the constructor for TransferClientComponent
still lists a removed dependency (`@param {Router} router Router`); update the
JSDoc block for the constructor to remove the stale `Router` param entry so the
doc matches the current constructor signature (check the constructor in
TransferClientComponent and its injected symbols like FormBuilder,
ClientsService, Dates, ActivatedRoute, SettingsService and remove the Router
line).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 79298424-a764-4a8b-8fb2-ad4519609e8d
📒 Files selected for processing (32)
src/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.spec.tssrc/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.tssrc/app/clients/clients-view/client-actions/activate-client/activate-client.component.spec.tssrc/app/clients/clients-view/client-actions/activate-client/activate-client.component.tssrc/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.spec.tssrc/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.tssrc/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.spec.tssrc/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.tssrc/app/clients/clients-view/client-actions/client-action-notifier.service.tssrc/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.spec.tssrc/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.tssrc/app/clients/clients-view/client-actions/close-client/close-client.component.spec.tssrc/app/clients/clients-view/client-actions/close-client/close-client.component.tssrc/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.spec.tssrc/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.tssrc/app/clients/clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.spec.tssrc/app/clients/clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.tssrc/app/clients/clients-view/client-actions/reject-client/reject-client.component.spec.tssrc/app/clients/clients-view/client-actions/reject-client/reject-client.component.tssrc/app/clients/clients-view/client-actions/take-survey/take-survey.component.spec.tssrc/app/clients/clients-view/client-actions/take-survey/take-survey.component.tssrc/app/clients/clients-view/client-actions/transfer-client/transfer-client.component.spec.tssrc/app/clients/clients-view/client-actions/transfer-client/transfer-client.component.tssrc/app/clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.spec.tssrc/app/clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.tssrc/app/clients/clients-view/client-actions/undo-client-transfer/undo-client-transfer.component.spec.tssrc/app/clients/clients-view/client-actions/undo-client-transfer/undo-client-transfer.component.tssrc/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.spec.tssrc/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.tssrc/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.spec.tssrc/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.tssrc/assets/translations/en-US.json
✅ Files skipped from review due to trivial changes (4)
- src/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.spec.ts
- src/assets/translations/en-US.json
- src/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.spec.ts
- src/app/clients/clients-view/client-actions/transfer-client/transfer-client.component.spec.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- src/app/clients/clients-view/client-actions/undo-client-transfer/undo-client-transfer.component.ts
- src/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.ts
- src/app/clients/clients-view/client-actions/close-client/close-client.component.spec.ts
- src/app/clients/clients-view/client-actions/activate-client/activate-client.component.spec.ts
- src/app/clients/clients-view/client-actions/undo-client-transfer/undo-client-transfer.component.spec.ts
- src/app/clients/clients-view/client-actions/reject-client/reject-client.component.ts
- src/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.spec.ts
- src/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.ts
- src/app/clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.spec.ts
- src/app/clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.ts
- src/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.spec.ts
- src/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.spec.ts
- src/app/clients/clients-view/client-actions/reject-client/reject-client.component.spec.ts
- src/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.ts
...lients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.spec.ts
Show resolved
Hide resolved
...lients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.spec.ts
Outdated
Show resolved
Hide resolved
src/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.ts
Outdated
Show resolved
Hide resolved
src/app/clients/clients-view/client-actions/take-survey/take-survey.component.ts
Outdated
Show resolved
Hide resolved
src/app/clients/clients-view/client-actions/transfer-client/transfer-client.component.ts
Outdated
Show resolved
Hide resolved
Agaba-Ed
force-pushed
the
add-client-action-success-notifications
branch
from
62cbcbd to
e293d02
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.spec.ts (1)
19-23: Improve type safety of test mocks by using narrowPick<>types instead ofas anycasts.The test mocks for
ClientsService,SettingsService,Dates, andClientActionNotifierServicecurrently useas anycasts. Since the tests only consume specific methods (executeClientCommand,formatDate,notifyAndNavigate) and properties (language,dateFormat,businessDate), usingPick<...>for each service will:
- Provide compile-time safety if service APIs change
- Make test dependencies explicit and self-documenting
- Align with the strict typing guideline for
src/app/**/*.ts♻️ Suggested refactor
- let clientsService: jest.Mocked<ClientsService>; - let settingsService: SettingsService; - let dates: jest.Mocked<Dates>; - let notifier: jest.Mocked<ClientActionNotifierService>; + let clientsService: jest.Mocked<Pick<ClientsService, 'executeClientCommand'>>; + let settingsService: Pick<SettingsService, 'language' | 'dateFormat' | 'businessDate'>; + let dates: jest.Mocked<Pick<Dates, 'formatDate'>>; + let notifier: jest.Mocked<Pick<ClientActionNotifierService, 'notifyAndNavigate'>>; @@ - clientsService = { - executeClientCommand: jest.fn(() => of({})) - } as any; + clientsService = { + executeClientCommand: jest.fn(() => of({})) + }; @@ - settingsService = { + settingsService = { language: { code: 'en' }, dateFormat: 'dd MMMM yyyy', businessDate - } as any; + }; @@ - dates = { + dates = { formatDate: jest.fn(() => '20 March 2026') - } as any; + }; @@ - notifier = { notifyAndNavigate: jest.fn() } as any; + notifier = { notifyAndNavigate: jest.fn() };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.spec.ts` around lines 19 - 23, The tests currently cast mocks to any; replace those with narrow Pick<>-based types to improve type safety: declare clientsService as jest.Mocked<Pick<ClientsService, 'executeClientCommand'>>, settingsService as Pick<SettingsService, 'language' | 'dateFormat'> (wrapped in jest.Mocked if you need jest.fn on methods), dates as jest.Mocked<Pick<Dates, 'formatDate' | 'businessDate'>>, and notifier as jest.Mocked<Pick<ClientActionNotifierService, 'notifyAndNavigate'>>; update the mock object initializations to implement only those named methods/properties (using jest.fn() where appropriate) instead of casting with as any so the compiler enforces the limited API used by the tests (references: executeClientCommand, formatDate, notifyAndNavigate, language, dateFormat, businessDate).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/app/clients/clients-view/client-actions/reject-client/reject-client.component.ts`:
- Around line 103-105: The subscription to clientsService.executeClientCommand
in reject-client.component.ts currently only handles success; add an error
handler to the Observable subscription (i.e., provide a second callback to
subscribe or use .subscribe({ next, error })) that calls the notifier with a
failure translation key (e.g., 'clients.actions.reject.failure') and does not
navigate, so users see an error message; follow the same pattern for the other
five components (add-client-collateral.component.ts where createClientCollateral
is used, add-client-charge.component.ts where createClientCharge is used,
activate-client.component.ts, undo-client-rejection.component.ts,
withdraw-client.component.ts) ensuring each failure uses the appropriate
'clients.actions.<action>.failure' key (add keys if missing) and surfaces the
error to the user via notifier.
---
Nitpick comments:
In
`@src/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.spec.ts`:
- Around line 19-23: The tests currently cast mocks to any; replace those with
narrow Pick<>-based types to improve type safety: declare clientsService as
jest.Mocked<Pick<ClientsService, 'executeClientCommand'>>, settingsService as
Pick<SettingsService, 'language' | 'dateFormat'> (wrapped in jest.Mocked if you
need jest.fn on methods), dates as jest.Mocked<Pick<Dates, 'formatDate' |
'businessDate'>>, and notifier as jest.Mocked<Pick<ClientActionNotifierService,
'notifyAndNavigate'>>; update the mock object initializations to implement only
those named methods/properties (using jest.fn() where appropriate) instead of
casting with as any so the compiler enforces the limited API used by the tests
(references: executeClientCommand, formatDate, notifyAndNavigate, language,
dateFormat, businessDate).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 400c77d6-6446-4d1c-b302-05aff33bf06a
📒 Files selected for processing (33)
src/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.spec.tssrc/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.tssrc/app/clients/clients-view/client-actions/activate-client/activate-client.component.spec.tssrc/app/clients/clients-view/client-actions/activate-client/activate-client.component.tssrc/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.spec.tssrc/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.tssrc/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.spec.tssrc/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.tssrc/app/clients/clients-view/client-actions/client-action-notifier.service.spec.tssrc/app/clients/clients-view/client-actions/client-action-notifier.service.tssrc/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.spec.tssrc/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.tssrc/app/clients/clients-view/client-actions/close-client/close-client.component.spec.tssrc/app/clients/clients-view/client-actions/close-client/close-client.component.tssrc/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.spec.tssrc/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.tssrc/app/clients/clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.spec.tssrc/app/clients/clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.tssrc/app/clients/clients-view/client-actions/reject-client/reject-client.component.spec.tssrc/app/clients/clients-view/client-actions/reject-client/reject-client.component.tssrc/app/clients/clients-view/client-actions/take-survey/take-survey.component.spec.tssrc/app/clients/clients-view/client-actions/take-survey/take-survey.component.tssrc/app/clients/clients-view/client-actions/transfer-client/transfer-client.component.spec.tssrc/app/clients/clients-view/client-actions/transfer-client/transfer-client.component.tssrc/app/clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.spec.tssrc/app/clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.tssrc/app/clients/clients-view/client-actions/undo-client-transfer/undo-client-transfer.component.spec.tssrc/app/clients/clients-view/client-actions/undo-client-transfer/undo-client-transfer.component.tssrc/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.spec.tssrc/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.tssrc/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.spec.tssrc/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.tssrc/assets/translations/en-US.json
✅ Files skipped from review due to trivial changes (8)
- src/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.spec.ts
- src/app/clients/clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.spec.ts
- src/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.spec.ts
- src/app/clients/clients-view/client-actions/take-survey/take-survey.component.spec.ts
- src/app/clients/clients-view/client-actions/undo-client-transfer/undo-client-transfer.component.spec.ts
- src/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.spec.ts
- src/app/clients/clients-view/client-actions/transfer-client/transfer-client.component.spec.ts
- src/app/clients/clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- src/app/clients/clients-view/client-actions/close-client/close-client.component.spec.ts
- src/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.ts
- src/app/clients/clients-view/client-actions/undo-client-transfer/undo-client-transfer.component.ts
- src/app/clients/clients-view/client-actions/activate-client/activate-client.component.spec.ts
- src/app/clients/clients-view/client-actions/close-client/close-client.component.ts
- src/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.ts
- src/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.spec.ts
- src/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.spec.ts
- src/app/clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.spec.ts
- src/app/clients/clients-view/client-actions/take-survey/take-survey.component.ts
- src/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.ts
- src/app/clients/clients-view/client-actions/client-action-notifier.service.ts
- src/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.ts
- src/assets/translations/en-US.json
- src/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.spec.ts
- src/app/clients/clients-view/client-actions/transfer-client/transfer-client.component.ts
src/app/clients/clients-view/client-actions/reject-client/reject-client.component.ts
Outdated
Show resolved
Hide resolved
IOhacker
requested changes
Mar 30, 2026
Contributor
IOhacker
left a comment
IOhacker
left a comment
There was a problem hiding this comment.
Missing translation for the other languages supported by Mifos WebApp
Agaba-Ed
force-pushed
the
add-client-action-success-notifications
branch
from
e293d02 to
a23b5a4
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (1)
src/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.spec.ts (1)
1-12:⚠️ Potential issue | 🟠 MajorAdd the MPL header at the top of this new spec file.
This file still starts directly with imports and is missing the repository license header.
🧾 Proposed fix
+/** + * Copyright since 2025 Mifos Initiative + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + import { ComponentFixture, TestBed } from '@angular/core/testing';As per coding guidelines, "Always run
npm run headers:checkandnpm run headers:addto ensure new files have the correct open source license file headers".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.spec.ts` around lines 1 - 12, This new spec file is missing the project's MPL license header; open the file that contains the imports and test setup for ClientAssignStaffComponent (the file with imports like ClientAssignStaffComponent, ClientsService, ClientActionNotifierService and the jest test imports) and prepend the standard MPL repository header comment block to the very top of the file (use the same header used across the repo); afterwards run the repository header tooling (npm run headers:add or headers:check) to ensure the header matches project standards.
🧹 Nitpick comments (4)
src/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.ts (1)
51-58: Clean up stale constructor JSDoc.The docblock still lists
Routereven though it is no longer injected.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.ts` around lines 51 - 58, Update the stale JSDoc above the constructor in WithdrawClientComponent to match the actual injected parameters: remove the obsolete "Router" entry and ensure the docblock lists only FormBuilder, ClientsService, Dates (dateUtils), ActivatedRoute (route), and SettingsService (settingsService) as in the constructor signature; adjust any parameter names in the comments to exactly match the constructor parameter names so the JSDoc accurately reflects the code.src/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.ts (1)
60-67: Update constructor JSDoc to match current injections.
@param {Router} routeris stale and should be removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.ts` around lines 60 - 67, The JSDoc for the AddClientChargeComponent constructor is out of sync: remove the stale "@param {Router} router" entry and ensure the remaining `@param` tags match the actual constructor parameters in add-client-charge.component.ts (e.g., FormBuilder formBuilder, ActivatedRoute route, Dates dateUtils, ClientsService clientsService, SettingsService settingsService) so the docblock reflects the current injections used by the constructor.src/app/clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.ts (1)
49-55: Update stale constructor JSDoc params.
@param {Router} routerno longer matches the injected dependencies and should be removed to avoid misleading docs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.ts` around lines 49 - 55, Update the constructor JSDoc in undo-client-rejection.component.ts to remove the stale "@param {Router} router" entry and ensure the remaining `@param` tags match the actual injected constructor parameters (e.g., FormBuilder, clientsService, Dates, ActivatedRoute); also fix any typos in param descriptions (e.g., "Cliens Service" → "Clients Service") so the JSDoc accurately reflects the constructor signature.src/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.spec.ts (1)
28-43: Replaceas anytype casts with properly typed mock objects.Lines 28–42 use
as anyto bypass type safety on incomplete mock objects. Since variables are declared with full interface types (e.g.,jest.Mocked<ClientsService>on line 19), the partial assignments need explicit casting. Useas Partial<T>or declare variables asjest.Mocked<Partial<T>>to preserve compile-time checks and prevent hidden contract drift.Example:
// Instead of: clientsService = { executeClientCommand: jest.fn() } as any; // Use: clientsService = { executeClientCommand: jest.fn() } as jest.Mocked<Partial<ClientsService>>;Per coding guidelines (
src/app/**/*.ts), strict typing conventions are required.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.spec.ts` around lines 28 - 43, Replace the unsafe `as any` casts on the test mocks by typing them as partial mocked interfaces: assign clientsService as jest.Mocked<Partial<ClientsService>> (so executeClientCommand remains jest.fn()), settingsService as Partial/Mocked<Partial<SettingsService>> including language, dateFormat and businessDate, dates as jest.Mocked<Partial<DateService>> (with formatDate mocked), and notifier as jest.Mocked<Partial<NotifierService>>; this preserves the existing jest.fn() implementations while restoring compile-time type checks and matching the variable declarations (e.g., the earlier jest.Mocked<ClientsService> declaration).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.spec.ts`:
- Around line 1-14: This spec file is missing the required MPL repository
license header; prepend the standard MPL header comment block at the very top of
src/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.spec.ts
(before any imports) so the file passes headers:check, then re-run npm run
headers:check or npm run headers:add; no code changes to
AcceptClientTransferComponent, ClientsService, SettingsService, or the tests
themselves are needed—only add the repository's standard header comment to the
top of the file.
- Line 3: The submit() method currently subscribes to
clientCommandsService.executeClientCommand without an error handler, causing
unhandled RxJS errors in tests; update the subscribe call inside
AcceptClientTransferComponent.submit (the place calling executeClientCommand) to
include an error callback (e.g., add an object with error: () => { /* handle/log
and close/cleanup same as success path */ }) following the pattern used in
reactivate-client.component.ts, or alternatively adjust the spec mock to return
a non-throwing observable (e.g., of(null)) to suppress the error during test;
locate submit() and executeClientCommand references to apply the change.
In
`@src/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.spec.ts`:
- Around line 1-14: This new spec file is missing the required MPL license
header; open
src/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.spec.ts
and insert the standard MPL header comment block at the very top of the file
above the import statements (same header used in other repo files), then run the
repository header helper scripts (npm run headers:add and npm run headers:check)
to ensure it matches the project's license header conventions for files like
add-client-charge.component.spec.ts and related test files.
In
`@src/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.ts`:
- Around line 166-168: The submit() method currently subscribes to
clientsService.createClientCharge(this.clientId, clientCharge) with only a
success handler; add an error callback to that subscription so failures are
handled like other components (see take-survey.component.ts). In the error
handler call the notifier error method used across this directory (e.g.,
notifier.notifyError or notifier.notify with a failure key such as
'clients.actions.addCharge.failure') and optionally stop navigation; keep the
existing success handler that calls
notifier.notifyAndNavigate('clients.actions.addCharge.success', this.route).
Ensure you reference the same Observable returned by createClientCharge and
implement both success and error callbacks in the subscribe call inside
submit().
In
`@src/app/clients/clients-view/client-actions/reject-client/reject-client.component.spec.ts`:
- Around line 1-14: This spec file is missing the required MPL license header;
add the standard MPL header block at the top of
src/app/clients/clients-view/client-actions/reject-client/reject-client.component.spec.ts
before any imports. Ensure the header matches the project's canonical MPL block
used elsewhere (same formatting and year/owner placeholders) so it will pass npm
run headers:check/npm run headers:add, then save and re-run the header check; no
code changes to RejectClientComponent, tests, or imports are needed.
- Line 3: The component's submit() subscribes to
clientsService.executeClientCommand() without handling errors, causing unhandled
RxJS errors in tests when the service returns throwError(); update submit() to
provide an error handler (e.g., .subscribe(result => {...}, error => {...})) or
use .pipe(catchError(err => { /* handle/log and return of(null) or EMPTY */ }))
before subscribe; target the submit() method and the call to
clientsService.executeClientCommand() to add the error callback or catchError
handling so tests that mock throwError() no longer crash.
In
`@src/app/clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.ts`:
- Around line 97-99: The submit() method currently subscribes to
clientsService.executeClientCommand(this.clientId, 'undoRejection', data) with
only a next handler; update the subscription to use the observer object pattern
(pass an object with next and error properties) similar to
transfer-client.component.ts and reactivate-client.component.ts so that on
success you call
this.notifier.notifyAndNavigate('clients.actions.undoRejection.success',
this.route) and on error you call
this.notifier.notify('clients.actions.undoRejection.failure') or an equivalent
notifier method and handle any route/state cleanup; locate the subscription in
undo-client-rejection.component.ts (submit method) and add the error handler to
prevent unhandled stream errors.
In
`@src/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.ts`:
- Around line 103-105: The submit() method only handles the success path for
clientsService.executeClientCommand(this.clientId, 'withdraw', data); update the
subscribe call to add an error handler so API failures don't fail silently: keep
the existing success callback that calls
notifier.notifyAndNavigate('clients.actions.withdraw.success', this.route) and
add an error callback that calls
notifier.notify('clients.actions.withdraw.failure') (or the appropriate failure
key) to surface the error; adjust tests to mock notifier.notify and assert the
error handler is invoked when clientsService.executeClientCommand() emits an
error.
In `@src/assets/translations/cs-CS.json`:
- Around line 4435-4440: Replace inconsistent Czech wording: update the
"undoTransfer" success message ("Převod klienta byl úspěšně vrácen.") and the
"undoRejection" success message ("Odmítnutí klienta bylo úspěšně vráceno.") to
use the consistent undo phrasing used across the file (e.g., change
"vrácen/vráceno" to "zrušen/zrušeno" or your project's standard token such as
"byl úspěšně zrušen"), and normalize collateral terminology by replacing any
occurrences of "Záruka" with the project's preferred term "kolaterál" (also
apply the same change to the nearby messages referenced in the comment for lines
4448–4450); update the JSON values for the keys "undoTransfer" and
"undoRejection" and any "Záruka" strings to the chosen consistent terms.
In `@src/assets/translations/es-MX.json`:
- Around line 4444-4446: Update the translation entries under the "assignStaff"
key to use "Asesor" instead of "Personal": change assignStaff.success to "Asesor
asignado al cliente con éxito." and assignStaff.failure to "Error al asignar
asesor al cliente." so they match the existing client/staff terminology; modify
the values for the "success" and "failure" keys accordingly.
---
Duplicate comments:
In
`@src/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.spec.ts`:
- Around line 1-12: This new spec file is missing the project's MPL license
header; open the file that contains the imports and test setup for
ClientAssignStaffComponent (the file with imports like
ClientAssignStaffComponent, ClientsService, ClientActionNotifierService and the
jest test imports) and prepend the standard MPL repository header comment block
to the very top of the file (use the same header used across the repo);
afterwards run the repository header tooling (npm run headers:add or
headers:check) to ensure the header matches project standards.
---
Nitpick comments:
In
`@src/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.ts`:
- Around line 60-67: The JSDoc for the AddClientChargeComponent constructor is
out of sync: remove the stale "@param {Router} router" entry and ensure the
remaining `@param` tags match the actual constructor parameters in
add-client-charge.component.ts (e.g., FormBuilder formBuilder, ActivatedRoute
route, Dates dateUtils, ClientsService clientsService, SettingsService
settingsService) so the docblock reflects the current injections used by the
constructor.
In
`@src/app/clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.ts`:
- Around line 49-55: Update the constructor JSDoc in
undo-client-rejection.component.ts to remove the stale "@param {Router} router"
entry and ensure the remaining `@param` tags match the actual injected constructor
parameters (e.g., FormBuilder, clientsService, Dates, ActivatedRoute); also fix
any typos in param descriptions (e.g., "Cliens Service" → "Clients Service") so
the JSDoc accurately reflects the constructor signature.
In
`@src/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.spec.ts`:
- Around line 28-43: Replace the unsafe `as any` casts on the test mocks by
typing them as partial mocked interfaces: assign clientsService as
jest.Mocked<Partial<ClientsService>> (so executeClientCommand remains
jest.fn()), settingsService as Partial/Mocked<Partial<SettingsService>>
including language, dateFormat and businessDate, dates as
jest.Mocked<Partial<DateService>> (with formatDate mocked), and notifier as
jest.Mocked<Partial<NotifierService>>; this preserves the existing jest.fn()
implementations while restoring compile-time type checks and matching the
variable declarations (e.g., the earlier jest.Mocked<ClientsService>
declaration).
In
`@src/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.ts`:
- Around line 51-58: Update the stale JSDoc above the constructor in
WithdrawClientComponent to match the actual injected parameters: remove the
obsolete "Router" entry and ensure the docblock lists only FormBuilder,
ClientsService, Dates (dateUtils), ActivatedRoute (route), and SettingsService
(settingsService) as in the constructor signature; adjust any parameter names in
the comments to exactly match the constructor parameter names so the JSDoc
accurately reflects the code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 34b5b587-6f71-4a26-9579-1582275ff46b
📒 Files selected for processing (45)
src/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.spec.tssrc/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.tssrc/app/clients/clients-view/client-actions/activate-client/activate-client.component.spec.tssrc/app/clients/clients-view/client-actions/activate-client/activate-client.component.tssrc/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.spec.tssrc/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.tssrc/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.spec.tssrc/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.tssrc/app/clients/clients-view/client-actions/client-action-notifier.service.spec.tssrc/app/clients/clients-view/client-actions/client-action-notifier.service.tssrc/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.spec.tssrc/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.tssrc/app/clients/clients-view/client-actions/close-client/close-client.component.spec.tssrc/app/clients/clients-view/client-actions/close-client/close-client.component.tssrc/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.spec.tssrc/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.tssrc/app/clients/clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.spec.tssrc/app/clients/clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.tssrc/app/clients/clients-view/client-actions/reject-client/reject-client.component.spec.tssrc/app/clients/clients-view/client-actions/reject-client/reject-client.component.tssrc/app/clients/clients-view/client-actions/take-survey/take-survey.component.spec.tssrc/app/clients/clients-view/client-actions/take-survey/take-survey.component.tssrc/app/clients/clients-view/client-actions/transfer-client/transfer-client.component.spec.tssrc/app/clients/clients-view/client-actions/transfer-client/transfer-client.component.tssrc/app/clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.spec.tssrc/app/clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.tssrc/app/clients/clients-view/client-actions/undo-client-transfer/undo-client-transfer.component.spec.tssrc/app/clients/clients-view/client-actions/undo-client-transfer/undo-client-transfer.component.tssrc/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.spec.tssrc/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.tssrc/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.spec.tssrc/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.tssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
✅ Files skipped from review due to trivial changes (9)
- src/app/clients/clients-view/client-actions/close-client/close-client.component.spec.ts
- src/app/clients/clients-view/client-actions/take-survey/take-survey.component.spec.ts
- src/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.spec.ts
- src/assets/translations/pt-PT.json
- src/assets/translations/de-DE.json
- src/assets/translations/sw-SW.json
- src/assets/translations/ne-NE.json
- src/assets/translations/it-IT.json
- src/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.spec.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- src/app/clients/clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.ts
- src/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.ts
- src/app/clients/clients-view/client-actions/activate-client/activate-client.component.spec.ts
- src/app/clients/clients-view/client-actions/client-action-notifier.service.spec.ts
- src/app/clients/clients-view/client-actions/undo-client-transfer/undo-client-transfer.component.spec.ts
- src/app/clients/clients-view/client-actions/reject-client/reject-client.component.ts
- src/app/clients/clients-view/client-actions/client-action-notifier.service.ts
- src/app/clients/clients-view/client-actions/close-client/close-client.component.ts
- src/app/clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.spec.ts
- src/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.ts
- src/app/clients/clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.spec.ts
- src/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.spec.ts
- src/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.ts
- src/app/clients/clients-view/client-actions/transfer-client/transfer-client.component.spec.ts
- src/app/clients/clients-view/client-actions/take-survey/take-survey.component.ts
- src/assets/translations/en-US.json
.../clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.spec.ts
Show resolved
Hide resolved
.../clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.spec.ts
Show resolved
Hide resolved
...pp/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.spec.ts
Show resolved
Hide resolved
src/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.ts
Outdated
Show resolved
Hide resolved
src/app/clients/clients-view/client-actions/reject-client/reject-client.component.spec.ts
Show resolved
Hide resolved
src/app/clients/clients-view/client-actions/reject-client/reject-client.component.spec.ts
Show resolved
Hide resolved
...clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.ts
Outdated
Show resolved
Hide resolved
src/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.ts
Outdated
Show resolved
Hide resolved
Agaba-Ed
force-pushed
the
add-client-action-success-notifications
branch
from
a23b5a4 to
be2e8c1
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (5)
src/assets/translations/es-CL.json (1)
4424-4433: Consider aligning terminology with existing ES-CL labels (Comisión,Asesor).To keep wording consistent across UI and snackbars, consider replacing “Cargo”/“Personal” with the terms already used in this locale.
Suggested wording tweak
"addCharge": { - "success": "Cargo del cliente agregado exitosamente." + "success": "Comisión del cliente agregada exitosamente." }, "addCollateral": { "success": "Garantía del cliente agregada exitosamente." }, "assignStaff": { - "success": "Personal asignado al cliente exitosamente.", - "failure": "Error al asignar personal al cliente." + "success": "Asesor asignado al cliente exitosamente.", + "failure": "Error al asignar asesor al cliente." },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-CL.json` around lines 4424 - 4433, Update the ES-CL translations to use the existing locale terminology: replace "Cargo" with "Comisión" in the "addCharge.success" message and replace "Personal" with "Asesor" in both "assignStaff.success" and "assignStaff.failure" so the snackbar text matches other UI labels; locate the keys "addCharge", "assignStaff" (and "addCollateral" if needed) and edit their "success"/"failure" string values accordingly to use "Comisión" and "Asesor".src/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.ts (1)
48-54: Remove outdated JSDoc reference to Router.The JSDoc comment at line 52 still mentions
@param {Router} router Router.but the Router dependency was removed from this component.📝 Proposed fix
/** * Retirives Collateral Form from `resolve` * `@param` {FormBuilder} formBuilder Form bUilder. * `@param` {ActivatedRoute} route Activated Route. - * `@param` {Router} router Router. * `@param` {ProductsService} productsService Products Service */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.ts` around lines 48 - 54, Update the JSDoc for the AddClientCollateralComponent constructor: remove the obsolete "@param {Router} router Router." entry so the comment matches the actual constructor dependencies (FormBuilder, ActivatedRoute, ProductsService) in add-client-collateral.component.ts and ensure the remaining `@param` lines reference the correct parameter names (formBuilder, route, productsService).src/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.spec.ts (1)
32-32: Includenotifyin the mock for error handling coverage.The notifier mock only includes
notifyAndNavigate. If the component handles errors withnotify, include it in the mock and verify the failure notification in the error test.🧪 Proposed fix
- notifier = { notifyAndNavigate: jest.fn() } as any; + notifier = { notifyAndNavigate: jest.fn(), notify: jest.fn() } as any;Also applies to: 91-97
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.spec.ts` at line 32, The notifier mock used in update-client-savings-account.component.spec.ts only defines notifyAndNavigate, so error branches that call notifier.notify aren't covered; add notify: jest.fn() to the mocked notifier (the existing notifier variable used in the test setup) and update the error test to expect notifier.notify to have been called with the appropriate failure message/args (verify in the test that mocks for notify are asserted alongside notifyAndNavigate where the component handles errors).src/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.spec.ts (1)
39-39: Includenotifyin the mock and update error test when component is fixed.The notifier mock is missing
notify. Once error handling is added to the component (as flagged in the component review), this test should verify thatnotifyis called with the failure key.🧪 Proposed fix
- notifier = { notifyAndNavigate: jest.fn() } as any; + notifier = { notifyAndNavigate: jest.fn(), notify: jest.fn() } as any;When the component's error handling is added:
it('should not notify and navigate if API call fails', () => { clientsService.createClientCollateral.mockReturnValueOnce(throwError(() => new Error('API error'))); component.clientCollateralForm.patchValue({ collateralId: 1, quantity: 2 }); component.submit(); expect(notifier.notifyAndNavigate).not.toHaveBeenCalled(); + expect(notifier.notify).toHaveBeenCalledWith('clients.actions.addCollateral.failure'); });Also applies to: 97-104
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.spec.ts` at line 39, The test's notifier mock only includes notifyAndNavigate but is missing the notify method and the error test isn't asserting notify usage; update the mock in add-client-collateral.component.spec.ts to include notify (e.g., notifier = { notifyAndNavigate: jest.fn(), notify: jest.fn() } as any) and, once the component's error handling is implemented, update the failing-path test (the test around lines 97-104) to assert that notifier.notify was called with the expected failure key (and any expected payload) instead of only relying on notifyAndNavigate.src/app/clients/clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.spec.ts (1)
41-41: Includenotifyin the mock and verify error notification.Same issue as other spec files: the notifier mock only has
notifyAndNavigate. If the component callsnotifyon error, include it in the mock and verify it's called in the failure test.🧪 Proposed fix
- notifier = { notifyAndNavigate: jest.fn() } as any; + notifier = { notifyAndNavigate: jest.fn(), notify: jest.fn() } as any;Also applies to: 92-98
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/clients/clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.spec.ts` at line 41, The notifier mock used in the reject-client-transfer unit tests only implements notifyAndNavigate but the component may call notify on errors; update the mock to include notify (e.g., notifier = { notifyAndNavigate: jest.fn(), notify: jest.fn() } as any) and in the failure test for RejectClientTransferComponent assert that notifier.notify was called with the expected error message/args (replace or add expectations alongside the existing notifyAndNavigate assertions so the spec verifies error notification behavior).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/app/clients/clients-view/client-actions/activate-client/activate-client.component.ts`:
- Around line 98-100: The subscribe call on
clientsService.executeClientCommand(this.clientId, 'activate', data) only
provides a success callback so API failures produce no user feedback; update the
subscription to include an error callback that calls the notifier with the
failure translation key (e.g.,
this.notifier.notify('clients.actions.activate.failure')) and does not navigate,
and keep the existing success path using
this.notifier.notifyAndNavigate('clients.actions.activate.success', this.route);
also add the clients.actions.activate.failure key to the translation bundles.
In
`@src/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.spec.ts`:
- Around line 1-13: The new spec file is missing the required Mozilla Public
License header; add the standard MPL header comment block to the top of
src/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.spec.ts
(above all imports) so it matches project licensing guidelines, then run the
repository tooling (npm run headers:add and verify with npm run headers:check)
to ensure the header is correctly formatted and applied for
AddClientCollateralComponent tests.
In
`@src/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.ts`:
- Around line 126-129: The submit() success-only subscription on
clientsService.createClientCollateral(this.clientId, clientCollateral) swallows
API errors; add an error handler to the Observable subscription so failures
trigger notifier.notify('clients.actions.addCollateral.failure') (similar to
withdraw-client/accept-client-transfer) and avoid silent failures—update the
subscribe(...) call in add-client-collateral.component.ts to provide both next
and error callbacks, using notifier.notify for errors and keeping
notifier.notifyAndNavigate(this.route) on success, referencing submit(),
clientsService.createClientCollateral, notifier.notifyAndNavigate, and
notifier.notify.
In
`@src/app/clients/clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.spec.ts`:
- Around line 1-13: Add the standard Mozilla Public License header to the top of
the new spec file (reject-client-transfer.component.spec.ts); place the MPL
header above the imports so the file begins with the required license block;
ensure the header format matches other files in the repo (e.g., existing
component/spec files referencing RejectClientTransferComponent and
ClientsService) and then run npm run headers:check and npm run headers:add to
validate/apply the correct header automatically.
In
`@src/app/clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.spec.ts`:
- Line 42: The test's notifier mock only defines notifyAndNavigate but the
component calls this.notifier.notify(...) on error, so update the spec's
notifier mock to include a notify jest.fn() as well (i.e., create notifier = {
notifyAndNavigate: jest.fn(), notify: jest.fn() } as any) so the failure branch
can be exercised and assertions can verify notify was called; adjust any
expectations to assert notify was invoked with
'clients.actions.undoRejection.failure' where appropriate.
- Around line 128-136: The test currently checks
clientsService.executeClientCommand failure path but only asserts
notifier.notifyAndNavigate was not called; add an assertion that notifier.notify
was called with the component's failure translation key to verify user-facing
error feedback. After calling fillValidForm() and component.submit(), assert
notifier.notify was invoked (once) with the expected failure translation string
used in the component (e.g., the key referenced in UndoClientRejectionComponent
such as "undoClientRejection.failure") and any expected params, while keeping
the existing expect(notifier.notifyAndNavigate).not.toHaveBeenCalled() check.
In
`@src/app/clients/clients-view/client-actions/undo-client-transfer/undo-client-transfer.component.spec.ts`:
- Around line 1-13: The file containing the Jest spec for
UndoClientTransferComponent is missing the required Mozilla Public License (MPL)
header; add the standard MPL header block at the top of the spec (the same
header used across the repo) so the file with UndoClientTransferComponent tests
conforms to header checks, then run the repository header tooling (npm run
headers:add to insert the standard header if needed and npm run headers:check to
verify) to ensure the change passes CI.
In
`@src/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.spec.ts`:
- Around line 1-11: The file is missing the required Mozilla Public License
header; add the standard MPL license header comment block at the very top of the
spec file (before any imports) in
src/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.spec.ts
so the file begins with the MPL header, then re-run the header tooling (npm run
headers:add or npm run headers:check) to ensure it matches the project's
standard header; no other code changes to UpdateClientSavingsAccountComponent
tests are needed.
In `@src/assets/translations/fr-FR.json`:
- Around line 4425-4427: The French locale under the "addCharge" object is
missing the failure key that AddClientChargeComponent expects
(clients.actions.addCharge.failure); add a "failure" entry alongside "success"
in src/assets/translations/fr-FR.json's "addCharge" object with an appropriate
French message (using the same i18n variable style as existing keys) so the
component can call clients.actions.addCharge.failure on error instead of falling
back to hardcoded text.
---
Nitpick comments:
In
`@src/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.spec.ts`:
- Line 39: The test's notifier mock only includes notifyAndNavigate but is
missing the notify method and the error test isn't asserting notify usage;
update the mock in add-client-collateral.component.spec.ts to include notify
(e.g., notifier = { notifyAndNavigate: jest.fn(), notify: jest.fn() } as any)
and, once the component's error handling is implemented, update the failing-path
test (the test around lines 97-104) to assert that notifier.notify was called
with the expected failure key (and any expected payload) instead of only relying
on notifyAndNavigate.
In
`@src/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.ts`:
- Around line 48-54: Update the JSDoc for the AddClientCollateralComponent
constructor: remove the obsolete "@param {Router} router Router." entry so the
comment matches the actual constructor dependencies (FormBuilder,
ActivatedRoute, ProductsService) in add-client-collateral.component.ts and
ensure the remaining `@param` lines reference the correct parameter names
(formBuilder, route, productsService).
In
`@src/app/clients/clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.spec.ts`:
- Line 41: The notifier mock used in the reject-client-transfer unit tests only
implements notifyAndNavigate but the component may call notify on errors; update
the mock to include notify (e.g., notifier = { notifyAndNavigate: jest.fn(),
notify: jest.fn() } as any) and in the failure test for
RejectClientTransferComponent assert that notifier.notify was called with the
expected error message/args (replace or add expectations alongside the existing
notifyAndNavigate assertions so the spec verifies error notification behavior).
In
`@src/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.spec.ts`:
- Line 32: The notifier mock used in
update-client-savings-account.component.spec.ts only defines notifyAndNavigate,
so error branches that call notifier.notify aren't covered; add notify:
jest.fn() to the mocked notifier (the existing notifier variable used in the
test setup) and update the error test to expect notifier.notify to have been
called with the appropriate failure message/args (verify in the test that mocks
for notify are asserted alongside notifyAndNavigate where the component handles
errors).
In `@src/assets/translations/es-CL.json`:
- Around line 4424-4433: Update the ES-CL translations to use the existing
locale terminology: replace "Cargo" with "Comisión" in the "addCharge.success"
message and replace "Personal" with "Asesor" in both "assignStaff.success" and
"assignStaff.failure" so the snackbar text matches other UI labels; locate the
keys "addCharge", "assignStaff" (and "addCollateral" if needed) and edit their
"success"/"failure" string values accordingly to use "Comisión" and "Asesor".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cca5c7da-7ecc-42a8-a0f4-240b05b9f0c1
📒 Files selected for processing (45)
src/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.spec.tssrc/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.tssrc/app/clients/clients-view/client-actions/activate-client/activate-client.component.spec.tssrc/app/clients/clients-view/client-actions/activate-client/activate-client.component.tssrc/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.spec.tssrc/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.tssrc/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.spec.tssrc/app/clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.tssrc/app/clients/clients-view/client-actions/client-action-notifier.service.spec.tssrc/app/clients/clients-view/client-actions/client-action-notifier.service.tssrc/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.spec.tssrc/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.tssrc/app/clients/clients-view/client-actions/close-client/close-client.component.spec.tssrc/app/clients/clients-view/client-actions/close-client/close-client.component.tssrc/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.spec.tssrc/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.tssrc/app/clients/clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.spec.tssrc/app/clients/clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.tssrc/app/clients/clients-view/client-actions/reject-client/reject-client.component.spec.tssrc/app/clients/clients-view/client-actions/reject-client/reject-client.component.tssrc/app/clients/clients-view/client-actions/take-survey/take-survey.component.spec.tssrc/app/clients/clients-view/client-actions/take-survey/take-survey.component.tssrc/app/clients/clients-view/client-actions/transfer-client/transfer-client.component.spec.tssrc/app/clients/clients-view/client-actions/transfer-client/transfer-client.component.tssrc/app/clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.spec.tssrc/app/clients/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.tssrc/app/clients/clients-view/client-actions/undo-client-transfer/undo-client-transfer.component.spec.tssrc/app/clients/clients-view/client-actions/undo-client-transfer/undo-client-transfer.component.tssrc/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.spec.tssrc/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.tssrc/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.spec.tssrc/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.tssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
✅ Files skipped from review due to trivial changes (6)
- src/app/clients/clients-view/client-actions/close-client/close-client.component.spec.ts
- src/app/clients/clients-view/client-actions/client-action-notifier.service.ts
- src/app/clients/clients-view/client-actions/reject-client/reject-client.component.spec.ts
- src/app/clients/clients-view/client-actions/withdraw-client/withdraw-client.component.spec.ts
- src/app/clients/clients-view/client-actions/transfer-client/transfer-client.component.spec.ts
- src/assets/translations/lt-LT.json
🚧 Files skipped from review as they are similar to previous changes (21)
- src/app/clients/clients-view/client-actions/undo-client-transfer/undo-client-transfer.component.ts
- src/app/clients/clients-view/client-actions/update-client-savings-account/update-client-savings-account.component.ts
- src/app/clients/clients-view/client-actions/client-assign-staff/client-assign-staff.component.ts
- src/app/clients/clients-view/client-actions/reactivate-client/reactivate-client.component.ts
- src/app/clients/clients-view/client-actions/add-client-charge/add-client-charge.component.spec.ts
- src/assets/translations/pt-PT.json
- src/app/clients/clients-view/client-actions/activate-client/activate-client.component.spec.ts
- src/assets/translations/sw-SW.json
- src/app/clients/clients-view/client-actions/transfer-client/transfer-client.component.ts
- src/app/clients/clients-view/client-actions/close-client/close-client.component.ts
- src/assets/translations/ne-NE.json
- src/assets/translations/ko-KO.json
- src/assets/translations/es-MX.json
- src/app/clients/clients-view/client-actions/take-survey/take-survey.component.spec.ts
- src/app/clients/clients-view/client-actions/accept-client-transfer/accept-client-transfer.component.spec.ts
- src/app/clients/clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.ts
- src/assets/translations/it-IT.json
- src/assets/translations/en-US.json
- src/assets/translations/lv-LV.json
- src/assets/translations/de-DE.json
- src/assets/translations/cs-CS.json
src/app/clients/clients-view/client-actions/activate-client/activate-client.component.ts
Outdated
Show resolved
Hide resolved
...ts/clients-view/client-actions/add-client-collateral/add-client-collateral.component.spec.ts
Show resolved
Hide resolved
...clients/clients-view/client-actions/add-client-collateral/add-client-collateral.component.ts
Outdated
Show resolved
Hide resolved
.../clients-view/client-actions/reject-client-transfer/reject-client-transfer.component.spec.ts
Show resolved
Hide resolved
...ts/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.spec.ts
Outdated
Show resolved
Hide resolved
...ts/clients-view/client-actions/undo-client-rejection/undo-client-rejection.component.spec.ts
Show resolved
Hide resolved
...ents/clients-view/client-actions/undo-client-transfer/undo-client-transfer.component.spec.ts
Show resolved
Hide resolved
...client-actions/update-client-savings-account/update-client-savings-account.component.spec.ts
Show resolved
Hide resolved
Agaba-Ed
force-pushed
the
add-client-action-success-notifications
branch
from
be2e8c1 to
7b9ec7a
Compare
Contributor
Author
|
@IOhacker I used Google Translate, so I’m not confident about the accuracy. Could you please re-review this PR if you get sometime? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Added success notification feedback to client action components across the clients module. Previously, when users performed actions such as accepting/rejecting transfers, activating/reactivating/closing clients, adding charges or collateral, assigning staff, taking surveys, and similar operations, there was no visual confirmation that the action had completed successfully. This change injects the
SnackbarServiceinto each component and triggers a success snackbar message upon a successful API response, improving UX clarity and consistency.Unit tests were also added or updated for all affected components to cover the new notification behavior.
Affected components:
accept-client-transfer,reject-client-transfer,undo-client-transferactivate-client,reactivate-client,close-client,reject-clientadd-client-charge,add-client-collateralclient-assign-staff,take-survey,transfer-client,undo-client-rejectionwithdraw-client,update-client-savings-accountRelated issues and discussion
#WEB-895
Screenshots, if any
_N/A — notification snackbars are behavioural, no layout changes.
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
If you have multiple commits please combine them into one commit by squashing them.
Read and understood the contribution guidelines at
web-app/.github/CONTRIBUTING.md.Summary by CodeRabbit
New Features
Behavioral Changes
Documentation
Tests