-
Notifications
You must be signed in to change notification settings - Fork 903
WEB-813: Working Capital loan account modification #3435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -59,8 +59,18 @@ export class ErrorHandlerInterceptor implements HttpInterceptor { | |||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // Combine both messages if both exist | ||||||||||||||||||||||||||||
| const errorMessage = nestedMessage ? `${topLevelMessage} ${nestedMessage}` : topLevelMessage; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| let errorMessage = nestedMessage ? `${topLevelMessage} ${nestedMessage}` : topLevelMessage; | ||||||||||||||||||||||||||||
| let parameterName: string | null = null; | ||||||||||||||||||||||||||||
| if (response.error.errors) { | ||||||||||||||||||||||||||||
| if (response.error.errors[0]) { | ||||||||||||||||||||||||||||
| errorMessage = | ||||||||||||||||||||||||||||
| response.error.errors[0].defaultUserMessage.replace(/\\./g, ' ') || | ||||||||||||||||||||||||||||
| response.error.errors[0].developerMessage.replace(/\\./g, ' '); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| if ('parameterName' in response.error.errors[0]) { | ||||||||||||||||||||||||||||
| parameterName = response.error.errors[0].parameterName; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
Comment on lines
+70
to
72
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: The 🐛 Proposed fix: move the check inside the existing null guard if (response.error.errors) {
if (response.error.errors[0]) {
errorMessage =
response.error.errors[0].defaultUserMessage.replace(/\\./g, ' ') ||
response.error.errors[0].developerMessage.replace(/\\./g, ' ');
- }
- if ('parameterName' in response.error.errors[0]) {
- parameterName = response.error.errors[0].parameterName;
+ if ('parameterName' in response.error.errors[0]) {
+ parameterName = response.error.errors[0].parameterName;
+ }
}
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| const isClientImage404 = status === 404 && request.url.includes('/clients/') && request.url.includes('/images'); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if (!environment.production && !isClientImage404) { | ||||||||||||||||||||||||||||
|
|
@@ -81,9 +91,12 @@ export class ErrorHandlerInterceptor implements HttpInterceptor { | |||||||||||||||||||||||||||
| message: this.translate.instant('errors.error.token.invalid.message') | ||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||
| } else if (status === 400) { | ||||||||||||||||||||||||||||
| const message = parameterName | ||||||||||||||||||||||||||||
| ? `[${parameterName}] ${errorMessage || 'Invalid parameters were passed in the request!'}` | ||||||||||||||||||||||||||||
| : `${errorMessage || 'Invalid parameters were passed in the request!'}`; | ||||||||||||||||||||||||||||
| this.alertService.alert({ | ||||||||||||||||||||||||||||
| type: this.translate.instant('errors.error.bad.request.type'), | ||||||||||||||||||||||||||||
| message: errorMessage || this.translate.instant('errors.error.bad.request.message') | ||||||||||||||||||||||||||||
| message: message || this.translate.instant('errors.error.bad.request.message') | ||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||
| } else if (status === 403) { | ||||||||||||||||||||||||||||
| this.alertService.alert({ | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| /** | ||
| * Copyright since 2025 Mifos Initiative | ||
| * | ||
| * This Source Code Form is subject to the terms of the Mozilla Public | ||
| * License, v. 2.0. If a copy of the MPL was not distributed with this | ||
| * file, You can obtain one at http://mozilla.org/MPL/2.0/. | ||
| */ | ||
|
|
||
| import { Directive, ElementRef, HostListener, inject } from '@angular/core'; | ||
|
|
||
| @Directive({ selector: '[mifosxPositiveNumber]' }) | ||
| export class PositiveNumberDirective { | ||
|
Comment on lines
+11
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check if other directives in the same module are standalone
fd -e ts -p 'directive' src/app/directives --exec grep -l "standalone.*true" {} \;
# Check the exact pattern used by existing directives
rg -A2 "@Directive" src/app/directives/Repository: openMF/web-app Length of output: 1313 🏁 Script executed: cat -n src/app/directives/directives.module.tsRepository: openMF/web-app Length of output: 1445 Missing The 🐛 Proposed fix-@Directive({ selector: '[mifosxPositiveNumber]' })
+@Directive({ selector: '[mifosxPositiveNumber]', standalone: true })
export class PositiveNumberDirective {🤖 Prompt for AI Agents |
||
| private el = inject(ElementRef); | ||
|
|
||
| @HostListener('keydown', ['$event']) | ||
| onKeyDown(event: KeyboardEvent): void { | ||
| const inputElement = this.el.nativeElement; | ||
|
|
||
| // Check if the key is ArrowUp or ArrowDown | ||
| if (event.key === 'ArrowUp' || event.key === 'ArrowDown') { | ||
| const currentValue = parseFloat(inputElement.value) || 0; | ||
|
|
||
| if (event.key === 'ArrowDown' && currentValue <= 0) { | ||
| // Prevent default if trying to decrement below or at 0 | ||
| event.preventDefault(); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| /** | ||
| * Copyright since 2025 Mifos Initiative | ||
| * | ||
| * This Source Code Form is subject to the terms of the Mozilla Public | ||
| * License, v. 2.0. If a copy of the MPL was not distributed with this | ||
| * file, You can obtain one at http://mozilla.org/MPL/2.0/. | ||
| */ | ||
|
|
||
| import { Injectable } from '@angular/core'; | ||
| import { BehaviorSubject } from 'rxjs'; | ||
| import { LOAN_PRODUCT_TYPE, LoanProductType } from 'app/products/loan-products/models/loan-product.model'; | ||
| import { ActivatedRouteSnapshot } from '@angular/router'; | ||
|
|
||
| @Injectable({ | ||
| providedIn: 'root' | ||
| }) | ||
| export class LoanBaseResolver { | ||
| productType = new BehaviorSubject<LoanProductType>(LOAN_PRODUCT_TYPE.LOAN); | ||
|
|
||
| constructor() {} | ||
|
|
||
| protected initialize(route: ActivatedRouteSnapshot): void { | ||
| const productType = route.queryParams['productType']; | ||
| if (productType !== null) { | ||
| if (productType === 'loan') { | ||
| this.productType.next(LOAN_PRODUCT_TYPE.LOAN); | ||
| } else if (productType === 'working-capital') { | ||
| this.productType.next(LOAN_PRODUCT_TYPE.WORKING_CAPITAL); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| get isWorkingCapital(): boolean { | ||
| return LOAN_PRODUCT_TYPE.WORKING_CAPITAL === this.productType.value; | ||
| } | ||
|
|
||
| get isLoanProduct(): boolean { | ||
| return LOAN_PRODUCT_TYPE.LOAN === this.productType.value; | ||
| } | ||
|
|
||
| get loanProductPath(): string { | ||
| return this.isLoanProduct ? 'loanproducts' : 'working-capital-loan-products'; | ||
| } | ||
|
|
||
| get loanAccountPath(): string { | ||
| return this.isLoanProduct ? 'loans' : 'working-capital-loans'; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,19 +10,25 @@ import { Injectable, inject } from '@angular/core'; | |
| import { ActivatedRouteSnapshot } from '@angular/router'; | ||
| import { Observable } from 'rxjs'; | ||
| import { LoansService } from '../loans.service'; | ||
| import { LoanBaseResolver } from './loan-base.resolver'; | ||
|
|
||
| @Injectable({ | ||
| providedIn: 'root' | ||
| }) | ||
| export class LoanDelinquencyActionsResolver { | ||
| export class LoanDelinquencyActionsResolver extends LoanBaseResolver { | ||
| private loansService = inject(LoansService); | ||
|
|
||
| constructor() { | ||
| super(); | ||
| } | ||
|
|
||
| /** | ||
| * Returns the Loans with Association data. | ||
| * @returns {Observable<any>} | ||
| */ | ||
| resolve(route: ActivatedRouteSnapshot): Observable<any> { | ||
| this.initialize(route); | ||
| const loanId = route.paramMap.get('loanId') || route.parent.paramMap.get('loanId'); | ||
| return this.loansService.getDelinquencyActions(loanId); | ||
| return this.loansService.getDelinquencyActions(this.loanAccountPath, loanId); | ||
| } | ||
|
Comment on lines
29
to
33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding loanId validation for consistency. Unlike 💡 Suggested fix for consistency resolve(route: ActivatedRouteSnapshot): Observable<any> {
this.initialize(route);
const loanId = route.paramMap.get('loanId') || route.parent.paramMap.get('loanId');
+ if (!loanId || isNaN(+loanId)) {
+ return of([]);
+ }
return this.loansService.getDelinquencyActions(this.loanAccountPath, loanId);
}Add 🤖 Prompt for AI Agents |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,20 +15,29 @@ import { Observable } from 'rxjs'; | |
|
|
||
| /** Custom Services */ | ||
| import { LoansService } from '../loans.service'; | ||
| import { LoanBaseResolver } from './loan-base.resolver'; | ||
|
|
||
| /** | ||
| * Loan Delinquency data resolver. | ||
| */ | ||
| @Injectable() | ||
| export class LoanDelinquencyDataResolver { | ||
| export class LoanDelinquencyDataResolver extends LoanBaseResolver { | ||
| private loansService = inject(LoansService); | ||
|
|
||
| constructor() { | ||
| super(); | ||
| } | ||
| /** | ||
| * Returns the Loans with Association data. | ||
| * @returns {Observable<any>} | ||
| */ | ||
| resolve(route: ActivatedRouteSnapshot): Observable<any> { | ||
| this.initialize(route); | ||
| const loanId = route.paramMap.get('loanId') || route.parent.paramMap.get('loanId'); | ||
| return this.loansService.getDelinquencyData(loanId); | ||
| if (!isNaN(+loanId)) { | ||
| if (this.isLoanProduct) { | ||
| return this.loansService.getDelinquencyData(loanId); | ||
| } | ||
| } | ||
| } | ||
|
Comment on lines
34
to
42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Return a safe fallback to prevent downstream NPE. When 🛠️ Suggested fix resolve(route: ActivatedRouteSnapshot): Observable<any> {
this.initialize(route);
const loanId = route.paramMap.get('loanId') || route.parent.paramMap.get('loanId');
if (!isNaN(+loanId)) {
if (this.isLoanProduct) {
return this.loansService.getDelinquencyData(loanId);
}
}
+ // Return empty object to prevent NPE in consumers
+ return of({});
}Add 🤖 Prompt for AI Agents |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,20 +15,28 @@ import { Observable } from 'rxjs'; | |
|
|
||
| /** Custom Services */ | ||
| import { LoansService } from '../loans.service'; | ||
| import { LoanBaseResolver } from './loan-base.resolver'; | ||
|
|
||
| /** | ||
| * Clients data resolver. | ||
| * Loan Delinquency Tags data resolver. | ||
| */ | ||
| @Injectable() | ||
| export class LoanDelinquencyTagsResolver { | ||
| export class LoanDelinquencyTagsResolver extends LoanBaseResolver { | ||
| private loansService = inject(LoansService); | ||
|
|
||
| constructor() { | ||
| super(); | ||
| } | ||
|
|
||
| /** | ||
| * Returns the Loans with Association data. | ||
| * @returns {Observable<any>} | ||
| */ | ||
| resolve(route: ActivatedRouteSnapshot): Observable<any> { | ||
| this.initialize(route); | ||
| const loanId = route.paramMap.get('loanId') || route.parent.paramMap.get('loanId'); | ||
| return this.loansService.getDelinquencyTags(loanId); | ||
| if (!isNaN(+loanId)) { | ||
| return this.loansService.getDelinquencyTags(this.loanAccountPath, loanId); | ||
| } | ||
| } | ||
|
Comment on lines
35
to
41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing return value may cause downstream NPE. When Consider returning an empty observable or a safe fallback object: 🛠️ Suggested fix resolve(route: ActivatedRouteSnapshot): Observable<any> {
this.initialize(route);
const loanId = route.paramMap.get('loanId') || route.parent.paramMap.get('loanId');
if (!isNaN(+loanId)) {
return this.loansService.getDelinquencyTags(this.loanAccountPath, loanId);
}
+ return of([]);
}Add 🤖 Prompt for AI Agents |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,20 +15,24 @@ import { Observable } from 'rxjs'; | |
|
|
||
| /** Custom Services */ | ||
| import { LoansService } from '../loans.service'; | ||
| import { LoanProductService } from 'app/products/loan-products/services/loan-product.service'; | ||
|
|
||
| /** | ||
| * Loan accounts template data resolver. | ||
| */ | ||
| @Injectable() | ||
| export class LoansAccountAndTemplateResolver { | ||
| private loansService = inject(LoansService); | ||
| private loanProductService = inject(LoanProductService); | ||
|
|
||
| /** | ||
| * Returns the loan account template data. | ||
| * @returns {Observable<any>} | ||
| */ | ||
| resolve(route: ActivatedRouteSnapshot): Observable<any> { | ||
| const loanId = route.paramMap.get('loanId') || route.parent.paramMap.get('loanId'); | ||
| return this.loansService.getLoansAccountAndTemplateResource(loanId); | ||
| return this.loanProductService.isLoanProduct | ||
| ? this.loansService.getLoansAccountAndTemplateResource(loanId) | ||
| : this.loansService.getWorkingCapitalLoanDetails(loanId); | ||
| } | ||
|
Comment on lines
32
to
37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check the route configuration to verify resolver ordering
rg -n "LoansAccountAndTemplateResolver" --type ts -A 5 -B 5Repository: openMF/web-app Length of output: 5266 🏁 Script executed: #!/bin/bash
# Check the full route configuration for edit-loans-account and surrounding routes
rg -n "edit-loans-account|LoanDetailsResolver" src/app/loans/loans-routing.module.ts -B 3 -A 8Repository: openMF/web-app Length of output: 5534 🏁 Script executed: #!/bin/bash
# Check the LoanProductService initialization and isLoanProduct property
rg -n "isLoanProduct|initialize" src/app/loans -B 2 -A 3 --type ts | head -100Repository: openMF/web-app Length of output: 9749 🏁 Script executed: #!/bin/bash
# Check if productType is used in edit-loans-account navigation or routes
rg -n "productType|edit-loans-account" src/app/loans --type ts | grep -A 3 -B 3 "edit-loans-account"Repository: openMF/web-app Length of output: 4331 🏁 Script executed: #!/bin/bash
# Find LoanProductService class and its properties
fd -e service.ts | xargs grep -l "LoanProductService" | head -5Repository: openMF/web-app Length of output: 121 🏁 Script executed: #!/bin/bash
# Search for resolveProductTypeDefault method implementation
rg -n "resolveProductTypeDefault" src/app --type ts -B 2 -A 10Repository: openMF/web-app Length of output: 6349 🏁 Script executed: #!/bin/bash
# Check LoanProductService for isLoanProduct property and initialization
rg -n "class LoanProductService|isLoanProduct|initialize\(" src/app/loans -B 2 -A 5 --type ts | grep -A 20 "class LoanProductService"Repository: openMF/web-app Length of output: 40 🏁 Script executed: #!/bin/bash
# Find and read the LoanProductService file
cat -n src/app/products/loan-products/services/loan-product.service.ts | head -150Repository: openMF/web-app Length of output: 2326 🏁 Script executed: #!/bin/bash
# Check for isLoanProduct and initialize method in the service
rg -n "isLoanProduct|initialize" src/app/products/loan-products/services/loan-product.service.ts -B 3 -A 5Repository: openMF/web-app Length of output: 1265 The resolver must initialize the product type from route parameters instead of relying on pre-existing service state. This resolver depends on If the resolver executes without the productType query parameter being present or the service not yet initialized from a previous page, The resolver should read const productType = LoanProductBaseComponent.resolveProductTypeDefault(route, 'loan');
this.loanProductService.initialize(productType);
return this.loanProductService.isLoanProduct
? this.loansService.getLoansAccountAndTemplateResource(loanId)
: this.loansService.getWorkingCapitalLoanDetails(loanId);🤖 Prompt for AI Agents |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regex pattern
/\\./gis likely incorrect and property access may throw.Two issues here:
Regex mismatch:
/\\./gmatches a literal backslash followed by any character, not a period. If the intent is to replace periods (.) with spaces, use/\./g. If the intent is to replace backslash-dot sequences (\.), use/\\\./g.Potential TypeError: If
defaultUserMessageisundefined, calling.replace()on it will throw before the||fallback is evaluated.🐛 Proposed fix with optional chaining and corrected regex
Assuming you want to replace periods with spaces:
Or if you want to replace backslash-dot (
\.) sequences with spaces:📝 Committable suggestion
🤖 Prompt for AI Agents