WEB-860: Present the GlobalisationCode and translated it to the local languages as part of the message instead of the fixed value for errors#3396
Open
EDSONZ-WASSWA 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 |
|---|---|
Error handling service src/app/core/error-handler/error-handler.service.ts |
Adds exported interfaces FineractErrorDetail and FineractErrorResponse; injects TranslateService; adds translateFineractError(errorResponse) to parse top-level and nested Fineract errors, dedupe messages, and getMessageForCode(code, defaultMessage) fallback. |
HTTP interceptor src/app/core/http/error-handler.interceptor.ts |
Injects and uses ErrorHandlerService to compute translated errorMessage from backend payloads; extracts developerMessage for non-prod logging; revises status-to-alert branches (400/401/403/404/500/default) to use new translated keys and improved invalid-token detection while preserving throw behavior and client-image 404 silencing. |
Translation files src/assets/translations/{cs-CS,de-DE,en-US,es-CL,es-MX,fr-FR,it-IT,ko-KO,lt-LT,lv-LV,ne-NE,pt-PT,sw-SW}.json |
Adds two new translation keys in each locale: error.msg.platform.service.unavailable and error.msg.database.type.not.supported with localized messages. |
Sequence Diagram
sequenceDiagram
participant Client as Client/HTTP Request
participant Interceptor as Error Interceptor
participant ErrorService as ErrorHandlerService
participant Translate as TranslateService
participant Alert as AlertService
Client->>Interceptor: receives HTTP Error Response
Interceptor->>ErrorService: translateFineractError(errorBody)
ErrorService->>ErrorService: parse top-level and nested errors, dedupe messages
ErrorService->>Translate: instant(translationKey) or return defaultMessage
Translate-->>ErrorService: translated strings
ErrorService-->>Interceptor: translatedErrorMessage
Interceptor->>Alert: show translatedErrorMessage (alert type based on status)
Alert-->>Client: display error to user
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
- fix: WEB-860 Present Globalisation Code translated to local languages … #3452: Also modifies HTTP error handling to translate backend
userMessageGlobalisationCodeincluding nestederrorsand compose displayed messages. - Web 429 implement centralized error handler service for user friendly api error messaging #2810: Introduces/changes the same ErrorHandlerService and interceptor usage for translated error messages and snackbars.
- WEB-917 feat: internationalize all user-facing error messages for multilingual support #3494: Adjusts error-handling codepaths and message translation/integration in the same modules.
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 directly describes the main change: extracting GlobalisationCode from backend errors, translating them to local languages, and displaying them as part of error messages instead of generic fixed values. |
| 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: 4
🧹 Nitpick comments (1)
src/app/core/error-handler/error-handler.service.ts (1)
200-235: Replaceanywith a typed Fineract error contract.The new translator path sits on core error handling; using
anyhere reduces safety and propagates weak typing downstream.♻️ Suggested typed refactor
+interface FineractErrorItem { + userMessageGlobalisationCode?: string; + defaultUserMessage?: string; +} + +interface FineractErrorResponse extends FineractErrorItem { + errors?: FineractErrorItem[]; +} + - translateFineractError(errorResponse: any): string { + translateFineractError(errorResponse: FineractErrorResponse | null | undefined): string { if (!errorResponse || typeof errorResponse !== 'object') { return ''; } @@ - if (Array.isArray(errorResponse.errors)) { - errorResponse.errors.forEach((error: any) => { + if (Array.isArray(errorResponse.errors)) { + errorResponse.errors.forEach((error: FineractErrorItem) => { if (!error || typeof error !== 'object') { return; }Based on learnings: “In TypeScript files … introduce specific interfaces/types for the response shapes and use proper typing instead of any.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/core/error-handler/error-handler.service.ts` around lines 200 - 235, translateFineractError currently accepts untyped errorResponse and uses any for nested errors; define a FineractErrorResponse interface (fields: userMessageGlobalisationCode?: string, defaultUserMessage?: string, errors?: FineractErrorDetail[]) and a FineractErrorDetail interface (same optional fields) in this file or a shared types file, then change the function signature to translateFineractError(errorResponse: FineractErrorResponse | null | undefined) and update the forEach callback to use (error: FineractErrorDetail) so all property accesses are type-checked while keeping the existing logic and still calling getMessageForCode.
🤖 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/core/http/error-handler.interceptor.ts`:
- Around line 50-55: The code currently includes errorBody.developerMessage in
the UI errorMessage; remove developerMessage from the user-facing message
assembly in error-handler.interceptor.ts and only surface translatedErrorMessage
or errorBody.defaultUserMessage (or response.message as final fallback) when
constructing errorMessage for UI. If developerMessage exists, ensure it's
recorded to logs only (e.g., send errorBody.developerMessage to the existing
logging path or errorHandlerService) rather than concatenating it into
errorMessage; keep translateFineractError(errorBody) and
errorBody.defaultUserMessage as the primary user-facing sources and log
developerMessage separately.
- Around line 90-93: Replace hardcoded English strings in the
ErrorHandlerInterceptor's alertService.alert calls with i18n lookups using
this.translate.instant; specifically change the alert type and message for
Internal Server Error to
this.translate.instant('error.resource.internalServerError.type') and
this.translate.instant('error.resource.internalServerError.message') (and apply
the same pattern to other status branches such as 401, 403, 400 and fallback 404
messages that currently use literal strings), updating the alertService.alert
invocations in the error-handler.interceptor (e.g., inside the handleError /
intercept branches) to use translate.instant keys, then run npm run
translations:extract to add the new keys to translation files.
In `@src/assets/translations/es-CL.json`:
- Line 6: Update the translation value for the key
"error.msg.database.type.not.supported" to use singular phrasing and include
terminal punctuation; locate that key in src/assets/translations/es-CL.json and
change the string from "El tipo de datos no es compatible" to a natural singular
form with a period at the end (e.g., "El tipo de dato no es compatible.").
In `@src/assets/translations/ne-NE.json`:
- Around line 5-6: The Nepali locale entries are written with Hindi-leaning
phrasing; update the two keys "error.msg.platform.service.unavailable" and
"error.msg.database.type.not.supported" to proper Nepali wording (e.g., use
phrases like "अनुरोध प्रस्तुत गर्न सक्षम छैन" / "कृपया केही समयपछि पुन: प्रयास
गर्नुहोस्" for the platform outage message and "डेटाबेस प्रकार समर्थन हुँदैन" or
"डेटाबेस प्रकार समर्थन गरिंदैन" for the unsupported DB message) so the ne-NE
file uses consistent Nepali phrasing instead of Hindi terms.
---
Nitpick comments:
In `@src/app/core/error-handler/error-handler.service.ts`:
- Around line 200-235: translateFineractError currently accepts untyped
errorResponse and uses any for nested errors; define a FineractErrorResponse
interface (fields: userMessageGlobalisationCode?: string, defaultUserMessage?:
string, errors?: FineractErrorDetail[]) and a FineractErrorDetail interface
(same optional fields) in this file or a shared types file, then change the
function signature to translateFineractError(errorResponse:
FineractErrorResponse | null | undefined) and update the forEach callback to use
(error: FineractErrorDetail) so all property accesses are type-checked while
keeping the existing logic and still calling getMessageForCode.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5aa5672b-efe1-48cc-a2e5-2e9a04cb022d
📒 Files selected for processing (15)
src/app/core/error-handler/error-handler.service.tssrc/app/core/http/error-handler.interceptor.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
EDSONZ-WASSWA
force-pushed
the
WEB-860-Present-the-GlobalisationCode-and-translated-it-to-the-local-languages
branch
from
ece690d to
908d688
Compare
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/core/http/error-handler.interceptor.ts`:
- Around line 69-73: The branch that detects an invalid one-time token should
not rely on the localized `errorMessage` string; update the conditional in
error-handler.interceptor (the block using `status === 403` and `errorMessage
=== 'The provided one time token is invalid'`) to instead check for a stable
backend identifier such as `body.userMessageGlobalisationCode` (or
`body.defaultUserMessage` as a fallback) before calling
`this.alertService.alert(...)`; ensure you still handle the case where those
fields are absent by falling back to the existing localized `errorMessage` check
only as last resort so non-English sessions route correctly.
- Around line 97-105: The current 500/501 branches call alertService.alert with
only the generic translate.instant messages, dropping the already-computed
translated backend detail (errorMessage) created earlier (see the errorMessage
computation). Update the 500 and 501 branches in error-handler.interceptor.ts so
that alertService.alert still uses the translated title via
translate.instant(...) for type but uses the computed errorMessage (or falls
back to the generic translate.instant message) for message; reference the
alertService.alert calls, the translate.instant invocations, and the
errorMessage variable to locate and adjust the logic accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6a2abf84-9834-411d-9a2a-51292045d180
📒 Files selected for processing (15)
src/app/core/error-handler/error-handler.service.tssrc/app/core/http/error-handler.interceptor.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 as they are similar to previous changes (9)
- src/assets/translations/es-CL.json
- src/assets/translations/es-MX.json
- src/assets/translations/lv-LV.json
- src/assets/translations/it-IT.json
- src/assets/translations/pt-PT.json
- src/assets/translations/de-DE.json
- src/assets/translations/lt-LT.json
- src/assets/translations/ne-NE.json
- src/app/core/error-handler/error-handler.service.ts
Contributor
|
Which is the ticket? |
Author
https://mifosforge.jira.com/issues?filter=-1&selectedIssue=WEB-860 |
IOhacker
requested changes
Mar 17, 2026
Contributor
IOhacker
left a comment
IOhacker
left a comment
There was a problem hiding this comment.
@EDSONZ-WASSWA please change the title of the PR as "WEB-860: Present the GlobalisationCode and translated it to the local languages as part of the message instead of the fixed value for errors"
EDSONZ-WASSWA
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/assets/translations/ko-KO.json`:
- Around line 5-6: This file fails the project's Prettier formatting check; run
the formatter (e.g., npx prettier --write .) to normalize spacing, trailing
commas/newline and overall JSON style so the translations remain valid for
ErrorHandlerService.getMessageForCode() lookups; after formatting, verify the
JSON still contains the keys "error.msg.platform.service.unavailable" and
"error.msg.database.type.not.supported" at the root and commit the formatted
file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aa323809-3c28-48d2-9f3d-2ba8827640db
📒 Files selected for processing (13)
src/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 as they are similar to previous changes (11)
- src/assets/translations/lt-LT.json
- src/assets/translations/lv-LV.json
- src/assets/translations/cs-CS.json
- src/assets/translations/es-MX.json
- src/assets/translations/en-US.json
- src/assets/translations/fr-FR.json
- src/assets/translations/sw-SW.json
- src/assets/translations/ne-NE.json
- src/assets/translations/de-DE.json
- src/assets/translations/pt-PT.json
- src/assets/translations/es-CL.json
EDSONZ-WASSWA
force-pushed
the
WEB-860-Present-the-GlobalisationCode-and-translated-it-to-the-local-languages
branch
from
8071ddd to
a8fe562
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/app/core/http/error-handler.interceptor.ts (1)
105-114:⚠️ Potential issue | 🟠 Major500/501 branches still drop translated backend detail.
These branches always show generic text and ignore the computed
errorMessage, which can regress the detailed translated-error behavior introduced by this PR.💡 Proposed fix
} else if (status === 500) { this.alertService.alert({ type: this.translate.instant('error.resource.internalServerError.type'), - message: this.translate.instant('error.resource.internalServerError.message') + message: errorMessage || this.translate.instant('error.resource.internalServerError.message') }); } else if (status === 501) { this.alertService.alert({ type: this.translate.instant('error.resource.notImplemented.type'), - message: this.translate.instant('error.resource.notImplemented.message') + message: errorMessage || this.translate.instant('error.resource.notImplemented.message') });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/core/http/error-handler.interceptor.ts` around lines 105 - 114, The 500 and 501 branches in the error handler (the status === 500 and status === 501 blocks) always set the alert message to a generic translate.instant string and ignore the previously computed errorMessage; update those branches in handleError/error-handler.interceptor to use the computed errorMessage (or combine the translated fallback with errorMessage) when calling alertService.alert so translated backend details are preserved (keep using translate.instant for the type field).
🧹 Nitpick comments (1)
src/app/core/http/error-handler.interceptor.ts (1)
50-53: AdddeveloperMessageto theFineractErrorResponseinterface and remove theanytype annotation.The
errorBodyvariable usesanytype, which bypasses strict typing. The code accessesdeveloperMessage(line 52) anddefaultUserMessage(line 53), but theFineractErrorResponseinterface only includesdefaultUserMessage, notdeveloperMessage. This gap should be fixed at the interface definition level rather than with a workaround.Update
FineractErrorResponseinsrc/app/core/error-handler/error-handler.service.tsto include the missing property:export interface FineractErrorResponse { userMessageGlobalisationCode?: string; defaultUserMessage?: string; developerMessage?: string; errors?: FineractErrorDetail[]; }Then change line 50 from
const errorBody: any = response.error;toconst errorBody: FineractErrorResponse | null = response.error || null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/core/http/error-handler.interceptor.ts` around lines 50 - 53, Update the FineractErrorResponse type and stop using any for errorBody: add developerMessage?: string to the FineractErrorResponse interface in error-handler.service.ts (keeping existing userMessageGlobalisationCode?, defaultUserMessage?, errors?: FineractErrorDetail[]), then in error-handler.interceptor.ts change the errorBody declaration to be typed as FineractErrorResponse | null (e.g., const errorBody: FineractErrorResponse | null = response.error || null) so developerMessage and defaultUserMessage are strongly typed when accessed by translateFineractError and the subsequent developerMessage/errorMessage logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/app/core/http/error-handler.interceptor.ts`:
- Around line 105-114: The 500 and 501 branches in the error handler (the status
=== 500 and status === 501 blocks) always set the alert message to a generic
translate.instant string and ignore the previously computed errorMessage; update
those branches in handleError/error-handler.interceptor to use the computed
errorMessage (or combine the translated fallback with errorMessage) when calling
alertService.alert so translated backend details are preserved (keep using
translate.instant for the type field).
---
Nitpick comments:
In `@src/app/core/http/error-handler.interceptor.ts`:
- Around line 50-53: Update the FineractErrorResponse type and stop using any
for errorBody: add developerMessage?: string to the FineractErrorResponse
interface in error-handler.service.ts (keeping existing
userMessageGlobalisationCode?, defaultUserMessage?, errors?:
FineractErrorDetail[]), then in error-handler.interceptor.ts change the
errorBody declaration to be typed as FineractErrorResponse | null (e.g., const
errorBody: FineractErrorResponse | null = response.error || null) so
developerMessage and defaultUserMessage are strongly typed when accessed by
translateFineractError and the subsequent developerMessage/errorMessage logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 66c2f2a6-f8c0-4238-ad83-2e054d1c2771
📒 Files selected for processing (15)
src/app/core/error-handler/error-handler.service.tssrc/app/core/http/error-handler.interceptor.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 as they are similar to previous changes (9)
- src/assets/translations/it-IT.json
- src/assets/translations/ko-KO.json
- src/assets/translations/cs-CS.json
- src/assets/translations/lv-LV.json
- src/assets/translations/fr-FR.json
- src/assets/translations/pt-PT.json
- src/assets/translations/sw-SW.json
- src/assets/translations/de-DE.json
- src/assets/translations/ne-NE.json
IOhacker
changed the title
Contributor
|
@EDSONZ-WASSWA I've resolved the merge conflicts with upstream/dev. To apply the fix to your PR branch, git fetch https://github.com/saksham869/web-app.git WEB-860-help What was resolved: Ready for re-review @IOhacker |
Contributor
|
@EDSONZ-WASSWA could you please resolve the conflicts? |
Author
@IOhacker I have resolved them thankyou |
Contributor
|
@EDSONZ-WASSWA plese squash and commit |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/core/http/error-handler.interceptor.ts (2)
90-121:⚠️ Potential issue | 🔴 CriticalFix merge-artifact corruption in the 401/400/403 condition chain—code is syntactically invalid.
Lines 91–94 have an unclosed
alertService.alert()object. Line 95 incorrectly opens a newif(should be} else if) instead of continuing the control flow. The block also contains:
- Duplicated
status === 400branches (lines 108 and 115)- Missing comma after the
messageproperty on line 118- Orphaned
typeandmessageproperties on lines 119–120 without proper nestingThis appears to be a partial merge conflict where old i18n key conventions (e.g.,
error.resource.authenticationError.*) were not fully replaced with new ones (e.g.,errors.error.auth.*). Consolidate the overlapping conditions and ensure all alert objects are properly closed.Proposed normalized control flow
- if (status === 401 || (environment.oauth.enabled && status === 400 && request.url.includes('/oauth/token'))) { - this.alertService.alert({ - type: this.translate.instant('error.resource.authenticationError.type'), - message: this.translate.instant('error.resource.authenticationError.message') - if (status === 401 || (environment.oauth.enabled && status === 400)) { - this.alertService.alert({ - type: this.translate.instant('errors.error.auth.type'), - message: this.translate.instant('errors.error.auth.message') - }); - } else if ( - status === 403 && - response.error?.errors?.[0]?.defaultUserMessage === 'The provided one time token is invalid' - ) { - this.alertService.alert({ - type: this.translate.instant('errors.error.token.invalid.type'), - message: this.translate.instant('errors.error.token.invalid.message') - }); - } else if (status === 400) { - const fallback = this.translate.instant('errors.interceptor.invalidParams'); - const message = parameterName ? `[${parameterName}] ${errorMessage || fallback}` : `${errorMessage || fallback}`; - this.alertService.alert({ - type: this.translate.instant('errors.error.bad.request.type'), - message: message || this.translate.instant('errors.error.bad.request.message') - }); - } else if (status === 400) { + if (status === 401 || (environment.oauth.enabled && status === 400 && request.url.includes('/oauth/token'))) { + this.alertService.alert({ + type: this.translate.instant('error.resource.authenticationError.type'), + message: this.translate.instant('error.resource.authenticationError.message') + }); + } else if (status === 400) { this.alertService.alert({ type: this.translate.instant('error.resource.badRequest.type'), message: errorMessage || this.translate.instant('error.resource.badRequest.message') - type: this.translate.instant('errors.error.unauthorized.type'), - message: errorMessage || this.translate.instant('errors.error.unauthorized.message') }); } else if (status === 403) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/core/http/error-handler.interceptor.ts` around lines 90 - 121, Fix the merge-artifact in error-handler.interceptor.ts by repairing the malformed control flow and alert objects: close the first this.alertService.alert({...}) call, change the stray `if (status === 401 || (environment.oauth.enabled && status === 400))` to `} else if (...)` to continue the chain, remove/merge the duplicated `else if (status === 400)` branches into a single 400 handler that uses `parameterName`/`errorMessage` and the correct i18n keys, add the missing comma after the `message` property, and remove the orphaned `type`/`message` lines; keep special-case checks for `environment.oauth.enabled && request.url.includes('/oauth/token')` and the 403 one-time-token check (`response.error?.errors?.[0]?.defaultUserMessage`) intact, and ensure every this.alertService.alert call is a well-formed object literal.
45-80:⚠️ Potential issue | 🔴 CriticalRemove the duplicate
errorMessagedeclaration and the legacy message-construction block; it violates TypeScript rules and can crash at runtime.The declaration at line 47 (
const errorMessage) is redeclared at line 69 (let errorMessage), which is a TypeScript compile-time error. The legacy block (lines 50–80) also contains unsafe.replace()calls on possibly undefined properties (defaultUserMessageanddeveloperMessage), which will throw at runtime.Additionally, the status-handling section (lines 90–169) contains merge artifacts with duplicate blocks and malformed object literals that prevent compilation. The 401, 400, 404, 500, and unknown error branches have duplicate keys and inconsistent i18n namespaces.
Replace the entire legacy block with the simplified approach relying on
translatedErrorMessagefromerrorHandlerService, and clean up duplicate status branches to use consistent i18n keys and includeerrorMessagefallbacks where appropriate.Key fixes needed
- Remove lines 50–80 (legacy top-level and nested message translation block)
- Consolidate duplicate status blocks (401, 400, 404, 500, unknown error)
- Ensure all error branches include
errorMessagefallback where end-users need detail- Replace
anytype on line 44 with a proper error interface for strict typing🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/core/http/error-handler.interceptor.ts` around lines 45 - 80, Remove the legacy nested translation/message-construction block and the duplicate errorMessage declaration; keep a single errorMessage variable populated from translatedErrorMessage (from errorHandlerService.translateFineractError) or fallback to response.error?.defaultUserMessage || response.message, and drop the manual nested parsing using translate.instant and response.error.errors[0]. Also remove unsafe .replace() calls on possibly undefined defaultUserMessage/developerMessage and instead guard properties before manipulating them (check existence of response.error.errors[0] and its fields). Consolidate the duplicated status-handling branches (401, 400, 404, 500, unknown) so each status uses consistent i18n keys and always includes the errorMessage fallback for user-facing text, and replace the loose any type used for the response/error with a strict interface (e.g., ErrorResponse) to enforce safe property access and prevent redeclaration/merge issues.
♻️ Duplicate comments (1)
src/app/core/http/error-handler.interceptor.ts (1)
151-162:⚠️ Potential issue | 🟠 MajorKeep translated backend detail for 500/501 alerts instead of always showing generic text.
Current 500/501 branches use generic messages only, which regresses the new detailed translated-error behavior.
Proposed fix
} else if (status === 500) { this.alertService.alert({ type: this.translate.instant('error.resource.internalServerError.type'), - message: this.translate.instant('error.resource.internalServerError.message') + message: errorMessage || this.translate.instant('error.resource.internalServerError.message') }); } else if (status === 501) { this.alertService.alert({ - type: this.translate.instant('errors.error.resource.notImplemented.type'), - message: this.translate.instant('errors.error.resource.notImplemented.message') + type: this.translate.instant('error.resource.notImplemented.type'), + message: errorMessage || this.translate.instant('error.resource.notImplemented.message') });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/core/http/error-handler.interceptor.ts` around lines 151 - 162, The 500/501 branches in error-handler.interceptor.ts currently replace detailed backend translations with generic keys; update the status === 500 and status === 501 branches to pass the backend-provided translated title/message into this.alertService.alert (instead of always calling this.translate.instant('errors.error.server.internal.*') or other generic keys). Locate where the interceptor parses backend errors (the variable that holds backend translation, e.g., parsedBackendError, translatedError, or backendErrorTranslation) and use its title/message for the alert payload while falling back to this.translate.instant(...) only if those backend fields are missing; keep the calls to this.alertService.alert and this.translate.instant as shown.
🧹 Nitpick comments (1)
src/app/core/http/error-handler.interceptor.ts (1)
44-44: Replaceanywith a typed error payload to preserve strict TypeScript safety.
const errorBody: anyweakens checks around optional fields (developerMessage,errors,parameterName) and makes this method easier to break silently.Typed payload example
+interface InterceptorErrorDetail { + userMessageGlobalisationCode?: string; + defaultUserMessage?: string; + developerMessage?: string; + parameterName?: string; +} + +interface InterceptorErrorBody { + userMessageGlobalisationCode?: string; + defaultUserMessage?: string; + developerMessage?: string; + errors?: InterceptorErrorDetail[]; +} ... - const errorBody: any = response.error; + const errorBody: InterceptorErrorBody | undefined = + response.error && typeof response.error === 'object' ? (response.error as InterceptorErrorBody) : undefined;As per coding guidelines:
src/app/**/*.ts: Use TypeScript for all application code with strict typing conventions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/core/http/error-handler.interceptor.ts` at line 44, The line "const errorBody: any = response.error" weakens TypeScript safety; define a strict interface (e.g., ErrorPayload { developerMessage?: string; errors?: Record<string,string[]>; parameterName?: string; [key: string]: unknown }) and replace the any with that type, then cast or validate response.error into that ErrorPayload before use in the interceptor method (where errorBody is referenced) so optional fields are typed and checked rather than implicitly any.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/app/core/http/error-handler.interceptor.ts`:
- Around line 90-121: Fix the merge-artifact in error-handler.interceptor.ts by
repairing the malformed control flow and alert objects: close the first
this.alertService.alert({...}) call, change the stray `if (status === 401 ||
(environment.oauth.enabled && status === 400))` to `} else if (...)` to continue
the chain, remove/merge the duplicated `else if (status === 400)` branches into
a single 400 handler that uses `parameterName`/`errorMessage` and the correct
i18n keys, add the missing comma after the `message` property, and remove the
orphaned `type`/`message` lines; keep special-case checks for
`environment.oauth.enabled && request.url.includes('/oauth/token')` and the 403
one-time-token check (`response.error?.errors?.[0]?.defaultUserMessage`) intact,
and ensure every this.alertService.alert call is a well-formed object literal.
- Around line 45-80: Remove the legacy nested translation/message-construction
block and the duplicate errorMessage declaration; keep a single errorMessage
variable populated from translatedErrorMessage (from
errorHandlerService.translateFineractError) or fallback to
response.error?.defaultUserMessage || response.message, and drop the manual
nested parsing using translate.instant and response.error.errors[0]. Also remove
unsafe .replace() calls on possibly undefined
defaultUserMessage/developerMessage and instead guard properties before
manipulating them (check existence of response.error.errors[0] and its fields).
Consolidate the duplicated status-handling branches (401, 400, 404, 500,
unknown) so each status uses consistent i18n keys and always includes the
errorMessage fallback for user-facing text, and replace the loose any type used
for the response/error with a strict interface (e.g., ErrorResponse) to enforce
safe property access and prevent redeclaration/merge issues.
---
Duplicate comments:
In `@src/app/core/http/error-handler.interceptor.ts`:
- Around line 151-162: The 500/501 branches in error-handler.interceptor.ts
currently replace detailed backend translations with generic keys; update the
status === 500 and status === 501 branches to pass the backend-provided
translated title/message into this.alertService.alert (instead of always calling
this.translate.instant('errors.error.server.internal.*') or other generic keys).
Locate where the interceptor parses backend errors (the variable that holds
backend translation, e.g., parsedBackendError, translatedError, or
backendErrorTranslation) and use its title/message for the alert payload while
falling back to this.translate.instant(...) only if those backend fields are
missing; keep the calls to this.alertService.alert and this.translate.instant as
shown.
---
Nitpick comments:
In `@src/app/core/http/error-handler.interceptor.ts`:
- Line 44: The line "const errorBody: any = response.error" weakens TypeScript
safety; define a strict interface (e.g., ErrorPayload { developerMessage?:
string; errors?: Record<string,string[]>; parameterName?: string; [key: string]:
unknown }) and replace the any with that type, then cast or validate
response.error into that ErrorPayload before use in the interceptor method
(where errorBody is referenced) so optional fields are typed and checked rather
than implicitly any.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2f0c0847-1f95-4b39-80b4-5d7fb3b25929
📒 Files selected for processing (2)
src/app/core/error-handler/error-handler.service.tssrc/app/core/http/error-handler.interceptor.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/core/error-handler/error-handler.service.ts
Author
Hay @saksham869 I have fixed the merge conflicts |
…guages (openMF#3385) fix-Present the GlobalisationCode and translated it to the local languages as part of the message modified: src/app/core/error-handler/error-handler.service.ts modified: src/app/core/http/error-handler.interceptor.ts
EDSONZ-WASSWA
force-pushed
the
WEB-860-Present-the-GlobalisationCode-and-translated-it-to-the-local-languages
branch
from
5864f57 to
edeab1e
Compare
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
4 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.
…uages as part of the message
Description
Before, the Mifos X Web App showed a generic message like “Unknown Error. Please try again later” whenever an API call failed. Even though Apache Fineract returned detailed error information, the UI ignored it, making it hard for users to understand what went wrong.
Now, the app extracts those backend error codes, translates them, and displays a clear, meaningful message. This makes errors easier to understand and much simpler to debug.
Related issues and discussion
#{Issue Number}
https://mifosforge.jira.com/issues?filter=-1&selectedIssue=WEB-860
Screenshots, if any
before
After
please @IOhacker please have a look at this
Summary by CodeRabbit
Improvements
New Translation Strings