WEB-862: Handle API errors using userMessageGlobalisationCode with lo…#3411
WEB-862: Handle API errors using userMessageGlobalisationCode with lo…#3411samruddhikasar05-bit wants to merge 1 commit intoopenMF:devfrom
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Error Handler Enhancement src/app/loans/loans-view/loans-view.component.ts |
Updated getEntityDataTableChecks() error callback to extract error details from err.error?.errors, translate each entry's userMessageGlobalisationCode, concatenate messages with spaces, and display via alert() instead of silently failing. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
- WEB-654 fix: show only product-specific datatables in loan account view #3105: Introduced the original
getEntityDataTableChecks()subscription anddatatablesReadyfallback logic infilterDatatablesByProduct(), which this PR extends with enhanced error messaging.
Suggested reviewers
- 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 specifically describes the main change: adding API error handling using userMessageGlobalisationCode with localization. |
| 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
📝 Coding Plan
- Generate coding plan for human review comments
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.
Tip
CodeRabbit can enforce grammar and style rules using `languagetool`.
Configure the reviews.tools.languagetool setting to enable/disable rules and categories. Refer to the LanguageTool Community to learn more.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/loans/loans-view/loans-view.component.ts`:
- Line 253: Replace the raw browser alert call at the end of the handler with
the app's alertService pattern: remove alert(message) in loans-view.component
(the method where alert(message) is invoked) and call this.alertService.alert({
type: '<appropriate-type>', message }) instead, matching how
transactions-tab.component.ts and view-transaction.component.ts use
this.alertService; ensure the component has alertService injected in the
constructor if not already.
- Around line 249-251: Replace the incorrect reference this.translate.instant
with the injected translateService (this.translateService.instant) and ensure
translateService is defined on the component (constructor injection already
present as translateService); when building the message from errors use a
fallback chain: prefer error.userMessageGlobalisationCode (translated),
otherwise error.defaultUserMessage, otherwise error.developerMessage, and
finally a generic text; and replace the native alert() call with the app's
notification service (e.g., this.notificationService.error or similar) to
display the assembled message so the UI uses the proper notifier.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 66547569-fb36-4ae1-a7f7-3a5dbfeb41fa
📒 Files selected for processing (1)
src/app/loans/loans-view/loans-view.component.ts
| const message = errors | ||
| .map(e => this.translate.instant(e.userMessageGlobalisationCode)) | ||
| .join(' '); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify incorrect member usage and compare expected error fields.
rg -nP 'private\s+translate(Service)?\s*=' src/app/loans/loans-view/loans-view.component.ts
rg -nP '\bthis\.translate\.instant\s*\(' src/app/loans/loans-view/loans-view.component.ts
rg -nP 'defaultUserMessage|developerMessage|userMessageGlobalisationCode' src/app/core/http/error-handler.interceptor.ts src/app/loans/loans-view/loans-view.component.tsRepository: openMF/web-app
Length of output: 605
🏁 Script executed:
# Read the full code section around lines 243-255 to see alert() and complete logic
head -n 255 src/app/loans/loans-view/loans-view.component.ts | tail -n 20Repository: openMF/web-app
Length of output: 580
🏁 Script executed:
# Also check for other uses of translate service in this file
rg -n 'this\.(translate|translateService)\.' src/app/loans/loans-view/loans-view.component.tsRepository: openMF/web-app
Length of output: 563
🏁 Script executed:
# Verify the error response structure more thoroughly
cat -n src/app/core/http/error-handler.interceptor.ts | head -n 60 | tail -n 20Repository: openMF/web-app
Length of output: 885
Fix undeclared translate service reference and add error message fallback with proper notification.
Line 250 uses this.translate.instant(...), but the component declares this.translateService (Line 83). This reference will fail type-checking. Additionally, line 253 uses native alert() instead of a notification service, and the error mapping lacks fallback for defaultUserMessage or developerMessage when userMessageGlobalisationCode is missing.
Proposed fix
const message = errors
- .map(e => this.translate.instant(e.userMessageGlobalisationCode))
+ .map((e: { userMessageGlobalisationCode?: string; defaultUserMessage?: string; developerMessage?: string }) => {
+ const key = e.userMessageGlobalisationCode;
+ return key
+ ? this.translateService.instant(key)
+ : (e.defaultUserMessage ?? e.developerMessage ?? '');
+ })
+ .filter(Boolean)
.join(' ');
- alert(message);
+ // Use notification service instead of native alert
+ // this.notificationService.showError(message);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/loans/loans-view/loans-view.component.ts` around lines 249 - 251,
Replace the incorrect reference this.translate.instant with the injected
translateService (this.translateService.instant) and ensure translateService is
defined on the component (constructor injection already present as
translateService); when building the message from errors use a fallback chain:
prefer error.userMessageGlobalisationCode (translated), otherwise
error.defaultUserMessage, otherwise error.developerMessage, and finally a
generic text; and replace the native alert() call with the app's notification
service (e.g., this.notificationService.error or similar) to display the
assembled message so the UI uses the proper notifier.
| .map(e => this.translate.instant(e.userMessageGlobalisationCode)) | ||
| .join(' '); | ||
|
|
||
| alert(message); // or notification service if exists |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for native alerts and existing app-level alert patterns to align this call.
rg -nP '\balert\s*\(' src/app --type=ts
rg -nP 'alertService\.alert|MatSnackBar|MatDialog' src/app/loans src/app/shared --type=tsRepository: openMF/web-app
Length of output: 30334
Replace native alert() with the app's alertService notification pattern.
Line 253 uses browser alert(), which is blocking and inconsistent with the established pattern in the loans module. Other components in this module use this.alertService.alert({ type: '...', message: ... }) (see sibling components like transactions-tab.component.ts and view-transaction.component.ts). Replace the native alert with the app's alertService for consistent UX behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/loans/loans-view/loans-view.component.ts` at line 253, Replace the
raw browser alert call at the end of the handler with the app's alertService
pattern: remove alert(message) in loans-view.component (the method where
alert(message) is invoked) and call this.alertService.alert({ type:
'<appropriate-type>', message }) instead, matching how
transactions-tab.component.ts and view-transaction.component.ts use
this.alertService; ensure the component has alertService injected in the
constructor if not already.
1 participant
…calisation
Description
Describe the changes made and why they were made instead of how they were made. List any dependencies that are required for this change.
Related issues and discussion
#{Issue Number}
Screenshots, if any
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