WEB-916 Align Loan Product Currency Form with Product Form#3493
Merged
IOhacker merged 1 commit intoopenMF:devfrom Apr 26, 2026
Merged
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Template src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.html |
Replaced always-visible currency multiples input with a mat-checkbox bound to setMultiples; conditionally render inMultiplesOf/installmentInMultiplesOf; removed loanProductService.isLoanProduct guard; added tooltip and required/min error messages. |
Component logic src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts |
Added setMultiples form control and setupConditionalValidation() to toggle required + min(1) validators and clear values when unchecked; replaced installmentAmountInMultiplesOf with installmentInMultiplesOf; digits-after-decimal only set when editing template; subscription uses takeUntilDestroyed(this.destroyRef); loanProductCurrency now explicitly includes/nullifies multiples fields. |
Imports / DI src/app/.../loan-product-currency-step.component.ts |
Component now imports/injects DestroyRef (and template references updated to use MatCheckbox), to manage lifecycle of the setMultiples subscription. |
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
- WEB-632-loan-product-default-empty-multiples-fields #3084: Implements a
setMultiplescheckbox and conditional rendering/validation for currency/installment multiples in the same component. - WEB-566 Validation missing negative values accepted in currency in multiples of installment in multiples of and decimal places fields #2981: Modifies currency-related form controls and validation (digitsAfterDecimal, inMultiplesOf, installment multiples) in the same component.
- WEB-632 Set installment in multiples of and decimal places fields to empty by default in loan product form #3053: Adjusts initialization/defaults for
inMultiplesOfanddigitsAfterDecimalwithin the loan-product-currency-step component.
Suggested reviewers
- IOhacker
- alberto-art3ch
🚥 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 summarizes the main change: aligning the Loan Product Currency form with the Product form by adding a conditional checkbox control for the currency multiples field. |
| 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: 2
🤖 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-currency-step/loan-product-currency-step.component.html`:
- Around line 54-80: The change removed the companion installment-multiple
control when setMultiples is true; restore a second form field and form control
for the installment multiple (add a formControlName like
"installmentInMultiplesOf" alongside the existing "inMultiplesOf" in the
loanProductCurrencyForm when loanProductCurrencyForm.value.setMultiples is
true), include the same number input, min=1 and required validators and
corresponding mat-error blocks (check for hasError('min') and
hasError('required')), ensure the form group initialization adds this control,
and update the TS payload builder (the function that assembles the loan product
payload) to include the installment multiple property when setMultiples is
enabled so both the loan and its installment values are preserved.
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts`:
- Around line 112-121: The loanProductCurrency getter currently omits
inMultiplesOf when the multiples checkbox is unchecked, which fails to clear the
server value on edit; update the getter (loanProductCurrency) to always include
the inMultiplesOf property: if the form control that toggles multiples (e.g.,
loanProductCurrencyForm.controls['setMultiples'] or formValue.setMultiples) is
false, set result.inMultiplesOf = null, otherwise set it to
formValue.inMultiplesOf (keeping the existing guard for empty/undefined values)
so the payload explicitly signals clearing the field.
🪄 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: 426faddc-7814-4489-9a0b-718c3a50670c
📒 Files selected for processing (2)
src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts
JaySoni1
force-pushed
the
WEB-916-align-loan-product-currency-form-with-product-form
branch
from
9dae486 to
ad59ebb
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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-currency-step/loan-product-currency-step.component.ts`:
- Around line 58-63: The checkbox initialization only reads
loanProductsTemplate.inMultiplesOf, so setMultiples should be derived from
either persisted field to avoid wiping installmentInMultiplesOf; change the
patchValue call in loanProductCurrencyForm.patchValue to set setMultiples:
!!(this.loanProductsTemplate.inMultiplesOf ||
this.loanProductsTemplate.installmentInMultiplesOf) so the form reflects an
existing installmentInMultiplesOf and preserves it during serialization by
LoanProductCurrency logic.
🪄 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: 0e1a6019-374f-41eb-b15f-d72844660607
📒 Files selected for processing (2)
src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.html
JaySoni1
force-pushed
the
WEB-916-align-loan-product-currency-form-with-product-form
branch
from
ad59ebb to
1c1768c
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts (1)
129-132: Consider defining an interface for the return type.Using
anyforresultbypasses type checking. Consider defining aLoanProductCurrencyPayloadinterface to ensure field names and types remain consistent with the API contract.interface LoanProductCurrencyPayload { currencyCode: string; digitsAfterDecimal: string | number; inMultiplesOf: number | null; installmentInMultiplesOf: number | 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-currency-step/loan-product-currency-step.component.ts` around lines 129 - 132, Replace the ad-hoc any-typed result with a typed payload: declare a LoanProductCurrencyPayload interface (as suggested) and use it for the result variable in loan-product-currency-step.component.ts so the object returned from the currency step has typed properties currencyCode: string, digitsAfterDecimal: string | number, inMultiplesOf: number | null and installmentInMultiplesOf: number | null; update the const result declaration to be typed as LoanProductCurrencyPayload and populate the additional fields (inMultiplesOf, installmentInMultiplesOf) with the appropriate values or null to match the API contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts`:
- Around line 129-132: Replace the ad-hoc any-typed result with a typed payload:
declare a LoanProductCurrencyPayload interface (as suggested) and use it for the
result variable in loan-product-currency-step.component.ts so the object
returned from the currency step has typed properties currencyCode: string,
digitsAfterDecimal: string | number, inMultiplesOf: number | null and
installmentInMultiplesOf: number | null; update the const result declaration to
be typed as LoanProductCurrencyPayload and populate the additional fields
(inMultiplesOf, installmentInMultiplesOf) with the appropriate values or null to
match the API contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 732c0910-9a3b-4917-a5da-54ed69a250a0
📒 Files selected for processing (2)
src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.ts
✅ Files skipped from review due to trivial changes (1)
- src/app/products/loan-products/loan-product-stepper/loan-product-currency-step/loan-product-currency-step.component.html
Contributor
|
@JaySoni1 thank you for your efforts! |
Contributor
Author
|
@IOhacker Thank You for the review |
This was referenced Apr 28, 2026
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
2 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.
Changes Made :-
-Aligned Loan Product Currency form with product by adding a conditional checkbox to control "Currency in multiples of" field visibility.
-Changed Decimal Places to default to empty .
WEB-916
Before :-
After :-
Summary by CodeRabbit
New Features
Bug Fixes