WEB-1022: First version with landing page, personal loan and advance loan - #3701
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
|
| Layer / File(s) | Summary |
|---|---|
Wizard payload configuration and normalization src/app/products/loan-products/wizard/loan-product.config.ts, src/app/products/loan-products/wizard/loan-product.config.spec.ts |
Defines wizard metadata, step configuration, template default merging, payload sanitization, and mode-specific create-payload building, with unit tests covering personal and custom-advanced cases. |
Wizard component, template, styles, and tests src/app/products/loan-products/wizard/loan-product-wizard.component.* |
Implements the standalone wizard with step visibility, field filtering, review rendering, submit payload construction, and unit coverage for advanced strategy, review, and payload behavior. |
Create flow, selection page, routing, and menu wiring src/app/products/loan-products/create-loan-product/*, src/app/products/products-routing.module.ts, src/app/products/products.module.ts, src/app/products/loan-products/loan-products.component.html, src/app/products/loan-products/loan-products-template.resolver.ts, src/app/products/loan-products/loan-products.ts |
Replaces the direct create page with a selection landing page, adds Classic and wizard-based create routes, registers the new component, updates the create menu, fixes template resolution, and makes charges mapping defensive. |
Classic loan product creation stepper src/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.* |
Adds the Classic stepper component, step wiring, advanced allocation handling, deferred-income handling, payload submission, and related styles. |
Summary gating and interest-rate terminology src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.*, src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.*, src/assets/translations/* |
Adds view-mode gating for loan product summary sections, updates interest-rate labels from nominal to annual, and adds the matching translation keys. |
Review support for summary and create flow src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.spec.ts, src/app/products/loan-products/wizard/loan-product-wizard.component.spec.ts, src/app/products/loan-products/wizard/loan-product.config.spec.ts |
Adds tests for summary gating, wizard review/payload behavior, and payload normalization rules. |
Standalone selection landing page src/app/products/loan-products/create-loan-product/loan-product-selection.component.* |
Adds the loan product creation landing page with selectable product cards and active/inactive CTA states. |
Estimated code review effort: 5 (Critical) | ~120 minutes
Loans Account Terms Step Currency Resolution
| Layer / File(s) | Summary |
|---|---|
Currency resolver and interest-rate field UI src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.* |
Adds resolveCurrency, uses it during initialization and changes, and updates the nominal interest-rate field to use a suffix and scoped spacing. |
Estimated code review effort: 2 (Simple) | ~15 minutes
Suggested reviewers: adamsaghy
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title matches the main change: a new landing page and new personal/advanced loan creation flows. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ 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.
3f6fa32 to
93c6090
Compare
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html (1)
11-19: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGating condition no longer matches the resolved currency it guards.
The wrapper now checks
loansAccountProductTemplate?.currency(the raw field) while the input still binds[currency]="currency", which is populated by the newresolveCurrency()fallback (currencyOptions, product.currencyOptions, etc.). If the raw template lacks a directcurrencyfield butresolveCurrency()successfully derives one from a fallback collection, this block will never render — silently hiding the Principal input even though a valid currency was resolved. The condition should check the resolvedcurrencyfield instead of the raw template property.🐛 Proposed fix
- `@if` (loansAccountProductTemplate?.currency) { + `@if` (currency) { <mifosx-input-amount class="flex-48" [currency]="currency"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html` around lines 11 - 19, The Principal input gate in loans-account-terms-step.component.html is checking the raw loansAccountProductTemplate?.currency instead of the resolved currency value used by mifosx-input-amount. Update the `@if` condition to use the component’s currency property so the block renders whenever resolveCurrency() successfully finds a fallback currency from currencyOptions or product.currencyOptions, keeping the guard aligned with the bound [currency].src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts (1)
206-217: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRe-resolve currency after the edit-mode data swap
this.currencyis set fromloansAccountProductTemplate/loansAccountTermsDatabeforeloansAccountTemplatereplaces the source in edit mode, so the amount field can keep the product currency instead of the account currency. Recompute it afterloansAccountTermsDatais finalized in bothngOnChangesandngOnInit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts` around lines 206 - 217, The currency is resolved before the edit-mode swap in loans-account-terms-step.component, so ngOnChanges and ngOnInit can leave this.currency based on loansAccountProductTemplate instead of the final loansAccountTemplate. Update the logic in ngOnChanges and ngOnInit to finalize loansAccountTermsData first, then call resolveCurrency using the selected source so the amount field reflects the account currency in edit mode.
🧹 Nitpick comments (12)
src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.scss (1)
34-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded pixel value and custom class deviate from spacing/theming guideline.
margin-left: 1pxis a bespoke value not aligned to the 8px grid, and.interest-rate-fieldis a custom class rather than reusing theme variables. As per coding guidelines,src/**/*.{scss,html}should "Stick to the 8px grid system for visual design and spacing" and "Leverage SCSS variables defined insrc/main.scssandsrc/theme/mifosx-theme.scssrather than generating custom classes and explicit pixel values."Given this is a tiny cosmetic suffix-spacing tweak, consider using an existing spacing variable if one is close enough (e.g., a small predefined gap token), or confirm the 1px value is intentional/necessary for this MDC suffix alignment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.scss` around lines 34 - 36, The suffix spacing tweak in loans-account-terms-step.component.scss uses a hardcoded 1px margin and a custom .interest-rate-field selector, which should be aligned with the spacing/theming rules. Update the style to reuse an existing spacing token or theme variable from src/main.scss or src/theme/mifosx-theme.scss where possible, and only keep the 1px adjustment if it is explicitly required for the MAT/MDC suffix alignment. Ensure the change is localized around .interest-rate-field .mat-mdc-form-field-text-suffix and avoids introducing bespoke spacing values.Source: Coding guidelines
src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts (1)
515-544: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
resolveCurrencyonly checks the first element of each fallback collection.If
collection[0]lacks acode, later valid entries in the same array are ignored even though other collections might also be skipped. Consider scanning the whole collection (collection.find(c => c?.code)) rather than just index 0.Also,
source: anybypasses strict typing; consider a narrower type/interface for the shapes this method actually receives (template, account data, etc.) per the project's strict-typing convention forsrc/app/**/*.ts.♻️ Suggested tweak for full-collection scan
for (const collection of currencyOptionCollections) { if (Array.isArray(collection) && collection.length > 0) { - const firstCurrency = collection[0]; - if (firstCurrency?.code) { + const firstCurrency = collection.find((c: any) => c?.code); + if (firstCurrency) { return { ...firstCurrency, displaySymbol: firstCurrency.displaySymbol || firstCurrency.displayLabel || firstCurrency.code }; } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts` around lines 515 - 544, The resolveCurrency helper currently only inspects the first item in each fallback collection, so valid currencies later in the array can be missed; update the fallback search in resolveCurrency to scan each collection for the first entry with a code instead of using index 0. While you’re there, replace the loose any on source with a narrower interface/type that matches the actual shapes this method consumes (such as template/account/product currency data) to align with the project’s strict-typing convention.Source: Path instructions
src/app/shared/input-amount/input-amount.component.scss (1)
24-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant default declarations.
opacity: 1;andvisibility: visible;are already the browser defaults for aspan; unless this is meant to override a hidden state elsewhere, they add no effect.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/shared/input-amount/input-amount.component.scss` around lines 24 - 32, The .input-indicator rule in input-amount.component.scss includes redundant default declarations; remove the explicit opacity and visibility settings unless they are needed to override a hidden state elsewhere. Keep the selector and its other layout/style properties intact, and only preserve those declarations if a corresponding hidden/disabled state in the same component depends on them.src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts (1)
695-710: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate the four section-visibility getters.
showDownPaymentsSection,showInterestRecalculationSection,showGuaranteeSection, andshowTrancheSectionare identical except for the field they read (!this.isViewAction || !!this.loanProduct.<field>). Consider a single parametrized helper, similar tohideToggleInView, to avoid the duplicated boolean expression as more sections get added later.♻️ Suggested consolidation
- get showDownPaymentsSection(): boolean { - return !this.isViewAction || !!this.loanProduct.enableDownPayment; - } - - get showInterestRecalculationSection(): boolean { - return !this.isViewAction || !!this.loanProduct.isInterestRecalculationEnabled; - } - - get showGuaranteeSection(): boolean { - return !this.isViewAction || !!this.loanProduct.holdGuaranteeFunds; - } - - get showTrancheSection(): boolean { - return !this.isViewAction || !!this.loanProduct.multiDisburseLoan; - } + showSectionInView(value: unknown): boolean { + return !this.isViewAction || !!value; + } + + get showDownPaymentsSection(): boolean { + return this.showSectionInView(this.loanProduct.enableDownPayment); + } + + get showInterestRecalculationSection(): boolean { + return this.showSectionInView(this.loanProduct.isInterestRecalculationEnabled); + } + + get showGuaranteeSection(): boolean { + return this.showSectionInView(this.loanProduct.holdGuaranteeFunds); + } + + get showTrancheSection(): boolean { + return this.showSectionInView(this.loanProduct.multiDisburseLoan); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts` around lines 695 - 710, Consolidate the duplicated section-visibility logic in loan-product-summary.component.ts: the getters showDownPaymentsSection, showInterestRecalculationSection, showGuaranteeSection, and showTrancheSection all repeat the same !this.isViewAction || !!this.loanProduct.<field> check. Add a small parametrized helper, similar to hideToggleInView, that accepts the loanProduct field name or accessor and returns the shared boolean result, then have each getter delegate to it so the repeated expression lives in one place.src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.html (1)
502-513: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMerge duplicate
enableDownPaymentconditions.Two adjacent
@if (loanProduct.enableDownPayment)blocks could be combined into one for readability.♻️ Suggested merge
- `@if` (loanProduct.enableDownPayment) { - <div class="flex-fill layout-row-wrap responsive-column"> - <span class="flex-40">{{ 'labels.inputs.Disbursed Amount Percentage Down Payment' | translate }} (%):</span> - <span class="flex-60">{{ loanProduct.disbursedAmountPercentageForDownPayment }} %</span> - </div> - } - `@if` (loanProduct.enableDownPayment) { - <div class="flex-fill layout-row-wrap responsive-column"> - <span class="flex-40">{{ 'labels.inputs.Enable Auto Repayment for Down Payment' | translate }}:</span> - <span class="flex-60">{{ loanProduct.enableAutoRepaymentForDownPayment | yesNo }}</span> - </div> - } + `@if` (loanProduct.enableDownPayment) { + <div class="flex-fill layout-row-wrap responsive-column"> + <span class="flex-40">{{ 'labels.inputs.Disbursed Amount Percentage Down Payment' | translate }} (%):</span> + <span class="flex-60">{{ loanProduct.disbursedAmountPercentageForDownPayment }} %</span> + </div> + <div class="flex-fill layout-row-wrap responsive-column"> + <span class="flex-40">{{ 'labels.inputs.Enable Auto Repayment for Down Payment' | translate }}:</span> + <span class="flex-60">{{ loanProduct.enableAutoRepaymentForDownPayment | yesNo }}</span> + </div> + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.html` around lines 502 - 513, The template has two adjacent checks on loanProduct.enableDownPayment that should be merged into a single conditional block for readability. In loan-product-summary.component.html, combine the duplicated `@if` around the disbursed amount percentage and the auto repayment section so the related markup sits under one loanProduct.enableDownPayment guard without changing the displayed content.src/app/products/loan-products/wizard/loan-product-wizard.component.spec.ts (1)
44-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd one template-render smoke test for the standalone wizard.
The suite only constructs the class, so template binding failures such as unsafe
loanProductsTemplate.*dereferences are not exercised. Add aTestBed.createComponent(LoanProductWizardComponent)smoke test with minimal resolver data. As per path instructions, tests should use stable selectors and minimal brittle timing dependencies.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/products/loan-products/wizard/loan-product-wizard.component.spec.ts` around lines 44 - 70, The current spec only instantiates LoanProductWizardComponent directly, so template binding errors in the standalone view are not covered. Add a smoke test that uses TestBed.createComponent(LoanProductWizardComponent) with minimal resolver/mock data and stable selectors to verify the template renders without unsafe loanProductsTemplate dereferences. Keep the existing createComponent helper for class-only setup, and add one render-focused test that exercises the component template with minimal timing dependence.Source: Path instructions
src/app/products/loan-products/wizard/loan-product-wizard.component.ts (1)
71-72: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftReplace new
anyboundaries with explicit wizard/template/payload types.The new component relies on
anyfor the template input, payload building, form control map, and create response. Define focused interfaces for the template fields consumed here and the create response shape. Based on learnings, API response shapes should use specific interfaces/types instead ofany; as per coding guidelines,src/app/**/*.tsshould use strict typing conventions.Also applies to: 274-287, 539-543, 558-559
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/products/loan-products/wizard/loan-product-wizard.component.ts` around lines 71 - 72, Replace the new any-based boundaries in LoanProductWizardComponent with explicit types: define focused interfaces for the loanProductsTemplate input, the payload/form control map used by the wizard, and the create response returned by the API. Update the relevant members and methods in loan-product-wizard.component.ts to use these concrete types instead of any, and keep the typings aligned with the fields actually consumed in the component so the wizard flow remains strictly typed throughout.Sources: Coding guidelines, Learnings
src/app/products/loan-products/wizard/loan-product-wizard.component.scss (1)
9-156: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftUse the 8px grid and theme variables for the wizard styles.
Several spacing values fall off-grid (
1.25rem,0.35rem,0.75rem) and colors are custom literals. Normalize spacing to 8px increments and reuse variables/tokens fromsrc/main.scss/src/theme/mifosx-theme.scss. As per coding guidelines, styling should stick to the 8px grid and leverage existing SCSS variables.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/products/loan-products/wizard/loan-product-wizard.component.scss` around lines 9 - 156, The wizard SCSS uses off-grid spacing values and hardcoded color literals; update the styles in loan-product-wizard.component.scss to follow the 8px spacing grid and replace custom backgrounds/opacities with existing theme variables/tokens from main.scss and mifosx-theme.scss. Focus on the affected selectors like .wizard-shell, .landing-page__eyebrow, .review-chip, .step-panel, .review-banner, .review-metric, and .review-defaults-pill, normalizing spacing to existing increments and reusing shared SCSS variables instead of literal rgb/hex values.Source: Coding guidelines
src/app/products/loan-products/wizard/loan-product.config.ts (1)
123-735: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftMove wizard UI copy to translation keys.
These product names, descriptions, labels, placeholders, hints, and option labels are user-facing but hardcoded in the TS config. Store translation keys here and resolve them in the component/template. 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 current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/products/loan-products/wizard/loan-product.config.ts` around lines 123 - 735, The loan product wizard config still hardcodes user-facing copy across PRODUCT_CARDS, VALUE_MAP, and FORM_STEPS, including names, descriptions, labels, placeholders, hints, and option text. Replace these literals with translation keys in loan-product.config.ts and update the wizard component/template to resolve them through `@ngx-translate/core`. Use the existing symbols PRODUCT_CARDS, VALUE_MAP, and FORM_STEPS to locate and convert every visible string.Source: Coding guidelines
src/app/products/loan-products/wizard/loan-product-wizard.component.html (1)
13-137: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftTranslate all visible wizard text.
Static text like “Multi-step form”, review metric labels, default notices, action buttons, plus
field.label/option.labelvalues, should be translation keys resolved through@ngx-translate/core. As per coding guidelines, user-facing strings must use i18n variables andnpm run translations:extractshould be run when strings are added.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/products/loan-products/wizard/loan-product-wizard.component.html` around lines 13 - 137, The loan-product wizard template still contains hardcoded user-facing text, including the stepper header, review labels, default notice, buttons, and option/field labels. Update the loan-product-wizard.component.html bindings to resolve all visible copy through `@ngx-translate/core` translation keys instead of raw strings, using the existing template symbols like visibleSteps, reviewGroups, field.label, and option.label as the places to swap in translated values. After adding or changing any strings, make sure the new translation keys are included and run npm run translations:extract.Source: Coding guidelines
src/app/products/loan-products/create-loan-product/create-loan-product.component.ts (1)
51-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse translation keys for
pageTitle.Both title variants are user-facing strings rendered by the template. Store/resolve translation keys instead of hardcoded English. As per coding guidelines, user-facing strings must use proper i18n variables from
@ngx-translate/core.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/products/loan-products/create-loan-product/create-loan-product.component.ts` around lines 51 - 54, The `CreateLoanProductComponent` is setting `pageTitle` with hardcoded user-facing English text instead of i18n keys. Update the `pageTitle` assignment logic in the component to store translation keys for both `custom-advanced` and default loan creation titles, and resolve them through `@ngx-translate/core` in the component/template flow. Use the existing `profileMode`/`pageTitle` logic and keep the route-based selection intact, but replace the literal strings with translation key references.Source: Coding guidelines
src/app/products/loan-products/create-loan-product/loan-product-selection.component.html (1)
17-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider Angular Material chip/badge components instead of custom
span/div.The version badge and status chip are custom-styled elements;
<mat-chip>(fromMatChipsModule) would align better with the guideline to prefer Material elements over native HTML where possible.Also applies to: 35-37
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/products/loan-products/create-loan-product/loan-product-selection.component.html` around lines 17 - 20, The version/status badge in loan-product-selection.component.html is using custom div/span markup instead of Angular Material. Update the badge blocks referenced by the landing-page__badge and the related status chip markup to use Material chip components from MatChipsModule, keeping the same displayed text and styling intent while replacing the native elements with the appropriate mat-chip structure.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.html`:
- Line 317: The loan-product-summary header is now hardcoded instead of using
the translated label, which removes i18n support. Update the heading in
loan-product-summary.component.html to use the existing translation
key/translate binding pattern used by this component for other user-facing
labels, keeping the text sourced through `@ngx-translate/core` rather than inline
English. Focus on the “Annual interest rate by loan cycle” heading and restore
the same translation approach used elsewhere in the template.
- Around line 172-178: The "Annual interest rate:" text in
loanProductSummaryComponent is hardcoded and bypasses translation. Update the
label in loan-product-summary.component.html to use the same `@ngx-translate/core`
pattern as the surrounding labels in this block, referencing the appropriate
labels.inputs key instead of a raw string. Keep the change localized to the
loanProduct.isLinkedToFloatingInterestRates section so all user-facing text
remains i18n-friendly.
In
`@src/app/products/loan-products/create-loan-product/loan-product-selection.component.html`:
- Around line 12-19: The loan product selection view still contains multiple
user-facing hardcoded strings, so update the template and its backing component
to use `@ngx-translate/core` keys instead of literal text. In
loan-product-selection.component.html and the related TypeScript for the product
cards/labels, replace "Products", "Loan product creation", "Select a product
below.", "Version 2", "Personal and Custom Loan enabled", "Product", the
Active/Disabled status text, the ctaLabel fallback "Create Personal Loan", and
"Coming soon" with translated values wired through existing translation
utilities. Then add/extract the new keys via the translation pipeline so the
component consistently uses i18n across all user-facing copy.
- Line 49: The fallback in loan-product-selection.component.html is too specific
and can mislabel any future active product card missing ctaLabel. Update the
template used by LoanProductSelectionComponent so it does not hardcode "Create
Personal Loan" as the generic default; instead, ensure each PRODUCT_CARDS entry
provides an explicit ctaLabel or derive the fallback from the selected
product/card data in a way that matches the current item.
In
`@src/app/products/loan-products/create-loan-product/loan-product-selection.component.scss`:
- Line 29: Update the spacing in loan-product-selection.component.scss to follow
the 8px grid by replacing the non-conforming rem values with 8px-based
increments. Review the affected selectors around the referenced rules, including
the ones using 0.35rem, 0.75rem, 1.25rem, and 0.25rem, and adjust them to the
nearest consistent spacing tokens so the styles stay visually aligned. Keep the
changes scoped to the existing SCSS rules in this component.
- Line 24: Replace the hardcoded hex fallbacks in
loan-product-selection.component.scss with shared theme tokens or SCSS variables
defined in src/main.scss and src/theme/mifosx-theme.scss. Update the styles
around the affected selectors in loan-product-selection.component.scss to use
the existing theme-based variables for surface container and primary colors
instead of `var(--mat-sys-..., #...)`, and apply the same change to the other
flagged style rules in this component so the styling stays aligned with the app
theme.
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html`:
- Line 244: The loan product terms step still contains hardcoded English labels
and validation text instead of localized strings. Update the template in the
loan-product-terms-step component to replace “Annual interest rate (%)”, the
default/min/max error text, and “Annual interest rate by loan cycle” with
ngx-translate keys, and make sure the corresponding translated entries are used
consistently throughout this step.
In
`@src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts`:
- Line 576: The loan-cycle heading is hardcoded instead of using localization.
Update the heading used in loan-product-terms-step.component and its matching
template in loan-product-terms-step.component.html to read from a translation
key, following the same pattern as the other loan-product labels. Use the
existing loan-product terms step symbols to locate the title definition and the
template binding, and replace the literal “Annual interest rate by loan cycle”
with a translatable key/value lookup.
In `@src/app/products/loan-products/wizard/loan-product-wizard.component.html`:
- Around line 22-33: The payment-allocation step in
loan-product-wizard.component.html is binding directly to loanProductsTemplate
properties before the template is guaranteed to exist. Guard the entire
payment-allocation block or its inputs with an existence check on
loanProductsTemplate so mifosx-loan-product-payment-strategy-step only renders
once resolver data is available, while keeping the existing
advancedPaymentAllocations and advancedCreditAllocations bindings intact.
In `@src/app/products/loan-products/wizard/loan-product-wizard.component.ts`:
- Around line 274-278: The submit flow in loan-product-wizard.component.ts
currently ignores the validation metadata from FORM_STEPS, so required and
maxLength rules never block the reactive form. Add Angular validators to the
form controls by deriving them from the config in FORM_STEPS, wire that
validation into the wizard’s form lifecycle, and make buildPayloadForSubmit or
the submit path refuse to proceed when the form is invalid. Use the existing
loan-product-wizard component methods and form setup code that builds the step
controls so the validation is enforced before payload assembly.
- Around line 533-537: The interestLabel getter in
loan-product-wizard.component.ts is treating 0 as empty because it uses a truthy
check on interestRatePerPeriod, so zero-interest products render as —. Update
the logic to use the same null/undefined/empty check pattern as
formattedPrincipal, and keep interestLabel returning 0% with the formatted
period when the rate is zero while preserving the fallback only for missing
values.
In `@src/app/products/loan-products/wizard/loan-product.config.ts`:
- Around line 671-682: The Charges step is still collecting free-text names
instead of charge IDs, but `buildChargeReferences` only works with actual charge
references. Update the loan product config fields in `loan-product.config.ts` so
`chargeName` and `overdueCharge` are removed or replaced with selectable charge
ID inputs, and ensure any payload assembly that consumes these values uses
template charge IDs rather than user-entered text. Also review the related
Charges step section noted in the comment and align it with the same ID-based
reference flow.
- Around line 1110-1136: The `loan-product.config.ts` merge in the
`defaults`/`merged` flow is still letting hidden defaults override
Custom/Advanced user input because `defaults` is spread after
`formValuesWithTemplateDefaults`. Update the `custom-advanced` branch so only
truly hidden/non-editable fields remain in `defaults` and visible fields like
`description`, `includeInBorrowerCycle`, `loanScheduleType`,
`loanScheduleProcessingType`, and `canUseForTopup` are not forced back from
`HIDDEN_DEFAULTS`. Adjust the `merged` construction so user-entered form values
win over defaults, while preserving the existing
`sanitizeCreateLoanProductPayload` behavior for derived fields like
`daysInYearCustomStrategy`.
- Around line 530-536: The daysInYearCustomStrategy select is using an option
value that does not match the backend normalization map, so the chosen value can
bypass enum translation in the create payload. Update the options in
loan-product.config.ts for the daysInYearCustomStrategy field so they align with
ENUM_CODE_MAPPINGS.daysInYearCustomStrategy, and make sure the select values use
the exact backend enum-friendly strings that the mapping logic expects.
In `@src/app/products/products-routing.module.ts`:
- Around line 175-177: The create route is now pointing to
LoanProductSelectionComponent, which only drives the loan flow and removes the
working-capital path. Update the routing in products-routing.module.ts so the
create entry can still reach the working-capital creation form, and adjust
LoanProductSelectionComponent/its template to either branch by product type or
expose a working-capital option instead of hardcoding only the loan path.
In `@src/app/products/products.module.ts`:
- Line 134: `LoanProductSelectionComponent` is a standalone component, so it
should not be listed in `ProductsModule.declarations`. Update `ProductsModule`
to remove that declaration and, if the module needs to use it directly, add
`LoanProductSelectionComponent` to the module’s imports instead; otherwise keep
it only route-accessible. Use the `ProductsModule` and
`LoanProductSelectionComponent` symbols to locate the change.
In `@src/app/shared/input-amount/input-amount.component.html`:
- Line 12: The input-amount.component.html template still contains unresolved
git merge conflict markers, which will break Angular template parsing. Clean up
all conflict artifacts in the template by removing every <<<<<<<, =======, and
>>>>>>> marker and leaving one fully resolved version of each affected block.
Verify the fix in the input-amount component template so the shipped HTML is
valid and compiles.
- Around line 41-54: Restore the missing currency null-guards in
InputAmountComponent’s template so optional currency access is safe. In the
template branches around the `currency.code` and `currency.displaySymbol`
bindings, bring back the conditional rendering used by the earlier `@if
(currency)` / `@else` flow, and ensure `displayHint` only reveals the hint when
`currency` is present. Update the `mifosxFormatAmount`, `matPrefix`,
`matSuffix`, and hint text usage so `InputAmountComponent` never dereferences
`currency` unless it has been checked first.
In `@src/assets/translations/en-US.json`:
- Around line 586-591: Resolve the merge conflict markers in the en-US
translations JSON so the file remains valid JSON. In the buttons section of
src/assets/translations/en-US.json, remove the conflict artifacts from the added
loan product entries and keep only the intended keys from the merge (refer to
the translation keys like "Create Loan Product (Classic)" and "Create Loan
Product (Convenient)"). Also avoid duplicating "Create Working Capital Product"
since it already exists later in the same buttons block.
---
Outside diff comments:
In
`@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html`:
- Around line 11-19: The Principal input gate in
loans-account-terms-step.component.html is checking the raw
loansAccountProductTemplate?.currency instead of the resolved currency value
used by mifosx-input-amount. Update the `@if` condition to use the component’s
currency property so the block renders whenever resolveCurrency() successfully
finds a fallback currency from currencyOptions or product.currencyOptions,
keeping the guard aligned with the bound [currency].
In
`@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts`:
- Around line 206-217: The currency is resolved before the edit-mode swap in
loans-account-terms-step.component, so ngOnChanges and ngOnInit can leave
this.currency based on loansAccountProductTemplate instead of the final
loansAccountTemplate. Update the logic in ngOnChanges and ngOnInit to finalize
loansAccountTermsData first, then call resolveCurrency using the selected source
so the amount field reflects the account currency in edit mode.
---
Nitpick comments:
In
`@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.scss`:
- Around line 34-36: The suffix spacing tweak in
loans-account-terms-step.component.scss uses a hardcoded 1px margin and a custom
.interest-rate-field selector, which should be aligned with the spacing/theming
rules. Update the style to reuse an existing spacing token or theme variable
from src/main.scss or src/theme/mifosx-theme.scss where possible, and only keep
the 1px adjustment if it is explicitly required for the MAT/MDC suffix
alignment. Ensure the change is localized around .interest-rate-field
.mat-mdc-form-field-text-suffix and avoids introducing bespoke spacing values.
In
`@src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts`:
- Around line 515-544: The resolveCurrency helper currently only inspects the
first item in each fallback collection, so valid currencies later in the array
can be missed; update the fallback search in resolveCurrency to scan each
collection for the first entry with a code instead of using index 0. While
you’re there, replace the loose any on source with a narrower interface/type
that matches the actual shapes this method consumes (such as
template/account/product currency data) to align with the project’s
strict-typing convention.
In
`@src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.html`:
- Around line 502-513: The template has two adjacent checks on
loanProduct.enableDownPayment that should be merged into a single conditional
block for readability. In loan-product-summary.component.html, combine the
duplicated `@if` around the disbursed amount percentage and the auto repayment
section so the related markup sits under one loanProduct.enableDownPayment guard
without changing the displayed content.
In
`@src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts`:
- Around line 695-710: Consolidate the duplicated section-visibility logic in
loan-product-summary.component.ts: the getters showDownPaymentsSection,
showInterestRecalculationSection, showGuaranteeSection, and showTrancheSection
all repeat the same !this.isViewAction || !!this.loanProduct.<field> check. Add
a small parametrized helper, similar to hideToggleInView, that accepts the
loanProduct field name or accessor and returns the shared boolean result, then
have each getter delegate to it so the repeated expression lives in one place.
In
`@src/app/products/loan-products/create-loan-product/create-loan-product.component.ts`:
- Around line 51-54: The `CreateLoanProductComponent` is setting `pageTitle`
with hardcoded user-facing English text instead of i18n keys. Update the
`pageTitle` assignment logic in the component to store translation keys for both
`custom-advanced` and default loan creation titles, and resolve them through
`@ngx-translate/core` in the component/template flow. Use the existing
`profileMode`/`pageTitle` logic and keep the route-based selection intact, but
replace the literal strings with translation key references.
In
`@src/app/products/loan-products/create-loan-product/loan-product-selection.component.html`:
- Around line 17-20: The version/status badge in
loan-product-selection.component.html is using custom div/span markup instead of
Angular Material. Update the badge blocks referenced by the landing-page__badge
and the related status chip markup to use Material chip components from
MatChipsModule, keeping the same displayed text and styling intent while
replacing the native elements with the appropriate mat-chip structure.
In `@src/app/products/loan-products/wizard/loan-product-wizard.component.html`:
- Around line 13-137: The loan-product wizard template still contains hardcoded
user-facing text, including the stepper header, review labels, default notice,
buttons, and option/field labels. Update the loan-product-wizard.component.html
bindings to resolve all visible copy through `@ngx-translate/core` translation
keys instead of raw strings, using the existing template symbols like
visibleSteps, reviewGroups, field.label, and option.label as the places to swap
in translated values. After adding or changing any strings, make sure the new
translation keys are included and run npm run translations:extract.
In `@src/app/products/loan-products/wizard/loan-product-wizard.component.scss`:
- Around line 9-156: The wizard SCSS uses off-grid spacing values and hardcoded
color literals; update the styles in loan-product-wizard.component.scss to
follow the 8px spacing grid and replace custom backgrounds/opacities with
existing theme variables/tokens from main.scss and mifosx-theme.scss. Focus on
the affected selectors like .wizard-shell, .landing-page__eyebrow, .review-chip,
.step-panel, .review-banner, .review-metric, and .review-defaults-pill,
normalizing spacing to existing increments and reusing shared SCSS variables
instead of literal rgb/hex values.
In `@src/app/products/loan-products/wizard/loan-product-wizard.component.spec.ts`:
- Around line 44-70: The current spec only instantiates
LoanProductWizardComponent directly, so template binding errors in the
standalone view are not covered. Add a smoke test that uses
TestBed.createComponent(LoanProductWizardComponent) with minimal resolver/mock
data and stable selectors to verify the template renders without unsafe
loanProductsTemplate dereferences. Keep the existing createComponent helper for
class-only setup, and add one render-focused test that exercises the component
template with minimal timing dependence.
In `@src/app/products/loan-products/wizard/loan-product-wizard.component.ts`:
- Around line 71-72: Replace the new any-based boundaries in
LoanProductWizardComponent with explicit types: define focused interfaces for
the loanProductsTemplate input, the payload/form control map used by the wizard,
and the create response returned by the API. Update the relevant members and
methods in loan-product-wizard.component.ts to use these concrete types instead
of any, and keep the typings aligned with the fields actually consumed in the
component so the wizard flow remains strictly typed throughout.
In `@src/app/products/loan-products/wizard/loan-product.config.ts`:
- Around line 123-735: The loan product wizard config still hardcodes
user-facing copy across PRODUCT_CARDS, VALUE_MAP, and FORM_STEPS, including
names, descriptions, labels, placeholders, hints, and option text. Replace these
literals with translation keys in loan-product.config.ts and update the wizard
component/template to resolve them through `@ngx-translate/core`. Use the existing
symbols PRODUCT_CARDS, VALUE_MAP, and FORM_STEPS to locate and convert every
visible string.
In `@src/app/shared/input-amount/input-amount.component.scss`:
- Around line 24-32: The .input-indicator rule in input-amount.component.scss
includes redundant default declarations; remove the explicit opacity and
visibility settings unless they are needed to override a hidden state elsewhere.
Keep the selector and its other layout/style properties intact, and only
preserve those declarations if a corresponding hidden/disabled state in the same
component depends on them.
🪄 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: df53412a-9c7b-425e-b777-9a9a80a20163
📒 Files selected for processing (30)
src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.scsssrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.tssrc/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.spec.tssrc/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.tssrc/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.htmlsrc/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.scsssrc/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.tssrc/app/products/loan-products/create-loan-product/create-loan-product.component.htmlsrc/app/products/loan-products/create-loan-product/create-loan-product.component.tssrc/app/products/loan-products/create-loan-product/loan-product-selection.component.htmlsrc/app/products/loan-products/create-loan-product/loan-product-selection.component.scsssrc/app/products/loan-products/create-loan-product/loan-product-selection.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.tssrc/app/products/loan-products/loan-products-template.resolver.tssrc/app/products/loan-products/loan-products.component.htmlsrc/app/products/loan-products/loan-products.tssrc/app/products/loan-products/wizard/loan-product-wizard.component.htmlsrc/app/products/loan-products/wizard/loan-product-wizard.component.scsssrc/app/products/loan-products/wizard/loan-product-wizard.component.spec.tssrc/app/products/loan-products/wizard/loan-product-wizard.component.tssrc/app/products/loan-products/wizard/loan-product.config.spec.tssrc/app/products/loan-products/wizard/loan-product.config.tssrc/app/products/products-routing.module.tssrc/app/products/products.module.tssrc/app/shared/input-amount/input-amount.component.htmlsrc/app/shared/input-amount/input-amount.component.scsssrc/assets/translations/en-US.json
|
Hello @IOhacker! |
IOhacker
left a comment
There was a problem hiding this comment.
LGTM - Close the comments please
|
@YousufFFFF please resolve the comments |
93c6090 to
d774f4c
Compare
|
Hello @IOhacker! |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/app/products/loan-products/wizard/loan-product-wizard.component.scss (1)
17-23: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign spacing values to the 8px grid system.
Per coding guidelines, stick to the 8px grid system for visual design and spacing. Several values are off-grid:
0.35rem(5.6px) — lines 18, 27 → use0.25rem(4px) or0.5rem(8px)0.08em(line 20) — use0.0625em(1px) or0.125em(2px) if on a 4px base0.8rem(12.8px) — lines 28, 124 → use0.75rem(12px) or0.875rem(14px)0.4rem(6.4px) — line 134 → use0.5rem(8px)1.05rem(16.8px) — line 128 → use1rem(16px)Also applies to: 124-125, 128-129, 134-135, 164-167
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/products/loan-products/wizard/loan-product-wizard.component.scss` around lines 17 - 23, Update the spacing values in loan-product-wizard.component.scss to match the 8px grid system. Review the affected selectors around landing-page__eyebrow and the other flagged blocks, and replace off-grid values like 0.35rem, 0.08em, 0.8rem, 0.4rem, and 1.05rem with approved grid-aligned equivalents. Keep the changes consistent across the referenced styles so the wizard’s typography and spacing follow the same design scale.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/products/loan-products/wizard/loan-product-wizard.component.html`:
- Around line 9-153: The wizard template still has hardcoded user-facing copy,
so update the `loan-product-wizard.component.html` labels to use the same
`translate` pipe pattern already used in
`create-loan-product-classic.component.html`. Replace the static strings in the
header, review metrics, defaults pill, ID prefix, and step actions with
translation keys, and keep the existing components/conditions intact while
wiring the text through `@ngx-translate/core`; after adding any new keys, run
`npm run translations:extract`.
In `@src/app/products/loan-products/wizard/loan-product-wizard.component.scss`:
- Around line 25-30: The review styles in loan-product-wizard.component.scss are
using hardcoded opacity-based color values instead of shared theme variables.
Replace the rgb(255 255 255 / X%) usages in the review-chip, review-banner,
review-metric, review-section, and review-defaults-pill selectors with SCSS
theme variables or existing CSS custom properties from src/main.scss and
src/theme/mifosx-theme.scss, keeping the existing Material system variable usage
unchanged. Ensure the new values are consistent with the app theme and reused
across these selectors rather than defined ad hoc.
---
Nitpick comments:
In `@src/app/products/loan-products/wizard/loan-product-wizard.component.scss`:
- Around line 17-23: Update the spacing values in
loan-product-wizard.component.scss to match the 8px grid system. Review the
affected selectors around landing-page__eyebrow and the other flagged blocks,
and replace off-grid values like 0.35rem, 0.08em, 0.8rem, 0.4rem, and 1.05rem
with approved grid-aligned equivalents. Keep the changes consistent across the
referenced styles so the wizard’s typography and spacing follow the same design
scale.
🪄 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: 5544ce2b-001a-4d96-9ced-11eb4f904e41
📒 Files selected for processing (40)
src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.scsssrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.tssrc/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.spec.tssrc/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.tssrc/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.htmlsrc/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.scsssrc/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.tssrc/app/products/loan-products/create-loan-product/create-loan-product.component.htmlsrc/app/products/loan-products/create-loan-product/create-loan-product.component.tssrc/app/products/loan-products/create-loan-product/loan-product-selection.component.htmlsrc/app/products/loan-products/create-loan-product/loan-product-selection.component.scsssrc/app/products/loan-products/create-loan-product/loan-product-selection.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.tssrc/app/products/loan-products/loan-products-template.resolver.tssrc/app/products/loan-products/loan-products.component.htmlsrc/app/products/loan-products/loan-products.tssrc/app/products/loan-products/wizard/loan-product-wizard.component.htmlsrc/app/products/loan-products/wizard/loan-product-wizard.component.scsssrc/app/products/loan-products/wizard/loan-product-wizard.component.spec.tssrc/app/products/loan-products/wizard/loan-product-wizard.component.tssrc/app/products/loan-products/wizard/loan-product.config.spec.tssrc/app/products/loan-products/wizard/loan-product.config.tssrc/app/products/products-routing.module.tssrc/app/products/products.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 (6)
- src/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.scss
- src/app/products/loan-products/create-loan-product/loan-product-selection.component.scss
- src/assets/translations/ne-NE.json
- src/assets/translations/lt-LT.json
- src/assets/translations/es-MX.json
- src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html
🚧 Files skipped from review as they are similar to previous changes (20)
- src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.scss
- src/app/products/loan-products/loan-products.ts
- src/app/products/loan-products/create-loan-product/loan-product-selection.component.ts
- src/app/products/loan-products/create-loan-product/loan-product-selection.component.html
- src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts
- src/app/products/loan-products/create-loan-product/create-loan-product.component.html
- src/app/products/loan-products/loan-products.component.html
- src/app/products/loan-products/loan-products-template.resolver.ts
- src/app/products/products.module.ts
- src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.spec.ts
- src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts
- src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts
- src/app/products/products-routing.module.ts
- src/app/products/loan-products/wizard/loan-product-wizard.component.spec.ts
- src/app/products/loan-products/wizard/loan-product.config.spec.ts
- src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.html
- src/app/products/loan-products/create-loan-product/create-loan-product.component.ts
- src/app/products/loan-products/wizard/loan-product-wizard.component.ts
- src/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.ts
- src/app/products/loan-products/wizard/loan-product.config.ts
d774f4c to
5698047
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/app/products/loan-products/wizard/loan-product-wizard.component.scss (1)
25-30: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winHardcoded
rgb(255 255 255 / X%)colors still present.This was previously flagged: per coding guidelines, leverage SCSS variables from
src/main.scssandsrc/theme/mifosx-theme.scssrather than hardcoded color values. Thergb(255 255 255 / X%)usages in.review-chip,.review-banner,.review-metric,.review-section, and.review-defaults-pillremain unchanged.Also applies to: 86-87, 118-119, 136-137, 156-157
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/products/loan-products/wizard/loan-product-wizard.component.scss` around lines 25 - 30, The remaining hardcoded `rgb(255 255 255 / X%)` color values in the loan product wizard stylesheet should be replaced with SCSS theme variables. Update the `.review-chip`, `.review-banner`, `.review-metric`, `.review-section`, and `.review-defaults-pill` rules in `loan-product-wizard.component.scss` to use variables defined in `src/main.scss` or `src/theme/mifosx-theme.scss` instead of direct RGB literals, keeping the existing visual intent while aligning with the shared theme palette.Source: Coding guidelines
🧹 Nitpick comments (3)
src/app/products/loan-products/wizard/loan-product-wizard.component.scss (1)
9-167: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSeveral spacing values deviate from the 8px grid system.
Per coding guidelines, stick to the 8px grid system for visual design and spacing. The following values are off-grid (1rem = 16px, so valid steps are 0, 4px, 8px, 16px, 24px, 32px…):
0.35rem(5.6px) →0.25rem(4px) or0.5rem(8px)0.4rem(6.4px) →0.25rem(4px) or0.5rem(8px)0.75rem(12px) →0.5rem(8px) or1rem(16px)1.25rem(20px) →1rem(16px) or1.5rem(24px)0.8rem(12.8px) →0.75remis also off; use0.75rem→0.5remor1rem9rem(144px) →8rem(128px) or10rem(160px)1.05rem(16.8px) →1rem(16px)These appear in padding, gap, margin, font-size, flex-basis, and min-width declarations throughout the file.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/products/loan-products/wizard/loan-product-wizard.component.scss` around lines 9 - 167, Several spacing and sizing values in loan-product-wizard.component.scss are off the 8px grid; update the affected declarations in the wizard-shell, landing-page__eyebrow, review-chip, step-panel, field-grid, step-actions, review-banner, review-metric, review-metric__label, review-section__title, review-row, and review-defaults-pill styles to use grid-aligned values only. Replace the non-grid gap, padding, margin, font-size, flex-basis, and min-width values with the nearest valid 4px/8px increments while preserving the existing layout intent.Source: Coding guidelines
src/assets/translations/fr-FR.json (1)
581-582: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNew keys left untranslated (English placeholder values).
The newly added keys (
Create Loan Product (Classic),Create Loan Product (Convenient),Multi-step form,Annual interest rate,Annual interest rate by loan cycle,Schedule, hidden-variables text, template-defaults text) all use English strings as their French translations. This is likely intentional pending a translator pass, but worth confirming it isn't shipped as final copy.Also applies to: 1130-1130, 1598-1599, 2831-2831, 3385-3385, 3991-3991
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/assets/translations/fr-FR.json` around lines 581 - 582, The newly added translation entries in fr-FR.json are still using English placeholder values, so update the French strings for the affected keys rather than leaving the source text in place. Use the matching translation entries in the same locale file for keys like “Create Loan Product (Classic)”, “Create Loan Product (Convenient)”, “Multi-step form”, “Annual interest rate”, “Annual interest rate by loan cycle”, “Schedule”, and the hidden-variables/template-defaults text to make sure the French locale is not shipped with untranslated copy.src/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.ts (1)
350-398: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated navigation logic across the two submit paths.
submitLoanProductandsubmitWCProductdiffer only in the payload pre-processing and whetherqueryParams.productTypeis included. ThecreateLoanProduct(...).subscribe(... router.navigate(...))block is otherwise identical and could be extracted into a small helper to reduce drift.♻️ Suggested consolidation
private navigateToCreated(response: any, includeProductType: boolean): void { this.router.navigate(['../', response.resourceId], { ...(includeProductType ? { queryParams: { productType: this.loanProductService.productType.value } } : {}), relativeTo: this.route }); }Then each submit method calls
this.navigateToCreated(response, true/false)insidesubscribe.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.ts` around lines 350 - 398, The two submit flows duplicate the post-create navigation logic in submitLoanProduct and submitWCProduct, so extract the shared router.navigate behavior into a small helper on CreateLoanProductClassicComponent (for example, a method keyed off response.resourceId and an includeProductType flag). Keep the payload-specific preprocessing in each submit method, but have both subscribe handlers call the shared helper so the only difference remains whether queryParams.productType is added.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/products/loan-products/wizard/loan-product-wizard.component.scss`:
- Line 179: The selector in loan-product-wizard.component.scss uses ::ng-deep,
which stylelint flags as an unknown pseudo-element. If this deep selector is
required for the wizard-charges styling to reach the embedded Classic Charges
component, update the project’s stylelint config to ignore
selector-pseudo-element-no-unknown for ::ng-deep; otherwise, replace the deep
selector with a supported approach such as a shared class or HostBinding in the
related component.
In `@src/assets/translations/cs-CS.json`:
- Around line 582-583: The Czech translation file still contains multiple
English strings, so update the `cs-CS.json` entries for `Create Loan Product
(Classic)`, `Create Loan Product (Convenient)`, `Multi-step form`, `Annual
interest rate`, `Annual interest rate by loan cycle`, `Schedule`, `Hidden
variables and predefined values are included in the final payload
automatically.`, and `Template defaults are preloaded` with proper Czech text.
Make the changes in the corresponding translation keys in
`src/assets/translations/cs-CS.json` so the localized UI no longer falls back to
English.
---
Duplicate comments:
In `@src/app/products/loan-products/wizard/loan-product-wizard.component.scss`:
- Around line 25-30: The remaining hardcoded `rgb(255 255 255 / X%)` color
values in the loan product wizard stylesheet should be replaced with SCSS theme
variables. Update the `.review-chip`, `.review-banner`, `.review-metric`,
`.review-section`, and `.review-defaults-pill` rules in
`loan-product-wizard.component.scss` to use variables defined in `src/main.scss`
or `src/theme/mifosx-theme.scss` instead of direct RGB literals, keeping the
existing visual intent while aligning with the shared theme palette.
---
Nitpick comments:
In
`@src/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.ts`:
- Around line 350-398: The two submit flows duplicate the post-create navigation
logic in submitLoanProduct and submitWCProduct, so extract the shared
router.navigate behavior into a small helper on
CreateLoanProductClassicComponent (for example, a method keyed off
response.resourceId and an includeProductType flag). Keep the payload-specific
preprocessing in each submit method, but have both subscribe handlers call the
shared helper so the only difference remains whether queryParams.productType is
added.
In `@src/app/products/loan-products/wizard/loan-product-wizard.component.scss`:
- Around line 9-167: Several spacing and sizing values in
loan-product-wizard.component.scss are off the 8px grid; update the affected
declarations in the wizard-shell, landing-page__eyebrow, review-chip,
step-panel, field-grid, step-actions, review-banner, review-metric,
review-metric__label, review-section__title, review-row, and
review-defaults-pill styles to use grid-aligned values only. Replace the
non-grid gap, padding, margin, font-size, flex-basis, and min-width values with
the nearest valid 4px/8px increments while preserving the existing layout
intent.
In `@src/assets/translations/fr-FR.json`:
- Around line 581-582: The newly added translation entries in fr-FR.json are
still using English placeholder values, so update the French strings for the
affected keys rather than leaving the source text in place. Use the matching
translation entries in the same locale file for keys like “Create Loan Product
(Classic)”, “Create Loan Product (Convenient)”, “Multi-step form”, “Annual
interest rate”, “Annual interest rate by loan cycle”, “Schedule”, and the
hidden-variables/template-defaults text to make sure the French locale is not
shipped with untranslated copy.
🪄 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: 690ed35c-f14f-483d-a811-e4da8c05e976
📒 Files selected for processing (40)
src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.scsssrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.tssrc/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.spec.tssrc/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.tssrc/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.htmlsrc/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.scsssrc/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.tssrc/app/products/loan-products/create-loan-product/create-loan-product.component.htmlsrc/app/products/loan-products/create-loan-product/create-loan-product.component.tssrc/app/products/loan-products/create-loan-product/loan-product-selection.component.htmlsrc/app/products/loan-products/create-loan-product/loan-product-selection.component.scsssrc/app/products/loan-products/create-loan-product/loan-product-selection.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.tssrc/app/products/loan-products/loan-products-template.resolver.tssrc/app/products/loan-products/loan-products.component.htmlsrc/app/products/loan-products/loan-products.tssrc/app/products/loan-products/wizard/loan-product-wizard.component.htmlsrc/app/products/loan-products/wizard/loan-product-wizard.component.scsssrc/app/products/loan-products/wizard/loan-product-wizard.component.spec.tssrc/app/products/loan-products/wizard/loan-product-wizard.component.tssrc/app/products/loan-products/wizard/loan-product.config.spec.tssrc/app/products/loan-products/wizard/loan-product.config.tssrc/app/products/products-routing.module.tssrc/app/products/products.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 (10)
- src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.scss
- src/assets/translations/ne-NE.json
- src/assets/translations/en-US.json
- src/assets/translations/es-MX.json
- src/assets/translations/lt-LT.json
- src/assets/translations/de-DE.json
- src/assets/translations/ko-KO.json
- src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html
- src/assets/translations/es-CL.json
- src/assets/translations/sw-SW.json
🚧 Files skipped from review as they are similar to previous changes (23)
- src/app/products/loan-products/create-loan-product/loan-product-selection.component.ts
- src/app/products/loan-products/loan-products-template.resolver.ts
- src/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.scss
- src/app/products/loan-products/create-loan-product/create-loan-product.component.html
- src/app/products/loan-products/create-loan-product/loan-product-selection.component.html
- src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts
- src/app/products/loan-products/loan-products.component.html
- src/app/products/products.module.ts
- src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html
- src/app/products/loan-products/loan-products.ts
- src/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.html
- src/app/products/products-routing.module.ts
- src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts
- src/app/products/loan-products/create-loan-product/loan-product-selection.component.scss
- src/app/products/loan-products/wizard/loan-product-wizard.component.html
- src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.spec.ts
- src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts
- src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.html
- src/app/products/loan-products/wizard/loan-product-wizard.component.spec.ts
- src/app/products/loan-products/wizard/loan-product.config.spec.ts
- src/app/products/loan-products/wizard/loan-product.config.ts
- src/app/products/loan-products/wizard/loan-product-wizard.component.ts
- src/app/products/loan-products/create-loan-product/create-loan-product.component.ts
9124952 to
1c1871a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/app/products/loan-products/wizard/loan-product-wizard.component.scss (1)
11-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNormalize the remaining spacing to the 8px grid.
Several padding/gap/margin values here (
0.35rem,1.25rem,0.75rem,0.4rem,0.25rem) drift off the repo’s spacing rule; please replace them with 8px multiples or shared spacing tokens.Also applies to: 17-23, 43-47, 49-79, 81-87, 95-119, 132-167
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/products/loan-products/wizard/loan-product-wizard.component.scss` around lines 11 - 15, The remaining spacing in loan-product-wizard.component.scss is off the 8px grid; update the padding/gap/margin values to use 8px multiples or the existing shared spacing tokens instead of the nonconforming rem values. Check the wizard styles around the main container and related sections in loan-product-wizard.component.scss, and normalize all affected spacing declarations consistently across the stylesheet.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/assets/translations/en-US.json`:
- Around line 4572-4580: The `cbild.fields.nationalId` translation contains an
internal note in the user-facing label. Update the `cbild` entry in the
translations JSON so the `nationalId` string is clean end-user copy, removing
the “(required — hard veto)” fragment while preserving the intended meaning and
keeping the rest of the `fields` labels consistent.
- Around line 4567-4571: The messages object contains a duplicate
`unableToUpdateDiscount` key, so remove the repeated entry and keep only one
canonical definition in `en-US.json`. Update the surrounding translation block
so `validationSaved`, `workingCapitalDiscountUpdated`, and
`unableToUpdateDiscount` each appear once, and verify the `messages` object
remains valid JSON with no duplicated keys.
---
Nitpick comments:
In `@src/app/products/loan-products/wizard/loan-product-wizard.component.scss`:
- Around line 11-15: The remaining spacing in loan-product-wizard.component.scss
is off the 8px grid; update the padding/gap/margin values to use 8px multiples
or the existing shared spacing tokens instead of the nonconforming rem values.
Check the wizard styles around the main container and related sections in
loan-product-wizard.component.scss, and normalize all affected spacing
declarations consistently across the stylesheet.
🪄 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: e9571be7-bbc4-4531-8f21-6410c6b7d93e
📒 Files selected for processing (40)
src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.scsssrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.tssrc/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.spec.tssrc/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.tssrc/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.htmlsrc/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.scsssrc/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.tssrc/app/products/loan-products/create-loan-product/create-loan-product.component.htmlsrc/app/products/loan-products/create-loan-product/create-loan-product.component.tssrc/app/products/loan-products/create-loan-product/loan-product-selection.component.htmlsrc/app/products/loan-products/create-loan-product/loan-product-selection.component.scsssrc/app/products/loan-products/create-loan-product/loan-product-selection.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.tssrc/app/products/loan-products/loan-products-template.resolver.tssrc/app/products/loan-products/loan-products.component.htmlsrc/app/products/loan-products/loan-products.tssrc/app/products/loan-products/wizard/loan-product-wizard.component.htmlsrc/app/products/loan-products/wizard/loan-product-wizard.component.scsssrc/app/products/loan-products/wizard/loan-product-wizard.component.spec.tssrc/app/products/loan-products/wizard/loan-product-wizard.component.tssrc/app/products/loan-products/wizard/loan-product.config.spec.tssrc/app/products/loan-products/wizard/loan-product.config.tssrc/app/products/products-routing.module.tssrc/app/products/products.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 (11)
- src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.scss
- src/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.scss
- src/app/products/loan-products/loan-products.ts
- src/assets/translations/es-MX.json
- src/assets/translations/ko-KO.json
- src/assets/translations/ne-NE.json
- src/assets/translations/lv-LV.json
- src/assets/translations/pt-PT.json
- src/assets/translations/de-DE.json
- src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html
- src/assets/translations/fr-FR.json
🚧 Files skipped from review as they are similar to previous changes (27)
- src/app/products/loan-products/create-loan-product/loan-product-selection.component.ts
- src/app/products/loan-products/create-loan-product/loan-product-selection.component.html
- src/app/products/products.module.ts
- src/app/products/loan-products/loan-products-template.resolver.ts
- src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts
- src/app/products/loan-products/create-loan-product/create-loan-product.component.html
- src/assets/translations/es-CL.json
- src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html
- src/assets/translations/cs-CS.json
- src/assets/translations/sw-SW.json
- src/app/products/products-routing.module.ts
- src/assets/translations/it-IT.json
- src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts
- src/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.html
- src/assets/translations/lt-LT.json
- src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts
- src/app/products/loan-products/create-loan-product/create-loan-product.component.ts
- src/app/products/loan-products/loan-products.component.html
- src/app/products/loan-products/create-loan-product/loan-product-selection.component.scss
- src/app/products/loan-products/wizard/loan-product-wizard.component.html
- src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.html
- src/app/products/loan-products/wizard/loan-product.config.ts
- src/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.ts
- src/app/products/loan-products/wizard/loan-product-wizard.component.ts
- src/app/products/loan-products/wizard/loan-product.config.spec.ts
- src/app/products/loan-products/wizard/loan-product-wizard.component.spec.ts
- src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.spec.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/app/products/loan-products/wizard/loan-product-wizard.component.scss (1)
11-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNormalize the remaining spacing to the 8px grid.
Several padding/gap/margin values here (
0.35rem,1.25rem,0.75rem,0.4rem,0.25rem) drift off the repo’s spacing rule; please replace them with 8px multiples or shared spacing tokens.Also applies to: 17-23, 43-47, 49-79, 81-87, 95-119, 132-167
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/products/loan-products/wizard/loan-product-wizard.component.scss` around lines 11 - 15, The remaining spacing in loan-product-wizard.component.scss is off the 8px grid; update the padding/gap/margin values to use 8px multiples or the existing shared spacing tokens instead of the nonconforming rem values. Check the wizard styles around the main container and related sections in loan-product-wizard.component.scss, and normalize all affected spacing declarations consistently across the stylesheet.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/assets/translations/en-US.json`:
- Around line 4572-4580: The `cbild.fields.nationalId` translation contains an
internal note in the user-facing label. Update the `cbild` entry in the
translations JSON so the `nationalId` string is clean end-user copy, removing
the “(required — hard veto)” fragment while preserving the intended meaning and
keeping the rest of the `fields` labels consistent.
- Around line 4567-4571: The messages object contains a duplicate
`unableToUpdateDiscount` key, so remove the repeated entry and keep only one
canonical definition in `en-US.json`. Update the surrounding translation block
so `validationSaved`, `workingCapitalDiscountUpdated`, and
`unableToUpdateDiscount` each appear once, and verify the `messages` object
remains valid JSON with no duplicated keys.
---
Nitpick comments:
In `@src/app/products/loan-products/wizard/loan-product-wizard.component.scss`:
- Around line 11-15: The remaining spacing in loan-product-wizard.component.scss
is off the 8px grid; update the padding/gap/margin values to use 8px multiples
or the existing shared spacing tokens instead of the nonconforming rem values.
Check the wizard styles around the main container and related sections in
loan-product-wizard.component.scss, and normalize all affected spacing
declarations consistently across the stylesheet.
🪄 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: e9571be7-bbc4-4531-8f21-6410c6b7d93e
📒 Files selected for processing (40)
src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.htmlsrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.scsssrc/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.tssrc/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.spec.tssrc/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.tssrc/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.htmlsrc/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.scsssrc/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.tssrc/app/products/loan-products/create-loan-product/create-loan-product.component.htmlsrc/app/products/loan-products/create-loan-product/create-loan-product.component.tssrc/app/products/loan-products/create-loan-product/loan-product-selection.component.htmlsrc/app/products/loan-products/create-loan-product/loan-product-selection.component.scsssrc/app/products/loan-products/create-loan-product/loan-product-selection.component.tssrc/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.htmlsrc/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.tssrc/app/products/loan-products/loan-products-template.resolver.tssrc/app/products/loan-products/loan-products.component.htmlsrc/app/products/loan-products/loan-products.tssrc/app/products/loan-products/wizard/loan-product-wizard.component.htmlsrc/app/products/loan-products/wizard/loan-product-wizard.component.scsssrc/app/products/loan-products/wizard/loan-product-wizard.component.spec.tssrc/app/products/loan-products/wizard/loan-product-wizard.component.tssrc/app/products/loan-products/wizard/loan-product.config.spec.tssrc/app/products/loan-products/wizard/loan-product.config.tssrc/app/products/products-routing.module.tssrc/app/products/products.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 (11)
- src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.scss
- src/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.scss
- src/app/products/loan-products/loan-products.ts
- src/assets/translations/es-MX.json
- src/assets/translations/ko-KO.json
- src/assets/translations/ne-NE.json
- src/assets/translations/lv-LV.json
- src/assets/translations/pt-PT.json
- src/assets/translations/de-DE.json
- src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.html
- src/assets/translations/fr-FR.json
🚧 Files skipped from review as they are similar to previous changes (27)
- src/app/products/loan-products/create-loan-product/loan-product-selection.component.ts
- src/app/products/loan-products/create-loan-product/loan-product-selection.component.html
- src/app/products/products.module.ts
- src/app/products/loan-products/loan-products-template.resolver.ts
- src/app/products/loan-products/loan-product-stepper/loan-product-terms-step/loan-product-terms-step.component.ts
- src/app/products/loan-products/create-loan-product/create-loan-product.component.html
- src/assets/translations/es-CL.json
- src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.html
- src/assets/translations/cs-CS.json
- src/assets/translations/sw-SW.json
- src/app/products/products-routing.module.ts
- src/assets/translations/it-IT.json
- src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.ts
- src/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.html
- src/assets/translations/lt-LT.json
- src/app/loans/loans-account-stepper/loans-account-terms-step/loans-account-terms-step.component.ts
- src/app/products/loan-products/create-loan-product/create-loan-product.component.ts
- src/app/products/loan-products/loan-products.component.html
- src/app/products/loan-products/create-loan-product/loan-product-selection.component.scss
- src/app/products/loan-products/wizard/loan-product-wizard.component.html
- src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.html
- src/app/products/loan-products/wizard/loan-product.config.ts
- src/app/products/loan-products/create-loan-product-classic/create-loan-product-classic.component.ts
- src/app/products/loan-products/wizard/loan-product-wizard.component.ts
- src/app/products/loan-products/wizard/loan-product.config.spec.ts
- src/app/products/loan-products/wizard/loan-product-wizard.component.spec.ts
- src/app/products/loan-products/common/loan-product-summary/loan-product-summary.component.spec.ts
🛑 Comments failed to post (2)
src/assets/translations/en-US.json (2)
4567-4571: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Duplicate JSON key
unableToUpdateDiscount.
unableToUpdateDiscountis defined twice in the samemessagesobject (line 4567 and line 4570). JSON parsers keep only the last occurrence, so this is dead/redundant data now, but it's fragile — if either copy is edited independently in the future, the change will silently be dropped.🐛 Proposed fix
"unableToUpdateDiscount": "Unable to update discount.", "validationSaved": "Validation data saved successfully.", ... - "workingCapitalDiscountUpdated": "Working capital discount was updated successfully.", - "unableToUpdateDiscount": "Unable to update discount." + "workingCapitalDiscountUpdated": "Working capital discount was updated successfully."📝 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."unableToUpdateDiscount": "Unable to update discount.", "validationSaved": "Validation data saved successfully.", "workingCapitalDiscountUpdated": "Working capital discount was updated successfully." },🧰 Tools
🪛 Biome (2.5.1)
[error] 4567-4567: The key unableToUpdateDiscount was already declared.
(lint/suspicious/noDuplicateObjectKeys)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/assets/translations/en-US.json` around lines 4567 - 4571, The messages object contains a duplicate `unableToUpdateDiscount` key, so remove the repeated entry and keep only one canonical definition in `en-US.json`. Update the surrounding translation block so `validationSaved`, `workingCapitalDiscountUpdated`, and `unableToUpdateDiscount` each appear once, and verify the `messages` object remains valid JSON with no duplicated keys.
4572-4580: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Internal-sounding note leaked into user-facing label.
cbild.fields.nationalIdreads"RFC Tax ID (required — hard veto)". The "(required — hard veto)" fragment looks like an internal business-rule/developer annotation rather than end-user copy, and would be confusing if surfaced in the UI as a field label.✏️ Proposed fix
- "nationalId": "RFC Tax ID (required — hard veto)", + "nationalId": "RFC Tax ID",📝 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."cbild": { "fields": { "nationalId": "RFC Tax ID", "dob": "Date of Birth", "firstName": "First Name", "lastName": "Last Name", "address": "Address", "phoneNumber": "Phone Number" },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/assets/translations/en-US.json` around lines 4572 - 4580, The `cbild.fields.nationalId` translation contains an internal note in the user-facing label. Update the `cbild` entry in the translations JSON so the `nationalId` string is clean end-user copy, removing the “(required — hard veto)” fragment while preserving the intended meaning and keeping the rest of the `fields` labels consistent.
Added more features to the personal loan creation Implement custom loan template selection and product descriptions Add classic and convenient loan product creation options Add classic loan product creation flow New features added Personal and Advance loan working state Final version for first push
1c1871a to
80554b1
Compare
|
@IOhacker I have fixed the comments and replied to each comment which I felt doesnt needs any changes. |
|
Good news @IOhacker I was right about the comments on which I commented and they all are marked as resolved now. |
Added more features to the personal loan creation Implement custom loan template selection and product descriptions Add classic and convenient loan product creation options Add classic loan product creation flow New features added Personal and Advance loan working state Final version for first push Co-authored-by: YousufFFFF <ansariyousuf365@gmail.com>
Description
This PR introduces a profile-driven loan product creation experience while preserving full compatibility with the existing Classic loan product flow.
Highlights
Added a new loan product creation landing page with:
Implemented a configurable profile-driven wizard supporting:
Improvements
Design considerations
Related issues and discussion
WEB-1022
Screenshots, if any
After Video:
Products.-.Google.Chrome.2026-07-03.03-39-21.mp4
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
Summary
New Features
Bug Fixes
Documentation