-
Notifications
You must be signed in to change notification settings - Fork 890
WEB-657: WorkingCapital Loan approve or reject actions #3416
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -84,10 +84,17 @@ export class RejectLoanComponent extends LoanAccountActionsBaseComponent impleme | |
| dateFormat, | ||
| locale | ||
| }; | ||
| const loanCommand: string = 'reject'; | ||
| if (this.isLoanProduct) { | ||
| this.loanService.loanActionButtons(this.loanId, 'reject', data).subscribe((response: any) => { | ||
| this.loanService.loanActionButtons(this.loanId, loanCommand, data).subscribe((response: any) => { | ||
| this.gotoLoanDefaultView(); | ||
| }); | ||
| } else if (this.isWorkingCapital) { | ||
| this.loanService | ||
| .applyWorkingCapitalLoanAccountCommand(this.loanId, loanCommand, data) | ||
| .subscribe((response: any) => { | ||
| this.gotoLoanDefaultView(); | ||
| }); | ||
| } | ||
|
Comment on lines
+87
to
98
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Add error handling and consider edge case coverage. Several concerns with this submission logic:
Proposed fix with error handling const loanCommand: string = 'reject';
if (this.isLoanProduct) {
- this.loanService.loanActionButtons(this.loanId, loanCommand, data).subscribe((response: any) => {
- this.gotoLoanDefaultView();
- });
+ this.loanService.loanActionButtons(this.loanId, loanCommand, data).subscribe({
+ next: () => this.gotoLoanDefaultView(),
+ error: (err) => console.error('Failed to reject loan', err)
+ });
} else if (this.isWorkingCapital) {
- this.loanService
- .applyWorkingCapitalLoanAccountCommand(this.loanId, loanCommand, data)
- .subscribe((response: any) => {
- this.gotoLoanDefaultView();
- });
+ this.loanService
+ .applyWorkingCapitalLoanAccountCommand(this.loanId, loanCommand, data)
+ .subscribe({
+ next: () => this.gotoLoanDefaultView(),
+ error: (err) => console.error('Failed to reject working capital loan', err)
+ });
+ } else {
+ console.warn('Unknown loan product type; reject action not performed');
}Consider also showing a user-facing error notification (e.g., via a toast/snackbar service) rather than just logging to console. Based on learnings: avoid 🤖 Prompt for AI Agents |
||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scope “Volume” label to Working Capital loans only.
Line 271 currently shows “Total Payment Volume” for every loan in this view. This should be conditional; non-working-capital loans should keep the amount label.
Proposed fix
🤖 Prompt for AI Agents