WEB-657: Loan view and Repayment for Working Capital - #3622
Merged
adamsaghy merged 1 commit intoJun 1, 2026
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 |
|---|---|
Base Resolver and Service Infrastructure src/app/loans/common-resolvers/loan-base.resolver.ts, src/app/loans/common-resolvers/loans-account-transaction.resolver.ts, src/app/loans/common-resolvers/loan-transactions.resolver.ts, src/app/loans/loans-routing.module.ts, src/app/loans/loans.service.ts |
Tightens loanAccountPath return type to string-literal union, extends LoansAccountTransactionResolver from LoanBaseResolver with route initialization and validation, introduces LoanTransactionsResolver for working-capital transaction fetching, wires new resolver into routing, and adds applyWorkingCapitalLoanActionCommand and getWorkingCapitalTransactions to LoansService. |
Loan Action Template Routing src/app/loans/common-resolvers/loan-action-button.resolver.ts |
Updates LoanActionButtonResolver.resolve() to conditionally select repayment template based on loanProductService.isLoanProduct, dispatching to loan-product or working-capital template service. |
Shared Account Header Component src/app/shared/account-header/account-header.component.ts, src/app/shared/account-header/account-header.component.html, src/app/shared/account-header/account-header.component.scss |
Introduces standalone AccountHeaderComponent with Material theming, status indicator tooltip support, and ng-content projection slots for reusable account headers across loan and account views. |
Loan Summary Balance Component src/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.ts, src/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.html, src/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.scss |
Extracts loan summary table into standalone component that conditionally renders loan-product (with optional adjustments column) or working-capital (reduced columns) summary tables based on loanProductService.isWorkingCapital. |
General Tab Component Refactor src/app/loans/loans-view/general-tab/general-tab.component.ts, src/app/loans/loans-view/general-tab/general-tab.component.html |
Removes inline loan summary table management and delegates to LoanSummaryBalanceComponentComponent, tracks hasChargeBack, updates template to render product-specific repayment/maturity fields (numberOfRepayments vs totalNoPayments), conditionally displays summary component. |
Loans View Header and Tabs src/app/loans/loans-view/loans-view.component.ts, src/app/loans/loans-view/loans-view.component.html, src/app/loans/loans-view/loans-view.component.scss |
Replaces header with AccountHeaderComponent structure, adds theme-aware styling for projected content, conditionally renders Dashboard tab for loan products, broadens Transactions tab visibility to include working-capital loans, simplifies Charges tab guard to remove product-type requirement. |
Make Repayment Form and Submission Flow src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts, src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.html |
Refactors form to use typed FormGroup/FormControl, adds DestroyRef/takeUntilDestroyed lifecycle management, introduces AlertService for penalty waiver failures, derives command from action name, conditionally loads penalties for loan products only, routes submission through submitCommandAction dispatching to loan-product or working-capital command services. |
Transactions Tab UI and Styling src/app/loans/loans-view/transactions-tab/transactions-tab.component.ts, src/app/loans/loans-view/transactions-tab/transactions-tab.component.html, src/app/loans/loans-view/transactions-tab/transactions-tab.component.scss |
Replaces checkbox filters with slide-toggle toolbar, dynamically populates displayedColumns based on loan vs working-capital product, types filter controls as FormControl<boolean>, adds row styling helpers (loanTransactionBadgeClass, loanTransactionBorderClass), adds working-capital transaction loading via paginator, overwrites SCSS with comprehensive design tokens, table styling, badge system, and alignment helpers. |
Transaction Detail View src/app/loans/loans-view/transactions/view-transaction/view-transaction.component.ts, src/app/loans/loans-view/transactions/view-transaction/view-transaction.component.html, src/app/loans/loans-view/transactions/view-transaction/view-transaction.component.scss |
Treats transactionType as nullable, conditionally copies transactionDate for working-capital loans, computes undo eligibility from `manuallyReversed |
Export Dialog and Layout src/app/loans/loans-view/transactions/export-transactions/export-transactions.component.html, src/app/loans/loans-view/transactions/export-transactions/export-transactions.component.scss |
Updates template and styles with responsive layout classes and form height constraint, adjusting button container spacing and padding. |
Working Capital Loan Balances Tab src/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.ts |
Refactors to manage parent route.data subscription using DestroyRef/takeUntilDestroyed, moves subscription from constructor to ngOnInit, implements OnInit interface, guards subscription with route.parent check. |
Theme and Button Styling src/app/shared/notifications-tray/notifications-tray.component.scss, src/app/shared/theme-toggle/theme-toggle.component.scss |
Adds Material icon button styling with transitions and hover states; updates theme-toggle button transitions to multi-property with semi-transparent background on hover. |
Locale Translations src/assets/translations/{cs-CS,de-DE,en-US,es-CL,es-MX,fr-FR,it-IT,ko-KO,lt-LT,lv-LV,ne-NE,pt-PT,sw-SW}.json |
Adds translation keys ("Discount Fee", "Waive loan charges", "User Profile", "Amount Breakdown", "Linked") across locales and corrects Disbursement capitalization in es-MX. |
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
- openMF/web-app#3100: Related changes to
LoanActionButtonResolverand repayment action handling. - openMF/web-app#3095: Also modifies
LoanActionButtonResolver.resolve()with overlapping action-branch changes. - openMF/web-app#2692: Related transaction tab updates affecting transaction-type visibility and styling.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Nitpick comments (7)
src/app/shared/theme-toggle/theme-toggle.component.scss (3)
11-17: ⚡ Quick winLeverage SCSS theme variables instead of hardcoded colors.
Similar to the notifications-tray component, this styling uses hardcoded color values (
rgb(255 255 255 / 72%),rgb(255 255 255 / 15%),#fff) instead of SCSS theme variables. As per coding guidelines, leverage SCSS variables fromsrc/main.scssandsrc/theme/mifosx-theme.scssrather than explicit color values.♻️ Suggested refactor using theme variables
:host button { - color: rgb(255 255 255 / 72%); + color: rgba($white, 0.72); border-radius: 50%; transition: background-color 0.2s ease, color 0.2s ease; &:hover { - background-color: rgb(255 255 255 / 15%); - color: `#fff`; + background-color: rgba($white, 0.15); + color: $white; } }Note: You'll need to import the color variables at the top of the file:
`@use` '../../../assets/styles/colours' as *;As per coding guidelines: Leverage SCSS variables defined in
src/main.scssandsrc/theme/mifosx-theme.scssrather than generating custom classes and explicit pixel values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/shared/theme-toggle/theme-toggle.component.scss` around lines 11 - 17, The SCSS in theme-toggle.component.scss uses hardcoded colors (e.g., rgb(255 255 255 / 72%), rgb(255 255 255 / 15%), `#fff`); update the styles in this file to use the project SCSS theme variables instead: import the colour variables (e.g., add `@use` '../../../assets/styles/colours' as *; at the top) and replace the hardcoded values in the theme-toggle selector and its &:hover rules with the corresponding variables from the colours/mifosx-theme files so the component follows the shared theme.
9-9: 💤 Low valueConsider using more specific MDC-based selector for consistency.
While
:host buttonworks, using:host button.mat-mdc-icon-buttonwould be more specific and align with the MDC-based Material selector pattern used innotifications-tray.component.scss. Based on learnings, MDC DOM classes should be preferred when styling Angular Material v20+ components.♻️ Suggested selector update
-:host button { +:host button.mat-mdc-icon-button { color: rgb(255 255 255 / 72%);Based on learnings: In Angular Material v20+ (MDC-based), prefer styling MDC DOM classes (e.g.,
.mat-mdc-icon-button) in SCSS instead of legacy selectors.🤖 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/theme-toggle/theme-toggle.component.scss` at line 9, The selector in theme-toggle.component.scss is currently using the generic host rule `:host button`; replace it with the MDC-specific selector `:host button.mat-mdc-icon-button` so styles target the Angular Material v20+ MDC icon button class consistently (update any related nested rules or specificity to match the new selector).
9-20: ⚖️ Poor tradeoffCode duplication with notifications-tray component.
This styling block is nearly identical to the one added in
notifications-tray.component.scss(lines 24-35), with only a slight difference in base color opacity (72% vs 80%). Consider extracting this common icon button hover pattern into a shared SCSS mixin to maintain consistency and reduce duplication.♻️ Example shared mixin approach
In a shared styles file (e.g.,
src/assets/styles/mixins.scss):`@mixin` icon-button-hover($base-opacity: 0.8) { color: rgba($white, $base-opacity); border-radius: 50%; transition: background-color 0.2s ease, color 0.2s ease; &:hover { background-color: rgba($white, 0.15); color: $white; } }Then in both component files:
`@use` '../../../assets/styles/mixins' as *; :host button.mat-mdc-icon-button { `@include` icon-button-hover(0.8); // or 0.72 for theme-toggle }🤖 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/theme-toggle/theme-toggle.component.scss` around lines 9 - 20, Extract the duplicated icon-button styles from theme-toggle.component.scss and notifications-tray.component.scss into a shared SCSS mixin (e.g., icon-button-hover) in a common mixins.scss, parameterize the base color opacity (default and override), and replace the duplicated blocks in both components by importing the mixins file and including the mixin (use the mixin name icon-button-hover with the appropriate opacity argument for ThemeToggle and NotificationsTray); update selectors to match existing button selector (e.g., :host button or :host button.mat-mdc-icon-button) so behavior and hover states remain identical.src/app/shared/notifications-tray/notifications-tray.component.scss (1)
24-35: ⚡ Quick winLeverage SCSS theme variables instead of hardcoded colors.
This block uses hardcoded color values (
rgb(255 255 255 / 80%),rgb(255 255 255 / 15%),#fff) instead of the SCSS variables defined in theme files. The rest of this file consistently uses variables like$white,$asbestos, etc. As per coding guidelines, SCSS variables fromsrc/main.scssandsrc/theme/mifosx-theme.scssshould be leveraged rather than explicit color values.♻️ Suggested refactor using theme variables
:host button.mat-mdc-icon-button { - color: rgb(255 255 255 / 80%); + color: rgba($white, 0.8); border-radius: 50%; transition: background-color 0.2s ease, color 0.2s ease; &:hover { - background-color: rgb(255 255 255 / 15%); - color: `#fff`; + background-color: rgba($white, 0.15); + color: $white; } }As per coding guidelines: Leverage SCSS variables defined in
src/main.scssandsrc/theme/mifosx-theme.scssrather than generating custom classes and explicit pixel values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/shared/notifications-tray/notifications-tray.component.scss` around lines 24 - 35, The :host button.mat-mdc-icon-button block uses hardcoded colors (rgb(...) and `#fff`); update it to use the theme SCSS variables (e.g. $white) and alpha via rgba() or existing opacity helper variables from your theme files: replace rgb(255 255 255 / 80%) with rgba($white, 0.8), rgb(255 255 255 / 15%) with rgba($white, 0.15), and `#fff` with $white in the :host button.mat-mdc-icon-button and its &:hover rule so the component follows the project's theme variables defined in src/main.scss and src/theme/mifosx-theme.scss.src/app/loans/loans-view/transactions/export-transactions/export-transactions.component.scss (2)
9-11: 💤 Low valueAdjust height to follow the 8px grid system.
The
170pxheight doesn't align with the 8px grid system (170 ÷ 8 = 21.25). Consider using168px(21 × 8) or176px(22 × 8) instead to maintain visual consistency across the application.As per coding guidelines: Stick to the 8px grid system for visual design and spacing.
📐 Proposed fix to align with 8px grid
form { - height: 170px; + height: 168px; }🤖 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-view/transactions/export-transactions/export-transactions.component.scss` around lines 9 - 11, The form selector in export-transactions.component.scss uses height: 170px which breaks the 8px grid; change the height value in the form rule (in export-transactions.component.scss) to an 8px-multiple such as 168px (21×8) or 176px (22×8) to restore grid alignment and visual consistency across the app.
15-15: ⚖️ Poor tradeoffConsider using SCSS variables instead of hardcoded percentages.
The padding values use hardcoded percentages rather than SCSS variables from the theme files. While functional, this approach makes it harder to maintain consistent spacing across the application and adapt to theme changes.
As per coding guidelines: Leverage SCSS variables defined in
src/main.scssandsrc/theme/mifosx-theme.scssrather than generating custom classes and explicit pixel values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/loans/loans-view/transactions/export-transactions/export-transactions.component.scss` at line 15, The rule uses hardcoded padding percentages in export-transactions.component.scss (padding: 1% 0 2%); replace those literal values with the project's SCSS spacing variables (from src/main.scss or src/theme/mifosx-theme.scss) to ensure consistent theming—e.g., map 1%/2% to the appropriate $spacing-* variables, import the theme file at the top of export-transactions.component.scss if not already imported, and update the selector (export-transactions component styles) to use the variables instead of literal percentages.src/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.ts (1)
36-36: ⚡ Quick winConsider introducing a typed interface for the route data.
The
loanDetailsData: anyparameter weakens type safety. While the retrieved learning notes that typing the full API response layer can be tracked as a separate cross-cutting refactor, introducing a simple interface here would provide immediate type safety benefits with minimal effort.🎯 Suggested typing improvement
Add an interface near the top of the file:
interface LoanBalancesRouteData { loanDetailsData: { currency: { code: string }; balance: WorkingCapitalBalances; }; }Then update the subscription:
- this.route.parent.data.pipe(takeUntilDestroyed(this.destroyRef)).subscribe((data: { loanDetailsData: any }) => { + this.route.parent.data.pipe(takeUntilDestroyed(this.destroyRef)).subscribe((data: LoanBalancesRouteData) => {Based on learnings: TypeScript files should introduce specific interfaces/types for response shapes instead of using
any, though full API response typing can be tracked as a separate enhancement.🤖 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-view/working-capital/loan-balances-tab/loan-balances-tab.component.ts` at line 36, Introduce a small interface to replace the use of `any` on the route data and update the observable subscription to use it: add an interface (e.g. `LoanBalancesRouteData`) near the top of the file describing `{ loanDetailsData: { currency: { code: string }; balance: WorkingCapitalBalances } }`, then change the subscription signature from `(data: { loanDetailsData: any })` to `(data: LoanBalancesRouteData)` in the `this.route.parent.data.pipe(...).subscribe(...)` call so `loanDetailsData` is strongly typed.
🤖 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/loans/common-resolvers/loans-account-transaction.resolver.ts`:
- Around line 36-43: The resolver's resolve method must reject null route params
before numeric coercion and always return an Observable; update resolve(route:
ActivatedRouteSnapshot) to first check that both loanId and transactionId are
not null (e.g., if (loanId == null || transactionId == null) return
throwError(() => new Error('Missing route params'))), then coerce to numbers and
validate with isNaN before calling
this.loansService.getLoansAccountTransaction(this.loanAccountPath, loanId,
transactionId); ensure every code path returns an Observable (use
throwError/EMPTY) so resolve never falls through returning undefined.
In `@src/app/loans/loans-view/general-tab/general-tab.component.ts`:
- Around line 77-82: The current use of this.loanDetails.transactions.some(...)
never returns true from the callback so it doesn't short-circuit; change it to
assign the boolean result to this.hasChargeBack by returning true when a match
is found (e.g., this.hasChargeBack =
this.loanDetails.transactions.some((transaction: any) => transaction.type.code
=== 'loanTransactionType.chargeback')), removing the manual flag set and the
ineffective return; update code around the some call in general-tab.component
(look for this.hasChargeBack and this.loanDetails.transactions.some)
accordingly.
In
`@src/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.ts`:
- Around line 50-56: The component currently uses any for the `@Input`() summary
and the MatTableDataSource which defeats TypeScript checks; introduce explicit
interfaces (e.g., LoanSummaryPayload and LoanSummaryRow) that describe the
summary API and each table row (fields like loanId, balance, currency, status,
chargeBackFlag, etc. matching the component's column usage), change the `@Input`()
signature to summary: LoanSummaryPayload | null and change dataSource to
MatTableDataSource<LoanSummaryRow>, update any code that maps/creates rows to
return LoanSummaryRow objects and adjust loanSummaryColumns typing if you can
narrow it (e.g., readonly string[] or a union of column keys) so the table
binding is strictly typed.
In
`@src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.html`:
- Around line 176-185: The CTA currently uses a hardcoded
*mifosxHasPermission="'REPAYMENT_LOAN'" which is incorrect for actions other
than repayment; update the template to derive the required permission from the
selected action/command (e.g. use a component getter or method that maps the
current command or actionName in make-repayment.component.ts to the correct
permission string) and bind that result to *mifosxHasPermission (keep the
existing [disabled]="!repaymentLoanForm.valid || isSubmitting" logic). Ensure
the mapping function is named clearly (e.g.
getPermissionForAction(command|actionName)) and referenced in the template so
the button only shows for the appropriate permission per action.
- Around line 64-69: Replace the hardcoded label on the mifosx-input-amount
component in make-repayment.component.html: change the inputLabel="'Transaction
Amount'" to use the ngx-translate key (e.g. a namespaced key like
'MAKE_REPAYMENT.TRANSACTION_AMOUNT') and the translate pipe or bound translate
value so the label is translated at runtime; update the translation JSON (e.g.
en.json) with the corresponding key and value to complete the i18n change.
In
`@src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts`:
- Around line 362-378: The error path of waivePenalties currently calls
submitCommandAction(data) while data.transactionAmount has already been reduced
by waived amounts; locate where data is constructed and used in
make-repayment.component.ts and on the waivePenalties(...).subscribe.error
handler either (a) abort the flow (do not call submitCommandAction) or (b)
restore the original transaction amount on data before calling
submitCommandAction; specifically update the waivePenalties error branch in the
block guarded by loanProductService.isLoanProduct && this.isRepayment() to
ensure submitCommandAction receives the unmodified amount (or skip submission) —
adjust references to data, submitCommandAction(), and waivePenalties()
accordingly.
- Around line 373-376: The alert message in make-repayment.component.ts is
hardcoded ("Some penalties could not be waived..."); replace it with an i18n
lookup using TranslateService from `@ngx-translate/core` (e.g., inject
TranslateService in the component and call
this.translate.instant('loan.repayment.penalties_not_waived') or
this.translate.get(...).subscribe(...)) and pass the translated string to
this.alertService.alert({ type: 'Warning', message: translatedText });
add/update the translation key (e.g., loan.repayment.penalties_not_waived) in
the appropriate translation files.
- Around line 105-115: resolveCommandFromActionName currently returns an empty
string on a miss which allows submitCommandAction to post an empty command;
update it to fail-fast by either (A) making the map exhaustive (e.g., add the
missing 'Capitalized Income Adjustment': 'capitalizedIncomeAdjustment' entry to
the map used in resolveCommandFromActionName) or (B) change
resolveCommandFromActionName to return undefined/null or throw when the
actionName is not found and then add a guard in submitCommandAction that checks
the resolved value (from resolveCommandFromActionName) and aborts/throws if
undefined so this.command is never posted when unmapped; reference
resolveCommandFromActionName, submitCommandAction, and isCapitalizedIncome when
implementing the fix.
In `@src/app/loans/loans-view/loans-view.component.html`:
- Line 39: Replace the hardcoded aria-label on the action button in
loans-view.component.html with a translated key using `@ngx-translate/core`; e.g.
change aria-label="Loan account actions" to a bound attribute like
[attr.aria-label]="'LOANS.ACTIONS_ARIA' | translate" on the same button element,
and add the LOANS.ACTIONS_ARIA entry to your locale JSON files (and update
LoansViewComponent only if you need component-specific keys or fallback
handling).
- Line 13: The status tooltip currently binds directly to
loanDetailsData.status.value, which bypasses loanStatusTooltip() and can
desynchronize tooltip text from the status-dot overrides (e.g.,
charge-off/overdue); update the [statusTooltip] binding to call the existing
loanStatusTooltip(...) method (or a computed wrapper that returns the same value
used by the status-dot) so the tooltip text and dot semantics remain
aligned—locate the template attribute [statusTooltip] and replace its expression
with loanStatusTooltip(...) or the component's computed property that
encapsulates the same logic.
In `@src/app/loans/loans-view/transactions-tab/transactions-tab.component.html`:
- Around line 206-207: The actions menu button (<button ...
[matMenuTriggerFor]="transactionMenu" class="action-button">) currently has an
empty aria-label; replace it with a meaningful, non-empty accessible label such
as aria-label="Open transaction actions menu" (or similar concise description)
so screen readers announce the button purpose; update the aria-label attribute
on the button element in transactions-tab.component.html accordingly.
In `@src/app/loans/loans-view/transactions-tab/transactions-tab.component.scss`:
- Around line 22-45: The SCSS uses hardcoded spacing/sizing that breaks the 8px
grid—replace explicit values in .toggle-group (gap: 1.5rem), the parent padding
(0.75rem 0 1rem), .export-button (letter-spacing and border-color), and
.table-wrapper (border-radius: 6px, box-shadow offsets) with the shared design
tokens/SCSS variables from src/main.scss and src/theme/mifosx-theme.scss and
adjust values to the 8px grid (multiples of 8px or the corresponding rem
variable); ensure you swap hardcoded colors/border variables for the theme
variables and use spacing variables (e.g., $space-1, $space-2 or equivalent) or
create/align to the nearest 8px-based token for all other occurrences mentioned
(lines 53-79, 86-94, 147-152, 288-290) so the classes .toggle-group,
.export-button, .table-wrapper and surrounding paddings follow the shared
tokens.
In `@src/app/loans/loans-view/transactions-tab/transactions-tab.component.ts`:
- Around line 326-329: The "Hide Reversed" filter currently only checks
transaction.manuallyReversed while loanTransactionBorderClass treats both
manuallyReversed and reversed as reversed; update the filtering logic that
builds the visible transactions (the function/method where you filter by
manuallyReversed) to also consider transaction.reversed (e.g., treat reversed ||
manuallyReversed as reversed) so the Hide Reversed behavior matches
loanTransactionBorderClass's row state for both API-reversed and
manually-reversed entries.
In
`@src/app/loans/loans-view/transactions/view-transaction/view-transaction.component.ts`:
- Line 91: The nullable property transactionType in ViewTransactionComponent is
dereferenced in the constructor and action methods (e.g.,
this.transactionType.reAge, reAmortize, contractTermination, value, isWriteOff,
and calls to allowUndoTransaction/allowChargebackTransaction) without null
checks; guard against a missing transactionData.type by either (a)
asserting/validating and throwing or logging + returning early if
transactionData.type is absent in the constructor, or (b) narrowing before each
use (if (this.transactionType) { ... }) so callers like
isWriteOff(this.transactionType) and
allowUndoTransaction/allowChargebackTransaction get a non-null
LoanTransactionType; locate references to transactionType, transactionData.type,
isWriteOff, allowUndoTransaction, allowChargebackTransaction and ensure
consistent null-handling or default assignment.
In
`@src/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.html`:
- Line 29: The template uses the custom pipe usage "{{ 'Payment Date' |
translateKey: 'inputs' }}" which is inconsistent with the rest of the table that
uses the standard "'labels.inputs.X' | translate" pattern; replace the
translateKey usage with the same pattern (e.g., use "'labels.inputs.paymentDate'
| translate" or the project's existing labels.inputs key for Payment Date) so
the column follows the same translate pipe as the other columns and update any
key name to match the translation file if necessary.
In `@src/app/loans/loans.service.ts`:
- Around line 651-652: Change getLoansAccountTransaction's productType parameter
from string to the narrower union 'loans' | 'working-capital-loans' and create
or reuse a shared exported type (e.g., LoanAccountPath) for that union; then
update loanAccountPath return types in LoanProductService (loan-product.service)
and LoanBaseResolver (loan-base.resolver) to return/declare that same
LoanAccountPath type so callers (like the resolver) are type-safe without casts
and all references (getLoansAccountTransaction, loanAccountPath,
LoanProductService, LoanBaseResolver) use the identical union type.
In `@src/app/shared/account-header/account-header.component.scss`:
- Around line 26-27: The status dot in account-header.component.scss currently
uses hardcoded width: 9px and height: 9px; change these to the 8px grid token
instead of explicit pixels (replace 9px with the appropriate design token SCSS
variable or mixin used in the repo, e.g., the 8px sizing token like $space-8 or
$size-8) so the status dot adheres to the 8px grid system; update both width and
height where they appear and ensure you import/use the existing token variable
consistent with other styles.
- Around line 17-18: The stylesheet uses unnecessary ::ng-deep on selectors and
a status dot sized off the 8px grid; remove the ::ng-deep prefix from the
.mat-mdc-card-title and .mat-mdc-card-subtitle selectors so they are scoped to
this component, and change the .status-dot dimensions from 9px to 8px (both
width and height) to comply with the 8px grid.
---
Nitpick comments:
In
`@src/app/loans/loans-view/transactions/export-transactions/export-transactions.component.scss`:
- Around line 9-11: The form selector in export-transactions.component.scss uses
height: 170px which breaks the 8px grid; change the height value in the form
rule (in export-transactions.component.scss) to an 8px-multiple such as 168px
(21×8) or 176px (22×8) to restore grid alignment and visual consistency across
the app.
- Line 15: The rule uses hardcoded padding percentages in
export-transactions.component.scss (padding: 1% 0 2%); replace those literal
values with the project's SCSS spacing variables (from src/main.scss or
src/theme/mifosx-theme.scss) to ensure consistent theming—e.g., map 1%/2% to the
appropriate $spacing-* variables, import the theme file at the top of
export-transactions.component.scss if not already imported, and update the
selector (export-transactions component styles) to use the variables instead of
literal percentages.
In
`@src/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.ts`:
- Line 36: Introduce a small interface to replace the use of `any` on the route
data and update the observable subscription to use it: add an interface (e.g.
`LoanBalancesRouteData`) near the top of the file describing `{ loanDetailsData:
{ currency: { code: string }; balance: WorkingCapitalBalances } }`, then change
the subscription signature from `(data: { loanDetailsData: any })` to `(data:
LoanBalancesRouteData)` in the `this.route.parent.data.pipe(...).subscribe(...)`
call so `loanDetailsData` is strongly typed.
In `@src/app/shared/notifications-tray/notifications-tray.component.scss`:
- Around line 24-35: The :host button.mat-mdc-icon-button block uses hardcoded
colors (rgb(...) and `#fff`); update it to use the theme SCSS variables (e.g.
$white) and alpha via rgba() or existing opacity helper variables from your
theme files: replace rgb(255 255 255 / 80%) with rgba($white, 0.8), rgb(255 255
255 / 15%) with rgba($white, 0.15), and `#fff` with $white in the :host
button.mat-mdc-icon-button and its &:hover rule so the component follows the
project's theme variables defined in src/main.scss and
src/theme/mifosx-theme.scss.
In `@src/app/shared/theme-toggle/theme-toggle.component.scss`:
- Around line 11-17: The SCSS in theme-toggle.component.scss uses hardcoded
colors (e.g., rgb(255 255 255 / 72%), rgb(255 255 255 / 15%), `#fff`); update the
styles in this file to use the project SCSS theme variables instead: import the
colour variables (e.g., add `@use` '../../../assets/styles/colours' as *; at the
top) and replace the hardcoded values in the theme-toggle selector and its
&:hover rules with the corresponding variables from the colours/mifosx-theme
files so the component follows the shared theme.
- Line 9: The selector in theme-toggle.component.scss is currently using the
generic host rule `:host button`; replace it with the MDC-specific selector
`:host button.mat-mdc-icon-button` so styles target the Angular Material v20+
MDC icon button class consistently (update any related nested rules or
specificity to match the new selector).
- Around line 9-20: Extract the duplicated icon-button styles from
theme-toggle.component.scss and notifications-tray.component.scss into a shared
SCSS mixin (e.g., icon-button-hover) in a common mixins.scss, parameterize the
base color opacity (default and override), and replace the duplicated blocks in
both components by importing the mixins file and including the mixin (use the
mixin name icon-button-hover with the appropriate opacity argument for
ThemeToggle and NotificationsTray); update selectors to match existing button
selector (e.g., :host button or :host button.mat-mdc-icon-button) so behavior
and hover states remain identical.
🪄 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: 86c7da73-1dee-43fb-b0d7-86d11fe0743a
📒 Files selected for processing (41)
src/app/loans/common-resolvers/loan-action-button.resolver.tssrc/app/loans/common-resolvers/loans-account-transaction.resolver.tssrc/app/loans/loans-view/general-tab/general-tab.component.htmlsrc/app/loans/loans-view/general-tab/general-tab.component.tssrc/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.htmlsrc/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.scsssrc/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.htmlsrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.tssrc/app/loans/loans-view/loans-view.component.htmlsrc/app/loans/loans-view/loans-view.component.scsssrc/app/loans/loans-view/loans-view.component.tssrc/app/loans/loans-view/transactions-tab/transactions-tab.component.htmlsrc/app/loans/loans-view/transactions-tab/transactions-tab.component.scsssrc/app/loans/loans-view/transactions-tab/transactions-tab.component.tssrc/app/loans/loans-view/transactions/export-transactions/export-transactions.component.htmlsrc/app/loans/loans-view/transactions/export-transactions/export-transactions.component.scsssrc/app/loans/loans-view/transactions/view-transaction/view-transaction.component.htmlsrc/app/loans/loans-view/transactions/view-transaction/view-transaction.component.scsssrc/app/loans/loans-view/transactions/view-transaction/view-transaction.component.tssrc/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.htmlsrc/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.tssrc/app/loans/loans.service.tssrc/app/shared/account-header/account-header.component.htmlsrc/app/shared/account-header/account-header.component.scsssrc/app/shared/account-header/account-header.component.tssrc/app/shared/notifications-tray/notifications-tray.component.scsssrc/app/shared/theme-toggle/theme-toggle.component.scsssrc/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
alberto-art3ch
force-pushed
the
WEB-657/loans-view-working-capital
branch
from
May 28, 2026 13:43
0d785cc to
42f456f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/app/loans/loans-view/transactions-tab/transactions-tab.component.html (1)
206-213:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThe
aria-labelis not properly bound for translation.The attribute
aria-label="'labels.inputs.Actions' | translate"is a literal string, not a binding. The translate pipe won't be executed, leaving the button with poor accessibility.♿ Proposed fix
<button mat-icon-button [matMenuTriggerFor]="transactionMenu" - aria-label="'labels.inputs.Actions' | translate" + [attr.aria-label]="'labels.inputs.Actions' | translate" class="action-button" >As per coding guidelines:
src/app/**/*.{html,ts}should "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/loans/loans-view/transactions-tab/transactions-tab.component.html` around lines 206 - 213, The aria-label on the actions button (the <button> with mat-icon-button and [matMenuTriggerFor]="transactionMenu") is currently a literal string so the translate pipe won't run; change it to a bound attribute so `@ngx-translate` can execute (use an attribute binding such as [attr.aria-label] with the translate pipe or the translate directive) so the translated label is applied for accessibility.src/app/loans/loans-view/loans-view.component.html (1)
35-46:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThe
aria-labelis not properly bound for translation.The attribute
aria-label="'labels.text.Loan Account Actions' | translate"is a literal string, not a binding. The translate pipe won't be executed.♿ Proposed fix
<button accountMenu mat-icon-button [matMenuTriggerFor]="accountMenu" - aria-label="'labels.text.Loan Account Actions' | translate" + [attr.aria-label]="'labels.text.Loan Account Actions' | translate" yPosition="below" >As per coding guidelines:
src/app/**/*.{html,ts}should "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/loans/loans-view/loans-view.component.html` around lines 35 - 46, The aria-label is currently a literal string so the translate pipe isn't executed; update the button element (the one with accountMenu and mat-icon-button and [matMenuTriggerFor]="accountMenu") to use a bound attribute so the translate pipe runs, e.g. use [attr.aria-label] or [aria-label] with the translate pipe (for example: [attr.aria-label]="'labels.text.Loan Account Actions' | translate") so the label is properly localized.
🤖 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.
Duplicate comments:
In `@src/app/loans/loans-view/loans-view.component.html`:
- Around line 35-46: The aria-label is currently a literal string so the
translate pipe isn't executed; update the button element (the one with
accountMenu and mat-icon-button and [matMenuTriggerFor]="accountMenu") to use a
bound attribute so the translate pipe runs, e.g. use [attr.aria-label] or
[aria-label] with the translate pipe (for example:
[attr.aria-label]="'labels.text.Loan Account Actions' | translate") so the label
is properly localized.
In `@src/app/loans/loans-view/transactions-tab/transactions-tab.component.html`:
- Around line 206-213: The aria-label on the actions button (the <button> with
mat-icon-button and [matMenuTriggerFor]="transactionMenu") is currently a
literal string so the translate pipe won't run; change it to a bound attribute
so `@ngx-translate` can execute (use an attribute binding such as
[attr.aria-label] with the translate pipe or the translate directive) so the
translated label is applied for accessibility.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dd20d4d3-efd1-439e-a98b-674a97bfae99
📒 Files selected for processing (41)
src/app/loans/common-resolvers/loan-action-button.resolver.tssrc/app/loans/common-resolvers/loans-account-transaction.resolver.tssrc/app/loans/loans-view/general-tab/general-tab.component.htmlsrc/app/loans/loans-view/general-tab/general-tab.component.tssrc/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.htmlsrc/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.scsssrc/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.htmlsrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.tssrc/app/loans/loans-view/loans-view.component.htmlsrc/app/loans/loans-view/loans-view.component.scsssrc/app/loans/loans-view/loans-view.component.tssrc/app/loans/loans-view/transactions-tab/transactions-tab.component.htmlsrc/app/loans/loans-view/transactions-tab/transactions-tab.component.scsssrc/app/loans/loans-view/transactions-tab/transactions-tab.component.tssrc/app/loans/loans-view/transactions/export-transactions/export-transactions.component.htmlsrc/app/loans/loans-view/transactions/export-transactions/export-transactions.component.scsssrc/app/loans/loans-view/transactions/view-transaction/view-transaction.component.htmlsrc/app/loans/loans-view/transactions/view-transaction/view-transaction.component.scsssrc/app/loans/loans-view/transactions/view-transaction/view-transaction.component.tssrc/app/loans/loans-view/working-capital/loan-amortization-schedule-tab/loan-amortization-schedule-tab.component.htmlsrc/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.tssrc/app/loans/loans.service.tssrc/app/shared/account-header/account-header.component.htmlsrc/app/shared/account-header/account-header.component.scsssrc/app/shared/account-header/account-header.component.tssrc/app/shared/notifications-tray/notifications-tray.component.scsssrc/app/shared/theme-toggle/theme-toggle.component.scsssrc/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 (9)
- src/assets/translations/ne-NE.json
- src/assets/translations/de-DE.json
- src/assets/translations/pt-PT.json
- src/assets/translations/lt-LT.json
- src/assets/translations/fr-FR.json
- src/assets/translations/en-US.json
- src/assets/translations/cs-CS.json
- src/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.scss
- src/app/loans/loans-view/transactions/export-transactions/export-transactions.component.scss
🚧 Files skipped from review as they are similar to previous changes (26)
- src/app/shared/account-header/account-header.component.html
- src/app/shared/notifications-tray/notifications-tray.component.scss
- src/app/shared/theme-toggle/theme-toggle.component.scss
- src/assets/translations/sw-SW.json
- src/app/loans/loans-view/transactions/export-transactions/export-transactions.component.html
- src/app/shared/account-header/account-header.component.ts
- src/assets/translations/lv-LV.json
- src/app/loans/common-resolvers/loans-account-transaction.resolver.ts
- src/assets/translations/es-MX.json
- src/app/loans/loans-view/transactions/view-transaction/view-transaction.component.scss
- src/app/loans/loans-view/general-tab/general-tab.component.html
- src/assets/translations/es-CL.json
- src/assets/translations/ko-KO.json
- src/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.html
- src/app/loans/loans-view/loans-view.component.scss
- src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.html
- src/assets/translations/it-IT.json
- src/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.ts
- src/app/loans/loans-view/transactions-tab/transactions-tab.component.scss
- src/app/loans/loans-view/loans-view.component.ts
- src/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.ts
- src/app/loans/loans-view/general-tab/general-tab.component.ts
- src/app/loans/loans-view/transactions/view-transaction/view-transaction.component.html
- src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts
- src/app/loans/loans-view/transactions-tab/transactions-tab.component.ts
- src/app/loans/loans-view/transactions/view-transaction/view-transaction.component.ts
alberto-art3ch
force-pushed
the
WEB-657/loans-view-working-capital
branch
from
May 28, 2026 17:11
42f456f to
41cd56d
Compare
|
Actionable comments posted: 0 |
alberto-art3ch
force-pushed
the
WEB-657/loans-view-working-capital
branch
from
June 1, 2026 00:50
41cd56d to
8d50df0
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/loans/common-resolvers/loan-transactions.resolver.ts`:
- Line 37: The current lookup for loanId dereferences route.parent directly
which can be null; change the expression in loan-transactions.resolver.ts that
defines loanId to guard route.parent (e.g., use optional chaining or an explicit
null check) so it reads the child param first and only calls
route.parent.paramMap.get('loanId') when route.parent is non-null; ensure the
symbol affected is the loanId assignment that references route.paramMap and
route.parent.
In `@src/app/shared/account-header/account-header.component.ts`:
- Around line 15-29: The `@Component` decorator for the account header defines an
imports array but is missing standalone: true, which is required for using
imports on a standalone component; update the `@Component` metadata for the
component with selector 'mifosx-account-header' (account-header.component.ts) to
include standalone: true so Angular treats the component as standalone and
accepts the imports array (i.e., add standalone: true alongside imports and
changeDetection).
In `@src/assets/translations/cs-CS.json`:
- Line 1017: The keys are in the wrong namespaces so runtime lookups fail: move
"Waive loan charges" from labels.catalogs into labels.inputs by adding it as
labels.inputs["Waive loan charges"] (or renaming the object entry under
labels.inputs) and move "User Profile" from labels.heading into labels.headings
as labels.headings["User Profile"] so the JSON matches the runtime lookup paths;
update or remove the old keys under labels.catalogs and labels.heading to avoid
duplicates.
🪄 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: 7eabd25f-2d7c-4822-b163-a4afe16d0ef4
📒 Files selected for processing (43)
src/app/loans/common-resolvers/loan-action-button.resolver.tssrc/app/loans/common-resolvers/loan-base.resolver.tssrc/app/loans/common-resolvers/loan-transactions.resolver.tssrc/app/loans/common-resolvers/loans-account-transaction.resolver.tssrc/app/loans/loans-routing.module.tssrc/app/loans/loans-view/general-tab/general-tab.component.htmlsrc/app/loans/loans-view/general-tab/general-tab.component.tssrc/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.htmlsrc/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.scsssrc/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.htmlsrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.tssrc/app/loans/loans-view/loans-view.component.htmlsrc/app/loans/loans-view/loans-view.component.scsssrc/app/loans/loans-view/loans-view.component.tssrc/app/loans/loans-view/transactions-tab/transactions-tab.component.htmlsrc/app/loans/loans-view/transactions-tab/transactions-tab.component.scsssrc/app/loans/loans-view/transactions-tab/transactions-tab.component.tssrc/app/loans/loans-view/transactions/export-transactions/export-transactions.component.htmlsrc/app/loans/loans-view/transactions/export-transactions/export-transactions.component.scsssrc/app/loans/loans-view/transactions/view-transaction/view-transaction.component.htmlsrc/app/loans/loans-view/transactions/view-transaction/view-transaction.component.scsssrc/app/loans/loans-view/transactions/view-transaction/view-transaction.component.tssrc/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.tssrc/app/loans/loans.service.tssrc/app/shared/account-header/account-header.component.htmlsrc/app/shared/account-header/account-header.component.scsssrc/app/shared/account-header/account-header.component.tssrc/app/shared/notifications-tray/notifications-tray.component.scsssrc/app/shared/theme-toggle/theme-toggle.component.scsssrc/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 (13)
- src/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.scss
- src/assets/translations/en-US.json
- src/app/loans/loans-view/transactions/export-transactions/export-transactions.component.scss
- src/app/loans/loans-view/transactions/export-transactions/export-transactions.component.html
- src/assets/translations/fr-FR.json
- src/app/loans/loans-view/loans-view.component.scss
- src/assets/translations/de-DE.json
- src/app/loans/loans-view/transactions/view-transaction/view-transaction.component.scss
- src/assets/translations/pt-PT.json
- src/assets/translations/lt-LT.json
- src/assets/translations/ne-NE.json
- src/assets/translations/es-MX.json
- src/app/shared/account-header/account-header.component.scss
🚧 Files skipped from review as they are similar to previous changes (19)
- src/app/shared/theme-toggle/theme-toggle.component.scss
- src/app/loans/common-resolvers/loan-base.resolver.ts
- src/app/loans/common-resolvers/loan-action-button.resolver.ts
- src/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.html
- src/app/shared/notifications-tray/notifications-tray.component.scss
- src/app/loans/loans-view/transactions/view-transaction/view-transaction.component.html
- src/app/loans/loans-view/loans-view.component.html
- src/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.ts
- src/app/shared/account-header/account-header.component.html
- src/app/loans/loans-view/loans-view.component.ts
- src/app/loans/loans-view/general-tab/general-tab.component.html
- src/app/loans/common-resolvers/loans-account-transaction.resolver.ts
- src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.html
- src/app/loans/loans-view/transactions/view-transaction/view-transaction.component.ts
- src/app/loans/loans-view/transactions-tab/transactions-tab.component.html
- src/app/loans/loans-view/general-tab/general-tab.component.ts
- src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts
- src/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.ts
- src/app/loans/loans-view/transactions-tab/transactions-tab.component.ts
alberto-art3ch
force-pushed
the
WEB-657/loans-view-working-capital
branch
from
June 1, 2026 02:54
8d50df0 to
ba3732e
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/app/loans/loans-view/transactions/view-transaction/view-transaction.component.scss (2)
12-71: 🏗️ Heavy liftExtract the duplicated design-token block into shared theme instead of redefining per component.
This
--ch-*/--tx-*token set (light + dark) is copied near-verbatim intotransactions-tab.component.scss, and the two have already drifted:--tx-reamortize-bgis#002420here but#002f2bin the transactions-tab file. Centralizing these tokens (and the shared badge/pill patterns) in the theme avoids future divergence and aligns with leveraging shared SCSS variables. As per coding guidelines: "Leverage SCSS variables defined insrc/main.scssandsrc/theme/mifosx-theme.scssrather than generating custom classes and explicit pixel values".🤖 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-view/transactions/view-transaction/view-transaction.component.scss` around lines 12 - 71, Duplicate design-token and badge/pill CSS defined in :host and :host-context(.dark-theme) (variables like --ch-*, --tx-*) should be extracted into the shared theme (e.g., main theme SCSS) and imported: create centralized token definitions for both light and dark palettes (including --tx-reamortize-bg) in the global theme file, move shared badge/pill styles to a shared partial, update view-transaction.component.scss and transactions-tab.component.scss to remove the full token blocks and rely on the imported theme variables, and ensure both components use the same shared badge/pill classes to avoid drift.
97-148: 💤 Low valueNormalize spacing/sizing to the 8px grid.
Values like
border-radius: 12px,padding: 0.5rem 0 1rem,padding: 3px 9px,font-size: 11px, andgap: 5pxdeviate from the 8px grid and shared tokens. Snap these to grid multiples / theme variables where available. As per coding guidelines: "Stick to the 8px grid system for visual design and spacing".🤖 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-view/transactions/view-transaction/view-transaction.component.scss` around lines 97 - 148, The CSS uses non-8px-grid values in .transaction-card and .txn-header (e.g., border-radius: 12px, padding: 0.5rem 0 1rem, small paddings/gaps and font-size 11px); update these to multiples of the 8px grid and shared design tokens where available (replace 12px with the standard radius token or 16px/8px multiple, convert padding values to 8px-based tokens or rems that equal 8px multiples, and snap font-size/gap to theme tokens or nearest 8px multiples), ensuring changes target .transaction-card, its card-tx--* modifiers and .txn-header so spacing/sizing aligns with the design system.
🤖 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.
Nitpick comments:
In
`@src/app/loans/loans-view/transactions/view-transaction/view-transaction.component.scss`:
- Around line 12-71: Duplicate design-token and badge/pill CSS defined in :host
and :host-context(.dark-theme) (variables like --ch-*, --tx-*) should be
extracted into the shared theme (e.g., main theme SCSS) and imported: create
centralized token definitions for both light and dark palettes (including
--tx-reamortize-bg) in the global theme file, move shared badge/pill styles to a
shared partial, update view-transaction.component.scss and
transactions-tab.component.scss to remove the full token blocks and rely on the
imported theme variables, and ensure both components use the same shared
badge/pill classes to avoid drift.
- Around line 97-148: The CSS uses non-8px-grid values in .transaction-card and
.txn-header (e.g., border-radius: 12px, padding: 0.5rem 0 1rem, small
paddings/gaps and font-size 11px); update these to multiples of the 8px grid and
shared design tokens where available (replace 12px with the standard radius
token or 16px/8px multiple, convert padding values to 8px-based tokens or rems
that equal 8px multiples, and snap font-size/gap to theme tokens or nearest 8px
multiples), ensuring changes target .transaction-card, its card-tx--* modifiers
and .txn-header so spacing/sizing aligns with the design system.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d0144b20-32e6-425b-9008-06b54972dfe6
📒 Files selected for processing (44)
src/app/loans/common-resolvers/loan-action-button.resolver.tssrc/app/loans/common-resolvers/loan-base.resolver.tssrc/app/loans/common-resolvers/loan-transactions.resolver.tssrc/app/loans/common-resolvers/loans-account-transaction.resolver.tssrc/app/loans/loans-routing.module.tssrc/app/loans/loans-view/general-tab/general-tab.component.htmlsrc/app/loans/loans-view/general-tab/general-tab.component.tssrc/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.htmlsrc/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.scsssrc/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.tssrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.htmlsrc/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.tssrc/app/loans/loans-view/loans-view.component.htmlsrc/app/loans/loans-view/loans-view.component.scsssrc/app/loans/loans-view/loans-view.component.tssrc/app/loans/loans-view/transactions-tab/transactions-tab.component.htmlsrc/app/loans/loans-view/transactions-tab/transactions-tab.component.scsssrc/app/loans/loans-view/transactions-tab/transactions-tab.component.tssrc/app/loans/loans-view/transactions/export-transactions/export-transactions.component.htmlsrc/app/loans/loans-view/transactions/export-transactions/export-transactions.component.scsssrc/app/loans/loans-view/transactions/view-transaction/view-transaction.component.htmlsrc/app/loans/loans-view/transactions/view-transaction/view-transaction.component.scsssrc/app/loans/loans-view/transactions/view-transaction/view-transaction.component.tssrc/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.tssrc/app/loans/loans.service.tssrc/app/shared/account-header/account-header.component.htmlsrc/app/shared/account-header/account-header.component.scsssrc/app/shared/account-header/account-header.component.tssrc/app/shared/notifications-tray/notifications-tray.component.scsssrc/app/shared/theme-toggle/theme-toggle.component.scsssrc/app/shared/transaction-payment-detail/transaction-payment-detail.component.htmlsrc/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 (12)
- src/app/shared/notifications-tray/notifications-tray.component.scss
- src/app/loans/loans-view/transactions/export-transactions/export-transactions.component.scss
- src/app/loans/loans-view/loans-view.component.scss
- src/assets/translations/pt-PT.json
- src/app/loans/loans-view/transactions/export-transactions/export-transactions.component.html
- src/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.scss
- src/assets/translations/es-MX.json
- src/assets/translations/lt-LT.json
- src/assets/translations/lv-LV.json
- src/assets/translations/ko-KO.json
- src/assets/translations/fr-FR.json
- src/app/shared/theme-toggle/theme-toggle.component.scss
🚧 Files skipped from review as they are similar to previous changes (24)
- src/app/shared/account-header/account-header.component.scss
- src/app/loans/loans-routing.module.ts
- src/app/loans/common-resolvers/loan-base.resolver.ts
- src/app/shared/account-header/account-header.component.ts
- src/app/loans/common-resolvers/loan-action-button.resolver.ts
- src/assets/translations/ne-NE.json
- src/assets/translations/cs-CS.json
- src/app/loans/common-resolvers/loan-transactions.resolver.ts
- src/app/shared/account-header/account-header.component.html
- src/assets/translations/de-DE.json
- src/assets/translations/it-IT.json
- src/assets/translations/sw-SW.json
- src/app/loans/loans-view/loans-view.component.html
- src/app/loans/loans-view/transactions/view-transaction/view-transaction.component.ts
- src/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.html
- src/app/loans/loans-view/general-tab/general-tab.component.html
- src/app/loans/loans-view/loans-view.component.ts
- src/app/loans/loans-view/working-capital/loan-balances-tab/loan-balances-tab.component.ts
- src/app/loans/loans-view/general-tab/loan-summary-balance-component/loan-summary-balance-component.component.ts
- src/app/loans/loans-view/general-tab/general-tab.component.ts
- src/app/loans/loans-view/transactions/view-transaction/view-transaction.component.html
- src/app/loans/loans-view/loan-account-actions/make-repayment/make-repayment.component.ts
- src/app/loans/loans-view/transactions-tab/transactions-tab.component.ts
- src/app/loans/loans-view/transactions-tab/transactions-tab.component.html
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Loan view enhanced and Repayment command for Working Capital
Related issues and discussion
WEB-657
Screenshots, if any
Working Capital Loan repayment
https://github.com/user-attachments/assets/84e98427-37b7-4f22-b247-0dc4a353dc2e
Loan view enhanced for Working Capital
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
If you have multiple commits please combine them into one commit by squashing them.
Read and understood the contribution guidelines at
web-app/.github/CONTRIBUTING.md.Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Localization