WEB-352: Re-Amortization Interest Handling configuration #2714
Conversation
WalkthroughAdds a resolver branch mapping "Re-Amortize" to the reAmortization template and extends the LoanReamortize component with two option lists, new form controls (interest handling, reason, note), trackBy helpers, and initialization of options from the component's dataObject. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Loan Actions UI
participant Resolver as LoanActionButtonResolver
participant Template as reAmortization Template
participant Component as LoanReamortizeComponent
User->>UI: Click "Re-Amortize"
UI->>Resolver: resolve(action="Re-Amortize")
Resolver-->>UI: returns "reAmortization" template
UI->>Template: render
Template->>Component: instantiate / ngOnInit
Component->>Component: read dataObject -> { reAmortizationReasonOptions, reAmortizationInterestHandlingOptions }
Component-->>Template: populate selects and initialize form controls
Template->>User: display form (interest handling, reason, note, externalId)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.html (2)
9-14: *Add trackBy function to ngFor for performance.The *ngFor directive is missing a trackBy function, which can lead to unnecessary DOM re-renders when the options array reference changes.
As per coding guidelines for Angular code, trackBy should be used on *ngFor loops.
Apply this diff to add trackBy:
- <mat-option - *ngFor="let reAmortizationInterestHandlingOption of reAmortizationInterestHandlingOptions" - [value]="reAmortizationInterestHandlingOption.id" - > + <mat-option + *ngFor="let reAmortizationInterestHandlingOption of reAmortizationInterestHandlingOptions; trackBy: trackByInterestHandlingId" + [value]="reAmortizationInterestHandlingOption.id" + >Then add the trackBy function in the TypeScript component:
trackByInterestHandlingId(index: number, item: OptionData): number { return item.id; }
23-28: *Add trackBy function to ngFor for performance.The *ngFor directive is missing a trackBy function, which can lead to unnecessary DOM re-renders when the options array reference changes.
As per coding guidelines for Angular code, trackBy should be used on *ngFor loops.
Apply this diff to add trackBy:
- <mat-option - *ngFor="let reAmortizationReasonOption of reAmortizationReasonOptions" - [value]="reAmortizationReasonOption.id" - > + <mat-option + *ngFor="let reAmortizationReasonOption of reAmortizationReasonOptions; trackBy: trackByReasonId" + [value]="reAmortizationReasonOption.id" + >Then add the trackBy function in the TypeScript component:
trackByReasonId(index: number, item: any): number { return item.id; }src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts (1)
35-36: Add defensive checks for dataObject properties.Direct property access on
dataObjectwithout null/undefined checks could lead to runtime errors if the properties don't exist or dataObject is undefined.Apply this diff to add defensive checks:
- this.reAmortizationReasonOptions = this.dataObject.reAmortizationReasonOptions; - this.reAmortizationInterestHandlingOptions = this.dataObject.reAmortizationInterestHandlingOptions; + this.reAmortizationReasonOptions = this.dataObject?.reAmortizationReasonOptions || []; + this.reAmortizationInterestHandlingOptions = this.dataObject?.reAmortizationInterestHandlingOptions || [];
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/loans/common-resolvers/loan-action-button.resolver.ts(1 hunks)src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.html(2 hunks)src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts(3 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/common-resolvers/loan-action-button.resolver.tssrc/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.htmlsrc/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts
🧬 Code graph analysis (1)
src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts (1)
src/app/shared/models/option-data.model.ts (1)
OptionData(1-5)
🔇 Additional comments (1)
src/app/loans/common-resolvers/loan-action-button.resolver.ts (1)
84-85: LGTM!The new 'Re-Amortize' action handler follows the established pattern consistently, using the appropriate camelCase naming convention for the template parameter.
src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts
Outdated
Show resolved
Hide resolved
src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts
Show resolved
Hide resolved
alberto-art3ch
force-pushed
the
WEB-352/re-amortization-interest-handling-configuration
branch
from
0827983 to
3cca951
Compare
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.html (1)
5-45: Form validation not properly implemented: missing validators and trackBy functions.The form has no validators applied to any control, so
reamortizeLoanForm.validis always true. The submit button's[disabled]="!reamortizeLoanForm.valid"binding will never actually disable the button. Additionally, both *ngFor loops (lines 10 and 24) are missingtrackByfunctions, which violates Angular performance best practices.Required fixes:
- Add validators to form controls, particularly
reasonCodeValueId(the "Reason" field should likely be required)- Add
trackByfunctions to both*ngForloops iterating overreAmortizationInterestHandlingOptionsandreAmortizationReasonOptions
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/loans/common-resolvers/loan-action-button.resolver.ts(1 hunks)src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.html(2 hunks)src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app/loans/common-resolvers/loan-action-button.resolver.ts
- src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts
🧰 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-view/loan-account-actions/loan-reamortize/loan-reamortize.component.html
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run Lint, Build and Deploy
🔇 Additional comments (1)
src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.html (1)
13-13: Translation approach is correct and intentional—no changes needed.The apparent "inconsistency" is actually correct implementation reflecting different data structures:
- Interest Handling uses
OptionDatatype with a.valueproperty that contains translation keys, requiring the| translateKey: 'catalogs'filter- Reason uses
CodeValuetype with a.nameproperty that contains plain display strings, requiring no translationEach dropdown appropriately uses the property available on its respective type. This is not an inconsistency but a correct alignment with the underlying data models.
src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.html
Outdated
Show resolved
Hide resolved
alberto-art3ch
force-pushed
the
WEB-352/re-amortization-interest-handling-configuration
branch
from
3cca951 to
4da458e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
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)
18-18: Define a proper interface fordataObjectto enforce strict type safety.The
@Input() dataObject: any;violates the strict type safety requirement for Angular code. Define a typed interface to catch potential errors at compile time.As per coding guidelines.
Apply this diff to add type safety:
+interface ReAmortizationData { + reAmortizationReasonOptions: CodeValue[]; + reAmortizationInterestHandlingOptions: OptionData[]; +} + - @Input() dataObject: any; + @Input() dataObject: ReAmortizationData;
🧹 Nitpick comments (2)
src/app/loans/common-resolvers/loan-action-button.resolver.ts (1)
84-85: LGTM! The new branch follows the established pattern.The 'Re-Amortize' action mapping is correctly implemented and consistent with other action handlers.
Optional: Consider refactoring the entire if-else chain to a map-based lookup for improved maintainability.
The resolver now contains 30+ action branches. A map structure would reduce duplication and improve maintainability:
private readonly ACTION_TEMPLATES = new Map<string, string>([ ['Make Repayment', 'repayment'], ['Re-Age', 'reAge'], ['Re-Amortize', 'reAmortization'], // ... other mappings ]); resolve(route: ActivatedRouteSnapshot): Observable<any> { const loanId = route.paramMap.get('loanId') || route.parent.paramMap.get('loanId'); const loanActionButton = route.paramMap.get('action'); const template = this.ACTION_TEMPLATES.get(loanActionButton); if (template) { return this.loansService.getLoanActionTemplate(loanId, template); } // Handle special cases... }src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts (1)
53-58: Add error handling to the submit subscription for better user experience.The
subscribecall lacks error handling, which means API failures will be silent and users won't receive feedback about what went wrong.Apply this diff to add error handling:
submit(): void { const data = this.reamortizeLoanForm.value; - 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: (response: any) => { + this.router.navigate(['../../transactions'], { relativeTo: this.route }); + }, + error: (error) => { + // Handle error (e.g., show error message to user) + console.error('Re-amortization failed:', error); + } + }); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/loans/common-resolvers/loan-action-button.resolver.ts(1 hunks)src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.html(2 hunks)src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.html
🧰 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-view/loan-account-actions/loan-reamortize/loan-reamortize.component.tssrc/app/loans/common-resolvers/loan-action-button.resolver.ts
🧬 Code graph analysis (1)
src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts (2)
src/app/shared/models/general.model.ts (1)
CodeValue(65-72)src/app/shared/models/option-data.model.ts (1)
OptionData(1-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run Lint, Build and Deploy
🔇 Additional comments (1)
src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts (1)
23-24: LGTM! Properties are now properly typed.The array properties use strict typing with
CodeValue[]andOptionData[]interfaces, correctly addressing the previous review feedback about usingany[].
src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts
Outdated
Show resolved
Hide resolved
src/app/loans/loans-view/loan-account-actions/loan-reamortize/loan-reamortize.component.ts
Show resolved
Hide resolved
alberto-art3ch
force-pushed
the
WEB-352/re-amortization-interest-handling-configuration
branch
from
4da458e to
7ab4b9b
Compare
2 participants
Description
User should be able to select the following Interest handling subtypes from the drop down during application of re-amortization to the loan account
Related issues and discussion
WEB-352
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