WEB-657: Working Capital product near breach configuration#3487
Open
alberto-art3ch wants to merge 1 commit intoopenMF:devfrom
Open
WEB-657: Working Capital product near breach configuration#3487alberto-art3ch wants to merge 1 commit intoopenMF:devfrom
alberto-art3ch wants to merge 1 commit intoopenMF:devfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Note
|
| Cohort / File(s) | Summary |
|---|---|
Models src/app/products/loan-products/models/loan-product.model.ts |
Replaced scalar near‑breach fields with nearBreach?: NearBreach; added exported NearBreach interface; added name to Breach. |
Loan Product Summary src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.html, src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts |
Template: added "Enable Near Breach" row and conditional Near Breach display using mifosx-breach-display. TS: added enableNearBreach() method, commented placeholder in setCurrentValues(), and getAccountingRuleName() returns '' for working-capital products. |
Settings Step (UI & Logic) src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html, ...component.ts |
Template: added nearBreachId selector, threshold, eval frequency and freq-type inputs with clear control. TS: switched form to use nearBreachId, added nearBreachOptions, selectedNearBreach getter, removed dynamic control creation for eval fields, extended clearProperty. |
Near Breach CRUD (List/Create/View/Edit) src/app/products/loan-products/working-capital/near-breach-configuration/* |
Added standalone components, templates, and styles for listing, creating, viewing and editing near‑breaches; reactive forms with validation, table with filter/pagination/sort, delete dialog behavior, resolvers wiring and permission gating. |
Resolvers .../near-breach-template.resolver.ts, .../near-breach.resolver.ts, .../near-breaches.resolver.ts |
Added three route resolvers: template, single near‑breach by id, and collection of near‑breaches (delegating to ProductsService). |
Routing & Navigation src/app/products/products-routing.module.ts, src/app/products/products.component.html, src/app/products/products.component.ts |
Added near-breach-configurations route subtree with providers; added navigation item and expanded arrowBooleans size from 12→13. |
Service API src/app/products/products.service.ts |
Added ProductsService methods: list/get/create/update/delete for working‑capital near‑breach endpoints. |
Shared UI / Display src/app/shared/loan/breach-display/*, src/app/shared/input-positive-integer/* |
Breach display: added nearBreach input and template blocks to render near‑breach name/frequency/threshold. Input component: removed right-input class on inputs. |
Breach Configurations src/app/products/loan-products/working-capital/breach-configuration/* |
Added name field to breach create/edit/view templates and table display; included name in filter predicate and adjusted displayedColumns. |
Icons & Translations src/app/shared/icons.module.ts, src/assets/translations/* |
Registered faCheckDouble icon; added extensive i18n keys/translations across many locales for Near Breach, labels, validation messages, remittance/recipient strings and related menu text. |
Styles src/app/products/loan-products/working-capital/near-breach-configuration/*.scss |
Added SCSS files for layout, table and content-wrapping styles for new components. |
Sequence Diagram(s)
sequenceDiagram
participant User
participant Router
participant Resolver as NearBreachesResolver
participant UI as NearBreachListComponent
participant Service as ProductsService
participant API
User->>Router: navigate to /products/near-breach-configurations
Router->>Resolver: resolve()
Resolver->>Service: getWorkingCapitalNearBreaches()
Service->>API: GET /working-capital/near-breach
API-->>Service: nearBreaches[]
Service-->>Resolver: nearBreaches[]
Resolver-->>Router: resolved data
Router->>UI: instantiate with data
UI->>UI: render table (sort/paginate/filter)
User->>UI: click Delete(row)
UI->>Service: deleteWrokingCapitalNearBreach(id)
Service->>API: DELETE /working-capital/near-breach/:id
API-->>Service: 204
Service-->>UI: success -> refresh
UI->>Service: getWorkingCapitalNearBreaches()
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
- WEB-813: Working Capital Breach configuration #3461: overlaps working‑capital breach feature changes (models, display, routing/services).
- WEB-711: Working Capital product #3159: related loan-product summary and working‑capital product handling adjustments.
- WEB-455: add full term tranches configuration #2853: touches loan-product settings flow and model changes similar to near‑breach form/fields.
Suggested reviewers
- adamsaghy
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly and specifically describes the main feature addition: 'Near Breach Configuration' support for 'Working Capital' products, which aligns with the PR's core objective of adding UI support for near-breach management. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/loan-products/common/loan-product-summary/loan-product-summary.component.ts`:
- Around line 542-548: In setCurrentValues() avoid remapping an already-resolved
object by checking the current type/value of
this.loanProduct.nearBreachEvalFrequencyType before calling optionDataLookUp:
only call optionDataLookUp(this.loanProduct.nearBreachEvalFrequencyType,
this.loanProductsTemplate.periodFrequencyTypeOptions) and assign the result when
the existing value is a primitive/id (e.g. string or number) or when
optionDataLookUp actually returns a non-null OptionData; if the value is already
an OptionData-like object (has expected keys/id) skip the lookup and do not
overwrite it. This change touches the block guarded by
this.loanProductService.isWorkingCapital and uses optionDataLookUp and
nearBreachEvalFrequencyType to locate the code to update.
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html`:
- Around line 124-128: The label for the mifosx-input-positive-integer used with
loanProductSettingsForm.controls.nearBreachEvalFrequency is hardcoded; replace
the literal "Near Breach Evaluation Frequency" with the appropriate i18n
translation key and use the ngx-translate mechanism (translate pipe or
TranslateService) so the [inputLabel] receives a translated string (e.g., bind
to the translation key for nearBreachEvalFrequency) instead of a plain English
literal.
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts`:
- Around line 962-965: In loan-product-settings-step.component.ts update the
reset behavior so breachId is set to null instead of an empty string: in the
loanProductSettingsForm.patchValue call (the block that currently patches
breachId: '' and enableNearBreach: false) change breachId to null so the
form/model sends a proper nullable numeric ID when clearing an existing breach;
ensure any downstream code that relies on form value checks for null instead of
empty string if necessary.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3f625f5a-75b1-451a-9e24-1dead706bcab
📒 Files selected for processing (4)
src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.htmlsrc/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts
Comment on lines
+124
to
+128
| <mifosx-input-positive-integer | ||
| class="flex-23" | ||
| [inputFormControl]="loanProductSettingsForm.controls.nearBreachEvalFrequency" | ||
| [inputLabel]="'Near Breach Evaluation Frequency'" | ||
| [isRequired]="true" |
There was a problem hiding this comment.
Localize the positive-integer label text.
Line 127 hardcodes a user-facing label string. Use the translation key here as well.
Suggested fix
<mifosx-input-positive-integer
class="flex-23"
[inputFormControl]="loanProductSettingsForm.controls.nearBreachEvalFrequency"
- [inputLabel]="'Near Breach Evaluation Frequency'"
+ [inputLabel]="'labels.inputs.Near Breach Evaluation Frequency' | translate"
[isRequired]="true"
[minVal]="'1'"
></mifosx-input-positive-integer>As per coding guidelines: "Use proper i18n variables from @ngx-translate/core for all user-facing strings instead of hardcoded text".
📝 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
| <mifosx-input-positive-integer | |
| class="flex-23" | |
| [inputFormControl]="loanProductSettingsForm.controls.nearBreachEvalFrequency" | |
| [inputLabel]="'Near Breach Evaluation Frequency'" | |
| [isRequired]="true" | |
| <mifosx-input-positive-integer | |
| class="flex-23" | |
| [inputFormControl]="loanProductSettingsForm.controls.nearBreachEvalFrequency" | |
| [inputLabel]="'labels.inputs.Near Breach Evaluation Frequency' | translate" | |
| [isRequired]="true" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html`
around lines 124 - 128, The label for the mifosx-input-positive-integer used
with loanProductSettingsForm.controls.nearBreachEvalFrequency is hardcoded;
replace the literal "Near Breach Evaluation Frequency" with the appropriate i18n
translation key and use the ngx-translate mechanism (translate pipe or
TranslateService) so the [inputLabel] receives a translated string (e.g., bind
to the translation key for nearBreachEvalFrequency) instead of a plain English
literal.
Comment on lines
+962
to
+965
| this.loanProductSettingsForm.patchValue({ | ||
| breachId: '', | ||
| enableNearBreach: false | ||
| }); |
There was a problem hiding this comment.
Reset breachId to null, not empty string.
Line 963 sets breachId to ''. For optional numeric IDs this can produce an invalid payload type when clearing an existing breach.
Suggested fix
- this.loanProductSettingsForm.patchValue({
- breachId: '',
+ this.loanProductSettingsForm.patchValue({
+ breachId: null,
enableNearBreach: false
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| this.loanProductSettingsForm.patchValue({ | |
| breachId: '', | |
| enableNearBreach: false | |
| }); | |
| this.loanProductSettingsForm.patchValue({ | |
| breachId: null, | |
| enableNearBreach: false | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts`
around lines 962 - 965, In loan-product-settings-step.component.ts update the
reset behavior so breachId is set to null instead of an empty string: in the
loanProductSettingsForm.patchValue call (the block that currently patches
breachId: '' and enableNearBreach: false) change breachId to null so the
form/model sends a proper nullable numeric ID when clearing an existing breach;
ensure any downstream code that relies on form value checks for null instead of
empty string if necessary.
alberto-art3ch
force-pushed
the
WEB-657/working-capital-product-near-breach-configuration
branch
from
7463fd8 to
bf29e21
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (5)
src/app/products/products.service.ts (1)
671-686: Fix theWrokingtypo in the newly added public method names.These method names are now part of the service API; keeping the typo will spread it further across components/resolvers.
🧹 Backward-compatible rename approach
- createWrokingCapitalNearBreach(payload: WorkingCapitalNearBreachPayload): Observable<WorkingCapitalNearBreach> { + createWorkingCapitalNearBreach(payload: WorkingCapitalNearBreachPayload): Observable<WorkingCapitalNearBreach> { return this.http.post(`/working-capital/near-breach`, payload); } - updateWrokingCapitalNearBreach(nearBreachId: number, payload: WorkingCapitalNearBreachPayload): Observable<WorkingCapitalNearBreach> { + updateWorkingCapitalNearBreach(nearBreachId: number, payload: WorkingCapitalNearBreachPayload): Observable<WorkingCapitalNearBreach> { return this.http.put(`/working-capital/near-breach/${nearBreachId}`, payload); } - deleteWrokingCapitalNearBreach(nearBreachId: number): Observable<void> { + deleteWorkingCapitalNearBreach(nearBreachId: number): Observable<void> { return this.http.delete(`/working-capital/near-breach/${nearBreachId}`); } + /** `@deprecated` use createWorkingCapitalNearBreach */ + createWrokingCapitalNearBreach(payload: WorkingCapitalNearBreachPayload): Observable<WorkingCapitalNearBreach> { + return this.createWorkingCapitalNearBreach(payload); + } + + /** `@deprecated` use updateWorkingCapitalNearBreach */ + updateWrokingCapitalNearBreach(nearBreachId: number, payload: WorkingCapitalNearBreachPayload): Observable<WorkingCapitalNearBreach> { + return this.updateWorkingCapitalNearBreach(nearBreachId, payload); + } + + /** `@deprecated` use deleteWorkingCapitalNearBreach */ + deleteWrokingCapitalNearBreach(nearBreachId: number): Observable<void> { + return this.deleteWorkingCapitalNearBreach(nearBreachId); + }🤖 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 671 - 686, The public method names contain a typo ("Wroking")—rename createWrokingCapitalNearBreach, updateWrokingCapitalNearBreach, and deleteWrokingCapitalNearBreach to createWorkingCapitalNearBreach, updateWorkingCapitalNearBreach, and deleteWorkingCapitalNearBreach respectively; update any internal references and tests to use the corrected names, and to remain backward-compatible add the old misspelled methods as thin deprecated wrappers that call the new correctly spelled methods (so consumers continue to work until they migrate).src/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.ts (1)
79-83: Consider moving route data subscription tongOnInit.Subscribing to route data in the constructor works but is unconventional. Moving it to
ngOnInitaligns with Angular lifecycle best practices.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.ts` around lines 79 - 83, Move the route.data subscription out of the constructor and into the component lifecycle: add OnInit to the class (import { OnInit } from '@angular/core' and implement OnInit), remove the subscription from the constructor, and put the subscription logic into ngOnInit where you set this.nearBreachesData from this.route.data (preserving the nullish fallback to []). Update the class signature to implement OnInit and ensure any necessary imports are added.src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.ts (1)
41-46: Move route data subscription tongOnInitfor consistency.Subscribing to route data in the constructor works but is unconventional. Moving it to
ngOnInitaligns with Angular lifecycle best practices and matches patterns used elsewhere in the codebase.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.ts` around lines 41 - 46, The subscription to route data should be moved out of the constructor into the component lifecycle: implement OnInit on the CreateNearBreachConfigurationComponent (add import and "implements OnInit"), remove the route.data.subscribe call from the constructor, and place that same subscription in ngOnInit so frequencyTypeOptions is set there; if you need to manage unsubscription consistently across the codebase, use the existing pattern (e.g., takeUntil or a Subscription field) when adding the subscription in ngOnInit.src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.html (2)
73-80: Add explicittype="submit"to the submit button.While the button will default to submit behavior inside a form, explicitly specifying
type="submit"improves clarity and ensures consistent behavior across browsers.♻️ Suggested fix
<button + type="submit" mat-raised-button color="primary" [disabled]="!nearBreachForm.valid" *mifosxHasPermission="'CREATE_WORKINGCAPITALNEARBREACH'" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.html` around lines 73 - 80, The submit button in create-near-breach-configuration.component.html currently relies on default form behavior; update the <button> element that uses mat-raised-button, color="primary", [disabled]="!nearBreachForm.valid" and *mifosxHasPermission="'CREATE_WORKINGCAPITALNEARBREACH'" to include an explicit type="submit" attribute so the form (nearBreachForm) submits consistently across browsers.
24-30: Hardcoded string should use i18n translation.The
inputLabelproperty uses a hardcoded string'Frequency'instead of using the translation pipe for consistency with other labels in this form.♻️ Suggested fix
<mifosx-input-positive-integer class="flex-48" [inputFormControl]="nearBreachForm.controls.nearBreachFrequency" - [inputLabel]="'Frequency'" + [inputLabel]="'labels.inputs.Frequency' | translate" [isRequired]="true" [minVal]="'1'" ></mifosx-input-positive-integer>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/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.html` around lines 24 - 30, The label for the mifosx-input-positive-integer in create-near-breach-configuration.component (property inputLabel on the component bound to nearBreachForm.controls.nearBreachFrequency) is hardcoded as 'Frequency'; replace it with a translated key using the ngx-translate pipe (e.g. use the appropriate i18n key like 'NEAR_BREACH.FREQUENCY' or an existing shared key) so the template binds [inputLabel] to the translation (use the | translate pipe) instead of the literal string.
🤖 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/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.ts`:
- Around line 66-74: The nearBreachThreshold form control uses Validators.min(0)
but the HTML input enforces min="0.01", causing inconsistent validation; update
the validator on the nearBreachThreshold control (the array that currently
includes amountValueValidator(), Validators.required, Validators.min(0),
Validators.max(100.0)) to use Validators.min(0.01) (or the same minimum used in
the template) so the reactive form and the input attribute match, keeping
amountValueValidator() and Validators.max(100.0) unchanged.
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.ts`:
- Around line 19-27: The `@Component` decorator for the
EditNearBreachConfigurationComponent (selector
'mifosx-edit-near-breach-configuration') is using the imports array but is
missing standalone: true; add standalone: true to the `@Component` metadata so the
component is declared as a standalone component (alongside the existing imports
and template/style fields).
- Line 42: Remove the debug console.log in EditNearBreachConfigurationComponent
(the line calling console.log(this.nearBreachData))—delete that statement (or
replace with the app's proper logger if persistent logging is required) so no
console.debug remains in edit-near-breach-configuration.component.ts.
- Around line 62-65: The form control nearBreachFrequencyType in
EditNearBreachConfigurationComponent is initialized with
this.nearBreachData!.frequencyType.code but the template mat-option binds to
frequencyType.id; change the initializer to
this.nearBreachData!.frequencyType.id so the form control value matches the
option values and the dropdown pre-selects correctly when editing.
- Around line 78-88: The submit() method accesses this.nearBreachData.id without
guarding against nearBreachData being null and logs errors to console; import
and inject ErrorHandlerService into the component, add a null-check at the start
of submit() (return early and call ErrorHandlerService.handleError or similar if
this.nearBreachData is null), and replace console.error in the
productsService.updateWrokingCapitalNearBreach subscription error handler with a
call to the injected ErrorHandlerService to match the create component's error
handling pattern.
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.ts`:
- Around line 107-121: The delete method currently nests subscriptions
(deleteDataTableDialogRef.afterClosed().subscribe ->
productsService.deleteWrokingCapitalNearBreach().subscribe ->
productsService.getWorkingCapitalNearBreaches().subscribe); refactor to use RxJS
operators: pipe the afterClosed() Observable, filter for response?.delete,
switchMap to productsService.deleteWrokingCapitalNearBreach(item.id), then
switchMap to productsService.getWorkingCapitalNearBreaches(), and subscribe once
to update this.nearBreachesData / this.dataSource; also add error handling via
catchError (or tap+finalize) to log or surface errors and consider take(1) to
auto-complete the stream. Ensure you reference the existing functions delete,
deleteDataTableDialogRef.afterClosed,
productsService.deleteWrokingCapitalNearBreach, and
productsService.getWorkingCapitalNearBreaches when making the change.
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/near-breach-template.resolver.ts`:
- Around line 16-29: The NearBreachTemplateResolver class is unused (routing
uses BreachTemplateResolver for near-breach create/edit), so either delete this
redundant resolver and remove its duplicate imports from the routing module, or
update the route configs to reference NearBreachTemplateResolver instead of
BreachTemplateResolver; locate the resolver by the class name
NearBreachTemplateResolver and its use of
ProductsService.getWorkingCapitalBreachTemplate(), and update the routing module
where BreachTemplateResolver is imported/declared to either remove the unused
import or swap it to NearBreachTemplateResolver accordingly.
In `@src/app/products/products-routing.module.ts`:
- Around line 1054-1057: The import NearBreachTemplateResolver is present but
not registered in the module's providers array, causing an unused-import or
missing-provider issue; either add NearBreachTemplateResolver to the providers
array alongside BreachResolver, NearBreachesResolver, and NearBreachResolver (so
the resolver is available for DI and route use), or remove the
NearBreachTemplateResolver import if it's not referenced by any route or code.
Locate the module where providers are declared (providers array) and update it
to include NearBreachTemplateResolver OR delete the unused
NearBreachTemplateResolver import to avoid confusion.
In `@src/assets/translations/es-CL.json`:
- Line 1214: Unify the Spanish term for "Near Breach" by replacing the
inconsistent "Cercano/Cercanos" translations with a single chosen term (e.g.,
"Inminente"/"Inminentes"); update the JSON entries that contain "Near Breach
Configuration" and the other "Near Breach" keys (the keys currently translated
with "cercano"/"cercanos") so they all use the same Spanish wording and
pluralization ("Inminente"/"Inminentes") to ensure consistent UI text.
---
Nitpick comments:
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.html`:
- Around line 73-80: The submit button in
create-near-breach-configuration.component.html currently relies on default form
behavior; update the <button> element that uses mat-raised-button,
color="primary", [disabled]="!nearBreachForm.valid" and
*mifosxHasPermission="'CREATE_WORKINGCAPITALNEARBREACH'" to include an explicit
type="submit" attribute so the form (nearBreachForm) submits consistently across
browsers.
- Around line 24-30: The label for the mifosx-input-positive-integer in
create-near-breach-configuration.component (property inputLabel on the component
bound to nearBreachForm.controls.nearBreachFrequency) is hardcoded as
'Frequency'; replace it with a translated key using the ngx-translate pipe (e.g.
use the appropriate i18n key like 'NEAR_BREACH.FREQUENCY' or an existing shared
key) so the template binds [inputLabel] to the translation (use the | translate
pipe) instead of the literal string.
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.ts`:
- Around line 41-46: The subscription to route data should be moved out of the
constructor into the component lifecycle: implement OnInit on the
CreateNearBreachConfigurationComponent (add import and "implements OnInit"),
remove the route.data.subscribe call from the constructor, and place that same
subscription in ngOnInit so frequencyTypeOptions is set there; if you need to
manage unsubscription consistently across the codebase, use the existing pattern
(e.g., takeUntil or a Subscription field) when adding the subscription in
ngOnInit.
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.ts`:
- Around line 79-83: Move the route.data subscription out of the constructor and
into the component lifecycle: add OnInit to the class (import { OnInit } from
'@angular/core' and implement OnInit), remove the subscription from the
constructor, and put the subscription logic into ngOnInit where you set
this.nearBreachesData from this.route.data (preserving the nullish fallback to
[]). Update the class signature to implement OnInit and ensure any necessary
imports are added.
In `@src/app/products/products.service.ts`:
- Around line 671-686: The public method names contain a typo ("Wroking")—rename
createWrokingCapitalNearBreach, updateWrokingCapitalNearBreach, and
deleteWrokingCapitalNearBreach to createWorkingCapitalNearBreach,
updateWorkingCapitalNearBreach, and deleteWorkingCapitalNearBreach respectively;
update any internal references and tests to use the corrected names, and to
remain backward-compatible add the old misspelled methods as thin deprecated
wrappers that call the new correctly spelled methods (so consumers continue to
work until they migrate).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 46216368-7f17-42a9-ab4c-2b22c1266073
📒 Files selected for processing (38)
src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.htmlsrc/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.tssrc/app/products/loan-products/models/loan-product.model.tssrc/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.htmlsrc/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.scsssrc/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.tssrc/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.htmlsrc/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.scsssrc/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.tssrc/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.htmlsrc/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.scsssrc/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.tssrc/app/products/loan-products/working-capital/near-breach-configuration/near-breach-template.resolver.tssrc/app/products/loan-products/working-capital/near-breach-configuration/near-breach.resolver.tssrc/app/products/loan-products/working-capital/near-breach-configuration/near-breaches.resolver.tssrc/app/products/loan-products/working-capital/near-breach-configuration/view-near-breach-configuration/view-near-breach-configuration.component.htmlsrc/app/products/loan-products/working-capital/near-breach-configuration/view-near-breach-configuration/view-near-breach-configuration.component.scsssrc/app/products/loan-products/working-capital/near-breach-configuration/view-near-breach-configuration/view-near-breach-configuration.component.tssrc/app/products/products-routing.module.tssrc/app/products/products.component.htmlsrc/app/products/products.component.tssrc/app/products/products.service.tssrc/app/shared/icons.module.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 due to trivial changes (17)
- src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.scss
- src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.scss
- src/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.scss
- src/app/products/loan-products/working-capital/near-breach-configuration/view-near-breach-configuration/view-near-breach-configuration.component.scss
- src/app/shared/icons.module.ts
- src/assets/translations/fr-FR.json
- src/assets/translations/lv-LV.json
- src/assets/translations/en-US.json
- src/assets/translations/de-DE.json
- src/assets/translations/it-IT.json
- src/assets/translations/pt-PT.json
- src/assets/translations/ko-KO.json
- src/assets/translations/ne-NE.json
- src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html
- src/assets/translations/lt-LT.json
- src/assets/translations/sw-SW.json
- src/assets/translations/cs-CS.json
🚧 Files skipped from review as they are similar to previous changes (3)
- src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts
- src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts
- src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.html
👮 Files not reviewed due to content moderation or server errors (4)
- src/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.html
- src/app/products/loan-products/working-capital/near-breach-configuration/view-near-breach-configuration/view-near-breach-configuration.component.ts
- src/assets/translations/es-MX.json
- src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.html
Comment on lines
+66
to
+74
| nearBreachThreshold: [ | ||
| '', | ||
| [ | ||
| amountValueValidator(), | ||
| Validators.required, | ||
| Validators.min(0), | ||
| Validators.max(100.0) | ||
| ] | ||
| ] |
There was a problem hiding this comment.
Validator mismatch: Validators.min(0) vs HTML min="0.01".
The TypeScript validator allows values from 0, but the HTML input specifies min="0.01". This inconsistency could cause confusing validation behavior. The threshold should likely not allow 0% (which would mean no threshold).
🔧 Suggested fix
nearBreachThreshold: [
'',
[
amountValueValidator(),
Validators.required,
- Validators.min(0),
+ Validators.min(0.01),
Validators.max(100.0)
]
]📝 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
| nearBreachThreshold: [ | |
| '', | |
| [ | |
| amountValueValidator(), | |
| Validators.required, | |
| Validators.min(0), | |
| Validators.max(100.0) | |
| ] | |
| ] | |
| nearBreachThreshold: [ | |
| '', | |
| [ | |
| amountValueValidator(), | |
| Validators.required, | |
| Validators.min(0.01), | |
| Validators.max(100.0) | |
| ] | |
| ] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.ts`
around lines 66 - 74, The nearBreachThreshold form control uses
Validators.min(0) but the HTML input enforces min="0.01", causing inconsistent
validation; update the validator on the nearBreachThreshold control (the array
that currently includes amountValueValidator(), Validators.required,
Validators.min(0), Validators.max(100.0)) to use Validators.min(0.01) (or the
same minimum used in the template) so the reactive form and the input attribute
match, keeping amountValueValidator() and Validators.max(100.0) unchanged.
Comment on lines
+19
to
+27
| @Component({ | ||
| selector: 'mifosx-edit-near-breach-configuration', | ||
| templateUrl: './edit-near-breach-configuration.component.html', | ||
| styleUrl: './edit-near-breach-configuration.component.scss', | ||
| imports: [ | ||
| ...STANDALONE_SHARED_IMPORTS, | ||
| InputPositiveIntegerComponent | ||
| ] | ||
| }) |
There was a problem hiding this comment.
Missing standalone: true in component decorator.
The component uses the imports array but is missing the standalone: true property. In Angular 15+, standalone components must explicitly declare standalone: true when using the imports array.
🐛 Proposed fix
`@Component`({
selector: 'mifosx-edit-near-breach-configuration',
templateUrl: './edit-near-breach-configuration.component.html',
styleUrl: './edit-near-breach-configuration.component.scss',
+ standalone: true,
imports: [
...STANDALONE_SHARED_IMPORTS,
InputPositiveIntegerComponent
]
})📝 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
| @Component({ | |
| selector: 'mifosx-edit-near-breach-configuration', | |
| templateUrl: './edit-near-breach-configuration.component.html', | |
| styleUrl: './edit-near-breach-configuration.component.scss', | |
| imports: [ | |
| ...STANDALONE_SHARED_IMPORTS, | |
| InputPositiveIntegerComponent | |
| ] | |
| }) | |
| `@Component`({ | |
| selector: 'mifosx-edit-near-breach-configuration', | |
| templateUrl: './edit-near-breach-configuration.component.html', | |
| styleUrl: './edit-near-breach-configuration.component.scss', | |
| standalone: true, | |
| imports: [ | |
| ...STANDALONE_SHARED_IMPORTS, | |
| InputPositiveIntegerComponent | |
| ] | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.ts`
around lines 19 - 27, The `@Component` decorator for the
EditNearBreachConfigurationComponent (selector
'mifosx-edit-near-breach-configuration') is using the imports array but is
missing standalone: true; add standalone: true to the `@Component` metadata so the
component is declared as a standalone component (alongside the existing imports
and template/style fields).
| ngOnInit(): void { | ||
| this.route.data.subscribe((data: { nearBreachData: NearBreach; breachTemplate: any }) => { | ||
| this.nearBreachData = data.nearBreachData; | ||
| console.log(this.nearBreachData); |
There was a problem hiding this comment.
Remove debug console.log statement.
Debug logging should be removed before merging.
🧹 Proposed fix
this.nearBreachData = data.nearBreachData;
- console.log(this.nearBreachData);
this.frequencyTypeOptions = data.breachTemplate.breachFrequencyTypeOptions || [];📝 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
| console.log(this.nearBreachData); | |
| this.nearBreachData = data.nearBreachData; | |
| this.frequencyTypeOptions = data.breachTemplate.breachFrequencyTypeOptions || []; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.ts`
at line 42, Remove the debug console.log in EditNearBreachConfigurationComponent
(the line calling console.log(this.nearBreachData))—delete that statement (or
replace with the app's proper logger if persistent logging is required) so no
console.debug remains in edit-near-breach-configuration.component.ts.
Comment on lines
+62
to
+65
| nearBreachFrequencyType: [ | ||
| this.nearBreachData!.frequencyType.code, | ||
| Validators.required | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify what value the mat-option uses in both create and edit templates
echo "=== Create template mat-option value ==="
rg -n "frequencyType\.(id|code)" --type html src/app/products/loan-products/working-capital/near-breach-configuration/Repository: openMF/web-app
Length of output: 1392
Change frequencyType.code to frequencyType.id in form initialization.
The mat-option elements in the template bind values to frequencyType.id, but the form control is initialized with frequencyType.code. This mismatch prevents the dropdown from pre-selecting the correct frequency type when editing.
🐛 Fix
nearBreachFrequencyType: [
- this.nearBreachData!.frequencyType.code,
+ this.nearBreachData!.frequencyType.id,
Validators.required
],📝 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
| nearBreachFrequencyType: [ | |
| this.nearBreachData!.frequencyType.code, | |
| Validators.required | |
| ], | |
| nearBreachFrequencyType: [ | |
| this.nearBreachData!.frequencyType.id, | |
| Validators.required | |
| ], |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.ts`
around lines 62 - 65, The form control nearBreachFrequencyType in
EditNearBreachConfigurationComponent is initialized with
this.nearBreachData!.frequencyType.code but the template mat-option binds to
frequencyType.id; change the initializer to
this.nearBreachData!.frequencyType.id so the form control value matches the
option values and the dropdown pre-selects correctly when editing.
Comment on lines
+78
to
+88
| submit(): void { | ||
| const payload = this.nearBreachForm.getRawValue(); | ||
| this.productsService.updateWrokingCapitalNearBreach(this.nearBreachData.id, payload).subscribe({ | ||
| next: () => { | ||
| this.router.navigate(['../'], { relativeTo: this.route }); | ||
| }, | ||
| error: (err) => { | ||
| console.error('Failed to update near breach configuration', err); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Inconsistent error handling and potential null dereference.
Two issues:
this.nearBreachData.idis accessed without null check, butnearBreachDatais typed asNearBreach | null(Line 36).- Error handling logs to console instead of using
ErrorHandlerServicelike the create component does.
🔧 Proposed fix
First, add the ErrorHandlerService import and injection:
import { InputPositiveIntegerComponent } from 'app/shared/input-positive-integer/input-positive-integer.component';
+import { ErrorHandlerService } from 'app/core/error-handler/error-handler.service';
+import { catchError } from 'rxjs'; private productsService = inject(ProductsService);
+ private errorHandler = inject(ErrorHandlerService);Then update the submit method:
submit(): void {
const payload = this.nearBreachForm.getRawValue();
- this.productsService.updateWrokingCapitalNearBreach(this.nearBreachData.id, payload).subscribe({
- next: () => {
- this.router.navigate(['../'], { relativeTo: this.route });
- },
- error: (err) => {
- console.error('Failed to update near breach configuration', err);
- }
- });
+ this.productsService
+ .updateWrokingCapitalNearBreach(this.nearBreachData!.id, payload)
+ .pipe(catchError((error) => this.errorHandler.handleError(error, 'Near Breach Configuration Update')))
+ .subscribe({
+ next: () => {
+ this.router.navigate(['../'], { relativeTo: this.route });
+ }
+ });
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.ts`
around lines 78 - 88, The submit() method accesses this.nearBreachData.id
without guarding against nearBreachData being null and logs errors to console;
import and inject ErrorHandlerService into the component, add a null-check at
the start of submit() (return early and call ErrorHandlerService.handleError or
similar if this.nearBreachData is null), and replace console.error in the
productsService.updateWrokingCapitalNearBreach subscription error handler with a
call to the injected ErrorHandlerService to match the create component's error
handling pattern.
Comment on lines
+107
to
+121
| delete(item: NearBreach): void { | ||
| const deleteDataTableDialogRef = this.dialog.open(DeleteDialogComponent, { | ||
| data: { deleteContext: ` the item of ${item.id}` } | ||
| }); | ||
| deleteDataTableDialogRef.afterClosed().subscribe((response: any) => { | ||
| if (response?.delete) { | ||
| this.productsService.deleteWrokingCapitalNearBreach(item.id).subscribe(() => { | ||
| this.productsService.getWorkingCapitalNearBreaches().subscribe((nearBreachesData: NearBreach[]) => { | ||
| this.nearBreachesData = nearBreachesData; | ||
| this.dataSource = new MatTableDataSource(this.nearBreachesData); | ||
| }); | ||
| }); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Nested subscriptions anti-pattern; missing error handling.
The delete method uses nested .subscribe() calls, which is an RxJS anti-pattern that makes error handling difficult and can lead to memory leaks. Use switchMap to chain the operations and add error handling.
♻️ Proposed refactor
First, add the imports:
import { MatDialog } from '@angular/material/dialog';
+import { filter, switchMap } from 'rxjs';Then refactor the delete method:
delete(item: NearBreach): void {
const deleteDataTableDialogRef = this.dialog.open(DeleteDialogComponent, {
data: { deleteContext: ` the item of ${item.id}` }
});
- deleteDataTableDialogRef.afterClosed().subscribe((response: any) => {
- if (response?.delete) {
- this.productsService.deleteWrokingCapitalNearBreach(item.id).subscribe(() => {
- this.productsService.getWorkingCapitalNearBreaches().subscribe((nearBreachesData: NearBreach[]) => {
- this.nearBreachesData = nearBreachesData;
- this.dataSource = new MatTableDataSource(this.nearBreachesData);
- });
- });
- }
- });
+ deleteDataTableDialogRef.afterClosed().pipe(
+ filter((response: any) => response?.delete),
+ switchMap(() => this.productsService.deleteWrokingCapitalNearBreach(item.id)),
+ switchMap(() => this.productsService.getWorkingCapitalNearBreaches())
+ ).subscribe({
+ next: (nearBreachesData: NearBreach[]) => {
+ this.nearBreachesData = nearBreachesData;
+ this.dataSource.data = this.nearBreachesData;
+ this.dataSource.paginator = this.paginator;
+ this.dataSource.sort = this.sort;
+ },
+ error: (err) => {
+ console.error('Failed to delete near breach configuration', err);
+ }
+ });
}As per coding guidelines: "Use RxJS Observables and route resolvers to fetch and pass data" and "verify... clean observable patterns."
📝 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
| delete(item: NearBreach): void { | |
| const deleteDataTableDialogRef = this.dialog.open(DeleteDialogComponent, { | |
| data: { deleteContext: ` the item of ${item.id}` } | |
| }); | |
| deleteDataTableDialogRef.afterClosed().subscribe((response: any) => { | |
| if (response?.delete) { | |
| this.productsService.deleteWrokingCapitalNearBreach(item.id).subscribe(() => { | |
| this.productsService.getWorkingCapitalNearBreaches().subscribe((nearBreachesData: NearBreach[]) => { | |
| this.nearBreachesData = nearBreachesData; | |
| this.dataSource = new MatTableDataSource(this.nearBreachesData); | |
| }); | |
| }); | |
| } | |
| }); | |
| } | |
| import { MatDialog } from '@angular/material/dialog'; | |
| import { filter, switchMap } from 'rxjs'; | |
| delete(item: NearBreach): void { | |
| const deleteDataTableDialogRef = this.dialog.open(DeleteDialogComponent, { | |
| data: { deleteContext: ` the item of ${item.id}` } | |
| }); | |
| deleteDataTableDialogRef.afterClosed().pipe( | |
| filter((response: any) => response?.delete), | |
| switchMap(() => this.productsService.deleteWrokingCapitalNearBreach(item.id)), | |
| switchMap(() => this.productsService.getWorkingCapitalNearBreaches()) | |
| ).subscribe({ | |
| next: (nearBreachesData: NearBreach[]) => { | |
| this.nearBreachesData = nearBreachesData; | |
| this.dataSource.data = this.nearBreachesData; | |
| this.dataSource.paginator = this.paginator; | |
| this.dataSource.sort = this.sort; | |
| }, | |
| error: (err) => { | |
| console.error('Failed to delete near breach configuration', err); | |
| } | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.ts`
around lines 107 - 121, The delete method currently nests subscriptions
(deleteDataTableDialogRef.afterClosed().subscribe ->
productsService.deleteWrokingCapitalNearBreach().subscribe ->
productsService.getWorkingCapitalNearBreaches().subscribe); refactor to use RxJS
operators: pipe the afterClosed() Observable, filter for response?.delete,
switchMap to productsService.deleteWrokingCapitalNearBreach(item.id), then
switchMap to productsService.getWorkingCapitalNearBreaches(), and subscribe once
to update this.nearBreachesData / this.dataSource; also add error handling via
catchError (or tap+finalize) to log or surface errors and consider take(1) to
auto-complete the stream. Ensure you reference the existing functions delete,
deleteDataTableDialogRef.afterClosed,
productsService.deleteWrokingCapitalNearBreach, and
productsService.getWorkingCapitalNearBreaches when making the change.
Comment on lines
+16
to
+29
| /** | ||
| * Near Breaches data resolver. | ||
| */ | ||
| @Injectable() | ||
| export class NearBreachTemplateResolver { | ||
| private productsService = inject(ProductsService); | ||
|
|
||
| /** | ||
| * Returns the Near Breach Template data. | ||
| * @returns {Observable<any>} | ||
| */ | ||
| resolve(): Observable<any> { | ||
| return this.productsService.getWorkingCapitalBreachTemplate(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there's a dedicated near breach template endpoint in ProductsService
rg -n "getWorkingCapitalNearBreachTemplate|NearBreachTemplate" --type=ts src/app/products/Repository: openMF/web-app
Length of output: 390
🏁 Script executed:
#!/bin/bash
# Check how NearBreachTemplateResolver is configured in routing
rg -A 10 -B 5 "NearBreachTemplateResolver" src/app/products/products-routing.module.ts
# Also check the ProductsService to understand getWorkingCapitalBreachTemplate()
rg -A 5 "getWorkingCapitalBreachTemplate" --type=ts src/app/products/Repository: openMF/web-app
Length of output: 2755
🏁 Script executed:
#!/bin/bash
# Get the full route configuration where NearBreachTemplateResolver is used
rg -B 5 -A 15 "path: 'near-breach-configuration'" src/app/products/products-routing.module.ts | grep -A 15 "NearBreachTemplateResolver"
# Check the create component to understand data usage
rg -n "breachTemplate\|templateData" src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/Repository: openMF/web-app
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Get broader context around near-breach routes in routing module
rg -n "near-breach-configuration\|NearBreachTemplateResolver" src/app/products/products-routing.module.ts -A 3 -B 3
# Check the create component file structure
fd "create-near-breach-configuration" --type d src/app/products/
# Check what data keys are used in route resolvers
rg "resolve:" src/app/products/products-routing.module.ts -A 2 -B 2 | grep -A 2 -B 2 "near-breach"Repository: openMF/web-app
Length of output: 164
🏁 Script executed:
#!/bin/bash
# Check routing file size
wc -l src/app/products/products-routing.module.ts
# Read the entire routing file to find route configuration (if reasonable size)
cat src/app/products/products-routing.module.ts | head -300Repository: openMF/web-app
Length of output: 20974
🏁 Script executed:
#!/bin/bash
# Search for near-breach routes with context
rg -n "path: 'near-breach" src/app/products/products-routing.module.ts -A 20
# Also search for how the resolver is mapped in resolve object
rg -n "breachTemplate:" src/app/products/products-routing.module.ts -B 5 -A 5Repository: openMF/web-app
Length of output: 2765
🏁 Script executed:
#!/bin/bash
# Search for all usages of NearBreachTemplateResolver throughout the codebase
rg "NearBreachTemplateResolver" --type=ts src/
# Also check if it's imported but not used in routing
grep -r "NearBreachTemplateResolver" src/app/products/products-routing.module.tsRepository: openMF/web-app
Length of output: 517
Remove unused NearBreachTemplateResolver—it's never referenced in routing.
The resolver is defined and imported twice in the routing module but is not used in any route configuration. The near-breach create and edit routes both use BreachTemplateResolver instead (lines 966, 986). Either remove the unused resolver and its duplicate imports, or clarify the intended usage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/near-breach-template.resolver.ts`
around lines 16 - 29, The NearBreachTemplateResolver class is unused (routing
uses BreachTemplateResolver for near-breach create/edit), so either delete this
redundant resolver and remove its duplicate imports from the routing module, or
update the route configs to reference NearBreachTemplateResolver instead of
BreachTemplateResolver; locate the resolver by the class name
NearBreachTemplateResolver and its use of
ProductsService.getWorkingCapitalBreachTemplate(), and update the routing module
where BreachTemplateResolver is imported/declared to either remove the unused
import or swap it to NearBreachTemplateResolver accordingly.
Comment on lines
+1054
to
1057
| BreachResolver, | ||
| NearBreachesResolver, | ||
| NearBreachResolver | ||
| ] |
There was a problem hiding this comment.
Missing NearBreachTemplateResolver in providers array.
NearBreachTemplateResolver is imported at line 147 but not added to the providers array. While the create route uses breachTemplate: BreachTemplateResolver (which is already registered), the import suggests NearBreachTemplateResolver was intended to be used but wasn't registered. If it's not needed, remove the import to avoid confusion.
🔧 Proposed fix: Either add to providers or remove unused import
Option 1: Add to providers if needed:
BreachesResolver,
BreachResolver,
NearBreachesResolver,
- NearBreachResolver
+ NearBreachResolver,
+ NearBreachTemplateResolver
]Option 2: Remove unused import at line 147 if not needed:
-import { NearBreachTemplateResolver } from './loan-products/working-capital/near-breach-configuration/near-breach-template.resolver';📝 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
| BreachResolver, | |
| NearBreachesResolver, | |
| NearBreachResolver | |
| ] | |
| BreachResolver, | |
| NearBreachesResolver, | |
| NearBreachResolver, | |
| NearBreachTemplateResolver | |
| ] |
🤖 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 1054 - 1057, The
import NearBreachTemplateResolver is present but not registered in the module's
providers array, causing an unused-import or missing-provider issue; either add
NearBreachTemplateResolver to the providers array alongside BreachResolver,
NearBreachesResolver, and NearBreachResolver (so the resolver is available for
DI and route use), or remove the NearBreachTemplateResolver import if it's not
referenced by any route or code. Locate the module where providers are declared
(providers array) and update it to include NearBreachTemplateResolver OR delete
the unused NearBreachTemplateResolver import to avoid confusion.
bf29e21 to
ade44ca
Compare
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/assets/translations/cs-CS.json (1)
5-6:⚠️ Potential issue | 🟡 MinorConsolidate duplicate notImplemented translations to avoid maintenance drift.
Two separate paths define the same error message:
- Flat key at line 5-6:
error.resource.notImplementedat root level- Nested object at lines 4816-4822:
error.resource.notImplementedunder theerrorobjectRemove the nested structure and keep the flat keys, which are the format used consistently across all language files and match the translation lookup pattern in the codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/cs-CS.json` around lines 5 - 6, There are duplicate translations for the same key: keep the flat keys "error.resource.notImplemented.type" and "error.resource.notImplemented.message" and remove the nested duplicate under the error object; locate the nested object named error that contains its own resource.notImplemented entries and delete those nested keys so only the flat key variants remain (ensure no other nested notImplemented keys are left to avoid lookup mismatches).src/assets/translations/lt-LT.json (1)
9-10:⚠️ Potential issue | 🟡 MinorUnify duplicated
notImplementederror phrasing across key paths.Line 9/Line 10 and Line 4531/Line 4532 currently carry different Lithuanian messages for the same semantic error. This can surface inconsistent UX depending on which key is consumed.
💡 Suggested alignment (example)
- "error.resource.notImplemented.type": "Neįdiegta klaida", - "error.resource.notImplemented.message": "Funkcionalumas neįdiegtas!", + "error.resource.notImplemented.type": "Neįgyvendintos klaidos", + "error.resource.notImplemented.message": "Neįgyvendinta funkcija!""error": { "resource": { "notImplemented": { "type": "Neįgyvendintos klaidos", "message": "Neįgyvendinta funkcija!" } } }Also applies to: 4528-4533
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/lt-LT.json` around lines 9 - 10, Different Lithuanian strings are used for the same semantic error across keys "error.resource.notImplemented.type" and "error.resource.notImplemented.message" (and the duplicate pair around the other block at lines 4528-4533); pick a single canonical Lithuanian phrasing for the type and message (e.g., make both occurrences use the same "Neįdiegta klaida" and "Funkcionalumas neįdiegtas!" or another agreed wording), then replace the duplicated entries so both key paths use the identical strings; ensure you update both the top occurrence (the keys "error.resource.notImplemented.type" / "error.resource.notImplemented.message") and the duplicate block (the other notImplemented.type / notImplemented.message entries) so UX is consistent.
🧹 Nitpick comments (1)
src/assets/translations/es-CL.json (1)
3359-3359: Keep “Loan Originator” terminology consistent with existing “Crédito” wording.Line 3359 and Line 3758 use “Préstamo”, while most surrounding strings use “Crédito”. Consider aligning these two labels for consistency.
Also applies to: 3758-3758
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-CL.json` at line 3359, The translation entries for "Edit Loan Originator" (and the other label at line 3758) use "Préstamo" but should match the project's existing "Crédito" terminology; update the Spanish (es-CL) values for the keys "Edit Loan Originator" and the other corresponding key to use "Editar Originador de Crédito" (or replace "Préstamo" with "Crédito") so the wording is consistent with surrounding strings.
🤖 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/loan-products/working-capital/breach-configuration/create-breach-configuration/create-breach-configuration.component.ts`:
- Around line 52-57: The form control "name" in
CreateBreachConfigurationComponent is using Validators.required but still allows
whitespace-only strings; update the form to reject such values by either adding
a custom nonWhitespace validator (e.g., export function
nonWhitespaceValidator(control: AbstractControl) { return control.value &&
control.value.trim() ? null : { whitespace: true }; } and include it alongside
Validators.required for the "name" control) or ensure the value is trimmed in
the submit handler (e.g., call
this.form.get('name')!.setValue(this.form.get('name')!.value.trim()) before
building the payload in the submit method) so whitespace-only names cannot be
submitted; reference the "name" FormControl and the component's
submit/build-payload method when applying the change.
In
`@src/app/products/loan-products/working-capital/breach-configuration/edit-breach-configuration/edit-breach-configuration.component.ts`:
- Around line 51-56: The FormControl for the breach "name" currently uses only
Validators.required and therefore allows whitespace-only values; update the
"name" control in the form initialization (the FormGroup where `name: [
this.breachData!.name, [ Validators.required ] ]` is defined) to include a
whitespace-checking validator — e.g., add Validators.pattern(/\S/) or a small
custom validator that fails when value.trim() is empty — so submissions with
only spaces are rejected; ensure the same validator is applied wherever the form
is built or patched (e.g., in the component's form setup method) so the UI
blocks whitespace-only names.
In `@src/app/products/products.component.html`:
- Around line 255-268: The toggle icon is not keyboard-accessible; import
MatIconButton from '@angular/material/button' into the component and replace the
bare <fa-icon> instances used with the arrow toggle (referenced by
arrowBooleans[12] and arrowBooleansToggle(12)) by a mat-icon-button wrapper that
contains the <fa-icon>, sets [attr.aria-expanded]="arrowBooleans[12]" and
preserves the (click)="arrowBooleansToggle(12); $event.stopPropagation()"
handler so the control gains proper button semantics and accessibility.
In `@src/assets/translations/cs-CS.json`:
- Around line 2896-2902: Remove the malformed nested object entry for the
"Loading vendors" key in the cs-CS translation JSON and replace it by deleting
that object so the translator resolves to the existing correct string key
"Loading vendors..." (the valid entry with ellipses). Locate the nested
empty-key structure under the top-level key "Loading vendors" and delete that
entire block so the lookup for labels.inputs.Loading vendors... resolves to the
existing "Loading vendors..." string "Načítání dodavatelů...".
In `@src/assets/translations/es-CL.json`:
- Around line 2883-2889: The translation entry for the "Loading vendors" key is
malformed due to three nested empty keys; update the JSON so "Loading vendors"
maps directly to the Spanish string (e.g., "Loading vendors": "Cargando
proveedores...") by removing the intermediate empty objects and ensuring the key
is a direct string value, so i18n lookups use the flat key "Loading vendors".
In `@src/assets/translations/es-MX.json`:
- Line 1218: Multiple translations for the concept “Near Breach” are
inconsistent across the locale; update the translation values for the keys
including "Near Breach Configuration" and the other occurrences at the noted
instances so they all use a single normalized Spanish term (e.g., change
"Configuración de Incumplimiento Cercano" and any "Cuasi
Incumplimiento"/"Incumplimiento Cercano" variants to "Incumplimiento
inminente"), ensuring every translation string that represents "Near Breach" is
replaced to the chosen normalized phrase for consistency.
- Line 3038: Update the translation value for the "Undo Write-off" key to use
the locale’s established verb “Reversar” instead of “Deshacer”, e.g. change
"Deshacer castigo contable" to "Reversar castigo contable" for the "Undo
Write-off" entry and any other occurrences of that key (there’s another instance
flagged) so action-verb wording is consistent across the locale.
In `@src/assets/translations/it-IT.json`:
- Line 1216: Multiple Italian translations for "Near Breach" are used (e.g.,
"quasi-violazione", "Violazione prossima", "Quasi Violazione") causing
inconsistency; pick a single canonical term (e.g., "Quasi violazione") and
replace the value for the "Near Breach Configuration" key and every other
translation entry that refers to "Near Breach" (the entries currently showing
the three variants) so all values use the same exact string and capitalization;
ensure you update all occurrences across the translations file where those
variants appear.
- Around line 2882-2888: The "Loading vendors" translation is incorrectly nested
under multiple empty-string keys; update the translation object so the "Loading
vendors" key maps directly to the string "Caricamento fornitori..." (i.e.,
replace the nested empty-string object structure with a flat key-value pair for
"Loading vendors") so lookups for "Loading vendors" return the string rather
than an object.
In `@src/assets/translations/lt-LT.json`:
- Around line 2881-2887: The "Loading vendors" translation entry in lt-LT.json
is malformed with nested empty keys, so look up for the "Loading vendors" key
and replace the nested empty-object structure with a simple string value
("Kraunami tiekėjai...") as the translation for that key (i.e., ensure "Loading
vendors": "Kraunami tiekėjai..." instead of the current nested objects) so
standard translation lookups will resolve correctly.
In `@src/assets/translations/lv-LV.json`:
- Around line 4528-4535: There are duplicate keys for the same concept:
errors.error.resource.notImplemented.type and
errors.error.resource.notImplemented.message already exist with different
Latvian wording, so either remove this new error.resource.notImplemented.* block
or make it reuse the exact same strings as the existing
errors.error.resource.notImplemented.* entries; locate the duplicate by the
object keys "error.resource.notImplemented.type" and
"error.resource.notImplemented.message" in the diff and consolidate to a single
source of truth (keep one set of translations and delete or map the duplicate).
- Line 1216: Pick a single Latvian translation (either the variant using
"Tuva/Tuvu" or "Gandrīz") and make it consistent across all translation keys
that refer to "Near Breach" (start by updating the value for "Near Breach
Configuration"); search the translations for all occurrences of the English
phrase "Near Breach" and replace any alternating values
("Tuva"/"Tuvu"/"Gandrīz") with your chosen term so headings, form fields,
actions and help text use the exact same Latvian word everywhere (also update
the other listed occurrences so they match).
- Around line 2882-2888: The "Loading vendors" entry is an object with empty
nested keys instead of a plain string; replace the nested object for the
"Loading vendors" key with a flat string value "Ielādē piegādātājus..." so the
JSON entry becomes "Loading vendors": "Ielādē piegādātājus..." (matching the
style used by placeholders.Loading vendors...).
In `@src/assets/translations/ne-NE.json`:
- Line 1215: Standardize the Nepali translation for the "Near Breach" concept by
picking one term (either "नजिकको उल्लङ्घन" or "लगभग उल्लंघन") and replacing all
occurrences so they are consistent; update the key "Near Breach Configuration"
and the other keys referenced (the occurrences at the other indices) to use the
chosen single Nepali term, ensuring headings, actions and list labels use the
identical phrase everywhere.
---
Outside diff comments:
In `@src/assets/translations/cs-CS.json`:
- Around line 5-6: There are duplicate translations for the same key: keep the
flat keys "error.resource.notImplemented.type" and
"error.resource.notImplemented.message" and remove the nested duplicate under
the error object; locate the nested object named error that contains its own
resource.notImplemented entries and delete those nested keys so only the flat
key variants remain (ensure no other nested notImplemented keys are left to
avoid lookup mismatches).
In `@src/assets/translations/lt-LT.json`:
- Around line 9-10: Different Lithuanian strings are used for the same semantic
error across keys "error.resource.notImplemented.type" and
"error.resource.notImplemented.message" (and the duplicate pair around the other
block at lines 4528-4533); pick a single canonical Lithuanian phrasing for the
type and message (e.g., make both occurrences use the same "Neįdiegta klaida"
and "Funkcionalumas neįdiegtas!" or another agreed wording), then replace the
duplicated entries so both key paths use the identical strings; ensure you
update both the top occurrence (the keys "error.resource.notImplemented.type" /
"error.resource.notImplemented.message") and the duplicate block (the other
notImplemented.type / notImplemented.message entries) so UX is consistent.
---
Nitpick comments:
In `@src/assets/translations/es-CL.json`:
- Line 3359: The translation entries for "Edit Loan Originator" (and the other
label at line 3758) use "Préstamo" but should match the project's existing
"Crédito" terminology; update the Spanish (es-CL) values for the keys "Edit Loan
Originator" and the other corresponding key to use "Editar Originador de
Crédito" (or replace "Préstamo" with "Crédito") so the wording is consistent
with surrounding strings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 37c9e136-95b5-46f5-ac1e-a7eba68d7574
📒 Files selected for processing (48)
src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.htmlsrc/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.tssrc/app/products/loan-products/models/loan-product.model.tssrc/app/products/loan-products/working-capital/breach-configuration/breach-configuration.component.htmlsrc/app/products/loan-products/working-capital/breach-configuration/breach-configuration.component.tssrc/app/products/loan-products/working-capital/breach-configuration/create-breach-configuration/create-breach-configuration.component.htmlsrc/app/products/loan-products/working-capital/breach-configuration/create-breach-configuration/create-breach-configuration.component.tssrc/app/products/loan-products/working-capital/breach-configuration/edit-breach-configuration/edit-breach-configuration.component.htmlsrc/app/products/loan-products/working-capital/breach-configuration/edit-breach-configuration/edit-breach-configuration.component.tssrc/app/products/loan-products/working-capital/breach-configuration/view-breach-configuration/view-breach-configuration.component.htmlsrc/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.htmlsrc/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.scsssrc/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.tssrc/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.htmlsrc/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.scsssrc/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.tssrc/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.htmlsrc/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.scsssrc/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.tssrc/app/products/loan-products/working-capital/near-breach-configuration/near-breach-template.resolver.tssrc/app/products/loan-products/working-capital/near-breach-configuration/near-breach.resolver.tssrc/app/products/loan-products/working-capital/near-breach-configuration/near-breaches.resolver.tssrc/app/products/loan-products/working-capital/near-breach-configuration/view-near-breach-configuration/view-near-breach-configuration.component.htmlsrc/app/products/loan-products/working-capital/near-breach-configuration/view-near-breach-configuration/view-near-breach-configuration.component.scsssrc/app/products/loan-products/working-capital/near-breach-configuration/view-near-breach-configuration/view-near-breach-configuration.component.tssrc/app/products/products-routing.module.tssrc/app/products/products.component.htmlsrc/app/products/products.component.tssrc/app/products/products.service.tssrc/app/shared/icons.module.tssrc/app/shared/input-positive-integer/input-positive-integer.component.htmlsrc/app/shared/loan/breach-display/breach-display.component.htmlsrc/app/shared/loan/breach-display/breach-display.component.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 with no reviewable changes (1)
- src/app/shared/input-positive-integer/input-positive-integer.component.html
✅ Files skipped from review due to trivial changes (15)
- src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.scss
- src/app/products/loan-products/working-capital/breach-configuration/view-breach-configuration/view-breach-configuration.component.html
- src/app/products/loan-products/working-capital/near-breach-configuration/near-breaches.resolver.ts
- src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.scss
- src/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.scss
- src/app/products/loan-products/working-capital/near-breach-configuration/view-near-breach-configuration/view-near-breach-configuration.component.scss
- src/app/products/loan-products/working-capital/near-breach-configuration/view-near-breach-configuration/view-near-breach-configuration.component.html
- src/app/products/products.component.ts
- src/app/products/products-routing.module.ts
- src/app/products/loan-products/working-capital/near-breach-configuration/near-breach.resolver.ts
- src/app/products/loan-products/working-capital/near-breach-configuration/near-breach-template.resolver.ts
- src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html
- src/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.html
- src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.html
- src/app/products/products.service.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- src/app/shared/icons.module.ts
- src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.html
- src/app/products/loan-products/working-capital/near-breach-configuration/view-near-breach-configuration/view-near-breach-configuration.component.ts
- src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts
- src/app/products/loan-products/models/loan-product.model.ts
- src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.ts
- src/assets/translations/fr-FR.json
- src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.html
- src/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.ts
- src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.ts
- src/assets/translations/sw-SW.json
- src/assets/translations/ko-KO.json
- src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts
- src/assets/translations/pt-PT.json
- src/assets/translations/de-DE.json
- src/assets/translations/en-US.json
Comment on lines
+52
to
+57
| name: [ | ||
| '', | ||
| [ | ||
| Validators.required | ||
| ] | ||
| ], |
There was a problem hiding this comment.
Reject whitespace-only breach names.
Validators.required still accepts values like ' ', so this new required field can be submitted as a blank-looking name and then render empty in the breach list/detail views. Add a non-whitespace validator, or trim before building the payload.
💡 Suggested fix
name: [
'',
[
- Validators.required
+ Validators.required,
+ Validators.pattern(/\S/)
]
],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/loan-products/working-capital/breach-configuration/create-breach-configuration/create-breach-configuration.component.ts`
around lines 52 - 57, The form control "name" in
CreateBreachConfigurationComponent is using Validators.required but still allows
whitespace-only strings; update the form to reject such values by either adding
a custom nonWhitespace validator (e.g., export function
nonWhitespaceValidator(control: AbstractControl) { return control.value &&
control.value.trim() ? null : { whitespace: true }; } and include it alongside
Validators.required for the "name" control) or ensure the value is trimmed in
the submit handler (e.g., call
this.form.get('name')!.setValue(this.form.get('name')!.value.trim()) before
building the payload in the submit method) so whitespace-only names cannot be
submitted; reference the "name" FormControl and the component's
submit/build-payload method when applying the change.
Comment on lines
+51
to
+56
| name: [ | ||
| this.breachData!.name, | ||
| [ | ||
| Validators.required | ||
| ] | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/app/products/loan-products/working-capital/breach-configuration/edit-breach-configuration/edit-breach-configuration.component.ts | head -80Repository: openMF/web-app
Length of output: 3425
🏁 Script executed:
# Check if the file exists and get its size
wc -l src/app/products/loan-products/working-capital/breach-configuration/edit-breach-configuration/edit-breach-configuration.component.tsRepository: openMF/web-app
Length of output: 193
🏁 Script executed:
# Search for the submit method to understand error handling
rg -A 10 "submit\(" src/app/products/loan-products/working-capital/breach-configuration/edit-breach-configuration/edit-breach-configuration.component.tsRepository: openMF/web-app
Length of output: 429
🏁 Script executed:
# Search for any custom validators in the codebase
rg "trimValidator|blankValidator|nonBlankValidator" src/app/Repository: openMF/web-app
Length of output: 40
🏁 Script executed:
# Check related components for form validation patterns
fd -i ".*component.ts" src/app/products/loan-products/ | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && rg -A 3 "Validators.required" {} | head -10'Repository: openMF/web-app
Length of output: 569
🌐 Web query:
Angular Validators.required whitespace-only string validation behavior
💡 Result:
Validators.required does not trim strings. It only flags a control as required/empty when the value is null/undefined or has length === 0. So a whitespace-only string like " " has length > 0 and therefore passes Validators.required (i.e., it’s considered valid). [1], [2]
To treat whitespace-only as invalid, add another validator, e.g.:
Validators.pattern(/\S/)(requires at least one non-whitespace character), or- a custom
noWhitespaceValidatorthat checksvalue.trim().length === 0. [3]
Sources:
[1] https://stackoverflow.com/questions/42184800/how-to-validate-formarray-length-in-angular2
[2] https://lightrun.com/answers/angular-angular-minlength-validator-treats-zero-value-as-zero-length (links to Angular validators.ts)
[3] https://stackoverflow.com/questions/39236992/how-to-validate-white-spaces-empty-spaces-angular-2
🏁 Script executed:
# Check Angular's source or documentation in the project
rg "Validators\.required" --type ts src/app/ -B 2 -A 2 | head -40Repository: openMF/web-app
Length of output: 2885
🏁 Script executed:
# Look for any related tests or usage patterns
rg -A 5 "trim\(\)" src/app/shared/validators/ 2>/dev/null | head -30Repository: openMF/web-app
Length of output: 40
Add whitespace-only validation to the breach name field.
Validators.required accepts whitespace-only strings (e.g., " "), allowing effectively empty breach names to pass form validation. Since the submit error path only logs to console with no user feedback, these invalid submissions create a poor experience. Add a validator to reject whitespace-only input.
Suggested validator change
+import { AbstractControl, ValidationErrors } from '@angular/forms';
+
+const nonBlankValidator = (control: AbstractControl): ValidationErrors | null =>
+ typeof control.value === 'string' && control.value.trim().length === 0 ? { blank: true } : null;
+
private initForm(): void {
this.breachForm = this.formBuilder.group({
name: [
this.breachData!.name,
[
- Validators.required
+ Validators.required,
+ nonBlankValidator
]
],📝 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
| name: [ | |
| this.breachData!.name, | |
| [ | |
| Validators.required | |
| ] | |
| ], | |
| import { AbstractControl, ValidationErrors } from '@angular/forms'; | |
| const nonBlankValidator = (control: AbstractControl): ValidationErrors | null => | |
| typeof control.value === 'string' && control.value.trim().length === 0 ? { blank: true } : null; | |
| private initForm(): void { | |
| this.breachForm = this.formBuilder.group({ | |
| name: [ | |
| this.breachData!.name, | |
| [ | |
| Validators.required, | |
| nonBlankValidator | |
| ] | |
| ], |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/loan-products/working-capital/breach-configuration/edit-breach-configuration/edit-breach-configuration.component.ts`
around lines 51 - 56, The FormControl for the breach "name" currently uses only
Validators.required and therefore allows whitespace-only values; update the
"name" control in the form initialization (the FormGroup where `name: [
this.breachData!.name, [ Validators.required ] ]` is defined) to include a
whitespace-checking validator — e.g., add Validators.pattern(/\S/) or a small
custom validator that fails when value.trim() is empty — so submissions with
only spaces are rejected; ensure the same validator is applied wherever the form
is built or patched (e.g., in the component's form setup method) so the UI
blocks whitespace-only names.
Comment on lines
+255
to
+268
| <div class="menu-right-section"> | ||
| @if (!arrowBooleans[12]) { | ||
| <fa-icon | ||
| (click)="arrowBooleansToggle(12); $event.stopPropagation()" | ||
| icon="arrow-down" | ||
| size="md" | ||
| ></fa-icon> | ||
| } | ||
| @if (arrowBooleans[12]) { | ||
| <fa-icon | ||
| (click)="arrowBooleansToggle(12); $event.stopPropagation()" | ||
| icon="arrow-up" | ||
| size="md" | ||
| ></fa-icon> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check imports in products component
head -50 src/app/products/products.component.ts | grep -i "import"Repository: openMF/web-app
Length of output: 765
🏁 Script executed:
# Search for mat-icon-button usage patterns in the codebase
rg "mat-icon-button" src/app --type html -A 3 -B 1 | head -80Repository: openMF/web-app
Length of output: 7553
🏁 Script executed:
# Search for expand/collapse controls in the codebase
rg "arrow-up|arrow-down|aria-expanded" src/app --type html -B 3 -A 3 | head -100Repository: openMF/web-app
Length of output: 7487
🏁 Script executed:
# Check the full products component import section
sed -n '1,80p' src/app/products/products.component.tsRepository: openMF/web-app
Length of output: 3387
Add MatIconButton import and wrap the icon in a button with proper accessibility attributes.
The toggle control is a clickable <fa-icon> without button semantics, making it inaccessible to keyboard users. Import MatIconButton from @angular/material/button and wrap the icon in a mat-icon-button with [attr.aria-expanded] to match the established pattern used in similar expand/collapse controls throughout the codebase.
💡 Suggested fix
- Add to component imports:
import { MatIconButton } from '@angular/material/button';- Update the template:
<div class="menu-right-section">
- `@if` (!arrowBooleans[12]) {
- <fa-icon
- (click)="arrowBooleansToggle(12); $event.stopPropagation()"
- icon="arrow-down"
- size="md"
- ></fa-icon>
- }
- `@if` (arrowBooleans[12]) {
- <fa-icon
- (click)="arrowBooleansToggle(12); $event.stopPropagation()"
- icon="arrow-up"
- size="md"
- ></fa-icon>
- }
+ <button
+ mat-icon-button
+ type="button"
+ [attr.aria-label]="'labels.heading.Near Breach Configuration' | translate"
+ [attr.aria-expanded]="arrowBooleans[12]"
+ (click)="arrowBooleansToggle(12); $event.stopPropagation()"
+ >
+ <fa-icon [icon]="arrowBooleans[12] ? 'arrow-up' : 'arrow-down'" size="md"></fa-icon>
+ </button>
</div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/products/products.component.html` around lines 255 - 268, The toggle
icon is not keyboard-accessible; import MatIconButton from
'@angular/material/button' into the component and replace the bare <fa-icon>
instances used with the arrow toggle (referenced by arrowBooleans[12] and
arrowBooleansToggle(12)) by a mat-icon-button wrapper that contains the
<fa-icon>, sets [attr.aria-expanded]="arrowBooleans[12]" and preserves the
(click)="arrowBooleansToggle(12); $event.stopPropagation()" handler so the
control gains proper button semantics and accessibility.
Comment on lines
+2896
to
+2902
| "Loading vendors": { | ||
| "": { | ||
| "": { | ||
| "": "Načítání dodavatelů..." | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify which translation key the remittance UI actually requests for the vendor-loading state.
rg -n --glob 'src/**/*.html' --glob 'src/**/*.ts' "Loading vendors|Failed to load vendors|Unable to load vendors|vendors"Repository: openMF/web-app
Length of output: 2181
🏁 Script executed:
# First, check the file size to determine how to read it
wc -l src/assets/translations/cs-CS.jsonRepository: openMF/web-app
Length of output: 97
🏁 Script executed:
# Read the specific lines from the JSON file with context
sed -n '2890,2910p' src/assets/translations/cs-CS.json | cat -nRepository: openMF/web-app
Length of output: 812
🏁 Script executed:
# Let me see more context to understand the full path hierarchy
sed -n '2870,2910p' src/assets/translations/cs-CS.json | cat -nRepository: openMF/web-app
Length of output: 1924
🏁 Script executed:
# Let me see more context to understand the full JSON path hierarchy
sed -n '2850,2900p' src/assets/translations/cs-CS.json | cat -nRepository: openMF/web-app
Length of output: 2813
🏁 Script executed:
# Let me look at the section header to understand the full path
sed -n '2800,2860p' src/assets/translations/cs-CS.json | cat -nRepository: openMF/web-app
Length of output: 4097
🏁 Script executed:
# Let me search for the "inputs" section header to understand the full hierarchy
rg -n '"inputs"' src/assets/translations/cs-CS.json | head -20Repository: openMF/web-app
Length of output: 78
🏁 Script executed:
# Let me find the parent section by looking backwards from line 2896
sed -n '1410,1430p' src/assets/translations/cs-CS.json | cat -nRepository: openMF/web-app
Length of output: 936
🏁 Script executed:
# Let me check the structure between line 1410 and line 2896 to find intermediate sections
sed -n '2850,2910p' src/assets/translations/cs-CS.json | cat -nRepository: openMF/web-app
Length of output: 3150
🏁 Script executed:
# Let me verify the exact translation key format by checking how the translate pipe is used
rg -n 'Loading vendors' src/app/remittances/remittance-stepper/search-remittance-step/ -A 1 -B 1Repository: openMF/web-app
Length of output: 493
🏁 Script executed:
# Let me verify the other translation keys that are used in the code
rg -n '"Failed to load vendors"' src/assets/translations/cs-CS.jsonRepository: openMF/web-app
Length of output: 129
🏁 Script executed:
# Let me check the English translation file to see if it has the same structure issue
sed -n '2896,2902p' src/assets/translations/en-US.json | cat -nRepository: openMF/web-app
Length of output: 373
🏁 Script executed:
# Let me search for "Loading vendors" in the English translation file
rg -n 'Loading vendors' src/assets/translations/en-US.jsonRepository: openMF/web-app
Length of output: 112
🏁 Script executed:
# Let me check the context around line 2856 in the English file
sed -n '2850,2910p' src/assets/translations/en-US.json | cat -nRepository: openMF/web-app
Length of output: 3020
🏁 Script executed:
# Let me verify the exact difference in structure by comparing a properly formatted entry
sed -n '2872,2876p' src/assets/translations/cs-CS.json | cat -nRepository: openMF/web-app
Length of output: 388
🏁 Script executed:
# Let me search for "Loading vendors" with the three dots in the Czech file
rg -n 'Loading vendors\.\.\.' src/assets/translations/cs-CS.jsonRepository: openMF/web-app
Length of output: 115
🏁 Script executed:
# Let me check the context around line 3056 to see where this proper entry is located
sed -n '3050,3065p' src/assets/translations/cs-CS.json | cat -nRepository: openMF/web-app
Length of output: 889
Remove the malformed "Loading vendors" entry at lines 2896-2902.
The entry is incorrectly structured as a nested object with empty-key nesting instead of a string value. The code requests 'labels.inputs.Loading vendors...' (with three dots), which would resolve to an object at this location, causing the loading text to fail to render. A correctly formatted version already exists at line 3056 as "Loading vendors...": "Načítání dodavatelů...". Remove the malformed duplicate entry.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/cs-CS.json` around lines 2896 - 2902, Remove the
malformed nested object entry for the "Loading vendors" key in the cs-CS
translation JSON and replace it by deleting that object so the translator
resolves to the existing correct string key "Loading vendors..." (the valid
entry with ellipses). Locate the nested empty-key structure under the top-level
key "Loading vendors" and delete that entire block so the lookup for
labels.inputs.Loading vendors... resolves to the existing "Loading vendors..."
string "Načítání dodavatelů...".
Comment on lines
+2883
to
+2889
| "Loading vendors": { | ||
| "": { | ||
| "": { | ||
| "": "Cargando proveedores..." | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
Fix malformed translation key structure for Loading vendors....
Line 2883 introduces nested empty keys ("Loading vendors" -> "" -> "" -> ""), which is brittle and can break i18n lookups.
💡 Proposed fix
- "Loading vendors": {
- "": {
- "": {
- "": "Cargando proveedores..."
- }
- }
- },
+ "Loading vendors...": "Cargando proveedores...",📝 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
| "Loading vendors": { | |
| "": { | |
| "": { | |
| "": "Cargando proveedores..." | |
| } | |
| } | |
| }, | |
| "Loading vendors...": "Cargando proveedores...", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/es-CL.json` around lines 2883 - 2889, The translation
entry for the "Loading vendors" key is malformed due to three nested empty keys;
update the JSON so "Loading vendors" maps directly to the Spanish string (e.g.,
"Loading vendors": "Cargando proveedores...") by removing the intermediate empty
objects and ensuring the key is a direct string value, so i18n lookups use the
flat key "Loading vendors".
Comment on lines
+2881
to
+2887
| "Loading vendors": { | ||
| "": { | ||
| "": { | ||
| "": "Kraunami tiekėjai..." | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
Fix malformed Loading vendors translation structure.
Line 2881 introduces nested empty keys, so the text will not resolve through standard translation lookups.
💡 Proposed fix
- "Loading vendors": {
- "": {
- "": {
- "": "Kraunami tiekėjai..."
- }
- }
- },
+ "Loading vendors...": "Kraunami tiekėjai...",📝 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
| "Loading vendors": { | |
| "": { | |
| "": { | |
| "": "Kraunami tiekėjai..." | |
| } | |
| } | |
| }, | |
| "Loading vendors...": "Kraunami tiekėjai...", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/lt-LT.json` around lines 2881 - 2887, The "Loading
vendors" translation entry in lt-LT.json is malformed with nested empty keys, so
look up for the "Loading vendors" key and replace the nested empty-object
structure with a simple string value ("Kraunami tiekėjai...") as the translation
for that key (i.e., ensure "Loading vendors": "Kraunami tiekėjai..." instead of
the current nested objects) so standard translation lookups will resolve
correctly.
| "Minimum Active Period": "Minimālais aktīvais periods", | ||
| "Minimum Deposit Term": "Minimālais depozīta termiņš", | ||
| "Moratorium": "Moratorijs", | ||
| "Near Breach Configuration": "Tuvas pārkāpuma konfigurācija", |
There was a problem hiding this comment.
Use one Latvian term for “Near Breach” everywhere.
These keys currently alternate between Tuva/Tuvu and Gandrīz, so the same feature is presented as different concepts across headings, form fields, actions, and help text. Please pick one translation and reuse it consistently.
Also applies to: 2239-2242, 3244-3244, 3315-3315, 3535-3535, 3758-3758
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/lv-LV.json` at line 1216, Pick a single Latvian
translation (either the variant using "Tuva/Tuvu" or "Gandrīz") and make it
consistent across all translation keys that refer to "Near Breach" (start by
updating the value for "Near Breach Configuration"); search the translations for
all occurrences of the English phrase "Near Breach" and replace any alternating
values ("Tuva"/"Tuvu"/"Gandrīz") with your chosen term so headings, form fields,
actions and help text use the exact same Latvian word everywhere (also update
the other listed occurrences so they match).
Comment on lines
+2882
to
+2888
| "Loading vendors": { | ||
| "": { | ||
| "": { | ||
| "": "Ielādē piegādātājus..." | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
Keep Loading vendors... as a plain string key.
This is the only non-string entry in inputs, and the empty-key nesting makes this label easy to miss or resolve as an object instead of text. A flat key here is safer and matches the existing placeholders.Loading vendors... entry.
Suggested fix
- "Loading vendors": {
- "": {
- "": {
- "": "Ielādē piegādātājus..."
- }
- }
- },
+ "Loading vendors...": "Ielādē piegādātājus...",📝 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
| "Loading vendors": { | |
| "": { | |
| "": { | |
| "": "Ielādē piegādātājus..." | |
| } | |
| } | |
| }, | |
| "Loading vendors...": "Ielādē piegādātājus...", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/lv-LV.json` around lines 2882 - 2888, The "Loading
vendors" entry is an object with empty nested keys instead of a plain string;
replace the nested object for the "Loading vendors" key with a flat string value
"Ielādē piegādātājus..." so the JSON entry becomes "Loading vendors": "Ielādē
piegādātājus..." (matching the style used by placeholders.Loading vendors...).
Comment on lines
+4528
to
4535
| "error": { | ||
| "resource": { | ||
| "notImplemented": { | ||
| "type": "Neimplementēta kļūda", | ||
| "message": "Neimplementēta funkcionalitāte!" | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Don’t fork the not-implemented copy.
errors.error.resource.notImplemented.type/message already exists at Lines 9-10, but this new error.resource.notImplemented.* branch uses different Latvian wording. If both namespaces are still consumed, the same failure will render two different messages. Reuse the same text in both places or keep a single source of truth.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/lv-LV.json` around lines 4528 - 4535, There are
duplicate keys for the same concept: errors.error.resource.notImplemented.type
and errors.error.resource.notImplemented.message already exist with different
Latvian wording, so either remove this new error.resource.notImplemented.* block
or make it reuse the exact same strings as the existing
errors.error.resource.notImplemented.* entries; locate the duplicate by the
object keys "error.resource.notImplemented.type" and
"error.resource.notImplemented.message" in the diff and consolidate to a single
source of truth (keep one set of translations and delete or map the duplicate).
| "Minimum Active Period": "न्यूनतम सक्रिय अवधि", | ||
| "Minimum Deposit Term": "न्यूनतम जम्मा अवधि", | ||
| "Moratorium": "मोरेटोरियम", | ||
| "Near Breach Configuration": "नजिकको उल्लङ्घन कन्फिगरेसन", |
There was a problem hiding this comment.
Use one Nepali term for “Near Breach” everywhere.
These keys currently mix नजिकको उल्लङ्घन and लगभग उल्लंघन, so the same feature will show up under different names across headings, actions, and list pages.
Also applies to: 2238-2238, 3245-3245, 3359-3359, 3535-3535, 3758-3758
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/ne-NE.json` at line 1215, Standardize the Nepali
translation for the "Near Breach" concept by picking one term (either "नजिकको
उल्लङ्घन" or "लगभग उल्लंघन") and replacing all occurrences so they are
consistent; update the key "Near Breach Configuration" and the other keys
referenced (the occurrences at the other indices) to use the chosen single
Nepali term, ensuring headings, actions and list labels use the identical phrase
everywhere.
eb6b14c to
eabfb48
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/assets/translations/cs-CS.json (1)
3033-3039:⚠️ Potential issue | 🟠 MajorFix malformed
Loading vendorstranslation value type.Line [3033] currently maps
Loading vendorsto a nested object, not a string. This can break UI rendering where a string is expected.💡 Suggested fix
- "Loading vendors": { - "": { - "": { - "": "Načítání dodavatelů..." - } - } - } + "Loading vendors...": "Načítání dodavatelů..."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/cs-CS.json` around lines 3033 - 3039, The "Loading vendors" translation key in src/assets/translations/cs-CS.json is incorrectly mapped to a nested object; replace that nested object value with a plain string "Načítání dodavatelů..." so the "Loading vendors" key maps directly to a string (not an object) to match expected translation value types used by the UI.
🧹 Nitpick comments (1)
src/assets/translations/de-DE.json (1)
4204-4207: Avoid duplicating message keys across namespacesThese new entries under
labels.messagesmirror keys already present in rootmessages. Keeping both copies invites drift and inconsistent wording. Prefer one canonical location.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/de-DE.json` around lines 4204 - 4207, Duplicate message keys ("validationSaved", "workingCapitalDiscountUpdated", "unableToUpdateDiscount") were added under the labels.messages namespace while identical keys already exist in the root messages; remove these three keys from labels.messages and reuse the existing entries in root messages to avoid drift, or if the intent is to centralize under labels.messages, move the canonical translations from root messages into labels.messages and delete the root duplicates—ensure only one canonical entry remains for each key.
🤖 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/assets/translations/de-DE.json`:
- Line 3305: Replace the incorrect German orthography "Verstoss" with the
correct "Verstoß" in this locale file; specifically update the translation value
for the key "Breach Configurations" (and any other keys whose values currently
contain "Verstoss") to use "Verstoß" so de-DE orthography is consistent across
the file.
- Around line 3035-3041: The translation for the key "Loading vendors" currently
has an invalid nested-object shape; replace its value with a simple localized
string (e.g., "Anbieter werden geladen...") so the key "Loading vendors" maps
directly to that string rather than to nested empty keys; update the JSON entry
for "Loading vendors" to be "Loading vendors": "Anbieter werden geladen..."
ensuring valid JSON string value for i18n lookups.
In `@src/assets/translations/pt-PT.json`:
- Line 1350: The translation uses two Portuguese nouns for "Breach" (violação
and incumprimento) which must be unified; pick one term (e.g., use
"incumprimento") and update all related translation values so the same noun
appears for both "Breach" and "Near Breach" variants (start with the key "Near
Breach Configuration" and then fix the other Breach/Near Breach keys referenced
in the comment), ensuring labels, headings and buttons use the identical
Portuguese noun across the file.
---
Duplicate comments:
In `@src/assets/translations/cs-CS.json`:
- Around line 3033-3039: The "Loading vendors" translation key in
src/assets/translations/cs-CS.json is incorrectly mapped to a nested object;
replace that nested object value with a plain string "Načítání dodavatelů..." so
the "Loading vendors" key maps directly to a string (not an object) to match
expected translation value types used by the UI.
---
Nitpick comments:
In `@src/assets/translations/de-DE.json`:
- Around line 4204-4207: Duplicate message keys ("validationSaved",
"workingCapitalDiscountUpdated", "unableToUpdateDiscount") were added under the
labels.messages namespace while identical keys already exist in the root
messages; remove these three keys from labels.messages and reuse the existing
entries in root messages to avoid drift, or if the intent is to centralize under
labels.messages, move the canonical translations from root messages into
labels.messages and delete the root duplicates—ensure only one canonical entry
remains for each key.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f429cf43-354f-4115-abea-2aa77a0e1fcc
📒 Files selected for processing (13)
src/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 due to trivial changes (3)
- src/assets/translations/es-MX.json
- src/assets/translations/en-US.json
- src/assets/translations/lv-LV.json
🚧 Files skipped from review as they are similar to previous changes (7)
- src/assets/translations/lt-LT.json
- src/assets/translations/es-CL.json
- src/assets/translations/sw-SW.json
- src/assets/translations/ko-KO.json
- src/assets/translations/it-IT.json
- src/assets/translations/fr-FR.json
- src/assets/translations/ne-NE.json
Comment on lines
+3035
to
+3041
| "Loading vendors": { | ||
| "": { | ||
| "": { | ||
| "": "Anbieter werden geladen..." | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
"Loading vendors" has an invalid translation value shape
The value is an object with nested empty keys instead of a localized string. This can break i18n lookup/rendering for that key.
💡 Proposed fix
- "Loading vendors": {
- "": {
- "": {
- "": "Anbieter werden geladen..."
- }
- }
- }
+ "Loading vendors": "Anbieter werden geladen..."📝 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
| "Loading vendors": { | |
| "": { | |
| "": { | |
| "": "Anbieter werden geladen..." | |
| } | |
| } | |
| } | |
| "Loading vendors": "Anbieter werden geladen..." |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/de-DE.json` around lines 3035 - 3041, The translation
for the key "Loading vendors" currently has an invalid nested-object shape;
replace its value with a simple localized string (e.g., "Anbieter werden
geladen...") so the key "Loading vendors" maps directly to that string rather
than to nested empty keys; update the JSON entry for "Loading vendors" to be
"Loading vendors": "Anbieter werden geladen..." ensuring valid JSON string value
for i18n lookups.
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (15)
src/assets/translations/lv-LV.json (3)
4663-4670:⚠️ Potential issue | 🟡 MinorAvoid forking
notImplementedwording across namespaces.
errors.error.resource.notImplemented.*already exists with different text; this newerror.resource.notImplemented.*copy introduces inconsistent user messaging.Suggested fix (align wording with existing keys)
"error": { "resource": { "notImplemented": { - "type": "Neimplementēta kļūda", - "message": "Neimplementēta funkcionalitāte!" + "type": "Nav ieviests", + "message": "Neieviesta funkcionalitāte!" } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/lv-LV.json` around lines 4663 - 4670, This adds a duplicate and inconsistent translation block at keys "error.resource.notImplemented.type" and "error.resource.notImplemented.message" that conflicts with the existing "errors.error.resource.notImplemented.*" entries; update this block to use the exact same wording as the existing "errors.error.resource.notImplemented.type" and "errors.error.resource.notImplemented.message" keys (or remove this duplicate block and reference the existing keys), ensuring the "notImplemented" messages are identical across namespaces to avoid inconsistent user messaging.
3017-3023:⚠️ Potential issue | 🟡 Minor
Loading vendorsshould be a plain string value, not a nested object.The current nested empty-key structure is brittle and can resolve incorrectly at runtime.
Suggested fix
- "Loading vendors": { - "": { - "": { - "": "Ielādē piegādātājus..." - } - } - }, + "Loading vendors": "Ielādē piegādātājus...",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/lv-LV.json` around lines 3017 - 3023, The "Loading vendors" translation is incorrectly stored as a nested object with empty keys; replace that nested structure under the "Loading vendors" key in src/assets/translations/lv-LV.json with a plain string value "Ielādē piegādātājus..." so the translation reads: "Loading vendors": "Ielādē piegādātājus..." (remove the nested empty-key object to avoid resolution issues).
1351-1351:⚠️ Potential issue | 🟡 MinorUse one Latvian term for “Near Breach” across all keys.
The file still mixes
Tuva/TuvuandGandrīz, so the same feature is presented inconsistently in headings, labels, actions, and descriptions.Suggested normalization (example using “Gandrīz”)
- "Near Breach Configuration": "Tuvas pārkāpuma konfigurācija", + "Near Breach Configuration": "Gandrīz pārkāpuma konfigurācija", - "Enable Near Breach": "Iespējot tuvo pārkāpumu", + "Enable Near Breach": "Iespējot gandrīz pārkāpumu", - "Near Breach": "Tuvu pārkāpumam", + "Near Breach": "Gandrīz pārkāpums", - "Near Breach Evaluation Frequency": "Tuva pārkāpuma novērtēšanas biežums", + "Near Breach Evaluation Frequency": "Gandrīz pārkāpuma novērtēšanas biežums", - "Near Breach Evaluation Frequency Type": "Tuva pārkāpuma novērtēšanas biežuma veids", + "Near Breach Evaluation Frequency Type": "Gandrīz pārkāpuma novērtēšanas biežuma veids", - "Near Breach Threshold": "Tuva pārkāpuma slieksnis", + "Near Breach Threshold": "Gandrīz pārkāpuma slieksnis"Also applies to: 1983-1983, 2374-2377, 3379-3379, 3450-3450, 3494-3494, 3670-3670, 3893-3893
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/lv-LV.json` at line 1351, Replace inconsistent Latvian variants for "Near Breach" in the translation values with a single chosen term (e.g., "Gandrīz"); specifically update the value for the key "Near Breach Configuration" and all other keys whose English key contains "Near Breach" so that any instances of "Tuva", "Tuvu", "Tuvas" are changed to the selected term (e.g., "Gandrīz") so the feature uses one consistent Latvian word across the file.src/assets/translations/es-MX.json (2)
1353-1353:⚠️ Potential issue | 🟡 MinorNormalize “Near Breach” wording across the locale.
Line 1353, Line 3390, Line 3504, Line 3682, and Line 3905 use different Spanish variants than the already-used “incumplimiento inminente” (for example at Line 2378), which makes the same feature appear inconsistent in UI flows.
💬 Suggested normalization
- "Near Breach Configuration": "Configuración de Incumplimiento Cercano", + "Near Breach Configuration": "Configuración de incumplimiento inminente", - "Create Near Breach": "Crear Cuasi Incumplimiento", + "Create Near Breach": "Crear incumplimiento inminente", - "Edit Near Breach": "Editar Cuasi Incumplimiento", + "Edit Near Breach": "Editar incumplimiento inminente", - "Near Breach Configurations": "Configuraciones de Cuasi Incumplimiento", + "Near Breach Configurations": "Configuraciones de incumplimiento inminente", - "View Near Breach": "Ver Cuasi Incumplimiento", + "View Near Breach": "Ver incumplimiento inminente"Also applies to: 3390-3390, 3504-3504, 3682-3682, 3905-3905
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-MX.json` at line 1353, The "Near Breach" translation is inconsistent; update the translation value for the "Near Breach Configuration" key (and any other keys at the referenced locations that use variants) to the normalized Spanish phrase "Configuración de incumplimiento inminente" so all occurrences match the existing usage of "incumplimiento inminente" (e.g., replace the value for "Near Breach Configuration" and the other listed entries at the same key string on lines noted in the locale file).
3173-3173:⚠️ Potential issue | 🟡 MinorKeep “Undo Write-off” verb consistent with locale actions.
Line 3173 uses “Deshacer…”, but this locale consistently uses “Reversar…” for undo actions.
📝 Suggested fix
- "Undo Write-off": "Deshacer castigo contable" + "Undo Write-off": "Reversar castigo contable"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-MX.json` at line 3173, The translation for the key "Undo Write-off" uses "Deshacer castigo contable" which is inconsistent with this locale's convention; update the value for the "Undo Write-off" key in src/assets/translations/es-MX.json to use "Reversar…" form (e.g., "Reversar castigo contable" or the established phrasing used elsewhere) so it matches other undo action translations like the locale's existing "Reversar..." entries.src/assets/translations/cs-CS.json (1)
3033-3039:⚠️ Potential issue | 🟠 MajorReplace the malformed
Loading vendorsentry with a string translation.This block is an object with empty nested keys, while the surrounding
labels.inputsentries are plain strings. Translate lookups will not resolve this loading label correctly in its current shape.🔧 Suggested fix
- "Loading vendors": { - "": { - "": { - "": "Načítání dodavatelů..." - } - } - } + "Loading vendors...": "Načítání dodavatelů..."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/cs-CS.json` around lines 3033 - 3039, The "Loading vendors" translation is malformed as a nested empty-object instead of a string; replace the entire value of the "Loading vendors" key with a direct string value "Načítání dodavatelů..." so it matches the surrounding labels.inputs style (i.e., "Loading vendors": "Načítání dodavatelů...") ensuring translation lookups resolve correctly.src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.ts (1)
66-72:⚠️ Potential issue | 🟡 MinorAlign the reactive min validator with the field minimum.
Validators.min(0)still accepts0, while the paired template minimum was previously set to0.01. That lets the form pass a value the UI rejects.🔧 Suggested fix
nearBreachThreshold: [ '', [ amountValueValidator(), Validators.required, - Validators.min(0), + Validators.min(0.01), Validators.max(this.maxThreshold) ] ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.ts` around lines 66 - 72, The reactive form uses Validators.min(0) for the nearBreachThreshold control which allows 0 but the template enforces a 0.01 minimum; change the validator to Validators.min(0.01) (or use a shared constant for the field minimum) and ensure amountValueValidator and any UI min attribute reference the same constant so the form and template validation align for nearBreachThreshold.src/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.ts (1)
107-121:⚠️ Potential issue | 🟠 MajorFlatten the delete flow and keep table bindings after refresh.
At Line 111-117, nested subscriptions make the flow hard to maintain, and recreating
MatTableDataSourceat Line 116 drops paginator/sort bindings unless reattached.♻️ Proposed refactor
+import { EMPTY, filter, switchMap, take, catchError } from 'rxjs'; @@ delete(item: NearBreach): void { const deleteDataTableDialogRef = this.dialog.open(DeleteDialogComponent, { data: { deleteContext: ` the item of ${item.id}` } }); - deleteDataTableDialogRef.afterClosed().subscribe((response: any) => { - if (response?.delete) { - this.productsService.deleteWrokingCapitalNearBreach(item.id).subscribe(() => { - this.productsService.getWorkingCapitalNearBreaches().subscribe((nearBreachesData: NearBreach[]) => { - this.nearBreachesData = nearBreachesData; - this.dataSource = new MatTableDataSource(this.nearBreachesData); - }); - }); - } - }); + deleteDataTableDialogRef + .afterClosed() + .pipe( + take(1), + filter((response: { delete?: boolean } | undefined) => !!response?.delete), + switchMap(() => this.productsService.deleteWrokingCapitalNearBreach(item.id)), + switchMap(() => this.productsService.getWorkingCapitalNearBreaches()), + catchError(() => EMPTY) + ) + .subscribe((nearBreachesData: NearBreach[]) => { + this.nearBreachesData = nearBreachesData; + this.dataSource.data = this.nearBreachesData; + this.dataSource.paginator = this.paginator; + this.dataSource.sort = this.sort; + }); }#!/bin/bash # Verify nested subscribe usage in this component rg -nP --type=ts -C2 'afterClosed\(\)\.subscribe|deleteWrokingCapitalNearBreach\([^)]*\)\.subscribe|getWorkingCapitalNearBreaches\(\)\.subscribe' src/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.tsAs per coding guidelines: "
src/app/**/*.{service.ts,component.ts,resolver.ts}: Use RxJS Observables..." and "src/app/**: verify ... clean observable patterns."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.ts` around lines 107 - 121, The delete flow nests subscriptions and recreates the MatTableDataSource (dropping paginator/sort); refactor the delete() method to flatten the stream: use the DeleteDialogComponent.afterClosed() observable, filter for response?.delete, then switchMap/concatMap to this.productsService.deleteWrokingCapitalNearBreach(item.id) and then switchMap to this.productsService.getWorkingCapitalNearBreaches(), subscribing once at the end (or return the composed observable). Instead of new MatTableDataSource(...), update the existing this.dataSource.data = nearBreachesData (so paginator and sort bindings on this.dataSource remain intact) and ensure you use take(1) or finalize as appropriate to complete the sequence.src/assets/translations/lt-LT.json (1)
3016-3022:⚠️ Potential issue | 🟠 MajorFix malformed
Loading vendorstranslation entry.At Line 3016-3022, the key is nested under empty-string objects, so normal translation lookups won’t resolve it.
💡 Proposed fix
- "Loading vendors": { - "": { - "": { - "": "Kraunami tiekėjai..." - } - } - }, + "Loading vendors...": "Kraunami tiekėjai...",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/lt-LT.json` around lines 3016 - 3022, The translation entry for the "Loading vendors" key is incorrectly nested under multiple empty-string objects, so translation lookups fail; locate the "Loading vendors" object and replace the nested empty-string structure with a direct string value (i.e., set "Loading vendors" to "Kraunami tiekėjai..." directly) so the key maps to a plain string rather than nested objects (look for the "Loading vendors" key in the lt-LT.json and update its value accordingly).src/app/products/products-routing.module.ts (1)
147-147:⚠️ Potential issue | 🟠 MajorPotential resolver mismatch in near-breach create/edit routes.
Line 147 imports
NearBreachTemplateResolver, but Line 966 and Line 986 still useBreachTemplateResolver. If near-breach has a distinct template contract, this can hydrate the form with the wrong payload.🔧 Proposed fix
- breachTemplate: BreachTemplateResolver + nearBreachTemplate: NearBreachTemplateResolver ... - breachTemplate: BreachTemplateResolver + nearBreachTemplate: NearBreachTemplateResolverBreachTemplateResolver, BreachesResolver, BreachResolver, NearBreachesResolver, - NearBreachResolver + NearBreachResolver, + NearBreachTemplateResolver#!/bin/bash # Verify whether near-breach has its own template resolver contract and service call. fd -i 'near-breach-template.resolver.ts' src/app/products fd -i 'breach-template.resolver.ts' src/app/products rg -n --type=ts -C3 'class (NearBreachTemplateResolver|BreachTemplateResolver)|resolve\(|getWorkingCapital.*Template' src/app/products/loan-products/working-capital rg -n --type=ts -C2 "path: 'near-breach-configurations'|nearBreachTemplate|breachTemplate" src/app/products/products-routing.module.tsAlso applies to: 965-967, 985-987
🤖 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` at line 147, The routes for 'near-breach-configurations' are wired to BreachTemplateResolver but you imported NearBreachTemplateResolver; update the route resolver references to use NearBreachTemplateResolver (or, if the near-breach flow intentionally uses the shared BreachTemplateResolver, remove the NearBreachTemplateResolver import and confirm contract compatibility). Specifically, change the resolver assigned to the create/edit near-breach routes (the route objects with path 'near-breach-configurations' / create/edit) to reference NearBreachTemplateResolver (or vice versa) and ensure the resolver class (NearBreachTemplateResolver vs BreachTemplateResolver) implements the expected resolve() payload for the near-breach form.src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.ts (4)
19-27:⚠️ Potential issue | 🔴 CriticalAdd
standalone: trueto this component.Line 23 uses
imports, so this component must be declared standalone.💡 Proposed fix
`@Component`({ selector: 'mifosx-edit-near-breach-configuration', templateUrl: './edit-near-breach-configuration.component.html', styleUrl: './edit-near-breach-configuration.component.scss', + standalone: true, imports: [ ...STANDALONE_SHARED_IMPORTS, InputPositiveIntegerComponent ] })#!/bin/bash # Verify this component uses `imports` but lacks `standalone: true` rg -n -C3 --type ts "selector: 'mifosx-edit-near-breach-configuration'|imports:|standalone:" \ src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.ts` around lines 19 - 27, The component decorated with `@Component` having selector 'mifosx-edit-near-breach-configuration' declares an imports array but is missing standalone: true; update the component metadata (the `@Component` decorator for the edit-near-breach-configuration component) to include standalone: true so Angular accepts the imports array (keep the existing imports: [...STANDALONE_SHARED_IMPORTS, InputPositiveIntegerComponent] intact).
43-43:⚠️ Potential issue | 🟡 MinorRemove debug logging before merge.
Line 43 leaves a
console.login production code path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.ts` at line 43, Remove the stray debug statement "console.log(this.nearBreachData)" from the edit-near-breach-configuration.component.ts (inside the component method where this.nearBreachData is referenced); either delete the console.log call or replace it with a proper production logger call if structured logging is required, ensuring no raw console output remains in the component lifecycle or event handler.
63-66:⚠️ Potential issue | 🟠 MajorUse
frequencyType.idfor form initialization.Line 64 initializes with
frequencyType.code, which is likely inconsistent with option values used by the select and can prevent correct preselection.💡 Proposed fix
nearBreachFrequencyType: [ - this.nearBreachData!.frequencyType.code, + this.nearBreachData!.frequencyType.id, Validators.required ],#!/bin/bash # Verify select option value binding in the paired template(s) rg -n -C2 --type html "nearBreachFrequencyType|frequencyType\.(id|code)" \ src/app/products/loan-products/working-capital/near-breach-configuration/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.ts` around lines 63 - 66, The form control nearBreachFrequencyType in EditNearBreachConfigurationComponent is initialized with this.nearBreachData!.frequencyType.code which mismatches the select option values; change the initialization to use this.nearBreachData!.frequencyType.id (keeping Validators.required) so the select preselects correctly and aligns with the template's option binding.
79-87:⚠️ Potential issue | 🟠 MajorGuard nullable data and replace
console.errorwith app error handling.Line 81 dereferences nullable
nearBreachData, and Line 86 bypasses centralized error handling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.ts` around lines 79 - 87, The submit() handler currently dereferences nullable nearBreachData and logs errors to console; update submit() to first guard that this.nearBreachData exists (e.g., return early or show validation if null) before using this.nearBreachData.id, then call this.productsService.updateWrokingCapitalNearBreach(this.nearBreachData.id, this.nearBreachForm.getRawValue()).subscribe(...) and replace the console.error in the error handler with the app's centralized error handling (e.g., call the injected error handler/service or this.toastService/errorService) and keep the success navigation using this.router.navigate([...], { relativeTo: this.route }).src/assets/translations/pt-PT.json (1)
1350-1350:⚠️ Potential issue | 🟡 MinorStandardize Breach/Near Breach terminology.
These lines still mix “violação” and “incumprimento” for the same concept, which makes the UI read inconsistently. Please use one term set across all related keys.
Also applies to: 2373-2376, 3302-3302, 3353-3353, 3380-3380, 3450-3450, 3470-3470, 3494-3494, 3670-3670, 3868-3868, 3893-3893
🤖 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 1350, The translation for "Near Breach Configuration" uses "incumprimento" while other related keys mix "violação" and "incumprimento"; pick the project's preferred term (use "violação" for consistency) and update the value for the "Near Breach Configuration" key and all related keys (the other entries referenced in the review) to use "violação" so the same terminology is used across the translations.
🧹 Nitpick comments (5)
src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.ts (1)
9-10: Prefer typed forms and resolver data in this new component.This screen is introduced with
UntypedFormBuilder,UntypedFormGroup, andbreachTemplate?: any, so the form payload and resolver contract stay unchecked. A small typed form model and template interface here would catch field drift at compile time.As per coding guidelines,
src/app/**/*.ts: "Use TypeScript for all application code with strict typing conventions".Also applies to: 33-45
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.ts` around lines 9 - 10, The component create-near-breach-configuration.component is using UntypedFormBuilder/UntypedFormGroup and a loosely typed breachTemplate?: any; replace these with a strictly typed form model interface (e.g., NearBreachFormModel) and use FormBuilder: FormBuilder and FormGroup<NearBreachFormModel> for the form property and controls, and type the resolver data/breachTemplate with a specific interface (e.g., BreachTemplate) returned by the route resolver; update the constructor injection (formBuilder: FormBuilder, route: ActivatedRoute) and any usages of form.get/rawValue and breachTemplate to use the typed shape so compile-time checks catch field drift.src/assets/translations/cs-CS.json (1)
3978-3978: Keep the rootpermissionscatalog in sync with these new keys.
REPORTINGandSUPER_USERare added only underlabels.permissions, but the rootpermissionsblock at Line 4268 still lacks both entries. Either mirror them there too or keep a single permission catalog.Also applies to: 4079-4079
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/cs-CS.json` at line 3978, The translations added new permission keys REPORTING and SUPER_USER under labels.permissions but the top-level permissions catalog (root "permissions" block) is missing these same keys; update the root permissions block to include REPORTING and SUPER_USER with matching values (or consolidate by removing duplicates and keeping a single canonical permissions catalog and referencing it from labels.permissions), ensuring the keys and values are consistent between labels.permissions and the root permissions block so both locations stay in sync.src/assets/translations/sw-SW.json (1)
9-10: Consolidate duplicatednotImplementedtranslation namespaces.You now have both
errors.error.resource.notImplemented.*anderror.resource.notImplemented.*with different wording. Keeping one canonical path (or same message text in both) avoids inconsistent alerts and maintenance drift.Also applies to: 4660-4665
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/sw-SW.json` around lines 9 - 10, There are duplicate translation keys for the same concept: "error.resource.notImplemented.type" / "error.resource.notImplemented.message" and the alternate "errors.error.resource.notImplemented.*"; pick a single canonical namespace (e.g., "error.resource.notImplemented.*" or "errors.error.resource.notImplemented.*"), merge the messages so both keys map to the same wording, remove or redirect the duplicate keys to the canonical ones, and ensure the entries referenced around lines 4660-4665 are updated to match the chosen canonical path and text.src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.html (1)
75-75: Use 8px-grid spacing utility instead of 5px gap class.
gap-5pxintroduces non-grid spacing and can create inconsistent layout rhythm.As per coding guidelines "Stick to the 8px grid system for visual design and spacing".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.html` at line 75, Replace the non-grid spacing class on the mat-card-actions element: remove the class gap-5px and use the project’s 8px-grid spacing utility instead (e.g., replace with gap-8px or the equivalent spacing-8 utility used across the codebase) in edit-near-breach-configuration.component.html so the mat-card-actions layout-row align-center responsive-column classes maintain consistent 8px-grid rhythm.src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.ts (1)
41-41: Replaceanyin route data with a typed contract.Line 41 weakens type safety with
breachTemplate: any; define a dedicated interface for resolved data.Based on learnings, “avoid using Observable as a project-wide pattern… introduce specific interfaces/types for the response shapes and use proper typing instead of any.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.ts` at line 41, Define a specific interface for the resolved route data (e.g., name it NearBreachRouteData) that includes the existing NearBreach type and the concrete shape of breachTemplate (replace any with proper fields or a dedicated BreachTemplate type), then update the subscribe call to use this interface: change this.route.data.subscribe((data: { nearBreachData: NearBreach; breachTemplate: any }) => ...) to use the new NearBreachRouteData type; also update the corresponding resolver and any places that construct or return the route data to return the typed shape so the type flows end-to-end (resolver method, route configuration, and the edit-near-breach-configuration component).
🤖 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/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts`:
- Line 468: The form control nearBreachId is being removed permanently via
removeControl('nearBreachId') and was initialized with an empty string; change
initialization to null (nearBreachId: [null]) and remove the call to
removeControl('nearBreachId') so the control remains present for subsequent
selections and preserves a numeric/nullable type; update any related logic that
assumed removal to instead set the control value to null when clearing a breach
(use formGroup.get('nearBreachId')?.setValue(null) or equivalent).
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.ts`:
- Around line 78-87: The submit() method currently posts getRawValue() even if
the form is invalid; update submit() to guard against invalid forms by checking
nearBreachForm.valid at the start, call nearBreachForm.markAllAsTouched()
(and/or markAsDirty) to trigger validation UI when invalid, and return early so
createWrokingCapitalNearBreach is only invoked for valid forms; keep the
existing error handling via errorHandler.handleError and router navigation
unchanged for the success path.
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.html`:
- Line 27: The template uses a hardcoded user-facing label
([inputLabel]="'Frequency'") which breaks i18n; change the binding to use the
translate key (e.g. replace the literal with the translate pipe or a component
property that yields translated text) in
edit-near-breach-configuration.component.html and add the corresponding key
(e.g. NEAR_BREACH.FREQUENCY) to your translation JSONs so inputLabel receives
the localized string.
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.ts`:
- Line 109: The hardcoded English delete context in the object passed to the
dialog (data: { deleteContext: ` the item of ${item.id}` }) must be replaced
with an i18n key and runtime interpolation; update
NearBreachConfigurationComponent so data.deleteContext is built from the
translation service (or translate pipe) using a key like
'nearBreach.deleteContext' and pass item.id as the interpolation value (e.g.,
translate.instant('nearBreach.deleteContext', { id: item.id }) or equivalent),
keeping the property name deleteContext and using item.id for the dynamic part.
In `@src/assets/translations/es-MX.json`:
- Line 3502: Update the Spanish (es-MX) translations for the keys "Edit Loan
Originator" and the other "Loan Originator" entry to use "Crédito" instead of
"Préstamo" so terminology matches the rest of the locale; locate the string
value for the key "Edit Loan Originator" (and the other "Loan Originator" key)
and replace "Editar Originador de Préstamo" (and any "Originador de Préstamo")
with "Editar Originador de Crédito" (and "Originador de Crédito") respectively.
In `@src/assets/translations/ne-NE.json`:
- Around line 3016-3022: The i18n entry for "Loading vendors" currently uses
nested empty keys (under the "Loading vendors" object) which breaks lookup;
replace that nested object with a single flat key mapping so the translation
lookup uses "Loading vendors" -> "विक्रेताहरू लोड हुँदैछ...". Locate the
"Loading vendors" entry and remove the inner empty objects, setting its value
directly to the Nepali string for vendor loading to restore a standard flat key
structure.
In `@src/assets/translations/pt-PT.json`:
- Around line 3016-3022: The translation entry for the key "Loading vendors" in
pt-PT.json is malformed as it contains nested empty-object keys instead of a
simple string; replace the nested object value for "Loading vendors" with the
actual translatable string (e.g., "A carregar fornecedores...") so consumers
receive a plain string for that key and not an object.
In `@src/assets/translations/sw-SW.json`:
- Around line 3014-3020: The translation for the key "Loading vendors" is
malformed as a nested empty-object structure; replace that object with a simple
string value so lookups return text (i.e., change the "Loading vendors" entry
under labels.inputs to a plain string like "Inapakia wauzaji..." instead of an
object). Locate the "Loading vendors" JSON key and set its value to the correct
string.
---
Duplicate comments:
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.ts`:
- Around line 66-72: The reactive form uses Validators.min(0) for the
nearBreachThreshold control which allows 0 but the template enforces a 0.01
minimum; change the validator to Validators.min(0.01) (or use a shared constant
for the field minimum) and ensure amountValueValidator and any UI min attribute
reference the same constant so the form and template validation align for
nearBreachThreshold.
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.ts`:
- Around line 19-27: The component decorated with `@Component` having selector
'mifosx-edit-near-breach-configuration' declares an imports array but is missing
standalone: true; update the component metadata (the `@Component` decorator for
the edit-near-breach-configuration component) to include standalone: true so
Angular accepts the imports array (keep the existing imports:
[...STANDALONE_SHARED_IMPORTS, InputPositiveIntegerComponent] intact).
- Line 43: Remove the stray debug statement "console.log(this.nearBreachData)"
from the edit-near-breach-configuration.component.ts (inside the component
method where this.nearBreachData is referenced); either delete the console.log
call or replace it with a proper production logger call if structured logging is
required, ensuring no raw console output remains in the component lifecycle or
event handler.
- Around line 63-66: The form control nearBreachFrequencyType in
EditNearBreachConfigurationComponent is initialized with
this.nearBreachData!.frequencyType.code which mismatches the select option
values; change the initialization to use this.nearBreachData!.frequencyType.id
(keeping Validators.required) so the select preselects correctly and aligns with
the template's option binding.
- Around line 79-87: The submit() handler currently dereferences nullable
nearBreachData and logs errors to console; update submit() to first guard that
this.nearBreachData exists (e.g., return early or show validation if null)
before using this.nearBreachData.id, then call
this.productsService.updateWrokingCapitalNearBreach(this.nearBreachData.id,
this.nearBreachForm.getRawValue()).subscribe(...) and replace the console.error
in the error handler with the app's centralized error handling (e.g., call the
injected error handler/service or this.toastService/errorService) and keep the
success navigation using this.router.navigate([...], { relativeTo: this.route
}).
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.ts`:
- Around line 107-121: The delete flow nests subscriptions and recreates the
MatTableDataSource (dropping paginator/sort); refactor the delete() method to
flatten the stream: use the DeleteDialogComponent.afterClosed() observable,
filter for response?.delete, then switchMap/concatMap to
this.productsService.deleteWrokingCapitalNearBreach(item.id) and then switchMap
to this.productsService.getWorkingCapitalNearBreaches(), subscribing once at the
end (or return the composed observable). Instead of new MatTableDataSource(...),
update the existing this.dataSource.data = nearBreachesData (so paginator and
sort bindings on this.dataSource remain intact) and ensure you use take(1) or
finalize as appropriate to complete the sequence.
In `@src/app/products/products-routing.module.ts`:
- Line 147: The routes for 'near-breach-configurations' are wired to
BreachTemplateResolver but you imported NearBreachTemplateResolver; update the
route resolver references to use NearBreachTemplateResolver (or, if the
near-breach flow intentionally uses the shared BreachTemplateResolver, remove
the NearBreachTemplateResolver import and confirm contract compatibility).
Specifically, change the resolver assigned to the create/edit near-breach routes
(the route objects with path 'near-breach-configurations' / create/edit) to
reference NearBreachTemplateResolver (or vice versa) and ensure the resolver
class (NearBreachTemplateResolver vs BreachTemplateResolver) implements the
expected resolve() payload for the near-breach form.
In `@src/assets/translations/cs-CS.json`:
- Around line 3033-3039: The "Loading vendors" translation is malformed as a
nested empty-object instead of a string; replace the entire value of the
"Loading vendors" key with a direct string value "Načítání dodavatelů..." so it
matches the surrounding labels.inputs style (i.e., "Loading vendors": "Načítání
dodavatelů...") ensuring translation lookups resolve correctly.
In `@src/assets/translations/es-MX.json`:
- Line 1353: The "Near Breach" translation is inconsistent; update the
translation value for the "Near Breach Configuration" key (and any other keys at
the referenced locations that use variants) to the normalized Spanish phrase
"Configuración de incumplimiento inminente" so all occurrences match the
existing usage of "incumplimiento inminente" (e.g., replace the value for "Near
Breach Configuration" and the other listed entries at the same key string on
lines noted in the locale file).
- Line 3173: The translation for the key "Undo Write-off" uses "Deshacer castigo
contable" which is inconsistent with this locale's convention; update the value
for the "Undo Write-off" key in src/assets/translations/es-MX.json to use
"Reversar…" form (e.g., "Reversar castigo contable" or the established phrasing
used elsewhere) so it matches other undo action translations like the locale's
existing "Reversar..." entries.
In `@src/assets/translations/lt-LT.json`:
- Around line 3016-3022: The translation entry for the "Loading vendors" key is
incorrectly nested under multiple empty-string objects, so translation lookups
fail; locate the "Loading vendors" object and replace the nested empty-string
structure with a direct string value (i.e., set "Loading vendors" to "Kraunami
tiekėjai..." directly) so the key maps to a plain string rather than nested
objects (look for the "Loading vendors" key in the lt-LT.json and update its
value accordingly).
In `@src/assets/translations/lv-LV.json`:
- Around line 4663-4670: This adds a duplicate and inconsistent translation
block at keys "error.resource.notImplemented.type" and
"error.resource.notImplemented.message" that conflicts with the existing
"errors.error.resource.notImplemented.*" entries; update this block to use the
exact same wording as the existing "errors.error.resource.notImplemented.type"
and "errors.error.resource.notImplemented.message" keys (or remove this
duplicate block and reference the existing keys), ensuring the "notImplemented"
messages are identical across namespaces to avoid inconsistent user messaging.
- Around line 3017-3023: The "Loading vendors" translation is incorrectly stored
as a nested object with empty keys; replace that nested structure under the
"Loading vendors" key in src/assets/translations/lv-LV.json with a plain string
value "Ielādē piegādātājus..." so the translation reads: "Loading vendors":
"Ielādē piegādātājus..." (remove the nested empty-key object to avoid resolution
issues).
- Line 1351: Replace inconsistent Latvian variants for "Near Breach" in the
translation values with a single chosen term (e.g., "Gandrīz"); specifically
update the value for the key "Near Breach Configuration" and all other keys
whose English key contains "Near Breach" so that any instances of "Tuva",
"Tuvu", "Tuvas" are changed to the selected term (e.g., "Gandrīz") so the
feature uses one consistent Latvian word across the file.
In `@src/assets/translations/pt-PT.json`:
- Line 1350: The translation for "Near Breach Configuration" uses
"incumprimento" while other related keys mix "violação" and "incumprimento";
pick the project's preferred term (use "violação" for consistency) and update
the value for the "Near Breach Configuration" key and all related keys (the
other entries referenced in the review) to use "violação" so the same
terminology is used across the translations.
---
Nitpick comments:
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.ts`:
- Around line 9-10: The component create-near-breach-configuration.component is
using UntypedFormBuilder/UntypedFormGroup and a loosely typed breachTemplate?:
any; replace these with a strictly typed form model interface (e.g.,
NearBreachFormModel) and use FormBuilder: FormBuilder and
FormGroup<NearBreachFormModel> for the form property and controls, and type the
resolver data/breachTemplate with a specific interface (e.g., BreachTemplate)
returned by the route resolver; update the constructor injection (formBuilder:
FormBuilder, route: ActivatedRoute) and any usages of form.get/rawValue and
breachTemplate to use the typed shape so compile-time checks catch field drift.
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.html`:
- Line 75: Replace the non-grid spacing class on the mat-card-actions element:
remove the class gap-5px and use the project’s 8px-grid spacing utility instead
(e.g., replace with gap-8px or the equivalent spacing-8 utility used across the
codebase) in edit-near-breach-configuration.component.html so the
mat-card-actions layout-row align-center responsive-column classes maintain
consistent 8px-grid rhythm.
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.ts`:
- Line 41: Define a specific interface for the resolved route data (e.g., name
it NearBreachRouteData) that includes the existing NearBreach type and the
concrete shape of breachTemplate (replace any with proper fields or a dedicated
BreachTemplate type), then update the subscribe call to use this interface:
change this.route.data.subscribe((data: { nearBreachData: NearBreach;
breachTemplate: any }) => ...) to use the new NearBreachRouteData type; also
update the corresponding resolver and any places that construct or return the
route data to return the typed shape so the type flows end-to-end (resolver
method, route configuration, and the edit-near-breach-configuration component).
In `@src/assets/translations/cs-CS.json`:
- Line 3978: The translations added new permission keys REPORTING and SUPER_USER
under labels.permissions but the top-level permissions catalog (root
"permissions" block) is missing these same keys; update the root permissions
block to include REPORTING and SUPER_USER with matching values (or consolidate
by removing duplicates and keeping a single canonical permissions catalog and
referencing it from labels.permissions), ensuring the keys and values are
consistent between labels.permissions and the root permissions block so both
locations stay in sync.
In `@src/assets/translations/sw-SW.json`:
- Around line 9-10: There are duplicate translation keys for the same concept:
"error.resource.notImplemented.type" / "error.resource.notImplemented.message"
and the alternate "errors.error.resource.notImplemented.*"; pick a single
canonical namespace (e.g., "error.resource.notImplemented.*" or
"errors.error.resource.notImplemented.*"), merge the messages so both keys map
to the same wording, remove or redirect the duplicate keys to the canonical
ones, and ensure the entries referenced around lines 4660-4665 are updated to
match the chosen canonical path and text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 723a0d89-b3ed-4876-ad58-807218c29df6
📒 Files selected for processing (48)
src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.htmlsrc/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.tssrc/app/products/loan-products/models/loan-product.model.tssrc/app/products/loan-products/working-capital/breach-configuration/breach-configuration.component.htmlsrc/app/products/loan-products/working-capital/breach-configuration/breach-configuration.component.tssrc/app/products/loan-products/working-capital/breach-configuration/create-breach-configuration/create-breach-configuration.component.htmlsrc/app/products/loan-products/working-capital/breach-configuration/create-breach-configuration/create-breach-configuration.component.tssrc/app/products/loan-products/working-capital/breach-configuration/edit-breach-configuration/edit-breach-configuration.component.htmlsrc/app/products/loan-products/working-capital/breach-configuration/edit-breach-configuration/edit-breach-configuration.component.tssrc/app/products/loan-products/working-capital/breach-configuration/view-breach-configuration/view-breach-configuration.component.htmlsrc/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.htmlsrc/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.scsssrc/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.tssrc/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.htmlsrc/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.scsssrc/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.tssrc/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.htmlsrc/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.scsssrc/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.tssrc/app/products/loan-products/working-capital/near-breach-configuration/near-breach-template.resolver.tssrc/app/products/loan-products/working-capital/near-breach-configuration/near-breach.resolver.tssrc/app/products/loan-products/working-capital/near-breach-configuration/near-breaches.resolver.tssrc/app/products/loan-products/working-capital/near-breach-configuration/view-near-breach-configuration/view-near-breach-configuration.component.htmlsrc/app/products/loan-products/working-capital/near-breach-configuration/view-near-breach-configuration/view-near-breach-configuration.component.scsssrc/app/products/loan-products/working-capital/near-breach-configuration/view-near-breach-configuration/view-near-breach-configuration.component.tssrc/app/products/products-routing.module.tssrc/app/products/products.component.htmlsrc/app/products/products.component.tssrc/app/products/products.service.tssrc/app/shared/icons.module.tssrc/app/shared/input-positive-integer/input-positive-integer.component.htmlsrc/app/shared/loan/breach-display/breach-display.component.htmlsrc/app/shared/loan/breach-display/breach-display.component.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 with no reviewable changes (1)
- src/app/shared/input-positive-integer/input-positive-integer.component.html
✅ Files skipped from review due to trivial changes (21)
- src/app/products/loan-products/working-capital/breach-configuration/edit-breach-configuration/edit-breach-configuration.component.ts
- src/app/products/loan-products/working-capital/breach-configuration/edit-breach-configuration/edit-breach-configuration.component.html
- src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.scss
- src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.scss
- src/app/products/products.component.html
- src/app/shared/icons.module.ts
- src/app/products/loan-products/working-capital/near-breach-configuration/near-breach-template.resolver.ts
- src/app/products/loan-products/working-capital/breach-configuration/create-breach-configuration/create-breach-configuration.component.html
- src/app/products/loan-products/working-capital/near-breach-configuration/view-near-breach-configuration/view-near-breach-configuration.component.scss
- src/app/products/loan-products/working-capital/breach-configuration/view-breach-configuration/view-breach-configuration.component.html
- src/app/products/loan-products/working-capital/near-breach-configuration/near-breach.resolver.ts
- src/app/products/loan-products/working-capital/near-breach-configuration/near-breaches.resolver.ts
- src/app/products/products.service.ts
- src/app/shared/loan/breach-display/breach-display.component.ts
- src/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.scss
- src/app/products/loan-products/working-capital/near-breach-configuration/view-near-breach-configuration/view-near-breach-configuration.component.html
- src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.html
- src/app/products/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.html
- src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.html
- src/assets/translations/de-DE.json
- src/assets/translations/es-CL.json
🚧 Files skipped from review as they are similar to previous changes (12)
- src/app/products/loan-products/working-capital/breach-configuration/create-breach-configuration/create-breach-configuration.component.ts
- src/app/products/products.component.ts
- src/app/products/loan-products/working-capital/breach-configuration/breach-configuration.component.html
- src/app/products/loan-products/working-capital/breach-configuration/breach-configuration.component.ts
- src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.html
- src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts
- src/app/products/loan-products/models/loan-product.model.ts
- src/app/shared/loan/breach-display/breach-display.component.html
- src/assets/translations/fr-FR.json
- src/assets/translations/en-US.json
- src/assets/translations/it-IT.json
- src/assets/translations/ko-KO.json
| delinquencyStartType: [''], | ||
| breachId: [''], | ||
| enableNearBreach: [false] | ||
| nearBreachId: [''] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts'
echo '== nearBreachId lifecycle in component =='
rg -n -C2 "nearBreachId|removeControl\\('nearBreachId'\\)|addControl\\('nearBreachId'\\)" "$FILE"
echo
echo '== template bindings =='
find src/app -name 'loan-product-settings-step.component.html' -print0 \
| xargs -0 -r rg -n -C2 'nearBreachId|formControlName'
echo
python - <<'PY'
from pathlib import Path
text = Path("src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts").read_text()
print("addControl('nearBreachId'):", text.count("addControl('nearBreachId'"))
print("removeControl('nearBreachId'):", text.count("removeControl('nearBreachId'"))
print("nearBreachId initialized with empty string:", "nearBreachId: ['']" in text)
PYRepository: openMF/web-app
Length of output: 25738
Remove removeControl('nearBreachId') and initialize the control with null instead of an empty string.
The form control is created once at line 468 but removed permanently at line 934 with no re-add path. After clearing a breach in the same session, subsequent near-breach selections lack a backing form control, and the template guard at line 112 fails silently. Additionally, initializing a numeric ID field with an empty string weakens the type contract.
Replace the empty string initialization and removeControl() call with null to keep the control persistent:
Proposed fix
- nearBreachId: ['']
+ nearBreachId: [null]
...
} else if (propertyName === 'breachId') {
this.loanProductSettingsForm.patchValue({
- breachId: ''
+ breachId: null,
+ nearBreachId: null
});
- this.loanProductSettingsForm.removeControl('nearBreachId');
} else if (propertyName === 'nearBreachId') {
this.loanProductSettingsForm.patchValue({
- nearBreachId: ''
+ nearBreachId: null
});
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-settings-step/loan-product-settings-step.component.ts`
at line 468, The form control nearBreachId is being removed permanently via
removeControl('nearBreachId') and was initialized with an empty string; change
initialization to null (nearBreachId: [null]) and remove the call to
removeControl('nearBreachId') so the control remains present for subsequent
selections and preserves a numeric/nullable type; update any related logic that
assumed removal to instead set the control value to null when clearing a breach
(use formGroup.get('nearBreachId')?.setValue(null) or equivalent).
Comment on lines
+78
to
+87
| submit(): void { | ||
| const payload = this.nearBreachForm.getRawValue(); | ||
| this.productsService | ||
| .createWrokingCapitalNearBreach(payload) | ||
| .pipe(catchError((error) => this.errorHandler.handleError(error, 'Near Breach Configuration Creation'))) | ||
| .subscribe({ | ||
| next: () => { | ||
| this.router.navigate(['../'], { relativeTo: this.route }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Guard submit() against invalid forms.
This posts getRawValue() unconditionally. If submission fires while the form is invalid, the user only gets a backend error instead of local validation feedback.
🔧 Suggested fix
submit(): void {
+ if (this.nearBreachForm.invalid) {
+ this.nearBreachForm.markAllAsTouched();
+ return;
+ }
+
const payload = this.nearBreachForm.getRawValue();
this.productsService🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/create-near-breach-configuration/create-near-breach-configuration.component.ts`
around lines 78 - 87, The submit() method currently posts getRawValue() even if
the form is invalid; update submit() to guard against invalid forms by checking
nearBreachForm.valid at the start, call nearBreachForm.markAllAsTouched()
(and/or markAsDirty) to trigger validation UI when invalid, and return early so
createWrokingCapitalNearBreach is only invoked for valid forms; keep the
existing error handling via errorHandler.handleError and router navigation
unchanged for the success path.
| <mifosx-input-positive-integer | ||
| class="flex-48" | ||
| [inputFormControl]="nearBreachForm.controls.nearBreachFrequency" | ||
| [inputLabel]="'Frequency'" |
There was a problem hiding this comment.
Hardcoded label blocks localization.
Line 27 uses a literal "Frequency" for a user-facing label; this should use a translate key to keep the form fully localized.
🌐 Proposed fix
- [inputLabel]="'Frequency'"
+ [inputLabel]="'labels.inputs.Frequency' | translate"As per coding guidelines "Use proper i18n variables from @ngx-translate/core for all user-facing strings instead of hardcoded text".
📝 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
| [inputLabel]="'Frequency'" | |
| [inputLabel]="'labels.inputs.Frequency' | translate" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/products/loan-products/working-capital/near-breach-configuration/edit-near-breach-configuration/edit-near-breach-configuration.component.html`
at line 27, The template uses a hardcoded user-facing label
([inputLabel]="'Frequency'") which breaks i18n; change the binding to use the
translate key (e.g. replace the literal with the translate pipe or a component
property that yields translated text) in
edit-near-breach-configuration.component.html and add the corresponding key
(e.g. NEAR_BREACH.FREQUENCY) to your translation JSONs so inputLabel receives
the localized string.
|
|
||
| delete(item: NearBreach): void { | ||
| const deleteDataTableDialogRef = this.dialog.open(DeleteDialogComponent, { | ||
| data: { deleteContext: ` the item of ${item.id}` } |
There was a problem hiding this comment.
Replace hardcoded delete context text with a translation key.
Line 109 builds a user-facing English string in TS. This should come from i18n keys to keep locale behavior consistent.
As per coding guidelines: "src/app/**/*.{html,ts}: Use proper i18n variables ... 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/loan-products/working-capital/near-breach-configuration/near-breach-configuration.component.ts`
at line 109, The hardcoded English delete context in the object passed to the
dialog (data: { deleteContext: ` the item of ${item.id}` }) must be replaced
with an i18n key and runtime interpolation; update
NearBreachConfigurationComponent so data.deleteContext is built from the
translation service (or translate pipe) using a key like
'nearBreach.deleteContext' and pass item.id as the interpolation value (e.g.,
translate.instant('nearBreach.deleteContext', { id: item.id }) or equivalent),
keeping the property name deleteContext and using item.id for the dynamic part.
| "Edit Group": "Editar grupo", | ||
| "Edit Holidays": "Editar días festivos", | ||
| "Edit Hook": "Editar gancho", | ||
| "Edit Loan Originator": "Editar Originador de Préstamo", |
There was a problem hiding this comment.
Align “Loan Originator” terminology with the rest of the locale.
Line 3502 and Line 3903 use “Préstamo”, while this file predominantly uses “Crédito” for “Loan”.
📝 Suggested fix
- "Edit Loan Originator": "Editar Originador de Préstamo",
+ "Edit Loan Originator": "Editar Originador de Crédito",
- "View Loan Originator": "Ver Originador de Préstamo",
+ "View Loan Originator": "Ver Originador de Crédito",Also applies to: 3903-3903
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/es-MX.json` at line 3502, Update the Spanish (es-MX)
translations for the keys "Edit Loan Originator" and the other "Loan Originator"
entry to use "Crédito" instead of "Préstamo" so terminology matches the rest of
the locale; locate the string value for the key "Edit Loan Originator" (and the
other "Loan Originator" key) and replace "Editar Originador de Préstamo" (and
any "Originador de Préstamo") with "Editar Originador de Crédito" (and
"Originador de Crédito") respectively.
Comment on lines
+3016
to
+3022
| "Loading vendors": { | ||
| "": { | ||
| "": { | ||
| "": "विक्रेताहरू लोड हुँदैछ..." | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
Fix malformed i18n key structure for vendor loading text.
Line 3016 introduces nested empty keys under "Loading vendors", which creates a brittle translation path and can fail standard key lookup. Use a single flat key string.
💡 Proposed fix
- "Loading vendors": {
- "": {
- "": {
- "": "विक्रेताहरू लोड हुँदैछ..."
- }
- }
- },
+ "Loading vendors...": "विक्रेताहरू लोड हुँदैछ...",📝 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
| "Loading vendors": { | |
| "": { | |
| "": { | |
| "": "विक्रेताहरू लोड हुँदैछ..." | |
| } | |
| } | |
| }, | |
| "Loading vendors...": "विक्रेताहरू लोड हुँदैछ...", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/ne-NE.json` around lines 3016 - 3022, The i18n entry
for "Loading vendors" currently uses nested empty keys (under the "Loading
vendors" object) which breaks lookup; replace that nested object with a single
flat key mapping so the translation lookup uses "Loading vendors" ->
"विक्रेताहरू लोड हुँदैछ...". Locate the "Loading vendors" entry and remove the
inner empty objects, setting its value directly to the Nepali string for vendor
loading to restore a standard flat key structure.
Comment on lines
+3016
to
+3022
| "Loading vendors": { | ||
| "": { | ||
| "": { | ||
| "": "A carregar fornecedores..." | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
Fix malformed translation value for Loading vendors.
Line 3016 currently maps to a nested object with empty keys, not a translatable string. This will break consumers expecting text.
💡 Proposed fix
- "Loading vendors": {
- "": {
- "": {
- "": "A carregar fornecedores..."
- }
- }
- },
+ "Loading vendors": "A carregar fornecedores...",📝 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
| "Loading vendors": { | |
| "": { | |
| "": { | |
| "": "A carregar fornecedores..." | |
| } | |
| } | |
| }, | |
| "Loading vendors": "A carregar fornecedores...", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/pt-PT.json` around lines 3016 - 3022, The translation
entry for the key "Loading vendors" in pt-PT.json is malformed as it contains
nested empty-object keys instead of a simple string; replace the nested object
value for "Loading vendors" with the actual translatable string (e.g., "A
carregar fornecedores...") so consumers receive a plain string for that key and
not an object.
Comment on lines
+3014
to
+3020
| "Loading vendors": { | ||
| "": { | ||
| "": { | ||
| "": "Inapakia wauzaji..." | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
Fix malformed Loading vendors translation value.
At Line 3014, labels.inputs.Loading vendors is an object with empty nested keys, not a string. Any direct lookup for this key will return an object instead of text.
💡 Proposed fix
- "Loading vendors": {
- "": {
- "": {
- "": "Inapakia wauzaji..."
- }
- }
- },
+ "Loading vendors": "Inapakia wauzaji...",📝 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
| "Loading vendors": { | |
| "": { | |
| "": { | |
| "": "Inapakia wauzaji..." | |
| } | |
| } | |
| }, | |
| "Loading vendors": "Inapakia wauzaji...", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/translations/sw-SW.json` around lines 3014 - 3020, The translation
for the key "Loading vendors" is malformed as a nested empty-object structure;
replace that object with a simple string value so lookups return text (i.e.,
change the "Loading vendors" entry under labels.inputs to a plain string like
"Inapakia wauzaji..." instead of an object). Locate the "Loading vendors" JSON
key and set its value to the correct string.
oleksii-novikov-onix
approved these changes
Apr 10, 2026
alberto-art3ch
force-pushed
the
WEB-657/working-capital-product-near-breach-configuration
branch
3 times, most recently
from
1f60f09 to
2e8f717
Compare
alberto-art3ch
force-pushed
the
WEB-657/working-capital-product-near-breach-configuration
branch
from
2e8f717 to
842ee6c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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
User should be able to configure Near Breach from UI. Configuration is similar to Breach management configuration page
Related issues and discussion
WEB-657
Screenshots
Screen.Recording.2026-04-08.at.12.27.13.AM.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
UI Enhancements
Localization