WEB-870: Delinquency management configuration for WC#3400
Open
alberto-art3ch wants to merge 1 commit intoopenMF:devfrom
Open
WEB-870: Delinquency management configuration for WC#3400alberto-art3ch wants to merge 1 commit intoopenMF:devfrom
alberto-art3ch wants to merge 1 commit intoopenMF:devfrom
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Base Component src/app/products/manage-delinquency-buckets/delinquency-base.component.ts |
New abstract base that reads bucketType from route query params, exposes delinquencyBucketType BehaviorSubject and helpers (isRegularBucket, isWorkingCapitalBucket, bucketTypeLabel, bucketType, getCatalogLabel, camalize). |
Models & Option Types src/app/products/manage-delinquency-buckets/models/delinquency-models.ts, src/app/shared/models/option-data.model.ts |
Added DELINQUENCY_BUCKET_TYPE const and DelinquencyBucketType type; added EnumOptionData interface. |
List & Navigation src/app/products/manage-delinquency-buckets/delinquency-bucket/delinquency-bucket.component.ts, .../delinquency-bucket/delinquency-bucket.component.html |
Component now extends base, adds bucketType column, paginator ViewChild and a permission-wrapped split Create menu that navigates with ?bucketType=. |
Create src/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.ts, .../create-bucket.component.html |
Now extends base; form and submit logic branch for regular vs working-capital; working-capital adds frequency/minimum-payment fields and options; added payloadData getter. |
Edit src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts, .../edit-bucket.component.html |
Now extends base; form branches and payload/ref validation for working-capital; adds frequencyTypeOptions, minimumPaymentOptions, validForm() and payloadData. |
View src/app/products/manage-delinquency-buckets/delinquency-bucket/view-bucket/view-bucket.component.ts, .../view-bucket.component.html |
Now extends base; shows either ranges (regular) or minimumPaymentPeriodAndRule (working-capital); edit link includes bucketType query param; conditional ranges sorting. |
Resolver & Routing src/app/products/manage-delinquency-buckets/delinquency-range/delinquency-range.component.resolver.ts, src/app/products/products-routing.module.ts |
Resolver branches on bucketType (working-capital returns template via getDelinquencyBucketsTemplate); route resolve keys renamed to delinquencyBucketsTemplateData for create/edit. |
Service src/app/products/products.service.ts |
Added getDelinquencyBucketsTemplate() to GET /delinquency/buckets/template. |
Translations src/assets/translations/* |
Added translation keys across locales: "Working Capital", "Delinquency Payment Rule", "Minimum Payment", "Minimum Payment Type". |
Minor UI translation tweak src/app/account-transfers/create-standing-instructions/create-standing-instructions.component.html |
Switched error translation key from labels.inputs.required to labels.commons.required. |
Sequence Diagram
sequenceDiagram
participant User
participant UI as Components
participant Router
participant Resolver
participant Service
User->>UI: Click Create (choose type)
UI->>Router: Navigate to /create?bucketType=regular|workingcapital
Router->>Resolver: resolve(delinquencyBucketsTemplateData)
alt bucketType == regular
Resolver->>Service: getDelinquencyRanges()
else bucketType == workingcapital
Resolver->>Service: getDelinquencyBucketsTemplate()
end
Service-->>Resolver: template/ranges
Resolver-->>Router: provide route data
Router->>UI: init component (bucketType)
UI->>UI: render form (Regular ranges OR Working Capital payment rule)
User->>UI: Submit
UI->>Service: create/updateDelinquencyBucket(payloadData)
Service-->>UI: response
UI->>Router: navigate to list (with bucketType)
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
- WEB-736 fix(i18n): translate remaining 'Delinquency' terms in Nepali … #3171: Related changes to Nepali translations (
ne-NE.json) for delinquency-related keys.
Suggested reviewers
- IOhacker
- gkbishnoi07
- 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 clearly and specifically describes the main change: introducing delinquency management configuration for the Working Capital (WC) product with two bucket types (Regular and Working Capital). It directly relates to the changeset's primary purpose. |
| 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
📝 Coding Plan
- Generate coding plan for human review comments
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: 17
🧹 Nitpick comments (4)
src/app/products/products.service.ts (1)
345-350: Consider typing the delinquency template response, but this refactoring may be outside the scope of this PR.Creating a dedicated
DelinquencyBucketsTemplateinterface for proper typing is technically sound—the template response contains properties likefrequencyTypeOptionsandminimumPaymentOptions(bothEnumOptionData[]) that the consuming components expect. However, this aligns with a broader project pattern:Observable<any>is used consistently across services and resolvers in the codebase. Typing the full API response layer is tracked as a significant cross-cutting refactoring effort rather than addressed in individual feature PRs. If this refactoring is in scope for your enhancement, proceed; otherwise, consider deferring it to a dedicated typing initiative.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/products.service.ts` around lines 345 - 350, The service method getDelinquencyBucketsTemplate currently returns Observable<any>; create a DelinquencyBucketsTemplate interface (including frequencyTypeOptions: EnumOptionData[] and minimumPaymentOptions: EnumOptionData[] plus any other known properties) and update getDelinquencyBucketsTemplate(): Observable<DelinquencyBucketsTemplate> to use that type; import or reference the existing EnumOptionData type where needed and ensure consuming components/resolvers still compile—if this typing change is out of scope, leave as-is but track the refactor for a dedicated typing effort.src/app/products/manage-delinquency-buckets/delinquency-bucket/delinquency-bucket.component.html (2)
16-22: Use full translation keys for these menu labels.Concatenating
Regular/Working CapitalwithDelinquency Buckethard-codes English word order into the template and won't localize cleanly in several languages. Please add dedicated keys for the full labels, then extract them.As per coding guidelines, "Use proper i18n variables from
@ngx-translate/corefor all user-facing strings instead of hardcoded text" and "Runnpm run translations:extractto extract i18n variables whenever new strings are added to code."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/manage-delinquency-buckets/delinquency-bucket/delinquency-bucket.component.html` around lines 16 - 22, Replace the concatenated translations in the two menu buttons with single full-label translation keys: stop using separate keys '{{ 'labels.catalogs.Regular' | translate }} {{ 'labels.inputs.Delinquency Bucket' | translate }}' and '{{ 'labels.catalogs.Working Capital' | translate }} {{ 'labels.inputs.Delinquency Bucket' | translate }}' and instead use new keys like 'labels.catalogs.DelinquencyBucket.Regular' and 'labels.catalogs.DelinquencyBucket.WorkingCapital' (or similar) directly in the template; add those new keys and values to your translation JSON files and then run the translations extractor (npm run translations:extract) so the i18n tool picks them up, and update the template buttons in delinquency-bucket.component.html to use the single-key translations via the translate pipe.
13-22: The new icon spacing is off-grid.
m-l-10andm-r-10introduce 10px gaps, which doesn't match the 8px spacing system used elsewhere. Please switch these new margins to an 8px-based utility or a token-backed style.As per coding guidelines, "Stick to the 8px grid system for visual design and spacing" and "Leverage SCSS variables defined in
src/main.scssandsrc/theme/mifosx-theme.scssrather than generating custom classes and explicit pixel values."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/manage-delinquency-buckets/delinquency-bucket/delinquency-bucket.component.html` around lines 13 - 22, Replace the non-8px spacing classes on the fa-icon instances (m-l-10 and m-r-10) inside the mat-menu buttons with the project’s 8px-grid spacing utilities or theme-backed tokens; locate the fa-icon elements in the mat-menu items (the two buttons using [routerLink]="['create']") and swap m-l-10/m-r-10 for the correct 8px utility class (e.g., m-l-8 / m-r-8) or apply the theme spacing token/mixin used elsewhere so spacing comes from the SCSS variables rather than hardcoded 10px values.src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts (1)
77-77: Please type the resolver payloads you just added.The new working-capital path dereferences
minimumPaymentPeriodAndRule,frequencyTypeOptions, andminimumPaymentOptionsthroughany, so a payload shape change will only fail at runtime. A small local interface fordelinquencyBucketanddelinquencyBucketsTemplateDatahere would give this branch compile-time checks without turning this PR into a full API typing sweep.Based on learnings: In TypeScript files (e.g., src/app/.../*.ts), avoid using Observable as a project-wide pattern for API responses. Treat typing the full API response layer as a cross-cutting refactor and track it as a separate enhancement backlog item rather than addressing it in individual bug fixes. 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: 97-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts` at line 77, The property delinquencyBucketData is currently typed as any and the component dereferences minimumPaymentPeriodAndRule, frequencyTypeOptions and minimumPaymentOptions through any; define small local interfaces (e.g., DelinquencyBucket and DelinquencyBucketsTemplateData) matching the resolver payload shape and replace any with those types for delinquencyBucketData and any resolver-returning members used in this component (also update usages in the working-capital branch between the earlier 97–115 block). Update the resolver method/type signatures and the component property types to use DelinquencyBucket/DelinquencyBucketsTemplateData so TypeScript enforces the presence and shapes of minimumPaymentPeriodAndRule, frequencyTypeOptions, and minimumPaymentOptions at compile time.
🤖 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/products/manage-delinquency-buckets/delinquency-base.component.ts`:
- Around line 39-45: bucketTypeLabel currently returns hardcoded English
strings; update it to use ngx-translate so labels are extracted and localized:
inject TranslateService into the component and replace the literal returns in
bucketTypeLabel(bucketType: number) with calls to TranslateService (e.g.
this.translate.instant('products.bucketType.regular') and
this.translate.instant('products.bucketType.workingCapital')), adding those
translation keys to the locale files; keep the same numeric checks (bucketType
=== 1 / === 2) and default behavior unchanged.
- Around line 39-57: The helpers bucketTypeLabel and bucketType currently check
for numeric values (1/2) but the model uses string bucket types ('regular' |
'workingcapital') and row.bucketType passes those strings; update these
functions to accept and handle the string values by returning 'Regular' for
'regular' and 'Working Capital' for 'workingcapital' (and returning
'regular'/'workingcapital' respectively in bucketType), and keep optional
backward-compatibility by also handling numeric 1/2 if needed so working-capital
rows no longer resolve to ''. Use the existing function names bucketTypeLabel
and bucketType to locate and modify the logic accordingly.
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.html`:
- Around line 118-120: The template is checking the wrong validation key: update
the mat-error condition for bucketForm.controls.minimumPayment from
hasError('pattern') to hasError('min') so the Validators.min(0.01) failure shows
the intended message; locate the minimumPayment control usage in the
create-bucket.component.html and replace the error key accordingly (keep the
same error text).
- Around line 88-89: Replace hardcoded English validation messages inside the
mat-error elements in create-bucket.component.html with translation keys and
switch any uses of labels.inputs.required to labels.commons.required;
specifically update the mat-error that currently renders " {{
'labels.inputs.Frequency' | translate }} must be a positive number." to use a
proper i18n key (e.g. errors.validation.mustBePositive combined with
'labels.inputs.Frequency' via translate) and change the two occurrences that use
labels.inputs.required to labels.commons.required so all user-facing strings use
the project's translation keys.
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.ts`:
- Around line 203-224: The getter payloadData builds the working-capital payload
incorrectly and can return undefined for unsupported types; change payloadData
(used with isRegularBucket and isWorkingCapitalBucket) to: for regular buckets
keep building ranges from rangesDataSource.rangeId; for working-capital buckets
construct minimumPaymentPeriodAndRule explicitly by pulling only the fields
frequency, frequencyType, minimumPayment, and minimumPaymentType from
this.bucketForm.value (and set name from payload.name separately); and make the
getter exhaustive by returning a sensible default or throwing for unknown bucket
types so it never falls through without a return.
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.html`:
- Around line 90-92: Replace hardcoded English validation messages in
edit-bucket.component.html with ngx-translate keys and ensure the
select-required messages use the existing key labels.commons.required (not
labels.inputs.required); specifically update the mat-error for
bucketForm.controls.frequency.pattern to use a translation key (e.g.,
labels.inputs.Frequency with | translate) instead of raw text, change the select
error messages at the referenced blocks (around lines 104-109 and 135-140) back
to labels.commons.required, and then run npm run translations:extract to pick up
any new keys.
- Around line 112-123: The template is checking for a 'pattern' error but the
FormControl 'minimumPayment' is validated with Validators.required and
Validators.min(0.01) on the 'bucketForm' control, so replace the pattern check
with the actual min validator: use
bucketForm.controls.minimumPayment.hasError('min') (and/or hasError('required')
as already present) to show a mat-error when the value is below 0.01; also
ensure you use Angular *ngIf syntax (e.g.,
*ngIf="bucketForm.controls.minimumPayment.hasError('min')") so the error becomes
visible and the form can recover.
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts`:
- Around line 127-134: The control array for the "name" FormControl in the
isRegularBucket branch is sparse (extra comma) causing Validators.required to be
treated as an async validator; update the FormBuilder call in
edit-bucket.component (the this.bucketForm = this.formBuilder.group(...) inside
the isRegularBucket check) so the "name" control is defined with the correct
control tuple: [ { value: this.delinquencyBucketData.name, disabled: true },
Validators.required ] (remove the stray comma and ensure Validators.required is
the sync validator slot), then run the linting scripts.
- Around line 226-247: The payloadData getter (in edit-bucket.component.ts)
currently returns for isRegularBucket and isWorkingCapitalBucket but can fall
through and return undefined; update the payloadData getter to include a final
fallback that throws a clear error (e.g., throw new Error('Unsupported
delinquency bucket type')) so callers like submit() never receive undefined and
the Biome useGetterReturn rule is satisfied; locate the getter named payloadData
and add the throw after the existing if/else branches.
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/view-bucket/view-bucket.component.html`:
- Around line 100-103: Replace the concatenated translation tokens '{{
'labels.inputs.Classification' | translate }} {{ 'labels.inputs.Name' |
translate }}' with a single dedicated translation key that represents the column
header (e.g. 'labels.inputs.ClassificationLabel'); update the template in
view-bucket.component.html to use that single key and add the new key/value to
your locale JSON files so translators can reorder/translate correctly; ensure
any CSS/class references (flex-60 mat-body-strong) remain unchanged and run the
app to verify the header renders from the new translation key.
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/view-bucket/view-bucket.component.ts`:
- Around line 41-45: The sorting currently only runs when this.isRegularBucket
is true, but the resolved payload contains the canonical bucket type; update the
condition before sorting the ranges in view-bucket.component.ts to fall back to
the resolved type by checking either this.isRegularBucket OR
this.delinquencyBucketData.bucketType (e.g., compare to the regular bucket
enum/string used in your app such as 'REGULAR' or the enum value) and ensure
delinquencyBucketData.ranges exists before sorting; change the conditional
around the sort that references isRegularBucket to use this combined check so
bookmarked/reloaded pages without the query param still normalize ranges.
- Line 17: The standalone ViewBucketComponent is importing FormatNumberPipe
directly but FormatNumberPipe is not declared standalone; update the pipe or the
component imports to fix it: either make FormatNumberPipe standalone by adding
standalone: true to its `@Pipe` decorator in src/app/pipes/format-number.pipe.ts
(keep the same selector/name) so it can be referenced directly in
view-bucket.component.ts, or remove the direct pipe import from
ViewBucketComponent and instead import the NgModule that declares
FormatNumberPipe (the module that exports the pipe) into the
ViewBucketComponent's imports array; reference FormatNumberPipe and the
ViewBucketComponent when applying the change.
In
`@src/app/products/manage-delinquency-buckets/delinquency-range/delinquency-range.component.resolver.ts`:
- Around line 31-43: The resolver's resolve() may return undefined for unknown
bucketType; update resolve (in delinquency-range.component.resolver.ts) to
always return an Observable by adding a fallback branch for unexpected
bucketType: for example call this.productsService.getDelinquencyRanges() or
return throwError/new Observable with an error so the method never returns
undefined; ensure the logic around DELINQUENCY_BUCKET_TYPE, the
delinquentcyRangeId handling, and calls to
productsService.getDelinquencyRanges(), getDelinquencyRange(), and
getDelinquencyBucketsTemplate() are preserved and used in the fallback.
In `@src/assets/translations/ko-KO.json`:
- Around line 887-888: The translation for the key "Working Capital" is
inconsistent with other entries (e.g., "Working Capital Product")—replace its
value "운영 자본" with the same terminology used elsewhere "운전자본" so the UI uses a
consistent Korean term; update the "Working Capital" entry value to "운전자본" and
scan for any other instances of "운영 자본" to harmonize them to "운전자본".
In `@src/assets/translations/ne-NE.json`:
- Around line 885-886: The translation for the key "Working Capital" currently
uses "चालु पूँजी"; update this value to the canonical Nepali term "कार्यशील
पूँजी" to match other product labels in the file and ensure consistent UI
wording (locate the "Working Capital" key in src/assets/translations/ne-NE.json
and replace its value).
In `@src/assets/translations/pt-PT.json`:
- Line 1760: The translation for the key "Delinquency Payment Rule" uses "Regra
de pagamento em atraso" but should use the existing terminology "inadimplência"
for consistency; update the value for the "Delinquency Payment Rule" JSON key to
use a phrase such as "Regra de pagamento de inadimplência" (or another
consistent phrase using "inadimplência") so it matches other Delinquency-related
labels.
In `@src/assets/translations/sw-SW.json`:
- Line 1757: The translation for the key "Delinquency Payment Rule" uses
"Ukiukaji wa Sheria" which is inconsistent with existing delinquency
terminology; update the value for "Delinquency Payment Rule" to use the same
Swahili delinquency phrase used elsewhere in this file (locate other keys like
"Delinquency", "Delinquent", or "Delinquency Policy" to copy the exact wording)
so the term is consistent across translations.
---
Nitpick comments:
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/delinquency-bucket.component.html`:
- Around line 16-22: Replace the concatenated translations in the two menu
buttons with single full-label translation keys: stop using separate keys '{{
'labels.catalogs.Regular' | translate }} {{ 'labels.inputs.Delinquency Bucket' |
translate }}' and '{{ 'labels.catalogs.Working Capital' | translate }} {{
'labels.inputs.Delinquency Bucket' | translate }}' and instead use new keys like
'labels.catalogs.DelinquencyBucket.Regular' and
'labels.catalogs.DelinquencyBucket.WorkingCapital' (or similar) directly in the
template; add those new keys and values to your translation JSON files and then
run the translations extractor (npm run translations:extract) so the i18n tool
picks them up, and update the template buttons in
delinquency-bucket.component.html to use the single-key translations via the
translate pipe.
- Around line 13-22: Replace the non-8px spacing classes on the fa-icon
instances (m-l-10 and m-r-10) inside the mat-menu buttons with the project’s
8px-grid spacing utilities or theme-backed tokens; locate the fa-icon elements
in the mat-menu items (the two buttons using [routerLink]="['create']") and swap
m-l-10/m-r-10 for the correct 8px utility class (e.g., m-l-8 / m-r-8) or apply
the theme spacing token/mixin used elsewhere so spacing comes from the SCSS
variables rather than hardcoded 10px values.
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts`:
- Line 77: The property delinquencyBucketData is currently typed as any and the
component dereferences minimumPaymentPeriodAndRule, frequencyTypeOptions and
minimumPaymentOptions through any; define small local interfaces (e.g.,
DelinquencyBucket and DelinquencyBucketsTemplateData) matching the resolver
payload shape and replace any with those types for delinquencyBucketData and any
resolver-returning members used in this component (also update usages in the
working-capital branch between the earlier 97–115 block). Update the resolver
method/type signatures and the component property types to use
DelinquencyBucket/DelinquencyBucketsTemplateData so TypeScript enforces the
presence and shapes of minimumPaymentPeriodAndRule, frequencyTypeOptions, and
minimumPaymentOptions at compile time.
In `@src/app/products/products.service.ts`:
- Around line 345-350: The service method getDelinquencyBucketsTemplate
currently returns Observable<any>; create a DelinquencyBucketsTemplate interface
(including frequencyTypeOptions: EnumOptionData[] and minimumPaymentOptions:
EnumOptionData[] plus any other known properties) and update
getDelinquencyBucketsTemplate(): Observable<DelinquencyBucketsTemplate> to use
that type; import or reference the existing EnumOptionData type where needed and
ensure consuming components/resolvers still compile—if this typing change is out
of scope, leave as-is but track the refactor for a dedicated typing effort.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9159079f-78ce-4bca-b5b5-5ee4a6851dbd
📒 Files selected for processing (27)
src/app/products/manage-delinquency-buckets/delinquency-base.component.tssrc/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.htmlsrc/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.tssrc/app/products/manage-delinquency-buckets/delinquency-bucket/delinquency-bucket.component.htmlsrc/app/products/manage-delinquency-buckets/delinquency-bucket/delinquency-bucket.component.tssrc/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.htmlsrc/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.tssrc/app/products/manage-delinquency-buckets/delinquency-bucket/view-bucket/view-bucket.component.htmlsrc/app/products/manage-delinquency-buckets/delinquency-bucket/view-bucket/view-bucket.component.tssrc/app/products/manage-delinquency-buckets/delinquency-range/delinquency-range.component.resolver.tssrc/app/products/manage-delinquency-buckets/models/delinquency-models.tssrc/app/products/products-routing.module.tssrc/app/products/products.service.tssrc/app/shared/models/option-data.model.tssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
Comment on lines
+39
to
+45
| bucketTypeLabel(bucketType: number): string { | ||
| if (bucketType === 1) { | ||
| return 'Regular'; | ||
| } | ||
| if (bucketType === 2) { | ||
| return 'Working Capital'; | ||
| } |
There was a problem hiding this comment.
Translate these bucket labels instead of hardcoding English.
Regular and Working Capital are rendered text, so they bypass the locale catalog and will not be picked up by translation extraction in this form.
As per coding guidelines: Use proper i18n variables from @ngx-translate/core for all user-facing strings instead of hardcoded text.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/products/manage-delinquency-buckets/delinquency-base.component.ts`
around lines 39 - 45, bucketTypeLabel currently returns hardcoded English
strings; update it to use ngx-translate so labels are extracted and localized:
inject TranslateService into the component and replace the literal returns in
bucketTypeLabel(bucketType: number) with calls to TranslateService (e.g.
this.translate.instant('products.bucketType.regular') and
this.translate.instant('products.bucketType.workingCapital')), adding those
translation keys to the locale files; keep the same numeric checks (bucketType
=== 1 / === 2) and default behavior unchanged.
Comment on lines
+39
to
+57
| bucketTypeLabel(bucketType: number): string { | ||
| if (bucketType === 1) { | ||
| return 'Regular'; | ||
| } | ||
| if (bucketType === 2) { | ||
| return 'Working Capital'; | ||
| } | ||
| return ''; | ||
| } | ||
|
|
||
| bucketType(bucketType: number): string { | ||
| if (bucketType === 1) { | ||
| return 'regular'; | ||
| } | ||
| if (bucketType === 2) { | ||
| return 'workingcapital'; | ||
| } | ||
| return ''; | ||
| } |
There was a problem hiding this comment.
Use DelinquencyBucketType strings in these helpers.
src/app/products/manage-delinquency-buckets/models/delinquency-models.ts (Lines 9-14) defines bucket types as 'regular' | 'workingcapital', and src/app/products/manage-delinquency-buckets/delinquency-bucket/delinquency-bucket.component.html (Line 54) passes row.bucketType straight into this code. With the current 1/2 checks, working-capital rows resolve to '', so edit/view falls back to the regular flow and then hits the regular-only ranges path in src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts (Lines 101-112) / src/app/products/manage-delinquency-buckets/delinquency-bucket/view-bucket/view-bucket.component.ts (Lines 37-46).
🛠️ Minimal fix
- bucketTypeLabel(bucketType: number): string {
- if (bucketType === 1) {
+ bucketTypeLabel(bucketType: DelinquencyBucketType): string {
+ if (bucketType === DELINQUENCY_BUCKET_TYPE.REGULAR) {
return 'Regular';
}
- if (bucketType === 2) {
+ if (bucketType === DELINQUENCY_BUCKET_TYPE.WORKING_CAPITAL) {
return 'Working Capital';
}
return '';
}
- bucketType(bucketType: number): string {
- if (bucketType === 1) {
- return 'regular';
- }
- if (bucketType === 2) {
- return 'workingcapital';
- }
- return '';
+ bucketType(bucketType: DelinquencyBucketType): DelinquencyBucketType {
+ return bucketType;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/products/manage-delinquency-buckets/delinquency-base.component.ts`
around lines 39 - 57, The helpers bucketTypeLabel and bucketType currently check
for numeric values (1/2) but the model uses string bucket types ('regular' |
'workingcapital') and row.bucketType passes those strings; update these
functions to accept and handle the string values by returning 'Regular' for
'regular' and 'Working Capital' for 'workingcapital' (and returning
'regular'/'workingcapital' respectively in bucketType), and keep optional
backward-compatibility by also handling numeric 1/2 if needed so working-capital
rows no longer resolve to ''. Use the existing function names bucketTypeLabel
and bucketType to locate and modify the logic accordingly.
...cts/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.html
Outdated
Show resolved
Hide resolved
Comment on lines
+118
to
+120
| @if (bucketForm.controls.minimumPayment.hasError('pattern')) { | ||
| <mat-error> {{ 'labels.inputs.Minimum Payment' | translate }} must be a positive number. </mat-error> | ||
| } |
There was a problem hiding this comment.
minimumPayment is checking the wrong validation error key.
The control uses Validators.min(0.01), but the template checks hasError('pattern'), so invalid min values won’t show the intended validation message.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.html`
around lines 118 - 120, The template is checking the wrong validation key:
update the mat-error condition for bucketForm.controls.minimumPayment from
hasError('pattern') to hasError('min') so the Validators.min(0.01) failure shows
the intended message; locate the minimumPayment control usage in the
create-bucket.component.html and replace the error key accordingly (keep the
same error text).
...ducts/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.ts
Show resolved
Hide resolved
...roducts/manage-delinquency-buckets/delinquency-range/delinquency-range.component.resolver.ts
Show resolved
Hide resolved
alberto-art3ch
force-pushed
the
WEB-870/wc-delinquency-management-configuration
branch
from
46cd085 to
230ff20
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/products/products-routing.module.ts (1)
547-552:⚠️ Potential issue | 🔴 CriticalResolve
delinquencyBucketsTemplateDatafrom the bucket-template endpoint, not the range resolver.Both bucket routes still wire this key to
DelinquencyRangeComponentsResolver, so the new/delinquency/buckets/templatepayload never reaches create/edit. That leaves the WC-only option lists introduced by this PR unavailable here. If the regular flow still needs ranges, resolve both payloads under separate keys.Also applies to: 572-575
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/products-routing.module.ts` around lines 547 - 552, Update the route resolve mapping so the delinquencyBucketsTemplateData key is resolved from the bucket-template endpoint resolver instead of DelinquencyRangeComponentsResolver: replace DelinquencyRangeComponentsResolver with the resolver that calls the bucket-template endpoint (so CreateBucketComponent's route and the corresponding EditBucketComponent route both receive the template payload). If the existing range payload is still required, keep DelinquencyRangeComponentsResolver under a separate key (e.g., delinquencyRangesData) so both payloads are resolved for the create and edit routes.
♻️ Duplicate comments (7)
src/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.html (1)
112-126:⚠️ Potential issue | 🟡 MinorVerify that the
minimumPaymenterror key matches the actual validator.This branch still checks
hasError('pattern'), but the field itself only advertises a min constraint. If the control is usingValidators.min(0.01), values like0or-1will never show this message.#!/bin/bash set -euo pipefail fd 'create-bucket.component.ts$' src/app/products/manage-delinquency-buckets | while read -r file; do echo "--- $file ---" rg -n -C3 'minimumPayment|Validators\.(required|min|pattern)' "$file" doneIf this only shows
Validators.min(0.01), switch the template check tohasError('min').🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.html` around lines 112 - 126, The template checks bucketForm.controls.minimumPayment.hasError('pattern') but the control uses Validators.min(0.01); update the error key check to match the validator (use hasError('min')) and adjust the mat-error message if needed to reference the min constraint; locate the check in create-bucket.component.html where bucketForm.controls.minimumPayment is referenced and replace the 'pattern' branch with a 'min' branch to align with Validators.min(0.01).src/app/products/manage-delinquency-buckets/delinquency-bucket/view-bucket/view-bucket.component.html (1)
99-103:⚠️ Potential issue | 🟡 MinorUse a dedicated translation key for this header.
Concatenating
ClassificationandNamestill produces awkward copy and prevents translators from reordering the phrase. Please replace it with a single key such aslabels.inputs.ClassificationName. As per coding guidelines "Use proper i18n variables from@ngx-translate/corefor all user-facing strings instead of hardcoded text".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/manage-delinquency-buckets/delinquency-bucket/view-bucket/view-bucket.component.html` around lines 99 - 103, Replace the concatenated translation calls inside the isRegularBucket block with a single translation key to allow proper i18n ordering: remove the two separate pipes ('labels.inputs.Classification' and 'labels.inputs.Name') used in the div governed by the isRegularBucket condition and replace them with a single translate key such as 'labels.inputs.ClassificationName' (update the template in view-bucket.component.html where isRegularBucket is referenced), then add that new key and its appropriate translations to the translations resource files so translators can provide a single coherent string.src/app/products/manage-delinquency-buckets/delinquency-bucket/view-bucket/view-bucket.component.ts (1)
39-45:⚠️ Potential issue | 🟠 MajorReinitialize from the resolved bucket before branching, and sort by the rendered field.
This still keys off query-param state, so reloading a WC bucket without
?bucketType=falls back to regular here. It also sorts onminimumAge, while the template rendersminimumAgeDays, so regular ranges will not normalize reliably.🛠️ Suggested fix
this.route.data.subscribe((data: { delinquencyBucket: any }) => { this.delinquencyBucketData = data.delinquencyBucket; - if (this.isRegularBucket) { + this.initialize(this.delinquencyBucketData.bucketType); + if (this.isRegularBucket && this.delinquencyBucketData.ranges) { this.delinquencyBucketData.ranges = this.delinquencyBucketData.ranges.sort( - (objA: { minimumAge: number }, objB: { minimumAge: number }) => objA.minimumAge - objB.minimumAge + (objA: { minimumAgeDays: number }, objB: { minimumAgeDays: number }) => + objA.minimumAgeDays - objB.minimumAgeDays ); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/manage-delinquency-buckets/delinquency-bucket/view-bucket/view-bucket.component.ts` around lines 39 - 45, Reinitialize component state from the resolved route data first (use the resolved object from this.route.data subscription rather than relying on query-param-derived isRegularBucket), then decide the branch from that resolved bucket; specifically, inside the this.route.data.subscribe callback assign this.delinquencyBucketData = data.delinquencyBucket, derive/update this.isRegularBucket from that resolved object, and when sorting regular bucket ranges sort by the rendered field minimumAgeDays (e.g. this.delinquencyBucketData.ranges.sort((a,b) => a.minimumAgeDays - b.minimumAgeDays)) while guarding for missing ranges.src/app/products/manage-delinquency-buckets/delinquency-base.component.ts (1)
39-57:⚠️ Potential issue | 🔴 CriticalHandle the string bucket types this feature introduced.
This base class initializes/query-strings with
'regular' | 'workingcapital', butbucketTypeLabel()andbucketType()still branch on1/2. That makes WC buckets resolve to'', which drops the label and loses the query param downstream.🛠️ Minimal fix
- bucketTypeLabel(bucketType: number): string { - if (bucketType === 1) { + bucketTypeLabel(bucketType: DelinquencyBucketType): string { + if (bucketType === DELINQUENCY_BUCKET_TYPE.REGULAR) { return 'Regular'; } - if (bucketType === 2) { + if (bucketType === DELINQUENCY_BUCKET_TYPE.WORKING_CAPITAL) { return 'Working Capital'; } return ''; } - bucketType(bucketType: number): string { - if (bucketType === 1) { - return 'regular'; - } - if (bucketType === 2) { - return 'workingcapital'; - } - return ''; + bucketType(bucketType: DelinquencyBucketType): DelinquencyBucketType { + return bucketType; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/manage-delinquency-buckets/delinquency-base.component.ts` around lines 39 - 57, bucketTypeLabel() and bucketType() still only handle numeric values (1/2) while the feature now uses string types 'regular' | 'workingcapital', causing working capital buckets to resolve to ''. Update both functions in delinquency-base.component.ts (bucketTypeLabel and bucketType) to handle the string values (and optionally accept legacy numeric values) — e.g., map 'regular' -> 'Regular'/'regular' and 'workingcapital' -> 'Working Capital'/'workingcapital', returning the empty string only for unknown inputs so labels and query params are preserved.src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts (2)
226-247:⚠️ Potential issue | 🟠 MajorHarden
payloadData: explicit WC rule object + fallback throw.Line 244 currently nests the full raw form under
minimumPaymentPeriodAndRule, and Line 247 still allows undefined return on unexpected state.Suggested fix
get payloadData() { - const bucketType: number = this.isRegularBucket ? 1 : 2; if (this.isRegularBucket) { - const ranges: any = []; + const ranges: number[] = []; this.rangesDataSource.forEach((item: any) => { ranges.push(item.id); }); return { - bucketType: bucketType, + bucketType: 1, ...this.bucketForm.value, - ranges: ranges + ranges }; - } else if (this.isWorkingCapitalBucket) { + } + + if (this.isWorkingCapitalBucket) { const payload = this.bucketForm.getRawValue(); - const bucketName = payload['name']; + const { name, frequency, frequencyType, minimumPayment, minimumPaymentType } = payload; return { - bucketType: bucketType, - name: bucketName, - minimumPaymentPeriodAndRule: payload + bucketType: 2, + name, + minimumPaymentPeriodAndRule: { + frequency, + frequencyType, + minimumPayment, + minimumPaymentType + } }; } + throw new Error(`Unsupported delinquency bucket type: ${this.delinquencyBucketType.value}`); }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/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts` around lines 226 - 247, The getter payloadData currently returns the entire raw form under minimumPaymentPeriodAndRule and can fall through returning undefined; update payloadData so that when isWorkingCapitalBucket is true you build an explicit, strongly-typed WC rule object from this.bucketForm.getRawValue() (pick and map only the expected fields into e.g. minimumPaymentPeriodAndRule: { period, ruleType, amount, ... } instead of the whole payload) and ensure the function always returns a value by adding a fallback that throws a clear Error when neither isRegularBucket nor isWorkingCapitalBucket is true; reference payloadData, isRegularBucket, isWorkingCapitalBucket, bucketForm and minimumPaymentPeriodAndRule to locate and fix the logic.
129-133:⚠️ Potential issue | 🟠 MajorRemove the sparse control tuple in regular-bucket form config.
Line 131 introduces an empty slot in the control array; this is lint-breaking and can misplace validator arguments.
Minimal fix
name: [ { value: this.delinquencyBucketData.name, disabled: true }, - , Validators.required ]As per coding guidelines "Run
npm run lint(eslint, stylelint, prettier, htmlhint) to ensure code meets quality standards before committing".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts` around lines 129 - 133, The form control definition for "name" in edit-bucket.component.ts contains a sparse tuple (an extra comma) that shifts validators into the third slot; remove the empty slot so the control tuple is either [ { value: this.delinquencyBucketData.name, disabled: true }, Validators.required ] or pass validators as an array/third argument correctly; update the 'name' entry in the FormBuilder/group config to remove the extra comma and ensure Validators.required is applied to the control.src/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.ts (1)
203-224:⚠️ Potential issue | 🟠 MajorMake
payloadDataexhaustive and build WC rule payload explicitly.At Line 221,
minimumPaymentPeriodAndRulecurrently receives the full form object (includingname). Also, Line 224 allows fallthrough with no return, sosubmit()can passundefined.Suggested fix
- get payloadData() { - const bucketType: number = this.isRegularBucket ? 1 : 2; + get payloadData() { if (this.isRegularBucket) { - const ranges: any = []; + const ranges: number[] = []; this.rangesDataSource.forEach((item: any) => { ranges.push(item.rangeId); }); return { - bucketType: bucketType, + bucketType: 1, ...this.bucketForm.value, - ranges: ranges + ranges }; - } else if (this.isWorkingCapitalBucket) { - const payload = this.bucketForm.value; - const bucketName = payload['name']; + } + + if (this.isWorkingCapitalBucket) { + const { name, frequency, frequencyType, minimumPayment, minimumPaymentType } = + this.bucketForm.getRawValue(); return { - bucketType: bucketType, - name: bucketName, - minimumPaymentPeriodAndRule: payload + bucketType: 2, + name, + minimumPaymentPeriodAndRule: { + frequency, + frequencyType, + minimumPayment, + minimumPaymentType + } }; } + + throw new Error(`Unsupported delinquency bucket type: ${this.delinquencyBucketType.value}`); }As per coding guidelines "Use TypeScript for all application code with strict typing conventions".
🧹 Nitpick comments (1)
src/app/products/products.service.ts (1)
345-350: Type the new bucket-template response.
getDelinquencyBucketsTemplate()adds a new API boundary but still returnsObservable<any>. A smallDelinquencyBucketTemplateDatainterface here would keep the resolver/create/edit flow strictly typed without turning this into a full service-wide typing cleanup. 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."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/products.service.ts` around lines 345 - 350, Add a specific response type for the new bucket-template API and use it in getDelinquencyBucketsTemplate(): define an interface named DelinquencyBucketTemplateData that matches the API shape (fields for buckets, rules, metadata, etc.), replace the return type Observable<any> with Observable<DelinquencyBucketTemplateData> in the getDelinquencyBucketsTemplate() method, and update any callers/resolvers/create/edit flows that consume this method to use the new type (e.g., adjust variable typings and return types) so the flow stays strictly typed without a broader service-wide refactor.
🤖 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/products/manage-delinquency-buckets/delinquency-base.component.ts`:
- Around line 59-65: getCatalogLabel currently assumes inputText.split('.') has
a second token and calls camalize(datas[1]) which can be undefined; update
getCatalogLabel to guard the split result (check that datas.length > 1 and
datas[1] is truthy) before calling camalize, and return a safe fallback (e.g.,
the original inputText or an empty string) when the second token is missing or
falsy; keep using the existing camalize(word) helper for valid tokens so
rendering won't break on unexpected enum/label formats.
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.html`:
- Around line 56-68: The delete icon button in create-bucket.component.html is
missing an accessible name; add an aria-label on the button element (e.g.,
aria-label="{{ 'tooltips.Delete' | translate }}" or a more specific translated
string like 'labels.actions.DeleteRange') so screen readers announce the action,
ensuring the element that calls deleteDelinquencyRange(rowIndex) remains
unchanged and the tooltip can stay for visual UX.
---
Outside diff comments:
In `@src/app/products/products-routing.module.ts`:
- Around line 547-552: Update the route resolve mapping so the
delinquencyBucketsTemplateData key is resolved from the bucket-template endpoint
resolver instead of DelinquencyRangeComponentsResolver: replace
DelinquencyRangeComponentsResolver with the resolver that calls the
bucket-template endpoint (so CreateBucketComponent's route and the corresponding
EditBucketComponent route both receive the template payload). If the existing
range payload is still required, keep DelinquencyRangeComponentsResolver under a
separate key (e.g., delinquencyRangesData) so both payloads are resolved for the
create and edit routes.
---
Duplicate comments:
In `@src/app/products/manage-delinquency-buckets/delinquency-base.component.ts`:
- Around line 39-57: bucketTypeLabel() and bucketType() still only handle
numeric values (1/2) while the feature now uses string types 'regular' |
'workingcapital', causing working capital buckets to resolve to ''. Update both
functions in delinquency-base.component.ts (bucketTypeLabel and bucketType) to
handle the string values (and optionally accept legacy numeric values) — e.g.,
map 'regular' -> 'Regular'/'regular' and 'workingcapital' -> 'Working
Capital'/'workingcapital', returning the empty string only for unknown inputs so
labels and query params are preserved.
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.html`:
- Around line 112-126: The template checks
bucketForm.controls.minimumPayment.hasError('pattern') but the control uses
Validators.min(0.01); update the error key check to match the validator (use
hasError('min')) and adjust the mat-error message if needed to reference the min
constraint; locate the check in create-bucket.component.html where
bucketForm.controls.minimumPayment is referenced and replace the 'pattern'
branch with a 'min' branch to align with Validators.min(0.01).
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts`:
- Around line 226-247: The getter payloadData currently returns the entire raw
form under minimumPaymentPeriodAndRule and can fall through returning undefined;
update payloadData so that when isWorkingCapitalBucket is true you build an
explicit, strongly-typed WC rule object from this.bucketForm.getRawValue() (pick
and map only the expected fields into e.g. minimumPaymentPeriodAndRule: {
period, ruleType, amount, ... } instead of the whole payload) and ensure the
function always returns a value by adding a fallback that throws a clear Error
when neither isRegularBucket nor isWorkingCapitalBucket is true; reference
payloadData, isRegularBucket, isWorkingCapitalBucket, bucketForm and
minimumPaymentPeriodAndRule to locate and fix the logic.
- Around line 129-133: The form control definition for "name" in
edit-bucket.component.ts contains a sparse tuple (an extra comma) that shifts
validators into the third slot; remove the empty slot so the control tuple is
either [ { value: this.delinquencyBucketData.name, disabled: true },
Validators.required ] or pass validators as an array/third argument correctly;
update the 'name' entry in the FormBuilder/group config to remove the extra
comma and ensure Validators.required is applied to the control.
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/view-bucket/view-bucket.component.html`:
- Around line 99-103: Replace the concatenated translation calls inside the
isRegularBucket block with a single translation key to allow proper i18n
ordering: remove the two separate pipes ('labels.inputs.Classification' and
'labels.inputs.Name') used in the div governed by the isRegularBucket condition
and replace them with a single translate key such as
'labels.inputs.ClassificationName' (update the template in
view-bucket.component.html where isRegularBucket is referenced), then add that
new key and its appropriate translations to the translations resource files so
translators can provide a single coherent string.
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/view-bucket/view-bucket.component.ts`:
- Around line 39-45: Reinitialize component state from the resolved route data
first (use the resolved object from this.route.data subscription rather than
relying on query-param-derived isRegularBucket), then decide the branch from
that resolved bucket; specifically, inside the this.route.data.subscribe
callback assign this.delinquencyBucketData = data.delinquencyBucket,
derive/update this.isRegularBucket from that resolved object, and when sorting
regular bucket ranges sort by the rendered field minimumAgeDays (e.g.
this.delinquencyBucketData.ranges.sort((a,b) => a.minimumAgeDays -
b.minimumAgeDays)) while guarding for missing ranges.
---
Nitpick comments:
In `@src/app/products/products.service.ts`:
- Around line 345-350: Add a specific response type for the new bucket-template
API and use it in getDelinquencyBucketsTemplate(): define an interface named
DelinquencyBucketTemplateData that matches the API shape (fields for buckets,
rules, metadata, etc.), replace the return type Observable<any> with
Observable<DelinquencyBucketTemplateData> in the getDelinquencyBucketsTemplate()
method, and update any callers/resolvers/create/edit flows that consume this
method to use the new type (e.g., adjust variable typings and return types) so
the flow stays strictly typed without a broader service-wide refactor.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: db3b8881-22f2-44d9-99ca-a120d812937f
📒 Files selected for processing (28)
src/app/account-transfers/create-standing-instructions/create-standing-instructions.component.htmlsrc/app/products/manage-delinquency-buckets/delinquency-base.component.tssrc/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.htmlsrc/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.tssrc/app/products/manage-delinquency-buckets/delinquency-bucket/delinquency-bucket.component.htmlsrc/app/products/manage-delinquency-buckets/delinquency-bucket/delinquency-bucket.component.tssrc/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.htmlsrc/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.tssrc/app/products/manage-delinquency-buckets/delinquency-bucket/view-bucket/view-bucket.component.htmlsrc/app/products/manage-delinquency-buckets/delinquency-bucket/view-bucket/view-bucket.component.tssrc/app/products/manage-delinquency-buckets/delinquency-range/delinquency-range.component.resolver.tssrc/app/products/manage-delinquency-buckets/models/delinquency-models.tssrc/app/products/products-routing.module.tssrc/app/products/products.service.tssrc/app/shared/models/option-data.model.tssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
🚧 Files skipped from review as they are similar to previous changes (16)
- src/app/products/manage-delinquency-buckets/delinquency-range/delinquency-range.component.resolver.ts
- src/assets/translations/pt-PT.json
- src/app/products/manage-delinquency-buckets/models/delinquency-models.ts
- src/assets/translations/en-US.json
- src/app/products/manage-delinquency-buckets/delinquency-bucket/delinquency-bucket.component.html
- src/assets/translations/es-CL.json
- src/assets/translations/ne-NE.json
- src/assets/translations/it-IT.json
- src/assets/translations/es-MX.json
- src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.html
- src/assets/translations/cs-CS.json
- src/assets/translations/fr-FR.json
- src/assets/translations/sw-SW.json
- src/assets/translations/ko-KO.json
- src/assets/translations/lv-LV.json
- src/assets/translations/lt-LT.json
Comment on lines
+59
to
+65
| getCatalogLabel(inputText: string): string { | ||
| const datas = inputText.split('.'); | ||
| return this.camalize(datas[1]); | ||
| } | ||
|
|
||
| camalize(word: string) { | ||
| return word.charAt(0).toUpperCase() + word.slice(1).toLowerCase(); |
There was a problem hiding this comment.
Guard getCatalogLabel() before camelizing.
This helper unconditionally reads the second token from split('.'). Any plain label or unexpected enum code from the new WC option data will call camalize(undefined) and break render-time translation.
🛡️ Minimal fix
getCatalogLabel(inputText: string): string {
- const datas = inputText.split('.');
- return this.camalize(datas[1]);
+ const [, catalogValue] = inputText.split('.');
+ return this.camalize(catalogValue ?? inputText);
}📝 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
| getCatalogLabel(inputText: string): string { | |
| const datas = inputText.split('.'); | |
| return this.camalize(datas[1]); | |
| } | |
| camalize(word: string) { | |
| return word.charAt(0).toUpperCase() + word.slice(1).toLowerCase(); | |
| getCatalogLabel(inputText: string): string { | |
| const [, catalogValue] = inputText.split('.'); | |
| return this.camalize(catalogValue ?? inputText); | |
| } | |
| camalize(word: string) { | |
| return word.charAt(0).toUpperCase() + word.slice(1).toLowerCase(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/products/manage-delinquency-buckets/delinquency-base.component.ts`
around lines 59 - 65, getCatalogLabel currently assumes inputText.split('.') has
a second token and calls camalize(datas[1]) which can be undefined; update
getCatalogLabel to guard the split result (check that datas.length > 1 and
datas[1] is truthy) before calling camalize, and return a safe fallback (e.g.,
the original inputText or an empty string) when the second token is missing or
falsy; keep using the existing camalize(word) helper for valid tokens so
rendering won't break on unexpected enum/label formats.
Comment on lines
+56
to
+68
| <ng-container matColumnDef="actions"> | ||
| <th mat-header-cell *matHeaderCellDef>{{ 'labels.inputs.Actions' | translate }}</th> | ||
| <td mat-cell *matCellDef="let row; let rowIndex = index"> | ||
| <button | ||
| type="button" | ||
| mat-icon-button | ||
| color="warn" | ||
| (click)="deleteDelinquencyRange(rowIndex)" | ||
| matTooltip="{{ 'tooltips.Delete' | translate }}" | ||
| matTooltipPosition="left" | ||
| > | ||
| <fa-icon icon="trash"></fa-icon> | ||
| </button> |
There was a problem hiding this comment.
Add an accessible name to the delete icon button.
matTooltip is not a reliable accessible label, so screen readers will announce this as an unlabeled button. Please add an aria-label for the delete action.
♿ Suggested fix
<button
type="button"
mat-icon-button
color="warn"
(click)="deleteDelinquencyRange(rowIndex)"
+ [attr.aria-label]="'tooltips.Delete' | translate"
matTooltip="{{ 'tooltips.Delete' | translate }}"
matTooltipPosition="left"
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.html`
around lines 56 - 68, The delete icon button in create-bucket.component.html is
missing an accessible name; add an aria-label on the button element (e.g.,
aria-label="{{ 'tooltips.Delete' | translate }}" or a more specific translated
string like 'labels.actions.DeleteRange') so screen readers announce the action,
ensuring the element that calls deleteDelinquencyRange(rowIndex) remains
unchanged and the tooltip can stay for visual UX.
alberto-art3ch
force-pushed
the
WEB-870/wc-delinquency-management-configuration
branch
from
230ff20 to
a2c2fad
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
src/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.ts (1)
208-230:⚠️ Potential issue | 🟠 Major
payloadDatagetter may returnundefinedfor unexpected bucket types.The getter has no fallback return statement. If
delinquencyBucketTypeis neither regular nor working capital (edge case), the getter returnsundefined, which would causecreateDelinquencyBucket()to fail silently or unpredictably.Also, Line 226 passes the entire
payload(includingname) intominimumPaymentPeriodAndRule, thoughnameis extracted separately. This duplicates the name field inside the rule object.🛡️ Proposed fix
get payloadData() { const bucketType: number = this.isRegularBucket ? 1 : 2; const ranges: any = []; this.rangesDataSource.forEach((item: any) => { ranges.push(item.rangeId); }); if (this.isRegularBucket) { return { bucketType: bucketType, ...this.bucketForm.value, ranges: ranges }; } else if (this.isWorkingCapitalBucket) { const payload = this.bucketForm.value; - const bucketName = payload['name']; + const { name, frequency, frequencyType, minimumPayment, minimumPaymentType } = payload; return { bucketType: bucketType, - name: bucketName, - minimumPaymentPeriodAndRule: payload, + name, + minimumPaymentPeriodAndRule: { + frequency, + frequencyType, + minimumPayment, + minimumPaymentType + }, ranges: ranges }; } + throw new Error(`Unsupported bucket type: ${this.delinquencyBucketType.value}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.ts` around lines 208 - 230, The payloadData getter can return undefined if neither isRegularBucket nor isWorkingCapitalBucket is true and also currently nests the name inside minimumPaymentPeriodAndRule; update the payloadData getter to always return a valid object (add a default/fallback return that throws or returns a safe payload) and when building the working-capital branch (isWorkingCapitalBucket) create minimumPaymentPeriodAndRule from bucketForm.value excluding the name field (use a copy without 'name') while still setting name at the top level; ensure ranges are built from rangesDataSource.map(item => item.rangeId) to keep behavior consistent.src/app/products/manage-delinquency-buckets/delinquency-bucket/view-bucket/view-bucket.component.ts (1)
16-27:⚠️ Potential issue | 🔴 CriticalMake
FormatNumberPipestandalone or remove from component imports.
FormatNumberPipeatsrc/app/pipes/format-number.pipe.tsis not declared withstandalone: truein its@Pipe()decorator, but the component is importing it in the standalone component's imports array (line 26). Addstandalone: trueto the pipe decorator:`@Pipe`({ name: 'formatNumber', standalone: true }) export class FormatNumberPipe implements PipeTransform {Or if the pipe should remain module-based, remove it from the component's imports array.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/manage-delinquency-buckets/delinquency-bucket/view-bucket/view-bucket.component.ts` around lines 16 - 27, The component ViewBucketComponent currently imports FormatNumberPipe in its standalone imports array (imports: [..., FormatNumberPipe]) but the pipe is not declared standalone; either make the pipe standalone by adding standalone: true to its `@Pipe` decorator in FormatNumberPipe (i.e. `@Pipe`({ name: 'formatNumber', standalone: true }) export class FormatNumberPipe ...) or remove FormatNumberPipe from the imports array in view-bucket.component.ts so the component relies on the module declaration; update the code accordingly to keep declarations consistent.src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.html (1)
69-74:⚠️ Potential issue | 🟡 MinorAdd
hasError('min')check for minimumPayment validation error.The template checks for
hasError('pattern')at lines 69-74, but the actual validator isValidators.min(0.01). The pattern error message will never display. Replace the pattern check with a min check, or add both checks to handle minimum value validation properly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.html` around lines 69 - 74, The template currently checks bucketForm.controls.minimumPayment.hasError('pattern') but the control is validated with Validators.min(0.01), so the min error will never be caught; update the edit-bucket.component.html check to use hasError('min') (or include both hasError('pattern') || hasError('min')) for the minimumPayment mat-error block so the correct validation message displays when minimum value validation (Validators.min(0.01)) fails.src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts (1)
232-254:⚠️ Potential issue | 🟡 MinorAdd fallback throw and avoid duplicate
namein payload.Two issues here:
The getter can fall through without returning if neither bucket type condition is met, causing
submit()to sendundefinedto the API.For working capital buckets,
minimumPaymentPeriodAndRule: payloadincludes thenamefield from the form's raw value, duplicating the top-levelname. Depending on API expectations, this could cause issues or send unnecessary data.🩹 Proposed fix
get payloadData() { const bucketType: number = this.isRegularBucket ? 1 : 2; const ranges: any = []; this.rangesDataSource.forEach((item: any) => { ranges.push(item.id); }); if (this.isRegularBucket) { return { bucketType: bucketType, ...this.bucketForm.value, ranges: ranges }; } else if (this.isWorkingCapitalBucket) { const payload = this.bucketForm.getRawValue(); const bucketName = payload['name']; + const { name, ...minimumPaymentPeriodAndRule } = payload; return { bucketType: bucketType, name: bucketName, - minimumPaymentPeriodAndRule: payload, + minimumPaymentPeriodAndRule, ranges: ranges }; } + throw new Error('Unsupported delinquency bucket type'); },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts` around lines 232 - 254, The payloadData getter can return undefined if neither isRegularBucket nor isWorkingCapitalBucket is true and for working-capital buckets it duplicates the name inside minimumPaymentPeriodAndRule; update the payloadData getter to (1) throw a clear error when no bucket type matches so submit() never sends undefined, and (2) for the isWorkingCapitalBucket branch create payload from this.bucketForm.getRawValue() but remove the name property before assigning minimumPaymentPeriodAndRule (keep name only at the top level). Reference: payloadData getter, isRegularBucket, isWorkingCapitalBucket, this.bucketForm.getRawValue(), and this.rangesDataSource.
🧹 Nitpick comments (4)
src/app/products/products.service.ts (1)
348-349: This is a refactoring improvement that should be tracked separately.While the coding guidelines encourage strict typing, the project currently accepts
Observable<any>as a pattern for API responses. Typing the full API response layer is better addressed as part of a broader cross-cutting refactoring effort rather than in individual PRs. If you're planning a dedicated enhancement for API response typing across the services, this method should be included in that scope.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/products.service.ts` around lines 348 - 349, The method getDelinquencyBucketsTemplate currently returns Observable<any> which we want to avoid long-term; create a tracked refactor instead of changing types here by adding a short TODO comment inside getDelinquencyBucketsTemplate referencing a new tracking ticket (e.g., "TODO: tighten API response types — TRACK-XXXX") and leave the return signature as Observable<any> for now; ensure the TODO mentions the function name getDelinquencyBucketsTemplate so it’s discoverable during the sweeping API-typing refactor.src/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.ts (1)
133-140: Redundant validators onfrequencyfield.The field has both
Validators.pattern('^(0*[1-9][0-9]*)$')(positive integers) andValidators.min(1). Since the pattern already enforces positive integers ≥1, themin(1)validator is redundant. Consider keeping only one for clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.ts` around lines 133 - 140, The frequency FormControl currently applies both Validators.pattern('^(0*[1-9][0-9]*)$') and Validators.min(1), which duplicate the same constraint; remove the redundant validator to clarify intent—either keep Validators.pattern('^(0*[1-9][0-9]*)$') for strict positive-integer formatting or keep Validators.min(1) if you want to allow non-integer numbers, and update the frequency control definition in create-bucket.component.ts accordingly (adjust the array that contains frequency validators).src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts (2)
97-122: Consider cleaning up route subscription to prevent potential memory leaks.The route data subscription in the constructor is never unsubscribed. While Angular's
ActivatedRouteobservables often complete when the component is destroyed, explicitly managing subscriptions usingtakeUntilDestroyed()(Angular 16+) ortakeUntilpattern is a safer practice.♻️ Example using takeUntilDestroyed
+ import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; + import { DestroyRef } from '@angular/core'; constructor() { super(); + const destroyRef = inject(DestroyRef); - this.route.data.subscribe((data: { delinquencyBucket: any; delinquencyBucketsTemplateData: any }) => { + this.route.data.pipe(takeUntilDestroyed(destroyRef)).subscribe((data: { delinquencyBucket: any; delinquencyBucketsTemplateData: any }) => { // ... }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts` around lines 97 - 122, The route.data subscription in the constructor (this.route.data.subscribe(...)) is never cleaned up; change it to use Angular's takeUntilDestroyed() (or the takeUntil + destroy$ pattern) so the observable is unsubscribed when the component is torn down: replace this.route.data.subscribe(...) with this.route.data.pipe(takeUntilDestroyed()).subscribe(...) (or pipe(takeUntil(this.destroy$)) and implement ngOnDestroy to next/complete destroy$), keeping the same handler body and retain references to delinquencyRangesData, delinquencyBucketData, delinquencyBucketId, rangesDataSource, delinquencyRangesIds, frequencyTypeOptions and minimumPaymentOptions.
228-230: RenamevalidForm()to reflect its actual purpose.The method returns whether the submit button should be disabled, not whether the form is valid. The current name is semantically inverted and misleading. Consider renaming to
submitDisabled()or inverting the return logic and keeping the name.♻️ Suggested rename
- validForm(): boolean { - return this.isRegularBucket ? !this.dataWasChanged : !this.bucketForm.valid; - } + submitDisabled(): boolean { + return this.isRegularBucket ? !this.dataWasChanged : !this.bucketForm.valid; + }Or, keep the name and invert the logic:
validForm(): boolean { - return this.isRegularBucket ? !this.dataWasChanged : !this.bucketForm.valid; + return this.isRegularBucket ? this.dataWasChanged : this.bucketForm.valid; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts` around lines 228 - 230, The method validForm() actually computes whether the submit button should be disabled; rename validForm() to submitDisabled() and update all references (templates, tests, and callers) accordingly; keep the existing logic using isRegularBucket, dataWasChanged and bucketForm.valid so submitDisabled() returns the current boolean for disabling the submit button. Ensure any bindings like [disabled]="validForm()" are updated to [disabled]="submitDisabled()".
🤖 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/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.html`:
- Around line 35-40: The template only checks
bucketForm.controls.frequency.hasError('pattern') but must also show messages
for 'min' and 'required'; update edit-bucket.component.html to add branches that
display a mat-error when bucketForm.controls.frequency.hasError('min') (e.g.,
"must be a positive number") and when hasError('required') (e.g., "is
required"). Locate the frequency field validators in the component (frequency:
[...] with Validators.pattern, Validators.min(1), Validators.required) and
consider simplifying by removing the redundant pattern validator and relying on
Validators.min(1) plus Validators.required if you want to enforce positive
integers.
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts`:
- Around line 260-266: The submit() method is missing error handling for the
updateDelinquencyBucket API call; add an error callback to the subscribe on
this.productsService.updateDelinquencyBucket(this.delinquencyBucketId,
this.payloadData) that prevents navigation on failure and surfaces feedback to
the user (e.g., call an existing notification/alert service like
this.alertService.error(err.message) or set a component error state such as
this.errorMessage = err.message), and optionally handle cleanup (hide any
loading indicator) in the error path so failures are visible and the
router.navigate([...]) only runs inside the success callback.
In `@src/assets/translations/pt-PT.json`:
- Line 889: The translation value for the "Working Capital" key is inconsistent
with other pt-PT entries; update the value for the "Working Capital" key in
src/assets/translations/pt-PT.json from "Capital de giro" to use the same
capitalization as other entries: "Capital de Giro".
---
Duplicate comments:
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.ts`:
- Around line 208-230: The payloadData getter can return undefined if neither
isRegularBucket nor isWorkingCapitalBucket is true and also currently nests the
name inside minimumPaymentPeriodAndRule; update the payloadData getter to always
return a valid object (add a default/fallback return that throws or returns a
safe payload) and when building the working-capital branch
(isWorkingCapitalBucket) create minimumPaymentPeriodAndRule from
bucketForm.value excluding the name field (use a copy without 'name') while
still setting name at the top level; ensure ranges are built from
rangesDataSource.map(item => item.rangeId) to keep behavior consistent.
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.html`:
- Around line 69-74: The template currently checks
bucketForm.controls.minimumPayment.hasError('pattern') but the control is
validated with Validators.min(0.01), so the min error will never be caught;
update the edit-bucket.component.html check to use hasError('min') (or include
both hasError('pattern') || hasError('min')) for the minimumPayment mat-error
block so the correct validation message displays when minimum value validation
(Validators.min(0.01)) fails.
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts`:
- Around line 232-254: The payloadData getter can return undefined if neither
isRegularBucket nor isWorkingCapitalBucket is true and for working-capital
buckets it duplicates the name inside minimumPaymentPeriodAndRule; update the
payloadData getter to (1) throw a clear error when no bucket type matches so
submit() never sends undefined, and (2) for the isWorkingCapitalBucket branch
create payload from this.bucketForm.getRawValue() but remove the name property
before assigning minimumPaymentPeriodAndRule (keep name only at the top level).
Reference: payloadData getter, isRegularBucket, isWorkingCapitalBucket,
this.bucketForm.getRawValue(), and this.rangesDataSource.
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/view-bucket/view-bucket.component.ts`:
- Around line 16-27: The component ViewBucketComponent currently imports
FormatNumberPipe in its standalone imports array (imports: [...,
FormatNumberPipe]) but the pipe is not declared standalone; either make the pipe
standalone by adding standalone: true to its `@Pipe` decorator in FormatNumberPipe
(i.e. `@Pipe`({ name: 'formatNumber', standalone: true }) export class
FormatNumberPipe ...) or remove FormatNumberPipe from the imports array in
view-bucket.component.ts so the component relies on the module declaration;
update the code accordingly to keep declarations consistent.
---
Nitpick comments:
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.ts`:
- Around line 133-140: The frequency FormControl currently applies both
Validators.pattern('^(0*[1-9][0-9]*)$') and Validators.min(1), which duplicate
the same constraint; remove the redundant validator to clarify intent—either
keep Validators.pattern('^(0*[1-9][0-9]*)$') for strict positive-integer
formatting or keep Validators.min(1) if you want to allow non-integer numbers,
and update the frequency control definition in create-bucket.component.ts
accordingly (adjust the array that contains frequency validators).
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts`:
- Around line 97-122: The route.data subscription in the constructor
(this.route.data.subscribe(...)) is never cleaned up; change it to use Angular's
takeUntilDestroyed() (or the takeUntil + destroy$ pattern) so the observable is
unsubscribed when the component is torn down: replace
this.route.data.subscribe(...) with
this.route.data.pipe(takeUntilDestroyed()).subscribe(...) (or
pipe(takeUntil(this.destroy$)) and implement ngOnDestroy to next/complete
destroy$), keeping the same handler body and retain references to
delinquencyRangesData, delinquencyBucketData, delinquencyBucketId,
rangesDataSource, delinquencyRangesIds, frequencyTypeOptions and
minimumPaymentOptions.
- Around line 228-230: The method validForm() actually computes whether the
submit button should be disabled; rename validForm() to submitDisabled() and
update all references (templates, tests, and callers) accordingly; keep the
existing logic using isRegularBucket, dataWasChanged and bucketForm.valid so
submitDisabled() returns the current boolean for disabling the submit button.
Ensure any bindings like [disabled]="validForm()" are updated to
[disabled]="submitDisabled()".
In `@src/app/products/products.service.ts`:
- Around line 348-349: The method getDelinquencyBucketsTemplate currently
returns Observable<any> which we want to avoid long-term; create a tracked
refactor instead of changing types here by adding a short TODO comment inside
getDelinquencyBucketsTemplate referencing a new tracking ticket (e.g., "TODO:
tighten API response types — TRACK-XXXX") and leave the return signature as
Observable<any> for now; ensure the TODO mentions the function name
getDelinquencyBucketsTemplate so it’s discoverable during the sweeping
API-typing refactor.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 155d9016-4693-4e64-82b3-83a484e66fac
📒 Files selected for processing (28)
src/app/account-transfers/create-standing-instructions/create-standing-instructions.component.htmlsrc/app/products/manage-delinquency-buckets/delinquency-base.component.tssrc/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.htmlsrc/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.tssrc/app/products/manage-delinquency-buckets/delinquency-bucket/delinquency-bucket.component.htmlsrc/app/products/manage-delinquency-buckets/delinquency-bucket/delinquency-bucket.component.tssrc/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.htmlsrc/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.tssrc/app/products/manage-delinquency-buckets/delinquency-bucket/view-bucket/view-bucket.component.htmlsrc/app/products/manage-delinquency-buckets/delinquency-bucket/view-bucket/view-bucket.component.tssrc/app/products/manage-delinquency-buckets/delinquency-range/delinquency-range.component.resolver.tssrc/app/products/manage-delinquency-buckets/models/delinquency-models.tssrc/app/products/products-routing.module.tssrc/app/products/products.service.tssrc/app/shared/models/option-data.model.tssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
🚧 Files skipped from review as they are similar to previous changes (17)
- src/assets/translations/fr-FR.json
- src/app/products/manage-delinquency-buckets/models/delinquency-models.ts
- src/app/shared/models/option-data.model.ts
- src/assets/translations/es-MX.json
- src/assets/translations/it-IT.json
- src/assets/translations/lv-LV.json
- src/assets/translations/ne-NE.json
- src/app/products/manage-delinquency-buckets/delinquency-bucket/create-bucket/create-bucket.component.html
- src/assets/translations/cs-CS.json
- src/assets/translations/de-DE.json
- src/assets/translations/sw-SW.json
- src/app/products/manage-delinquency-buckets/delinquency-range/delinquency-range.component.resolver.ts
- src/assets/translations/ko-KO.json
- src/assets/translations/en-US.json
- src/assets/translations/lt-LT.json
- src/assets/translations/es-CL.json
- src/app/account-transfers/create-standing-instructions/create-standing-instructions.component.html
Comment on lines
+35
to
+40
| @if (bucketForm.controls.frequency.hasError('pattern')) { | ||
| <mat-error> | ||
| {{ 'labels.inputs.Frequency' | translate }} {{ 'labels.commons.must be' | translate }} | ||
| {{ 'labels.commons.a positive number' | translate }} | ||
| </mat-error> | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the form setup for frequency in edit-bucket component
rg -n 'frequency.*Validators' src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts -A3Repository: openMF/web-app
Length of output: 40
🏁 Script executed:
#!/bin/bash
# First, find the TypeScript file and check its content
cat -n src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts | head -100Repository: openMF/web-app
Length of output: 4192
🏁 Script executed:
#!/bin/bash
# Search for frequency field setup more broadly
rg -n 'frequency' src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts -B2 -A2Repository: openMF/web-app
Length of output: 900
🏁 Script executed:
#!/bin/bash
# Look for form initialization/builder patterns
rg -n 'FormBuilder|FormControl|Validators' src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts -B1 -A1Repository: openMF/web-app
Length of output: 1221
Add error checks for min and required validators on frequency field.
The frequency field has three validators (pattern, min, required) but the template only checks for pattern errors. Add error checks for the min and required validators to ensure users see error messages when they enter 0 (min validation fails) or leave the field empty:
Frequency validators in component (lines 147-154)
frequency: [
this.delinquencyBucketData.minimumPaymentPeriodAndRule.frequency,
[
Validators.pattern('^(0*[1-9][0-9]*)$'),
Validators.min(1),
Validators.required
]
]Additionally, the pattern validator ^(0*[1-9][0-9]*)$ is redundant with Validators.min(1) since both enforce positive integers—consider simplifying to just the min validator.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.html`
around lines 35 - 40, The template only checks
bucketForm.controls.frequency.hasError('pattern') but must also show messages
for 'min' and 'required'; update edit-bucket.component.html to add branches that
display a mat-error when bucketForm.controls.frequency.hasError('min') (e.g.,
"must be a positive number") and when hasError('required') (e.g., "is
required"). Locate the frequency field validators in the component (frequency:
[...] with Validators.pattern, Validators.min(1), Validators.required) and
consider simplifying by removing the redundant pattern validator and relying on
Validators.min(1) plus Validators.required if you want to enforce positive
integers.
Comment on lines
+260
to
+266
| submit() { | ||
| this.productsService.updateDelinquencyBucket(this.delinquencyBucketId, this.payloadData).subscribe(() => { | ||
| this.router.navigate(['../'], { | ||
| queryParams: { bucketType: this.delinquencyBucketType.value }, | ||
| relativeTo: this.route | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Add error handling for the API call.
The subscribe() has no error callback. If updateDelinquencyBucket fails, the user receives no feedback and the navigation silently doesn't occur. Consider adding error handling to display an appropriate message.
🩹 Proposed fix
submit() {
- this.productsService.updateDelinquencyBucket(this.delinquencyBucketId, this.payloadData).subscribe(() => {
- this.router.navigate(['../'], {
- queryParams: { bucketType: this.delinquencyBucketType.value },
- relativeTo: this.route
- });
- });
+ this.productsService.updateDelinquencyBucket(this.delinquencyBucketId, this.payloadData).subscribe({
+ next: () => {
+ this.router.navigate(['../'], {
+ queryParams: { bucketType: this.delinquencyBucketType.value },
+ relativeTo: this.route
+ });
+ },
+ error: (error) => {
+ // Handle error - e.g., display notification to user
+ console.error('Failed to update delinquency bucket:', error);
+ }
+ });
}📝 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
| submit() { | |
| this.productsService.updateDelinquencyBucket(this.delinquencyBucketId, this.payloadData).subscribe(() => { | |
| this.router.navigate(['../'], { | |
| queryParams: { bucketType: this.delinquencyBucketType.value }, | |
| relativeTo: this.route | |
| }); | |
| } | |
| }); | |
| submit() { | |
| this.productsService.updateDelinquencyBucket(this.delinquencyBucketId, this.payloadData).subscribe({ | |
| next: () => { | |
| this.router.navigate(['../'], { | |
| queryParams: { bucketType: this.delinquencyBucketType.value }, | |
| relativeTo: this.route | |
| }); | |
| }, | |
| error: (error) => { | |
| // Handle error - e.g., display notification to user | |
| console.error('Failed to update delinquency bucket:', error); | |
| } | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/manage-delinquency-buckets/delinquency-bucket/edit-bucket/edit-bucket.component.ts`
around lines 260 - 266, The submit() method is missing error handling for the
updateDelinquencyBucket API call; add an error callback to the subscribe on
this.productsService.updateDelinquencyBucket(this.delinquencyBucketId,
this.payloadData) that prevents navigation on failure and surfaces feedback to
the user (e.g., call an existing notification/alert service like
this.alertService.error(err.message) or set a component error state such as
this.errorMessage = err.message), and optionally handle cleanup (hide any
loading indicator) in the error path so failures are visible and the
router.navigate([...]) only runs inside the success callback.
| "move to previous working day": "mover para o dia útil anterior", | ||
| "move to next meeting day": "mover para o próximo dia de reunião" | ||
| "move to next meeting day": "mover para o próximo dia de reunião", | ||
| "Working Capital": "Capital de giro" |
There was a problem hiding this comment.
Align capitalization with existing “Capital de Giro” usage.
This key uses "Capital de giro" while other pt-PT entries use "Capital de Giro", which can create inconsistent UI labels.
💡 Suggested fix
- "Working Capital": "Capital de giro"
+ "Working Capital": "Capital de Giro"📝 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
| "Working Capital": "Capital de giro" | |
| "Working Capital": "Capital de Giro" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/pt-PT.json` at line 889, The translation value for
the "Working Capital" key is inconsistent with other pt-PT entries; update the
value for the "Working Capital" key in src/assets/translations/pt-PT.json from
"Capital de giro" to use the same capitalization as other entries: "Capital de
Giro".
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
Due the new WC we require to have Delinquency Management - Configuration with new attributes to be consider and to have two Delinquency bucket types: Regular and Working Capital
WEB-870
Screenshots
Screen.Recording.2026-03-16.at.5.44.45.PM.mov
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
Translations