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 Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 resolver paths now may 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, .../loan-delinquency-action-reschedule-dialog.component.html, .../loan-delinquency-action-reschedule-dialog.component.scss |
New standalone Angular Material dialog with reactive form controls (minimumPayment, frequency, frequencyType); SCSS header/license added. |
Delinquency Tags Tab (UI + Logic) src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts, .../loan-delinquency-tags-tab.component.html |
Conditional rendering by product type; updated table cell bindings (classification, addedOn, liftedOn); added "Reschedule" action for working-capital, tightened action visibility, integrated dialog & reschedule flow, extended sendDelinquencyAction payload and call-site updates (now includes productType). |
Loans Service src/app/loans/loans.service.ts |
Changed getDelinquencyTags signature/path to accept only loanId and updated GET path; changed createDelinquencyActions signature to (productType, loanId, delinquencyActions) and updated POST path. |
Loan Terms Step (label) src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html |
Updated validation error translation key from labels.inputs.Repaid every to labels.inputs.Period Payment Frequency. |
Dialog Layout Adjustment src/app/loans/custom-dialog/loan-delinquency-action-dialog/loan-delinquency-action-dialog.component.html |
Moved layout class into inner container; form bindings and controls unchanged (structural/template refinement). |
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 (minimumPayment, frequency, frequencyType)
User->>Dialog: Fill form and submit
Dialog-->>TagsTab: Return {data: form}
TagsTab->>Service: createDelinquencyActions(productType, loanId, payload{reschedule fields})
Service->>Backend: POST /{productType}/{loanId}/delinquency-actions
Backend-->>Service: 200 OK
Service-->>TagsTab: Observable resolves
TagsTab->>Service: getDelinquencyActions(productType, loanId) (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-813: Working Capital loan account modification #3435: Overlapping resolver and LoansService changes for delinquency endpoints and product-type routing.
- WEB-670: Working Capital loan delinquency grace days #3440: Related modifications to loan-delinquency-data resolver control flow and numeric/product gating.
- WEB-711: Working Capital product #3159: Related product-aware delinquency API and loan-product path changes.
Suggested reviewers
- adamsaghy
🚥 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 title 'WEB-813: Working Capital loan delinquency actions' accurately reflects the main changes: introducing a new delinquency action (reschedule) specifically for Working Capital loans that allows changing minimum payment amounts. |
| 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).
src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts
Show resolved
Hide resolved
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
alberto-art3ch
force-pushed
the
WEB-813/working-capital-loan-delinquency-actions
branch
from
8b70279 to
577cda7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
src/app/loans/common-resolvers/loan-delinquency-tags.resolver.ts (1)
35-43:⚠️ Potential issue | 🟠 MajorResolver still falls through without an observable.
The non-loan-product and invalid-
loanIdpaths here still returnundefined, so this resolver does not consistently fulfill the Angular resolver contract. Please return an explicit observable for the no-data branch as well.🤖 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 - 43, The resolver resolve(route: ActivatedRouteSnapshot) can return undefined on non-loan-product or invalid loanId paths; update resolve to always return an Observable by handling those branches explicitly (after calling initialize(route) and computing loanId): if loanId is not a number or this.isLoanProduct is false, return an explicit Observable (e.g. of(null) or EMPTY) instead of falling through, while keeping the existing successful path that calls this.loansService.getDelinquencyTags(loanId).src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts (4)
158-163:⚠️ Potential issue | 🟠 MajorOnly derive working-capital pause availability from PAUSE actions with an end date.
The new
rescheduleaction does not supplyendDate, but this branch still parsescurrentLoanDelinquencyAction.endDateunconditionally. Once a reschedule becomes current,allowPausecan be computed incorrectly or fail on a missing date.🤖 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 branch for working-capital should only parse endDate for PAUSE actions: in the loan-delinquency-tags-tab component, update the logic that uses loanProductService.isWorkingCapital and currentLoanDelinquencyAction so you first check currentLoanDelinquencyAction.action === 'PAUSE' and that currentLoanDelinquencyAction.endDate is present before calling dateUtils.parseDate and dateUtils.isAfter; otherwise set allowPause = false (or the appropriate default). This prevents parsing a missing endDate for reschedule actions and ensures allowPause is only derived from PAUSE actions with an end date.
144-150:⚠️ Potential issue | 🟠 MajorDon't open the reschedule dialog until the frequency options are loaded.
frequencyTypeOptionsis filled asynchronously, butcreateDelinquencyActionReschedule()can still open immediately with an empty array. Guard the method and/or disable the button until that template request has resolved.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, The reschedule dialog can open before frequencyTypeOptions is loaded; modify the logic so createDelinquencyActionReschedule (and any button click handler) checks that frequencyTypeOptions is populated or that the template request from loansServices.getWorkingCapitalLoansAccountTemplate has completed, and disable the UI trigger until then. Concretely, add a boolean like isTemplateLoaded (set true in the subscribe callback where frequencyTypeOptions is assigned) and use it to guard createDelinquencyActionReschedule() and the button's disabled state, or alternatively have createDelinquencyActionReschedule early-return when frequencyTypeOptions is empty and show a loading/error state; ensure the same guard is applied for the other occurrence mentioned (lines 182-188).
174-179:⚠️ Potential issue | 🟠 MajorGuard both dialog callbacks before dereferencing
response.data.value.
afterClosed()can emitundefinedon cancel/backdrop close, and the reschedule path still passesnullinto parameters declared as non-nullable. Please handle the cancel path first and make the method signature match the nullable callers.Also applies to: 190-195, 217-223
🤖 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, Guard the dialog callbacks emitted by loanDelinquencyActionDialogRef.afterClosed().subscribe by first checking that the emitted response is defined before accessing response.data.value, and do the same for the other two afterClosed() subscriptions that follow the same pattern; if the response is undefined, simply return early. Also update the sendDelinquencyAction method signature to accept nullable parameters for the values that callers may pass as null (e.g., startDate?: Date | null, endDate?: Date | null, and any other nullable params), and ensure sendDelinquencyAction handles null/undefined inputs safely (avoid dereferencing without checks) so the reschedule/cancel paths no longer pass null into non-nullable parameters.
249-258:⚠️ Potential issue | 🟠 MajorReload the full delinquency state after submitting the action.
The backend re-evaluates balances and delinquency status for this flow, but this success handler only refreshes
loanDelinquencyActions.loanDelinquencyTagsandinstallmentLevelDelinquencywill stay stale until the page is reloaded.🤖 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, After createDelinquencyActions succeeds you only refresh loanDelinquencyActions, leaving loanDelinquencyTags and installmentLevelDelinquency stale; update the success handler in the createDelinquencyActions subscribe so it reloads the full delinquency state (either call the existing methods that fetch loanDelinquencyTags and installmentLevelDelinquency or add and call a single reloadDelinquencyState method) and then run validateDelinquencyActions; reference createDelinquencyActions, getDelinquencyActions, loanDelinquencyActions, loanDelinquencyTags, installmentLevelDelinquency, and validateDelinquencyActions when making the changes.
🧹 Nitpick comments (2)
src/app/loans/loans.service.ts (1)
113-114: Type the delinquency API surface instead of reintroducingany.These two changed methods are now the contract for the new delinquency/reschedule flow, but they still expose
Observable<any>/any.getDelinquencyTags()already feeds strongly-typed UI state, andcreateDelinquencyActions()now accepts a broader payload that would benefit from an explicit request type as well.Based on learnings: "When you encounter API responses, introduce specific interfaces/types for the response shapes and use proper typing instead of any, updating services and resolvers accordingly."
Also applies to: 136-137
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/loans/loans.service.ts` around lines 113 - 114, Replace the use of any by introducing explicit TypeScript interfaces for the delinquency API shapes (e.g., DelinquencyTag, DelinquencyActionRequest/Response) and update the service methods getDelinquencyTags and createDelinquencyActions to return Observable<DelinquencyTag[]> or the appropriate response type and to accept a properly-typed request parameter instead of any; change the this.http.get/post calls to use HttpClient generics (this.http.get<DelinquencyTag[]>(...), this.http.post<DelinquencyActionResponse>(...)) and then update any resolvers/consumers that call getDelinquencyTags/createDelinquencyActions to use the new types.src/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.ts (1)
39-53: Type the dialog data and form model.This new dialog is a payload boundary, but
inject(MAT_DIALOG_DATA)andUntypedFormGrouperase the shapes offrequencyTypeOptionsand the returned form values. A small dialog-data interface plus a typedFormGroupwould make the reschedule flow much safer.As per coding guidelines: "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/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.ts` around lines 39 - 53, Define a typed interface for the dialog payload (e.g., LoanDelinquencyActionRescheduleDialogData) and replace the untyped inject(MAT_DIALOG_DATA) usage with inject<LoanDelinquencyActionRescheduleDialogData>(MAT_DIALOG_DATA) so frequencyTypeOptions has a concrete type; change UntypedFormBuilder and UntypedFormGroup usage to FormBuilder and a typed FormGroup model (e.g., FormGroup<DelinquencyActionFormModel>) by declaring the shape of the form values and updating delinquencyActionForm and createDelinquencyActionForm to use those types; keep dialogRef typed as MatDialogRef<LoanDelinquencyActionRescheduleDialogComponent> and update any places that read this.data or this.delinquencyActionForm.value to use the new typed interfaces.
🤖 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.html`:
- Line 77: Replace the non-conforming utility class "gap-5px" in the
loan-delinquency-tags-tab component template (the div with classes "layout-row
m-t-20 m-b-10 align-end align-items-center gap-5px") with the repo's 8px grid
spacing utility/token (e.g., the standard "gap-8" or whichever 8px-based spacing
utility your project uses) so the element uses the 8px spacing system instead of
a custom 5px value.
---
Duplicate comments:
In `@src/app/loans/common-resolvers/loan-delinquency-tags.resolver.ts`:
- Around line 35-43: The resolver resolve(route: ActivatedRouteSnapshot) can
return undefined on non-loan-product or invalid loanId paths; update resolve to
always return an Observable by handling those branches explicitly (after calling
initialize(route) and computing loanId): if loanId is not a number or
this.isLoanProduct is false, return an explicit Observable (e.g. of(null) or
EMPTY) instead of falling through, while keeping the existing successful path
that calls this.loansService.getDelinquencyTags(loanId).
In
`@src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts`:
- Around line 158-163: The branch for working-capital should only parse endDate
for PAUSE actions: in the loan-delinquency-tags-tab component, update the logic
that uses loanProductService.isWorkingCapital and currentLoanDelinquencyAction
so you first check currentLoanDelinquencyAction.action === 'PAUSE' and that
currentLoanDelinquencyAction.endDate is present before calling
dateUtils.parseDate and dateUtils.isAfter; otherwise set allowPause = false (or
the appropriate default). This prevents parsing a missing endDate for reschedule
actions and ensures allowPause is only derived from PAUSE actions with an end
date.
- Around line 144-150: The reschedule dialog can open before
frequencyTypeOptions is loaded; modify the logic so
createDelinquencyActionReschedule (and any button click handler) checks that
frequencyTypeOptions is populated or that the template request from
loansServices.getWorkingCapitalLoansAccountTemplate has completed, and disable
the UI trigger until then. Concretely, add a boolean like isTemplateLoaded (set
true in the subscribe callback where frequencyTypeOptions is assigned) and use
it to guard createDelinquencyActionReschedule() and the button's disabled state,
or alternatively have createDelinquencyActionReschedule early-return when
frequencyTypeOptions is empty and show a loading/error state; ensure the same
guard is applied for the other occurrence mentioned (lines 182-188).
- Around line 174-179: Guard the dialog callbacks emitted by
loanDelinquencyActionDialogRef.afterClosed().subscribe by first checking that
the emitted response is defined before accessing response.data.value, and do the
same for the other two afterClosed() subscriptions that follow the same pattern;
if the response is undefined, simply return early. Also update the
sendDelinquencyAction method signature to accept nullable parameters for the
values that callers may pass as null (e.g., startDate?: Date | null, endDate?:
Date | null, and any other nullable params), and ensure sendDelinquencyAction
handles null/undefined inputs safely (avoid dereferencing without checks) so the
reschedule/cancel paths no longer pass null into non-nullable parameters.
- Around line 249-258: After createDelinquencyActions succeeds you only refresh
loanDelinquencyActions, leaving loanDelinquencyTags and
installmentLevelDelinquency stale; update the success handler in the
createDelinquencyActions subscribe so it reloads the full delinquency state
(either call the existing methods that fetch loanDelinquencyTags and
installmentLevelDelinquency or add and call a single reloadDelinquencyState
method) and then run validateDelinquencyActions; reference
createDelinquencyActions, getDelinquencyActions, loanDelinquencyActions,
loanDelinquencyTags, installmentLevelDelinquency, and validateDelinquencyActions
when making the changes.
---
Nitpick comments:
In
`@src/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.ts`:
- Around line 39-53: Define a typed interface for the dialog payload (e.g.,
LoanDelinquencyActionRescheduleDialogData) and replace the untyped
inject(MAT_DIALOG_DATA) usage with
inject<LoanDelinquencyActionRescheduleDialogData>(MAT_DIALOG_DATA) so
frequencyTypeOptions has a concrete type; change UntypedFormBuilder and
UntypedFormGroup usage to FormBuilder and a typed FormGroup model (e.g.,
FormGroup<DelinquencyActionFormModel>) by declaring the shape of the form values
and updating delinquencyActionForm and createDelinquencyActionForm to use those
types; keep dialogRef typed as
MatDialogRef<LoanDelinquencyActionRescheduleDialogComponent> and update any
places that read this.data or this.delinquencyActionForm.value to use the new
typed interfaces.
In `@src/app/loans/loans.service.ts`:
- Around line 113-114: Replace the use of any by introducing explicit TypeScript
interfaces for the delinquency API shapes (e.g., DelinquencyTag,
DelinquencyActionRequest/Response) and update the service methods
getDelinquencyTags and createDelinquencyActions to return
Observable<DelinquencyTag[]> or the appropriate response type and to accept a
properly-typed request parameter instead of any; change the this.http.get/post
calls to use HttpClient generics (this.http.get<DelinquencyTag[]>(...),
this.http.post<DelinquencyActionResponse>(...)) and then update any
resolvers/consumers that call getDelinquencyTags/createDelinquencyActions to use
the new types.
🪄 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: f55d537d-ba85-4975-95d3-2cb38fc2aa2c
📒 Files selected for processing (12)
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-dialog/loan-delinquency-action-dialog.component.htmlsrc/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/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.scss
- src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html
- src/app/loans/custom-dialog/loan-delinquency-action-dialog/loan-delinquency-action-dialog.component.html
- src/app/core/utils/dates.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.html
|
|
||
| @if (allowPause) { | ||
| <div class="layout-row m-t-20 m-b-10 align-end align-items-center"> | ||
| <div class="layout-row m-t-20 m-b-10 align-end align-items-center gap-5px"> |
There was a problem hiding this comment.
Avoid the 5px spacing utility here.
gap-5px breaks the repo’s 8px spacing system. Please swap it for an 8px-based token/utility instead.
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, Replace the non-conforming utility class "gap-5px" in the
loan-delinquency-tags-tab component template (the div with classes "layout-row
m-t-20 m-b-10 align-end align-items-center gap-5px") with the repo's 8px grid
spacing utility/token (e.g., the standard "gap-8" or whichever 8px-based spacing
utility your project uses) so the element uses the 8px spacing system instead of
a custom 5px value.
577cda7 to
fd079b0
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts (4)
190-195:⚠️ Potential issue | 🟠 MajorHandle dialog cancellation before reading form values.
The dialog has a Cancel path that closes without a payload, so this subscriber can throw on
response.data.value...when the user cancels or dismisses the dialog.Proposed fix
- 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; + loanDelinquencyActionDialogRef.afterClosed().subscribe((response?: { data?: { value?: any } }) => { + const formValue = response?.data?.value; + if (!formValue) { + return; + } + + const minimumPayment: number = formValue.minimumPayment; + const frequency: number = formValue.frequency; + const frequencyType: string = formValue.frequencyType; this.sendDelinquencyAction(action, null, null, minimumPayment, frequency, frequencyType); });🤖 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 subscriber on loanDelinquencyActionDialogRef.afterClosed() reads response.data.value without guarding against a cancel/close with no payload; update the callback in the afterClosed().subscribe handler to first check that response is truthy and that response.data and response.data.value exist (or use optional chaining) and return early if they don't, so only call sendDelinquencyAction(action, ...) when minimumPayment, frequency and frequencyType are present; reference the afterClosed subscription, the response variable, and sendDelinquencyAction when making the guard.
158-163:⚠️ Potential issue | 🟠 MajorOnly derive
allowPausefrom PAUSE actions that have an end date.After a reschedule becomes the latest delinquency action,
endDatecan be absent. Parsing it unconditionally here makes working-capital pause availability depend on an invalid date instead of the active action type.Proposed fix
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); + if (this.currentLoanDelinquencyAction.action === 'PAUSE' && this.currentLoanDelinquencyAction.endDate) { + const delinquencyActionEndDate = this.dateUtils.parseDate(this.currentLoanDelinquencyAction.endDate); + this.allowPause = this.dateUtils.isAfter(businessDate, delinquencyActionEndDate); + } else { + this.allowPause = false; + } }🤖 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 allowPause logic incorrectly parses endDate for working-capital loans even when the active action isn't a PAUSE or endDate is missing; update the branch in loan-delinquency-tags-tab.component.ts to first check that this.loanProductService.isWorkingCapital && this.currentLoanDelinquencyAction?.action === 'PAUSE' && this.currentLoanDelinquencyAction?.endDate is present before calling dateUtils.parseDate and dateUtils.isAfter; if any of those checks fail, set allowPause = false to avoid parsing null/undefined endDate.
144-150:⚠️ Potential issue | 🟠 MajorDon’t open Reschedule until the frequency options are loaded.
frequencyTypeOptionsis filled asynchronously here, butcreateDelinquencyActionReschedule()opens the dialog with whatever array exists at that moment. A fast click gets an empty select, and the later assignment on Line 149 replaces the array reference instead of updating the already-open dialog.Proposed fix
frequencyTypeOptions: StringEnumOptionData[] = []; + frequencyTypeOptionsLoaded = false; @@ .getWorkingCapitalLoansAccountTemplate(clientId, this.loanProductId) .subscribe((response: any) => { - this.frequencyTypeOptions = response.periodFrequencyTypeOptions; + this.frequencyTypeOptions = response?.periodFrequencyTypeOptions ?? []; + this.frequencyTypeOptionsLoaded = this.frequencyTypeOptions.length > 0; }); @@ createDelinquencyActionReschedule(): void { + if (!this.frequencyTypeOptionsLoaded) { + return; + } + const action = 'reschedule';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, The frequencyTypeOptions array is populated asynchronously by loansServices.getWorkingCapitalLoansAccountTemplate but createDelinquencyActionReschedule() may be called before the subscribe completes, causing an empty select or broken reference; to fix, defer opening the reschedule dialog until the options are ready: in the isWorkingCapital branch, move the dialog-open call (createDelinquencyActionReschedule) into the subscribe callback after assigning frequencyTypeOptions, or instead mutate the existing array reference (e.g., clear and push new items into this.frequencyTypeOptions) so any already-open dialog sees updates; apply the same change to the second occurrence around createDelinquencyActionReschedule at the 182-188 region.
249-258:⚠️ Potential issue | 🟠 MajorRefresh the full tab state after reschedule.
Reschedule can recompute balances, tags, and delinquency status, but this success path only reloads
loanDelinquencyActions. The rest of the tab stays stale until a manual refresh.Proposed fix
this.loansServices .createDelinquencyActions(this.loanProductService.loanAccountPath, this.loanId, payload) - .subscribe((result: any) => { + .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, After creating delinquency actions, the code only reloads getDelinquencyActions, leaving other tab state (balances, tags, delinquency status) stale; update the success path in the createDelinquencyActions subscribe to refresh the full tab state: invoke the existing methods that reload loan details/totals, tags, and delinquency actions (e.g., call whatever methods like loadLoanDetails/loadBalances/loadTags or a single refreshTabState helper), then set this.loanDelinquencyActions from getDelinquencyActions and call this.validateDelinquencyActions(); ensure all relevant reload calls run after createDelinquencyActions completes so the entire tab reflects recomputed state.
🤖 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/core/utils/dates.ts`:
- Around line 55-61: The isBefore and isAfter helpers currently compare full
timestamps; change both (isBefore and isAfter in src/app/core/utils/dates.ts) to
compare only calendar days by normalizing inputs to their date parts (year,
month, day) before comparing — e.g., derive a date-only representation from Date
arguments (using getFullYear/getMonth/getDate or constructing new Date(year,
month, day)) and then perform the < or > comparison on those normalized values
so times on the same business day are treated equal.
In
`@src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts`:
- Around line 126-134: The code dereferences data.loanDelinquencyData without
guarding it; make the whole delinquency payload null-safe by first checking
whether data.loanDelinquencyData exists before accessing its properties: only
set this.currency, this.installmentLevelDelinquency, and this.loanProductId when
data.loanDelinquencyData is truthy (use the already-declared loanDelinquencyData
variable as the guard), e.g., if (loanDelinquencyData) { this.currency =
loanDelinquencyData.currency; this.installmentLevelDelinquency =
loanDelinquencyData.installmentLevelDelinquency || []; if
(loanDelinquencyData.product) this.loanProductId =
loanDelinquencyData.product.id; } so no properties of data.loanDelinquencyData
are accessed when it is null/undefined.
---
Duplicate comments:
In
`@src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts`:
- Around line 190-195: The subscriber on
loanDelinquencyActionDialogRef.afterClosed() reads response.data.value without
guarding against a cancel/close with no payload; update the callback in the
afterClosed().subscribe handler to first check that response is truthy and that
response.data and response.data.value exist (or use optional chaining) and
return early if they don't, so only call sendDelinquencyAction(action, ...) when
minimumPayment, frequency and frequencyType are present; reference the
afterClosed subscription, the response variable, and sendDelinquencyAction when
making the guard.
- Around line 158-163: The allowPause logic incorrectly parses endDate for
working-capital loans even when the active action isn't a PAUSE or endDate is
missing; update the branch in loan-delinquency-tags-tab.component.ts to first
check that this.loanProductService.isWorkingCapital &&
this.currentLoanDelinquencyAction?.action === 'PAUSE' &&
this.currentLoanDelinquencyAction?.endDate is present before calling
dateUtils.parseDate and dateUtils.isAfter; if any of those checks fail, set
allowPause = false to avoid parsing null/undefined endDate.
- Around line 144-150: The frequencyTypeOptions array is populated
asynchronously by loansServices.getWorkingCapitalLoansAccountTemplate but
createDelinquencyActionReschedule() may be called before the subscribe
completes, causing an empty select or broken reference; to fix, defer opening
the reschedule dialog until the options are ready: in the isWorkingCapital
branch, move the dialog-open call (createDelinquencyActionReschedule) into the
subscribe callback after assigning frequencyTypeOptions, or instead mutate the
existing array reference (e.g., clear and push new items into
this.frequencyTypeOptions) so any already-open dialog sees updates; apply the
same change to the second occurrence around createDelinquencyActionReschedule at
the 182-188 region.
- Around line 249-258: After creating delinquency actions, the code only reloads
getDelinquencyActions, leaving other tab state (balances, tags, delinquency
status) stale; update the success path in the createDelinquencyActions subscribe
to refresh the full tab state: invoke the existing methods that reload loan
details/totals, tags, and delinquency actions (e.g., call whatever methods like
loadLoanDetails/loadBalances/loadTags or a single refreshTabState helper), then
set this.loanDelinquencyActions from getDelinquencyActions and call
this.validateDelinquencyActions(); ensure all relevant reload calls run after
createDelinquencyActions completes so the entire tab reflects recomputed state.
🪄 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: e08adc2e-5118-457b-aea5-7c00e5003660
📒 Files selected for processing (12)
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-dialog/loan-delinquency-action-dialog.component.htmlsrc/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 (2)
- src/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.scss
- 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 (7)
- src/app/loans/common-resolvers/loan-delinquency-data.resolver.ts
- src/app/loans/common-resolvers/loan-delinquency-tags.resolver.ts
- src/app/loans/common-resolvers/loan-delinquency-actions.resolver.ts
- src/app/loans/custom-dialog/loan-delinquency-action-dialog/loan-delinquency-action-dialog.component.html
- src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.html
- src/app/loans/custom-dialog/loan-delinquency-action-reschedule-dialog/loan-delinquency-action-reschedule-dialog.component.ts
- src/app/loans/loans.service.ts
src/app/loans/loans-view/loan-delinquency-tags-tab/loan-delinquency-tags-tab.component.ts
Outdated
Show resolved
Hide resolved
alberto-art3ch
force-pushed
the
WEB-813/working-capital-loan-delinquency-actions
branch
2 times, most recently
from
ffcd862 to
50dd821
Compare
alberto-art3ch
force-pushed
the
WEB-813/working-capital-loan-delinquency-actions
branch
from
50dd821 to
b5d209a
Compare
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