WEB-813: Working Capital loan delinquency actions#3460
Open
alberto-art3ch wants to merge 1 commit intoopenMF:devfrom
Open
WEB-813: Working Capital loan delinquency actions#3460alberto-art3ch wants to merge 1 commit intoopenMF:devfrom
alberto-art3ch wants to merge 1 commit intoopenMF:devfrom
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Date Utilities src/app/core/utils/dates.ts |
Added public comparison helpers isBefore(date1, date2) and isAfter(date1, date2). |
Loan Resolvers src/app/loans/common-resolvers/loan-delinquency-actions.resolver.ts, src/app/loans/common-resolvers/loan-delinquency-data.resolver.ts, src/app/loans/common-resolvers/loan-delinquency-tags.resolver.ts |
Added numeric-coercion checks and product-type conditional branching before calling LoansService; some paths may now return no value when conditions fail. |
Reschedule Dialog Component src/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.ts, ... .html, ... .scss |
New standalone Angular Material dialog component with reactive form (minimumPayment, frequency, frequencyType) and template; SCSS header/licence added. |
Delinquency Tags Tab (UI + Logic) src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts, ... .html |
UI now conditionally renders by product type, table columns changed (classification, addedOn, liftedOn), added Reschedule button for working-capital, tightened action visibility, integrated dialog and reschedule flow, and extended sendDelinquencyAction payload to include reschedule fields. |
Loans Service src/app/loans/loans.service.ts |
Changed getDelinquencyTags signature/path to accept only loanId and changed createDelinquencyActions signature to (productType, loanId, delinquencyActions) and corresponding endpoint path. |
Loan Terms Step (label) src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html |
Updated validation error label key from labels.inputs.Repaid every to labels.inputs.Period Payment Frequency. |
Sequence Diagram(s)
sequenceDiagram
participant User
participant TagsTab as "Delinquency Tags Tab"
participant Dialog as "Reschedule Dialog"
participant Service as "Loans Service"
participant Backend as "Backend API"
User->>TagsTab: Click Reschedule
TagsTab->>Dialog: open(RescheduleDialog, {data})
Dialog->>Dialog: initialize form (minPayment, frequency, type)
User->>Dialog: Fill form and submit
Dialog-->>TagsTab: Return {data: form}
TagsTab->>Service: createDelinquencyActions(productType, loanId, payload with reschedule fields)
Service->>Backend: POST /{productType}/{loanId}/delinquency-actions
Backend-->>Service: 200 OK
Service-->>TagsTab: Observable resolves
TagsTab->>Service: getDelinquencyActions(...) refresh
Service->>Backend: GET delinquency actions
Backend-->>Service: updated data
Service-->>TagsTab: Updated actions
TagsTab-->>User: UI refresh
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
- WEB-711: Working Capital product #3159: Overlapping changes to delinquency actions/tags, loans.service signatures, and product-type handling.
- WEB-813: Working Capital loan account modification #3435: Related resolver and service adjustments for loan vs working-capital path handling.
- WEB-813: Working Capital loan account creation #3257: Related product-aware service endpoints and loan-view action logic changes.
Suggested reviewers
- adamsaghy
- 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 PR title accurately summarizes the main feature: introducing a new delinquency action for Working Capital loans. It references the Jira ticket (WEB-813) and clearly indicates the primary change scope. |
| 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 (4)
src/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.html (2)
12-15: Consider adding validation error display forminimumPayment.The
minimumPaymentfield is marked as required in the component (per form builder), but unlike thefrequencyfield (lines 26-31), it lacks a<mat-error>block to display validation feedback to users. For consistency and better UX, consider adding error handling:♻️ Suggested enhancement
<mat-form-field class="flex-28"> <mat-label>{{ 'labels.inputs.Minimum Payment' | translate }}</mat-label> <input type="number" matInput formControlName="minimumPayment" /> + `@if` (delinquencyActionForm.controls.minimumPayment.hasError('required')) { + <mat-error> + {{ 'labels.inputs.Minimum Payment' | translate }} {{ 'labels.commons.is' | translate }} + <strong>{{ 'labels.commons.required' | translate }}</strong> + </mat-error> + } </mat-form-field>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.html` around lines 12 - 15, The template is missing a validation error display for the minimumPayment form control; add a <mat-error> under the input bound to formControlName="minimumPayment" that mirrors the frequency field's validation messaging (e.g., show a "required" message when minimumPayment.errors?.required and the control is touched or dirty) so users see feedback when LoanDelinquencyActionRescheduleDialogComponent's form requires this field.
37-41: Use unique identifier fortrackexpression.The
track frequencyTypetracks the entire object reference. For optimal change detection, track by a unique identifier:- `@for` (frequencyType of frequencyTypeOptions; track frequencyType) { + `@for` (frequencyType of frequencyTypeOptions; track frequencyType.id) {As per coding guidelines: "verify component separation, trackBy on *ngFor, strict type safety".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.html` around lines 37 - 41, The template is tracking the whole object with "track frequencyType"; change the *ngFor to use a trackBy function (e.g., "trackBy: trackByFrequencyType") and implement a strongly-typed method on the component (e.g., trackByFrequencyType(index: number, frequencyType: FrequencyType): string | number) that returns the unique id (frequencyType.id); update the component class where frequencyTypeOptions is defined to ensure FrequencyType typing and export the trackBy method so Angular can use it for optimal change detection.src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.html (1)
19-24: Simplify mutually exclusive conditionals with@else.The two
@ifblocks check opposite conditions formaximumAgeDays. Use@elsefor cleaner logic:♻️ Suggested refactor
`@if` (item.delinquencyRange.maximumAgeDays) { <span>( {{ item.delinquencyRange.minimumAgeDays }} - {{ item.delinquencyRange.maximumAgeDays }} )</span> - } - `@if` (!item.delinquencyRange.maximumAgeDays) { + } `@else` { <span>( {{ item.delinquencyRange.minimumAgeDays }} )</span> }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.html` around lines 19 - 24, The template currently uses two mutually exclusive conditionals on item.delinquencyRange.maximumAgeDays; replace the second "@if (!item.delinquencyRange.maximumAgeDays)" with an "@else" to simplify logic in loan-delinquency-tags-tab.component.html so the first block checks "@if (item.delinquencyRange.maximumAgeDays)" and the alternate branch uses "@else" to render the single-value span, keeping the same bindings (item.delinquencyRange.minimumAgeDays / maximumAgeDays).src/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.ts (1)
52-67: Consider adding numeric validators for payment and frequency fields.The
minimumPaymentandfrequencyfields accept numeric input but only validate forrequired. Consider adding minimum value validators to prevent invalid negative values:♻️ Suggested enhancement
createDelinquencyActionForm() { this.delinquencyActionForm = this.formBuilder.group({ minimumPayment: [ '', - Validators.required + [Validators.required, Validators.min(0)] ], frequency: [ '', - Validators.required + [Validators.required, Validators.min(1)] ], frequencyType: [ '', Validators.required ] }); }This ensures users cannot submit negative payment amounts or zero/negative frequency values.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.ts` around lines 52 - 67, The form created in createDelinquencyActionForm currently only checks for required values; update the delinquencyActionForm definition to add numeric validators for minimumPayment and frequency (e.g., Validators.min for a positive minimumPayment like Validators.min(0.01) and Validators.min(1) or Validators.min(0) as appropriate for frequency) and optionally a numeric pattern validator to ensure only numbers are accepted; adjust the formBuilder.group for the minimumPayment and frequency controls to include these validators so negative or zero values are rejected.
🤖 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/common-resolvers/loan-base.resolver.ts`:
- Around line 22-30: The initialize method currently only ignores null and can
leave this.productType (the BehaviorSubject) unchanged when the query param is
missing or unexpected; update initialize(route: ActivatedRouteSnapshot) to
always reset this.productType by reading route.queryParams['productType'] and
setting this.productType.next(LOAN_PRODUCT_TYPE.LOAN) as the default, then
override with LOAN_PRODUCT_TYPE.WORKING_CAPITAL if the param ===
'working-capital' (or LOAN if param === 'loan'), so any missing/unknown values
reliably revert to LOAN.
In `@src/app/loans/common-resolvers/loan-delinquency-tags.resolver.ts`:
- Around line 35-41: The resolver method resolve currently can return undefined
when loanId is non-numeric; update resolve in loan-delinquency-tags.resolver
(and the sibling resolvers loan-delinquency-data.resolver.ts and
loan-delinquency-actions.resolver.ts) to explicitly return an RxJS EMPTY
observable for the non-numeric branch: keep the existing initialize(route) and
loanId extraction, but replace the implicit undefined path with a return of
EMPTY so resolve always returns an Observable; target the resolve method and the
call to this.loansService.getDelinquencyTags(loanId) when making the change.
In
`@src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts`:
- Around line 249-258: The success handler after createDelinquencyActions
currently only re-fetches getDelinquencyActions and updates
loanDelinquencyActions, leaving loanDelinquencyTags and
installmentLevelDelinquency stale; replace the inner getDelinquencyActions call
with a call to reload() (inherited from LoanProductBaseComponent) so the
component fully refreshes state (loanDelinquencyActions, loanDelinquencyTags,
installmentLevelDelinquency) after createDelinquencyActions completes.
- Around line 190-195: Guard against dialog cancellation by checking the value
emitted from loanDelinquencyActionDialogRef.afterClosed() before accessing
response.data.value and only calling sendDelinquencyAction when the dialog
returned a valid payload; specifically, in the subscription to
loanDelinquencyActionDialogRef.afterClosed() validate that response is defined
and has the expected data/value properties and return early on cancel. Also fix
the type contract: either make the Date parameters in sendDelinquencyAction
nullable (e.g., allow Date | null) or ensure you pass real Date values when
calling it for the reschedule flow so the method signature and callers (the
reschedule branch around sendDelinquencyAction) match strict typing; apply the
same checks/fix to the other similar block referenced (lines ~217–246).
---
Nitpick comments:
In
`@src/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.html`:
- Around line 12-15: The template is missing a validation error display for the
minimumPayment form control; add a <mat-error> under the input bound to
formControlName="minimumPayment" that mirrors the frequency field's validation
messaging (e.g., show a "required" message when minimumPayment.errors?.required
and the control is touched or dirty) so users see feedback when
LoanDelinquencyActionRescheduleDialogComponent's form requires this field.
- Around line 37-41: The template is tracking the whole object with "track
frequencyType"; change the *ngFor to use a trackBy function (e.g., "trackBy:
trackByFrequencyType") and implement a strongly-typed method on the component
(e.g., trackByFrequencyType(index: number, frequencyType: FrequencyType): string
| number) that returns the unique id (frequencyType.id); update the component
class where frequencyTypeOptions is defined to ensure FrequencyType typing and
export the trackBy method so Angular can use it for optimal change detection.
In
`@src/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.ts`:
- Around line 52-67: The form created in createDelinquencyActionForm currently
only checks for required values; update the delinquencyActionForm definition to
add numeric validators for minimumPayment and frequency (e.g., Validators.min
for a positive minimumPayment like Validators.min(0.01) and Validators.min(1) or
Validators.min(0) as appropriate for frequency) and optionally a numeric pattern
validator to ensure only numbers are accepted; adjust the formBuilder.group for
the minimumPayment and frequency controls to include these validators so
negative or zero values are rejected.
In
`@src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.html`:
- Around line 19-24: The template currently uses two mutually exclusive
conditionals on item.delinquencyRange.maximumAgeDays; replace the second "@if
(!item.delinquencyRange.maximumAgeDays)" with an "@else" to simplify logic in
loan-delinquency-tags-tab.component.html so the first block checks "@if
(item.delinquencyRange.maximumAgeDays)" and the alternate branch uses "@else" to
render the single-value span, keeping the same bindings
(item.delinquencyRange.minimumAgeDays / maximumAgeDays).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 92372f55-cf17-4045-822f-a12ac72ecc4c
📒 Files selected for processing (13)
src/app/core/utils/dates.tssrc/app/loans/common-resolvers/loan-base.resolver.tssrc/app/loans/common-resolvers/loan-delinquency-actions.resolver.tssrc/app/loans/common-resolvers/loan-delinquency-data.resolver.tssrc/app/loans/common-resolvers/loan-delinquency-tags.resolver.tssrc/app/loans/common-resolvers/loan-details.resolver.tssrc/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.htmlsrc/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.scsssrc/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.tssrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.htmlsrc/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.tssrc/app/loans/loans.service.ts
Comment on lines
+22
to
+30
| protected initialize(route: ActivatedRouteSnapshot): void { | ||
| const productType = route.queryParams['productType']; | ||
| if (productType !== null) { | ||
| if (productType === 'loan') { | ||
| this.productType.next(LOAN_PRODUCT_TYPE.LOAN); | ||
| } else if (productType === 'working-capital') { | ||
| this.productType.next(LOAN_PRODUCT_TYPE.WORKING_CAPITAL); | ||
| } | ||
| } |
There was a problem hiding this comment.
Always reset productType during initialization.
Line 24 only filters out null, not the missing-param case, so this resolver keeps the previous BehaviorSubject value when productType is absent or unexpected. After a working-capital navigation, a plain loan route can keep resolving working-capital-loans endpoints until something else resets it. src/app/loans/common-resolvers/loan-details.resolver.ts:35-47 already defaults that case back to LOAN_PRODUCT_TYPE.LOAN, so this new base resolver can diverge from the rest of the page.
💡 Proposed fix
protected initialize(route: ActivatedRouteSnapshot): void {
- const productType = route.queryParams['productType'];
- if (productType !== null) {
- if (productType === 'loan') {
- this.productType.next(LOAN_PRODUCT_TYPE.LOAN);
- } else if (productType === 'working-capital') {
- this.productType.next(LOAN_PRODUCT_TYPE.WORKING_CAPITAL);
- }
- }
+ const productType = route.queryParamMap.get('productType');
+ this.productType.next(
+ productType === LOAN_PRODUCT_TYPE.WORKING_CAPITAL
+ ? LOAN_PRODUCT_TYPE.WORKING_CAPITAL
+ : LOAN_PRODUCT_TYPE.LOAN
+ );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| protected initialize(route: ActivatedRouteSnapshot): void { | |
| const productType = route.queryParams['productType']; | |
| if (productType !== null) { | |
| if (productType === 'loan') { | |
| this.productType.next(LOAN_PRODUCT_TYPE.LOAN); | |
| } else if (productType === 'working-capital') { | |
| this.productType.next(LOAN_PRODUCT_TYPE.WORKING_CAPITAL); | |
| } | |
| } | |
| protected initialize(route: ActivatedRouteSnapshot): void { | |
| const productType = route.queryParamMap.get('productType'); | |
| this.productType.next( | |
| productType === LOAN_PRODUCT_TYPE.WORKING_CAPITAL | |
| ? LOAN_PRODUCT_TYPE.WORKING_CAPITAL | |
| : LOAN_PRODUCT_TYPE.LOAN | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/loans/common-resolvers/loan-base.resolver.ts` around lines 22 - 30,
The initialize method currently only ignores null and can leave this.productType
(the BehaviorSubject) unchanged when the query param is missing or unexpected;
update initialize(route: ActivatedRouteSnapshot) to always reset
this.productType by reading route.queryParams['productType'] and setting
this.productType.next(LOAN_PRODUCT_TYPE.LOAN) as the default, then override with
LOAN_PRODUCT_TYPE.WORKING_CAPITAL if the param === 'working-capital' (or LOAN if
param === 'loan'), so any missing/unknown values reliably revert to LOAN.
Comment on lines
35
to
41
| resolve(route: ActivatedRouteSnapshot): Observable<any> { | ||
| this.initialize(route); | ||
| const loanId = route.paramMap.get('loanId') || route.parent.paramMap.get('loanId'); | ||
| return this.loansService.getDelinquencyTags(loanId); | ||
| if (!isNaN(+loanId)) { | ||
| return this.loansService.getDelinquencyTags(loanId); | ||
| } | ||
| } |
There was a problem hiding this comment.
Resolver may return undefined for non-numeric loanId.
When loanId fails the numeric check, resolve() implicitly returns undefined. Angular's router resolver contract expects an Observable, Promise, or a resolved value. Returning undefined may cause unpredictable behavior or silent failures in route resolution.
Consider returning EMPTY from RxJS to explicitly handle the non-numeric case:
🛡️ Suggested fix
+import { EMPTY, Observable } from 'rxjs';
resolve(route: ActivatedRouteSnapshot): Observable<any> {
this.initialize(route);
const loanId = route.paramMap.get('loanId') || route.parent.paramMap.get('loanId');
if (!isNaN(+loanId)) {
return this.loansService.getDelinquencyTags(loanId);
}
+ return EMPTY;
}This same pattern applies to sibling resolvers (loan-delinquency-data.resolver.ts, loan-delinquency-actions.resolver.ts).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/loans/common-resolvers/loan-delinquency-tags.resolver.ts` around
lines 35 - 41, The resolver method resolve currently can return undefined when
loanId is non-numeric; update resolve in loan-delinquency-tags.resolver (and the
sibling resolvers loan-delinquency-data.resolver.ts and
loan-delinquency-actions.resolver.ts) to explicitly return an RxJS EMPTY
observable for the non-numeric branch: keep the existing initialize(route) and
loanId extraction, but replace the implicit undefined path with a return of
EMPTY so resolve always returns an Observable; target the resolve method and the
call to this.loansService.getDelinquencyTags(loanId) when making the change.
Comment on lines
+190
to
+195
| loanDelinquencyActionDialogRef.afterClosed().subscribe((response: { data: any }) => { | ||
| const minimumPayment: number = response.data.value.minimumPayment; | ||
| const frequency: number = response.data.value.frequency; | ||
| const frequencyType: string = response.data.value.frequencyType; | ||
|
|
||
| this.sendDelinquencyAction(action, null, null, minimumPayment, frequency, frequencyType); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Dates.formatDate implementation:"
rg -n -C3 'formatDate\s*\(' src/app/core/utils/dates.ts
echo
echo "TypeScript nullability settings:"
rg -n --iglob 'tsconfig*.json' '"strict"\s*:|"strictNullChecks"\s*:'Repository: openMF/web-app
Length of output: 447
🏁 Script executed:
cat src/app/core/utils/dates.tsRepository: openMF/web-app
Length of output: 2557
🏁 Script executed:
cat tsconfig.json | grep -A 10 '"strict"'Repository: openMF/web-app
Length of output: 40
🏁 Script executed:
sed -n '180,260p' src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.tsRepository: openMF/web-app
Length of output: 2774
Guard against dialog cancellation in the reschedule flow.
When the dialog is cancelled or closed via backdrop, afterClosed() emits undefined, causing immediate crash on accessing response.data.value. Additionally, sendDelinquencyAction() declares non-nullable Date parameters but receives null for the reschedule action, violating the coding guideline for strict typing conventions. While the reschedule payload overwrites the initial payload (avoiding the formatDate(null) call), the type mismatch remains a contract violation.
💡 Proposed fix
loanDelinquencyActionDialogRef.afterClosed().subscribe((response?: { data?: { value?: any } }) => {
const formValue = response?.data?.value;
if (!formValue) {
return;
}
this.sendDelinquencyAction(action, null, null, formValue.minimumPayment, formValue.frequency, formValue.frequencyType);
});
}
sendDelinquencyAction(
action: string,
- startDate: Date,
- endDate: Date,
- minimumPayment: number,
- frequency: number,
- frequencyType: string
+ startDate: Date | null,
+ endDate: Date | null,
+ minimumPayment: number | null,
+ frequency: number | null,
+ frequencyType: string | null
): void {Note: TypeScript strict null checks are not enabled in the main tsconfig.json, so these violations are not caught at compile time. The crash on dialog cancellation is the immediate runtime risk; the type violations should be fixed to align with coding guidelines and prevent future regressions.
Also applies to: 217–246
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts`
around lines 190 - 195, Guard against dialog cancellation by checking the value
emitted from loanDelinquencyActionDialogRef.afterClosed() before accessing
response.data.value and only calling sendDelinquencyAction when the dialog
returned a valid payload; specifically, in the subscription to
loanDelinquencyActionDialogRef.afterClosed() validate that response is defined
and has the expected data/value properties and return early on cancel. Also fix
the type contract: either make the Date parameters in sendDelinquencyAction
nullable (e.g., allow Date | null) or ensure you pass real Date values when
calling it for the reschedule flow so the method signature and callers (the
reschedule branch around sendDelinquencyAction) match strict typing; apply the
same checks/fix to the other similar block referenced (lines ~217–246).
Comment on lines
+249
to
+258
| this.loansServices | ||
| .createDelinquencyActions(this.loanProductService.loanAccountPath, this.loanId, payload) | ||
| .subscribe((result: any) => { | ||
| this.loansServices | ||
| .getDelinquencyActions(this.loanProductService.loanAccountPath, this.loanId) | ||
| .subscribe((loanDelinquencyActions: LoanDelinquencyAction[]) => { | ||
| this.loanDelinquencyActions = loanDelinquencyActions; | ||
| this.validateDelinquencyActions(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Refresh the delinquency data, not just the actions list.
The new reschedule action can re-evaluate balances and delinquency status, but this success handler only refetches loanDelinquencyActions. That leaves loanDelinquencyTags and installmentLevelDelinquency stale until the user manually reloads the page. Since this component now extends LoanProductBaseComponent, reload() is already available here.
💡 Proposed fix
this.loansServices
.createDelinquencyActions(this.loanProductService.loanAccountPath, this.loanId, payload)
- .subscribe((result: any) => {
- this.loansServices
- .getDelinquencyActions(this.loanProductService.loanAccountPath, this.loanId)
- .subscribe((loanDelinquencyActions: LoanDelinquencyAction[]) => {
- this.loanDelinquencyActions = loanDelinquencyActions;
- this.validateDelinquencyActions();
- });
+ .subscribe(() => {
+ if (action === 'reschedule') {
+ this.reload();
+ return;
+ }
+
+ this.loansServices
+ .getDelinquencyActions(this.loanProductService.loanAccountPath, this.loanId)
+ .subscribe((loanDelinquencyActions: LoanDelinquencyAction[]) => {
+ this.loanDelinquencyActions = loanDelinquencyActions;
+ this.validateDelinquencyActions();
+ });
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| this.loansServices | |
| .createDelinquencyActions(this.loanProductService.loanAccountPath, this.loanId, payload) | |
| .subscribe((result: any) => { | |
| this.loansServices | |
| .getDelinquencyActions(this.loanProductService.loanAccountPath, this.loanId) | |
| .subscribe((loanDelinquencyActions: LoanDelinquencyAction[]) => { | |
| this.loanDelinquencyActions = loanDelinquencyActions; | |
| this.validateDelinquencyActions(); | |
| }); | |
| }); | |
| this.loansServices | |
| .createDelinquencyActions(this.loanProductService.loanAccountPath, this.loanId, payload) | |
| .subscribe(() => { | |
| if (action === 'reschedule') { | |
| this.reload(); | |
| return; | |
| } | |
| this.loansServices | |
| .getDelinquencyActions(this.loanProductService.loanAccountPath, this.loanId) | |
| .subscribe((loanDelinquencyActions: LoanDelinquencyAction[]) => { | |
| this.loanDelinquencyActions = loanDelinquencyActions; | |
| this.validateDelinquencyActions(); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts`
around lines 249 - 258, The success handler after createDelinquencyActions
currently only re-fetches getDelinquencyActions and updates
loanDelinquencyActions, leaving loanDelinquencyTags and
installmentLevelDelinquency stale; replace the inner getDelinquencyActions call
with a call to reload() (inherited from LoanProductBaseComponent) so the
component fully refreshes state (loanDelinquencyActions, loanDelinquencyTags,
installmentLevelDelinquency) after createDelinquencyActions completes.
alberto-art3ch
force-pushed
the
WEB-813/working-capital-loan-delinquency-actions
branch
from
0def1ca to
b33757e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts (3)
190-195:⚠️ Potential issue | 🟠 MajorHandle dialog cancellation before reading
response.data.value.
afterClosed()can emit no payload. This subscription still dereferencesresponse.data.value, so cancelling the new reschedule dialog via backdrop or Cancel crashes the tab. The same failure mode still exists in the pause flow on Line 174.Angular Material MatDialogRef afterClosed emitted value when a dialog is closed without passing a result🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts` around lines 190 - 195, The subscription to loanDelinquencyActionDialogRef.afterClosed() dereferences response.data.value without guarding for a null/undefined response when the dialog is cancelled; update the afterClosed() handlers (both the reschedule flow and the pause flow that also subscribes around line 174) to first check that response and response.data (and response.data.value) exist before reading minimumPayment, frequency, or frequencyType, and bail out (return) if not present so sendDelinquencyAction is only called with valid data.
217-246:⚠️ Potential issue | 🟠 MajorSplit payload construction by action and make the nullable inputs explicit.
createDelinquencyActionReschedule()passesnulldates, butsendDelinquencyAction()still declares non-nullDate/number/stringparameters and eagerly formatsstartDatebefore thereschedulebranch overwrites the payload. That makes every reschedule call depend onDates.formatDate(null, ...)and leaves the method contract inaccurate.#!/bin/bash set -euo pipefail sed -n '182,246p' src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts echo sed -n '1,220p' src/app/core/utils/dates.ts | rg -n 'formatDate|parseDate' echo rg -n --iglob 'tsconfig*.json' '"strict"\s*:|"strictNullChecks"\s*:'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/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts` around lines 217 - 246, The sendDelinquencyAction method currently declares non-nullable parameters and eagerly formats startDate, causing null to be passed into dateUtils.formatDate for reschedule flows; update sendDelinquencyAction signature to accept nullable types for startDate, endDate, minimumPayment, frequency, and frequencyType and stop formatting dates until you know the action requires them—build separate payload objects per action (e.g., for 'pause' include formatted startDate and formatted endDate, for 'reschedule' include minimumPayment, frequency, frequencyType) and only call this.dateUtils.formatDate when the corresponding date parameter is non-null; ensure createDelinquencyActionReschedule callers that pass null align with the new nullable types and adjust any type annotations or usages in loan-delinquency-tags-tab.component.ts accordingly.
249-258:⚠️ Potential issue | 🟠 MajorRefresh the full delinquency tab after a reschedule succeeds.
This handler only re-fetches
loanDelinquencyActions, but reschedule also changes tags, balances, and delinquency status. The actions table updates whileloanDelinquencyTagsandinstallmentLevelDelinquencystay stale until the user manually reloads the page.💡 Suggested change
this.loansServices .createDelinquencyActions(this.loanProductService.loanAccountPath, this.loanId, payload) - .subscribe((result: any) => { - this.loansServices - .getDelinquencyActions(this.loanProductService.loanAccountPath, this.loanId) - .subscribe((loanDelinquencyActions: LoanDelinquencyAction[]) => { - this.loanDelinquencyActions = loanDelinquencyActions; - this.validateDelinquencyActions(); - }); + .subscribe(() => { + if (action === 'reschedule') { + this.reload(); + return; + } + + this.loansServices + .getDelinquencyActions(this.loanProductService.loanAccountPath, this.loanId) + .subscribe((loanDelinquencyActions: LoanDelinquencyAction[]) => { + this.loanDelinquencyActions = loanDelinquencyActions; + this.validateDelinquencyActions(); + }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts` around lines 249 - 258, The current createDelinquencyActions success handler only re-fetches loanDelinquencyActions, leaving loanDelinquencyTags and installmentLevelDelinquency stale after a reschedule; inside the subscribe callback in loan-delinquency-tags-tab.component.ts (the createDelinquencyActions(...) .subscribe(...) block), after reloading getDelinquencyActions and calling validateDelinquencyActions(), also invoke the existing methods that refresh the rest of the delinquency tab state (e.g., call the component/service methods that fetch loanDelinquencyTags and installmentLevelDelinquency and any balance/status refresh — e.g., getDelinquencyTags(...), getInstallmentLevelDelinquency(...) or a central refreshDelinquencyTab() if present) so tags, installment-level delinquency and balances are re-fetched and UI stays consistent after a reschedule.
🧹 Nitpick comments (1)
src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.html (1)
77-77: Use an 8px-aligned gap utility instead ofgap-5px.
gap-5pxis off-grid relative to the rest of the layout helpers. Please switch this container to an 8px-based spacing token/class so the action bar stays aligned with the repo’s spacing system.As per coding guidelines:
src/**/*.{scss,html}: Stick to the 8px grid system for visual design and spacing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.html` at line 77, Replace the off-grid class on the action bar div (the element with class "layout-row m-t-20 m-b-10 align-end align-items-center gap-5px") by swapping out "gap-5px" for the 8px-aligned spacing utility your project uses (e.g., "gap-8px" or the project's equivalent "gap-8") so the container uses the 8px grid spacing token and stays aligned with the repo spacing system.
🤖 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/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts`:
- Around line 144-150: frequencyTypeOptions is populated asynchronously by
getWorkingCapitalLoansAccountTemplate but createDelinquencyActionReschedule can
open the reschedule dialog before that array is filled; update the component so
the dialog is not opened until options are ready—either (preferably) disable the
reschedule button until frequencyTypeOptions is populated (set a boolean like
frequencyOptionsLoaded when assigning response.periodFrequencyTypeOptions and
bind the button disabled state) and add a guard in
createDelinquencyActionReschedule to return/await if frequencyTypeOptions is
empty, or alternatively make createDelinquencyActionReschedule fetch
getWorkingCapitalLoansAccountTemplate on demand and open the dialog only after
the observable resolves; apply the same fix to the other similar block
referenced (createDelinquencyActionReschedule usage around the 182-188 area).
- Around line 158-163: The working-capital branch of allowPause uses
currentLoanDelinquencyAction.endDate unconditionally; change the logic in the
block that checks loanProductService.isWorkingCapital to first verify the
currentLoanDelinquencyAction.action === 'PAUSE' and that endDate is present
before calling dateUtils.parseDate/dateUtils.isAfter; if not a PAUSE action or
endDate is missing, set allowPause = false. Update any similar logic around the
reschedule payload handling (references: currentLoanDelinquencyAction,
allowPause, loanProductService.isWorkingCapital, dateUtils.parseDate,
dateUtils.isAfter) so pause availability is only derived from PAUSE actions with
a valid endDate.
---
Duplicate comments:
In
`@src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts`:
- Around line 190-195: The subscription to
loanDelinquencyActionDialogRef.afterClosed() dereferences response.data.value
without guarding for a null/undefined response when the dialog is cancelled;
update the afterClosed() handlers (both the reschedule flow and the pause flow
that also subscribes around line 174) to first check that response and
response.data (and response.data.value) exist before reading minimumPayment,
frequency, or frequencyType, and bail out (return) if not present so
sendDelinquencyAction is only called with valid data.
- Around line 217-246: The sendDelinquencyAction method currently declares
non-nullable parameters and eagerly formats startDate, causing null to be passed
into dateUtils.formatDate for reschedule flows; update sendDelinquencyAction
signature to accept nullable types for startDate, endDate, minimumPayment,
frequency, and frequencyType and stop formatting dates until you know the action
requires them—build separate payload objects per action (e.g., for 'pause'
include formatted startDate and formatted endDate, for 'reschedule' include
minimumPayment, frequency, frequencyType) and only call
this.dateUtils.formatDate when the corresponding date parameter is non-null;
ensure createDelinquencyActionReschedule callers that pass null align with the
new nullable types and adjust any type annotations or usages in
loan-delinquency-tags-tab.component.ts accordingly.
- Around line 249-258: The current createDelinquencyActions success handler only
re-fetches loanDelinquencyActions, leaving loanDelinquencyTags and
installmentLevelDelinquency stale after a reschedule; inside the subscribe
callback in loan-delinquency-tags-tab.component.ts (the
createDelinquencyActions(...) .subscribe(...) block), after reloading
getDelinquencyActions and calling validateDelinquencyActions(), also invoke the
existing methods that refresh the rest of the delinquency tab state (e.g., call
the component/service methods that fetch loanDelinquencyTags and
installmentLevelDelinquency and any balance/status refresh — e.g.,
getDelinquencyTags(...), getInstallmentLevelDelinquency(...) or a central
refreshDelinquencyTab() if present) so tags, installment-level delinquency and
balances are re-fetched and UI stays consistent after a reschedule.
---
Nitpick comments:
In
`@src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.html`:
- Line 77: Replace the off-grid class on the action bar div (the element with
class "layout-row m-t-20 m-b-10 align-end align-items-center gap-5px") by
swapping out "gap-5px" for the 8px-aligned spacing utility your project uses
(e.g., "gap-8px" or the project's equivalent "gap-8") so the container uses the
8px grid spacing token and stays aligned with the repo spacing system.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bcd7c9f9-4193-4db1-9e62-f9f1e90430f8
📒 Files selected for processing (10)
src/app/core/utils/dates.tssrc/app/loans/common-resolvers/loan-delinquency-actions.resolver.tssrc/app/loans/common-resolvers/loan-delinquency-data.resolver.tssrc/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.htmlsrc/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.scsssrc/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.tssrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.htmlsrc/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.tssrc/app/loans/loans.service.ts
✅ Files skipped from review due to trivial changes (3)
- src/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.scss
- src/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.html
- src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html
🚧 Files skipped from review as they are similar to previous changes (3)
- src/app/core/utils/dates.ts
- src/app/loans/loans.service.ts
- src/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.ts
Comment on lines
+144
to
+150
| if (this.loanProductService.isWorkingCapital) { | ||
| const clientId: number = this.route.parent.parent.snapshot.params['clientId']; | ||
| this.loansServices | ||
| .getWorkingCapitalLoansAccountTemplate(clientId, this.loanProductId) | ||
| .subscribe((response: any) => { | ||
| this.frequencyTypeOptions = response.periodFrequencyTypeOptions; | ||
| }); |
There was a problem hiding this comment.
Don’t open the reschedule dialog until frequencyTypeOptions are ready.
frequencyTypeOptions are loaded asynchronously in ngOnInit(), but createDelinquencyActionReschedule() opens the dialog with whatever array exists at that moment. If the user clicks early, the dialog gets an empty list and never sees the later response because Line 149 replaces the array reference. Either fetch the template before opening the dialog or disable the button until the options are loaded.
Also applies to: 182-188
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts`
around lines 144 - 150, frequencyTypeOptions is populated asynchronously by
getWorkingCapitalLoansAccountTemplate but createDelinquencyActionReschedule can
open the reschedule dialog before that array is filled; update the component so
the dialog is not opened until options are ready—either (preferably) disable the
reschedule button until frequencyTypeOptions is populated (set a boolean like
frequencyOptionsLoaded when assigning response.periodFrequencyTypeOptions and
bind the button disabled state) and add a guard in
createDelinquencyActionReschedule to return/await if frequencyTypeOptions is
empty, or alternatively make createDelinquencyActionReschedule fetch
getWorkingCapitalLoansAccountTemplate on demand and open the dialog only after
the observable resolves; apply the same fix to the other similar block
referenced (createDelinquencyActionReschedule usage around the 182-188 area).
Comment on lines
+158
to
+163
| if (this.loanProductService.isLoanProduct) { | ||
| this.allowPause = this.currentLoanDelinquencyAction.action === 'RESUME'; | ||
| } else if (this.loanProductService.isWorkingCapital) { | ||
| const delinquencyActionEndDate = this.dateUtils.parseDate(this.currentLoanDelinquencyAction.endDate); | ||
| this.allowPause = this.dateUtils.isAfter(businessDate, delinquencyActionEndDate); | ||
| } |
There was a problem hiding this comment.
Guard the working-capital pause logic for non-PAUSE actions.
The new reschedule payload on Lines 239-246 sends no dates. After that action becomes the current action, Line 161 parses currentLoanDelinquencyAction.endDate unconditionally, which can leave allowPause wrong or break the pause-state calculation if endDate is absent. Only derive pause availability from PAUSE actions that actually have an end date.
Also applies to: 239-246
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts`
around lines 158 - 163, The working-capital branch of allowPause uses
currentLoanDelinquencyAction.endDate unconditionally; change the logic in the
block that checks loanProductService.isWorkingCapital to first verify the
currentLoanDelinquencyAction.action === 'PAUSE' and that endDate is present
before calling dateUtils.parseDate/dateUtils.isAfter; if not a PAUSE action or
endDate is missing, set allowPause = false. Update any similar logic around the
reschedule payload handling (references: currentLoanDelinquencyAction,
allowPause, loanProductService.isWorkingCapital, dateUtils.parseDate,
dateUtils.isAfter) so pause availability is only derived from PAUSE actions with
a valid endDate.
alberto-art3ch
force-pushed
the
WEB-813/working-capital-loan-delinquency-actions
branch
from
b33757e to
8b70279
Compare
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 (1)
src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts (1)
174-179:⚠️ Potential issue | 🟠 MajorGuard against dialog cancellation in the pause flow.
The
afterClosed()subscription accessesresponse.data.valuewithout checking if the user cancelled the dialog. When cancelled,responsemay beundefined, causing a runtime error.🛡️ Proposed fix
loanDelinquencyActionDialogRef.afterClosed().subscribe((response: { data: any }) => { + if (!response?.data?.value) { + return; + } const startDate: Date = response.data.value.startDate; const endDate: Date = response.data.value.endDate; this.sendDelinquencyAction(action, startDate, endDate, null, null, null); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts` around lines 174 - 179, The afterClosed() callback for loanDelinquencyActionDialogRef accesses response.data.value without guarding for cancellation; update the subscription in loan-delinquency-tags-tab.component (the loanDelinquencyActionDialogRef.afterClosed().subscribe handler) to first check that response and response.data (or response.data.value) are defined (e.g., if (!response || !response.data || !response.data.value) return;) or use optional chaining to extract startDate/endDate only when present, and only then call this.sendDelinquencyAction(action, startDate, endDate, ...); this prevents runtime errors when the dialog is cancelled.
🧹 Nitpick comments (2)
src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.html (1)
77-77: Use 8px grid-aligned spacing.The
gap-5pxclass doesn't follow the 8px grid system specified in the coding guidelines. Consider usinggap-8pxor a spacing variable that aligns with the grid.As per coding guidelines: "Stick to the 8px grid system for visual design and spacing."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.html` at line 77, The layout row uses a non‑standard spacing class "gap-5px" which violates the 8px grid guideline; update the div that contains classes "layout-row m-t-20 m-b-10 align-end align-items-center gap-5px" to use an 8px-aligned spacing token (e.g., replace "gap-5px" with "gap-8px" or the project spacing variable/class that represents 8px) so the spacing follows the design system.src/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.ts (1)
52-66: Consider adding numeric validation for monetary and frequency fields.The form controls
minimumPaymentandfrequencyonly haveValidators.required. For better input validation, consider addingValidators.min(0)orValidators.min(1)to ensure positive values, especially sinceminimumPaymenthandles monetary amounts.💡 Suggested enhancement
createDelinquencyActionForm() { this.delinquencyActionForm = this.formBuilder.group({ minimumPayment: [ '', - Validators.required + [Validators.required, Validators.min(0)] ], frequency: [ '', - Validators.required + [Validators.required, Validators.min(1)] ], frequencyType: [ '', Validators.required ] }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.ts` around lines 52 - 66, The form currently built in createDelinquencyActionForm sets Validators.required only for minimumPayment and frequency; update the delinquencyActionForm builder to add numeric range validators (e.g., Validators.min(0) or Validators.min(1) and optionally Validators.pattern(/^\d+(\.\d{1,2})?$/) for monetary formatting) to the minimumPayment control and Validators.min(1) (and/or a digits pattern) to the frequency control so they reject non-positive or non-numeric input; keep frequencyType as-is.
🤖 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/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts`:
- Around line 174-179: The afterClosed() callback for
loanDelinquencyActionDialogRef accesses response.data.value without guarding for
cancellation; update the subscription in loan-delinquency-tags-tab.component
(the loanDelinquencyActionDialogRef.afterClosed().subscribe handler) to first
check that response and response.data (or response.data.value) are defined
(e.g., if (!response || !response.data || !response.data.value) return;) or use
optional chaining to extract startDate/endDate only when present, and only then
call this.sendDelinquencyAction(action, startDate, endDate, ...); this prevents
runtime errors when the dialog is cancelled.
---
Nitpick comments:
In
`@src/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.ts`:
- Around line 52-66: The form currently built in createDelinquencyActionForm
sets Validators.required only for minimumPayment and frequency; update the
delinquencyActionForm builder to add numeric range validators (e.g.,
Validators.min(0) or Validators.min(1) and optionally
Validators.pattern(/^\d+(\.\d{1,2})?$/) for monetary formatting) to the
minimumPayment control and Validators.min(1) (and/or a digits pattern) to the
frequency control so they reject non-positive or non-numeric input; keep
frequencyType as-is.
In
`@src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.html`:
- Line 77: The layout row uses a non‑standard spacing class "gap-5px" which
violates the 8px grid guideline; update the div that contains classes
"layout-row m-t-20 m-b-10 align-end align-items-center gap-5px" to use an
8px-aligned spacing token (e.g., replace "gap-5px" with "gap-8px" or the project
spacing variable/class that represents 8px) so the spacing follows the design
system.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5dc664ef-87ee-4abc-8f54-2bb9c2207821
📒 Files selected for processing (11)
src/app/core/utils/dates.tssrc/app/loans/common-resolvers/loan-delinquency-actions.resolver.tssrc/app/loans/common-resolvers/loan-delinquency-data.resolver.tssrc/app/loans/common-resolvers/loan-delinquency-tags.resolver.tssrc/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.htmlsrc/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.scsssrc/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.tssrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.htmlsrc/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.tssrc/app/loans/loans.service.ts
✅ Files skipped from review due to trivial changes (4)
- src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html
- src/app/loans/common-resolvers/loan-delinquency-data.resolver.ts
- src/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.scss
- src/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.html
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app/core/utils/dates.ts
- src/app/loans/loans.service.ts
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
1 participant
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.
Description
For Working Capital loans we would like to introduce a new delinquency action which allows us to change on the minimum payment amount.
Based on the provided date it target the appropriate minimum payment - or breach ranges and update the minimum payment amount. All consecutive periods to be updated with the new value.
Once it was updated, we need to reevaluate these ranges and update (if applicable) the balances and delinquency status.
Related issues and discussion
WEB-813
Screenshots
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
New Features
Bug Fixes
UI Improvements