Cherry-pick dev-angular-19 changes back to dev#2865
Conversation
reslove the issue
…Collection Sheet instead of Collection Sheet
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Loan re-amortization preview feature src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.html, src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts |
Added Preview button in template mirroring Submit button enabled/permission logic. Added MatDialog and SettingsService injection, private data preparation helpers (prepareReAmortizeData, prepareReAmortizePreviewData), preview() method to fetch preview and open dialog, and updated submit() to use data preparation helper. |
Re-amortization preview dialog component src/app/loans/loans-view/loan-account-actions/loan-reamortize/re-amortize-preview-dialog/re-amortize-preview-dialog.component.html, src/app/loans/loans-view/loan-account-actions/loan-reamortize/re-amortize-preview-dialog/re-amortize-preview-dialog.component.ts |
Added new standalone dialog component with ReAmortizePreviewDialogData interface. Template includes dialog title, repayment-schedule-tab content binding, and Go back action button. Component exposes repaymentSchedule and currencyCode properties and close() method. |
Loans service src/app/loans/loans.service.ts |
Added getReAmortizePreview() method that constructs HttpParams from data object (excluding null/undefined/empty values) and issues GET request to /loans/{loanId}/transactions/reamortized-preview. |
Translation updates src/app/core/shell/sidenav/sidenav.component.html |
Changed Collection Sheet tooltip translation key from "tooltips.Collection Sheet" to "labels.menus.Collection Sheet". |
Multi-language "Savings Account Details" translations 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 |
Added "Savings Account Details" translation entry in 13 language files with locale-specific translations. |
Sequence Diagram
sequenceDiagram
actor User
participant LoanReamortizeComponent
participant LoansService
participant API
participant ReAmortizePreviewDialogComponent
participant MatDialog
User->>LoanReamortizeComponent: Click Preview button
LoanReamortizeComponent->>LoanReamortizeComponent: preview()
LoanReamortizeComponent->>LoanReamortizeComponent: prepareReAmortizePreviewData()
LoanReamortizeComponent->>LoansService: getReAmortizePreview(loanId, data)
LoansService->>API: GET /loans/{id}/transactions/reamortized-preview
API-->>LoansService: repaymentSchedule
LoansService-->>LoanReamortizeComponent: Observable<repaymentSchedule>
LoanReamortizeComponent->>MatDialog: open(ReAmortizePreviewDialogComponent, data)
MatDialog->>ReAmortizePreviewDialogComponent: inject dialog data
ReAmortizePreviewDialogComponent-->>User: Display preview with repayment schedule
User->>ReAmortizePreviewDialogComponent: Click Go back
ReAmortizePreviewDialogComponent->>MatDialog: close()
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60–75 minutes
- Data preparation methods (
prepareReAmortizeData,prepareReAmortizePreviewData): Requires verification of field mapping, normalization logic for reAmortizationInterestHandling, and locale/dateFormat handling - Preview dialog component wiring: New interface (ReAmortizePreviewDialogData) and standalone component setup require validation of dependency injection and data binding
- Service method pattern: getReAmortizePreview() uses HttpParams construction; ensure consistency with existing patterns (e.g., getReAgePreview)
- Integration: Verify proper error handling in preview() method, currencyCode validation, and dialog lifecycle management
Possibly related PRs
- WEB-481 :- a translation problem in mifos home page Tooltip text shows tooltips.Collection Sheet instead of Collection Sheet #2863: Updates the same Collection Sheet tooltip translation key in sidenav.component.html (tooltips.Collection Sheet → labels.menus.Collection Sheet)
- WEB-477: Re-amortization:- Preview Schedule API during re-amortization #2855: Adds identical re-amortization preview workflow—Preview button, preview() method, ReAmortizePreviewDialogComponent, and getReAmortizePreview() service method
- WEB-478-fix/i18n/savings-account-details-missing #2856: Adds the same "Savings Account Details" translation entries across all language files
Suggested reviewers
- alberto-art3ch
- IOhacker
- gkbishnoi07
Pre-merge checks and finishing touches
❌ Failed checks (1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check | ❓ Inconclusive | The title 'Cherry-pick dev-angular-19 changes back to dev' is vague and does not clearly describe the actual changes in the pull request, which include loan re-amortization preview functionality, translation updates, and minor UI adjustments. | Provide a more specific title that describes the main feature or change, such as 'Add loan re-amortization preview dialog and update translations' or similar. |
✅ 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. |
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
dev-angular-19-pr
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts (1)
127-132: Add error handling tosubmit().The
submit()method has no error handler, which could leave users without feedback if the API call fails. This is inconsistent with the error handling inpreview().submit(): void { const data = this.prepareReAmortizeData(); - this.loanService.submitLoanActionButton(this.loanId, data, 'reAmortize').subscribe((response: any) => { - this.router.navigate(['../../transactions'], { relativeTo: this.route }); + this.loanService.submitLoanActionButton(this.loanId, data, 'reAmortize').subscribe({ + next: () => { + this.router.navigate(['../../transactions'], { relativeTo: this.route }); + }, + error: (error) => { + console.error('Error submitting re-amortize:', error); + // Consider adding user-facing error notification + } }); }
🧹 Nitpick comments (2)
src/app/loans/loans.service.ts (1)
261-277: Consider extracting common param-building logic.The
getReAmortizePreviewcorrectly mirrorsgetReAgePreview, but both methods have identical param-building logic (lines 270-274 vs 252-256). Consider extracting to a private helper for DRY.+ private buildHttpParams(data: any): HttpParams { + let httpParams = new HttpParams(); + Object.keys(data).forEach((key) => { + if (data[key] !== null && data[key] !== undefined && data[key] !== '') { + httpParams = httpParams.set(key, data[key].toString()); + } + }); + return httpParams; + } + getReAgePreview(loanId: string, data: any): Observable<any> { - let httpParams = new HttpParams(); - - Object.keys(data).forEach((key) => { - if (data[key] !== null && data[key] !== undefined && data[key] !== '') { - httpParams = httpParams.set(key, data[key].toString()); - } - }); - + const httpParams = this.buildHttpParams(data); return this.http.get(`/loans/${loanId}/transactions/reage-preview`, { params: httpParams }); } getReAmortizePreview(loanId: string, data: any): Observable<any> { - let httpParams = new HttpParams(); - - Object.keys(data).forEach((key) => { - if (data[key] !== null && data[key] !== undefined && data[key] !== '') { - httpParams = httpParams.set(key, data[key].toString()); - } - }); - + const httpParams = this.buildHttpParams(data); return this.http.get(`/loans/${loanId}/transactions/reamortized-preview`, { params: httpParams }); }src/app/loans/loans-view/loan-account-actions/loan-reamortize/re-amortize-preview-dialog/re-amortize-preview-dialog.component.ts (1)
1-1: Remove unusedInjectimport.The
Injectdecorator is imported but not used since the component uses the functionalinject()approach.-import { Component, inject, Inject } from '@angular/core'; +import { Component, inject } from '@angular/core';
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
.github/workflows/build.ymlis excluded by!.github/**and included by**/*.github/workflows/create-docker-hub-image.ymlis excluded by!.github/**and included by**/*.github/workflows/test.ymlis excluded by!.github/**and included by**/*
📒 Files selected for processing (19)
src/app/core/shell/sidenav/sidenav.component.html(1 hunks)src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.html(1 hunks)src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts(3 hunks)src/app/loans/loans-view/loan-account-actions/loan-reamortize/re-amortize-preview-dialog/re-amortize-preview-dialog.component.html(1 hunks)src/app/loans/loans-view/loan-account-actions/loan-reamortize/re-amortize-preview-dialog/re-amortize-preview-dialog.component.ts(1 hunks)src/app/loans/loans.service.ts(1 hunks)src/assets/translations/cs-CS.json(1 hunks)src/assets/translations/de-DE.json(1 hunks)src/assets/translations/en-US.json(1 hunks)src/assets/translations/es-CL.json(1 hunks)src/assets/translations/es-MX.json(1 hunks)src/assets/translations/fr-FR.json(1 hunks)src/assets/translations/it-IT.json(1 hunks)src/assets/translations/ko-KO.json(1 hunks)src/assets/translations/lt-LT.json(1 hunks)src/assets/translations/lv-LV.json(1 hunks)src/assets/translations/ne-NE.json(1 hunks)src/assets/translations/pt-PT.json(1 hunks)src/assets/translations/sw-SW.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/app/**
⚙️ CodeRabbit configuration file
src/app/**: For Angular code: verify component separation, trackBy on *ngFor,
strict type safety, and clean observable patterns.
Files:
src/app/loans/loans.service.tssrc/app/loans/loans-view/loan-account-actions/loan-reamortize/re-amortize-preview-dialog/re-amortize-preview-dialog.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-reamortize/re-amortize-preview-dialog/re-amortize-preview-dialog.component.tssrc/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.htmlsrc/app/core/shell/sidenav/sidenav.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts
🧬 Code graph analysis (2)
src/app/loans/loans-view/loan-account-actions/loan-reamortize/re-amortize-preview-dialog/re-amortize-preview-dialog.component.ts (3)
src/app/loans/models/loan-account.model.ts (1)
RepaymentSchedule(55-73)src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts (1)
Component(13-141)src/app/standalone-shared.module.ts (1)
STANDALONE_SHARED_IMPORTS(21-51)
src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts (2)
src/app/settings/settings.service.ts (1)
dateFormat(109-111)src/app/loans/models/loan-account.model.ts (1)
RepaymentSchedule(55-73)
🔇 Additional comments (18)
src/app/core/shell/sidenav/sidenav.component.html (1)
100-110: Tooltip translation key inconsistency with other menu items.Line 102 uses
'labels.menus.Collection Sheet'for the tooltip, while all other menu items (Dashboard, Navigation, Checker Inbox, Individual Collection Sheet, Notifications, etc.) use the'tooltips.X'pattern. This creates a mismatch in the translation key naming convention.Verify that:
- The new translation key
'labels.menus.Collection Sheet'exists in all translation files- Whether other tooltip keys should also be updated as part of a broader refactoring (to avoid inconsistency across the sidenav)
src/assets/translations/fr-FR.json (1)
2747-2747: Translation addition looks good.The new "Savings Account Details" entry is correctly placed in the "text" section, maintains consistent JSON formatting with surrounding entries, and provides an appropriate French translation.
src/assets/translations/sw-SW.json (1)
2746-2746: Straightforward translation addition for Swahili locale. ✓The new entry "Maelezo ya Akaunti ya Akiba" is a valid, well-formatted Swahili translation appropriately placed within the
labels.textsection. The translation accurately conveys "Savings Account Details" and maintains consistency with the surrounding entries.src/assets/translations/de-DE.json (1)
2747-2747: Translation entry correctly added and follows established conventions.The new translation entry
"Savings Account Details": "Sparkonto-Details"is properly formatted, semantically accurate, and consistent with existing similar entries in the file (e.g.,"Savings Details": "Spardetails"). The German terminology is appropriate and uses correct titlecase capitalization matching the English source.src/assets/translations/lv-LV.json (1)
2747-2747: Translation entry is properly formatted and linguistically accurate.The addition of
"Savings Account Details": "Krājkonta informācija"follows the established translation pattern and correctly conveys the English meaning in Latvian. The entry is well-positioned within the text section and maintains proper JSON formatting.Consider verifying that this translation entry has been consistently added across all other locale files included in this PR (en-US, cs-CS, de-DE, es-CL, es-MX, fr-FR, it-IT, ko-KO, lt-LT, ne-NE, pt-PT, sw-SW) to ensure complete i18n coverage.
src/assets/translations/es-MX.json (1)
2749-2749: Translation entry looks good.The new "Savings Account Details" translation is properly placed in the
textsection and the Spanish translation ("Detalles de la cuenta de ahorros") is accurate and consistent with existing localization patterns in the file.src/assets/translations/ko-KO.json (1)
2747-2747: Translation entry correctly added to Korean locale file.The new "Savings Account Details" translation entry is properly formatted and placed in the correct section of the translation file. The Korean translation "저축 계좌 세부 정보" accurately conveys the meaning of the English key.
src/assets/translations/es-CL.json (1)
2746-2748: Translation entry added: "Savings Account Details"The new translation key "Savings Account Details" with Spanish value "Detalles de la cuenta de ahorros" has been correctly added to the es-CL.json locale file. The entry is properly formatted, placed logically within the "text" section, and the translation is grammatically accurate.
Per the PR summary, this translation key should be added consistently across all supported locale files. You may want to verify that all 12+ locale files mentioned in the PR context (cs-CS, de-DE, en-US, es-MX, fr-FR, it-IT, ko-KO, lt-LT, lv-LV, ne-NE, pt-PT, sw-SW) have this entry added with appropriate translations.
src/assets/translations/cs-CS.json (1)
2747-2747: Translation addition looks good.The new "Savings Account Details" entry is properly added with a semantically correct Czech translation. The placement and formatting are consistent with existing entries in the text dictionary.
src/assets/translations/it-IT.json (1)
2747-2747: Translation addition is accurate and properly positioned.The new "Savings Account Details" entry has been correctly translated to Italian ("Dettagli del conto di risparmio"), maintains proper alphabetical ordering between "All Savings" and "Allocate Cash", and follows the consistent JSON formatting of the file.
As a minor observation: please verify that this translation key is referenced in the actual application code (component templates, services, etc.) as part of the loan re-amortization preview feature, and confirm translations have been added to other language files mentioned in the PR summary (en-US, es-MX, etc.).
src/assets/translations/lt-LT.json (1)
2747-2747: ✓ Translation entry is well-formed and appropriate.The new "Savings Account Details" entry is correctly translated to Lithuanian ("Taupomosios sąskaitos informacija") with proper grammar, appropriate lexical choice (using genitive case for the compound noun), and correct JSON formatting. The entry is logically placed within the text section alongside related savings-related translations.
src/assets/translations/en-US.json (1)
2757-2757: Translation entry added correctly. The new "Savings Account Details" entry is properly formatted, correctly positioned in the text section of the labels, and follows the established translation pattern in the file. No issues identified.src/assets/translations/ne-NE.json (1)
2747-2747: ✓ Translation addition approved.The new "Savings Account Details" entry is correctly added to the Nepali translation file with proper JSON syntax and accurate Nepali translation ("बचत खाता विवरण"). The placement within the "text" section aligns with the UI label convention and maintains alphabetical ordering. This change is part of the coordinated multilingual update across all supported locales.
src/assets/translations/pt-PT.json (1)
2747-2747: Translation addition is correct and properly formatted.The new entry "Savings Account Details" → "Detalhes da Conta Poupança" is grammatically correct Portuguese and consistently placed within the
labels.textsection alongside similar UI display strings. JSON formatting is valid and maintains proper nesting and comma placement.src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.html (1)
63-72: LGTM!The Preview button is well-implemented with proper
type="button"to prevent form submission, consistent disabled state binding, and appropriate permission checks matching the Submit button.src/app/loans/loans-view/loan-account-actions/loan-reamortize/re-amortize-preview-dialog/re-amortize-preview-dialog.component.html (1)
1-16: LGTM!The dialog template follows Material Dialog best practices with proper structure (title, content, actions) and correct translation key usage.
src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts (1)
71-94: LGTM - data preparation logic is correct.The
prepareReAmortizePreviewData()properly handles the object-to-id transformation and fallback to 'default' value. The conditional check at line 82 correctly preserves0as a valid id.src/app/loans/loans-view/loan-account-actions/loan-reamortize/re-amortize-preview-dialog/re-amortize-preview-dialog.component.ts (1)
32-45: LGTM!The component implementation is clean, uses modern
inject()pattern with proper typing, and exposesreadonlyfields for the template. Good use of the exported interface for type safety.
| preview(): void { | ||
| if (this.reamortizeLoanForm.invalid) { | ||
| return; | ||
| } | ||
| const data = this.prepareReAmortizePreviewData(); | ||
|
|
||
| this.loanService.getReAmortizePreview(this.loanId, data).subscribe({ | ||
| next: (response: RepaymentSchedule) => { | ||
| const currencyCode = response.currency?.code; | ||
|
|
||
| if (!currencyCode) { | ||
| console.error('Currency code is not available in API response'); | ||
| return; | ||
| } | ||
|
|
||
| this.dialog.open(ReAmortizePreviewDialogComponent, { | ||
| data: { | ||
| repaymentSchedule: response, | ||
| currencyCode: currencyCode | ||
| }, | ||
| width: '95%', | ||
| maxWidth: '1400px', | ||
| height: '90vh' | ||
| }); | ||
| }, | ||
| error: (error) => { | ||
| console.error('Error loading re-amortize preview:', error); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Add user-facing error feedback.
The preview() method logs errors to console but provides no feedback to the user when:
- Currency code is missing (lines 106-109)
- API call fails (lines 121-123)
Users will see no indication that something went wrong.
Consider using a snackbar or toast notification:
+ private snackBar = inject(MatSnackBar); // Add to imports and inject
preview(): void {
// ...
this.loanService.getReAmortizePreview(this.loanId, data).subscribe({
next: (response: RepaymentSchedule) => {
const currencyCode = response.currency?.code;
if (!currencyCode) {
console.error('Currency code is not available in API response');
+ this.snackBar.open('Unable to load preview: missing currency information', 'Close', { duration: 5000 });
return;
}
// ...
},
error: (error) => {
console.error('Error loading re-amortize preview:', error);
+ this.snackBar.open('Error loading preview. Please try again.', 'Close', { duration: 5000 });
}
});
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts
around lines 96 to 125, the preview() method only logs errors to the console and
gives no user-facing feedback when the currency code is missing or when the API
call fails; inject and use the app's snackbar/toast service (e.g., MatSnackBar
or the project's notification service) in the component constructor, then
replace the console.error branches: when currency is absent show a user-friendly
error toast like "Unable to preview: missing currency information" and return,
and in the API error handler show a toast like "Failed to load re-amortize
preview. Please try again." (optionally include a brief error detail in the
toast or log it to console for debugging).
5 participants
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
Release Notes
New Features
UI Updates
✏️ Tip: You can customize this high-level summary in your review settings.