Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Loan Error Handling src/app/loans/loans-view/loans-view.component.ts |
Modified filterDatatablesByProduct() error handler to extract error messages from API response, translate each userMessageGlobalisationCode via i18n, concatenate results, and display via alert instead of silent fallback. |
Savings Route Metadata src/app/savings/savings-routing.module.ts |
Updated route data.title and data.breadcrumb for the create child route from hardcoded strings ('Create Savings Account') to i18n translation keys ('labels.createSavingsAccount'). |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
- WEB-594 Translate breadcrumbs account transfers using the i-18-n translation-key #3008: Updates route metadata to use i18n translation keys in other routing modules, following the same pattern as the savings routing changes.
- WEB-654 fix: show only product-specific datatables in loan account view #3105: Directly modifies the
filterDatatablesByProductmethod, establishing the baseline that this PR's error handling enhancements build upon.
Suggested reviewers
- IOhacker
- alberto-art3ch
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check | The PR title 'Web 542 fix breadcrumb' refers to only one aspect of the changeset. The PR actually contains two distinct changes: breadcrumb/title translation key updates (WEB-542) and API error handling improvements (WEB-862), but the title mentions only the breadcrumb fix. | Update the title to reflect both main changes or prioritize the most significant change. Consider: 'WEB-542: Fix breadcrumb and title translation keys' or include both commits in the scope description. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| 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
You can get early access to new features in CodeRabbit.
Enable the early_access setting to enable early access features such as new models, tools, and more.
IOhacker
left a comment
There was a problem hiding this comment.
Please add in the title the "WEB-542: The Savings product creation section displays text in English: Institution–Client—Create Savings Account—Summary."
Make sure to send only one commit, so then squash and commit
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/app/loans/loans-view/loans-view.component.ts (1)
243-255: Inconsistent indentation in the error handler block.The error handler body uses 2-space indentation starting at the function level, while the rest of the file consistently uses 8-space indentation inside the
subscribecallbacks. Runnpx prettier --write .to fix formatting as per coding guidelines.🤖 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 243 - 255, The error handler in the subscribe callback in loans-view.component.ts is mis-indented (uses 2-space indentation) causing inconsistent formatting with other subscribe callbacks that use 8-space indentation; re-indent the entire error: (err) => { ... } block (including setting this.datatablesReady, extraction of errors, message construction, and alert call) to match the 8-space indentation style used in the surrounding subscribe callbacks and then run npx prettier --write . to apply project formatting rules (target symbols: the subscribe callback's error handler and the this.datatablesReady assignment).
🤖 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`:
- Around line 249-251: The mapping that builds the const message from the errors
array calls this.translate.instant with e.userMessageGlobalisationCode without
checking it; update the mapping over errors in loans-view.component (where const
message is created) to defensively handle missing or falsy
userMessageGlobalisationCode by providing a safe fallback (e.g., a default
translation key or skipping that error) before calling this.translate.instant,
so that this.translate.instant never receives undefined and the resulting
message string is well-formed.
- Around line 243-255: The error handler in loans-view.component.ts uses
this.translate.instant and alert() but the TranslateService is injected as
translateService and the app uses AlertService; update the error callback to
call this.translateService.instant(...) instead of this.translate.instant,
import and inject AlertService (e.g., private alertService =
inject(AlertService) or via constructor) if not present, and replace native
alert(message) with this.alertService.alert(message) (keeping the existing
datatablesReady assignment and mapping logic intact).
---
Nitpick comments:
In `@src/app/loans/loans-view/loans-view.component.ts`:
- Around line 243-255: The error handler in the subscribe callback in
loans-view.component.ts is mis-indented (uses 2-space indentation) causing
inconsistent formatting with other subscribe callbacks that use 8-space
indentation; re-indent the entire error: (err) => { ... } block (including
setting this.datatablesReady, extraction of errors, message construction, and
alert call) to match the 8-space indentation style used in the surrounding
subscribe callbacks and then run npx prettier --write . to apply project
formatting rules (target symbols: the subscribe callback's error handler and the
this.datatablesReady assignment).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 996396ce-2d52-4b0b-8006-bdbbbcbf6b96
📒 Files selected for processing (2)
src/app/loans/loans-view/loans-view.component.tssrc/app/savings/savings-routing.module.ts
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
Bug Fixes
Chores