WEB-861 - Implement live dashboard metrics and align UI#3386
Merged
IOhacker merged 1 commit intoopenMF:devfrom Mar 16, 2026
Merged
WEB-861 - Implement live dashboard metrics and align UI#3386IOhacker merged 1 commit intoopenMF:devfrom
IOhacker merged 1 commit intoopenMF:devfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Note
|
| Cohort / File(s) | Summary |
|---|---|
UI Styling Updates src/app/home/dashboard/dashboard.component.scss |
Replaces single .card container with .dashboard-card layout; introduces .summary-bar with .summary-tile flex components featuring hover transforms; adds responsive .charts-grid with .full-width and .half-width variants; transitions from column-based structure to modular tile-based dashboard with 32px gap spacing. |
Component Logic & Service Integration src/app/home/dashboard/dashboard.component.ts |
Injects DashboardService; adds public state properties: activeLoans, totalCollections, parRatio, newClients; implements fetchDashboardData method that processes HTTP response from service to calculate loan summaries; refactors getRecentActivities and getFrequentActivities to use non-mutating operations and exclude route paths (/login, /home, /dashboard). |
New Dashboard Service src/app/home/dashboard/dashboard.service.ts |
Creates new DashboardService with getActiveLoansSummary method; constructs HttpParams with report filters (office, currency, fund, purpose, type, officer, product) and performs HTTP GET request to /runreports endpoint with 'Active Loans - Summary' report name. |
Sequence Diagram
sequenceDiagram
participant DC as DashboardComponent
participant DS as DashboardService
participant HC as HttpClient
participant API as /runreports API
DC->>DC: ngOnInit()
DC->>DC: fetchDashboardData()
DC->>DS: getActiveLoansSummary()
DS->>DS: Build HttpParams<br/>(filters: office, currency, fund, etc.)
DS->>HC: GET /runreports/{Encoded Report Name}
HC->>API: HTTP GET Request
API-->>HC: Response (data rows, column headers)
HC-->>DS: Observable<any>
DS-->>DC: Return Observable
DC->>DC: Process response<br/>(locate columns, accumulate totals)
DC->>DC: Update state<br/>(activeLoans, totalCollections,<br/>parRatio, newClients)
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~22 minutes
Suggested reviewers
- IOhacker
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title directly references the PR objective of implementing live dashboard metrics and aligning the UI, matching the primary changes across all modified files. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
📝 Coding Plan
- Generate coding plan for human review comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
Member
Author
|
Hi @IOhacker, I've added the dashboard metric tiles as discussed. For this PR, the data is static so we can confirm the UI and layout look good. I'll handle the API integration in the next step! Ready for review. |
HarshDeep-dev
force-pushed
the
feat/dashboard-metrics
branch
from
307cea8 to
8d408c4
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
src/app/groups/groups.component.html (1)
29-29: Remove redundant null check and consolidate async subscriptions forrecords$.
records$is a numericObservable<number>initialized with 0, so the null check is unnecessary. The current template subscripted torecords$multiple times (lines 29, 40, and 80), creating redundant subscriptions. Consolidate the async binding usingng-containerwith theassyntax for lines 29 and 40.Suggested refactor
+ <ng-container *ngIf="dataSource?.records$ | async as total"> <mifosx-empty-state - *ngIf="(dataSource?.records$ | async) === null || (dataSource?.records$ | async) === 0" + *ngIf="total === 0" [title]="'labels.text.No Groups Found' | translate" [description]="'labels.text.No Groups Found Description' | translate" [icon]="'users'" > <button mat-raised-button color="primary" [routerLink]="['create']"> <fa-icon icon="plus" class="m-r-10"></fa-icon> {{ 'labels.buttons.Create Your First Group' | translate }} </button> </mifosx-empty-state> <table *ngIf="total > 0" mat-table [dataSource]="dataSource" matSort class="bordered-table"> <!-- table columns --> </table> + </ng-container>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/groups/groups.component.html` at line 29, Remove the redundant null check on dataSource?.records$ (records$ is an Observable<number> initialized to 0) and consolidate the multiple async bindings by creating a single ng-container that subscribes once using the "as" syntax (e.g., bind dataSource?.records$ | async to a local template variable like records) and then replace subsequent uses with that local variable for the checks and displays; update template references to use records === 0 for the empty-state logic and eliminate the repeated (dataSource?.records$ | async) subscriptions in the component template.src/app/shared/components/empty-state/empty-state.component.ts (1)
11-12: Type the icon input asIconProp, notstring.
fa-iconaccepts Font Awesome lookups/definitions, so the current type loses template safety and prevents callers from passing tuple orIconDefinitionvalues.Suggested fix
import { FaIconComponent } from '@fortawesome/angular-fontawesome'; +import { IconProp } from '@fortawesome/fontawesome-svg-core'; @@ - `@Input`() icon: string = ''; + `@Input`() icon: IconProp | null = null;Also applies to: 26-26
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/shared/components/empty-state/empty-state.component.ts` around lines 11 - 12, The icon Input in EmptyStateComponent is currently typed as string which prevents passing FontAwesome tuples or IconDefinition; update the Input type(s) named icon (and any other similar Input like secondaryIcon if present) to use IconProp from '@fortawesome/fontawesome-svg-core' and update the import (ensure IconProp is imported) so the fa-icon binding accepts lookups/definitions and restores template type safety.src/app/home/dashboard/dashboard.component.scss (1)
10-28: Please align this styling with the shared theme and 8px spacing scale.This block hardcodes both colors (
#fff,#f2f7fb,#555,#2b5b9c,#999) and off-scale spacing values (30px,20px,25px,15px,40px,10px). That makes the dashboard drift from the app theme and the repository’s spacing system.As per coding guidelines "Stick to the 8px grid system for visual design and spacing" and "Leverage SCSS variables defined in
src/main.scssandsrc/theme/mifosx-theme.scssrather than generating custom classes and explicit pixel values".Also applies to: 41-54, 75-85, 89-102
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/dashboard/dashboard.component.scss` around lines 10 - 28, The styles hardcode colors and non-8px spacing in .dashboard-card, .summary-bar, and .summary-tile; replace color literals (`#fff`, `#f2f7fb`, `#555`, `#2b5b9c`, `#999`) with the corresponding SCSS theme variables from main.scss / mifosx-theme.scss (e.g., $color-background, $color-surface, $color-text, $color-primary, $color-muted) and convert all pixel spacing (30px, 20px, 25px, 15px, 40px, 10px) to multiples of the 8px grid (8, 16, 24, 32, 40, etc.) using spacing variables (e.g., $space-8, $space-16) or calc-based helpers; ensure box-shadow and border-radius use theme tokens (e.g., $shadow-none, $radius-lg) and update the rules inside .dashboard-card, .summary-bar, .summary-tile (and similar blocks noted at lines 41-54, 75-85, 89-102) to reference these variables so the dashboard conforms to the shared theme and 8px spacing system.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/groups/groups.component.html`:
- Around line 34-37: The empty-state "Create Your First Group" button is shown
regardless of permissions; add the same permission gating used by the primary
action so users without CREATE_GROUP can't see it. Wrap the button element
containing routerLink "['create']" and the label "labels.buttons.Create Your
First Group" with the same check (for example
*ngIf="hasPermission('CREATE_GROUP')" or the existing permission pipe/variable
your app uses) referencing the permission identifier CREATE_GROUP so the CTA is
hidden when the user lacks that permission.
- Line 39: Remove the stray Markdown fence marker (```) from the template (line
39) so Angular parsing isn't broken; add the same permission guard
(*mifosxHasPermission="'CREATE_GROUP'") to the empty-state "create" button so it
matches the top-level create button and enforces consistent authorization; and
replace the duplicate async subscribes of (dataSource?.records$ | async) by
capturing that observable once in a template variable (via a single *ngIf or
async pipe assignment) and reuse that variable for the table *ngIf and any other
checks to avoid multiple subscriptions (affecting expressions that reference
dataSource?.records$ and the empty-state render).
In `@src/app/groups/groups.component.ts`:
- Around line 124-132: The current ngAfterViewInit block only wires sort events
if this.sort is present at that time, so when the table appears later the
MatSort headers never trigger loadGroupsPage; refactor to attach the sort
subscription from the MatSort `@ViewChild` setter (or a view-change hook) so that
whenever the sort instance is assigned you subscribe: ensure the setter
unsubscribes any previous subscription, call this.paginator.pageIndex = 0 on
sort.sortChange, and merge(sort.sortChange, this.paginator.page).pipe(tap(() =>
this.loadGroupsPage())).subscribe(); keep the existing paginator-only
subscription as a fallback but remove the conditional branch in ngAfterViewInit
so subscriptions are consistently established when sort becomes available.
In `@src/app/home/dashboard/dashboard.component.html`:
- Around line 12-29: The new dashboard template in dashboard.component.html
introduces four new translatable labels ("Active Loans", "Total Collections",
"Par Ratio", "New Clients") but the locale JSONs are missing matching keys; add
appropriate entries for these keys in each translation file (e.g.,
src/assets/translations/*.json) using the same i18n keys you use in the
template, then run npm run translations:extract to update extraction and verify
translations are present (ensure keys match the strings used in
dashboard.component.html so `@ngx-translate/core` resolves them instead of falling
back to English).
In `@src/app/shared/components/empty-state/empty-state.component.scss`:
- Line 13: Line 13 uses padding: 60px 24px which breaks the 8px design grid;
update the vertical padding to an 8px multiple (e.g., change the top/bottom
value from 60px to 64px) in
src/app/shared/components/empty-state/empty-state.component.scss (the
EmptyStateComponent styles) so the rule becomes 64px 24px to comply with the
8px-grid spacing guideline.
In `@src/assets/translations/cs-CS.json`:
- Line 353: The Czech locale contains English placeholder values (e.g., the key
"Create Your First Group") that must be translated into Czech; replace the
English strings with their proper Czech translations for "Create Your First
Group" and the other English entries noted (same file's other English
placeholders) so the cs-CS.json contains only Czech values, and run the
i18n/lint checks to ensure no remaining English placeholders remain.
In `@src/assets/translations/es-CL.json`:
- Around line 3029-3030: The empty-state description key "No Groups Found
Description" is written in a formal imperative; change its text to match the
informal tone used by the button ("Crea...") so both are consistent—update the
value of "No Groups Found Description" to an informal phrasing (e.g., use
"Intenta..." or "Prueba...") that aligns with the button copy and verify the
button string key that contains "Crea..." also uses the same informal register.
In `@src/assets/translations/pt-PT.json`:
- Line 353: The Portuguese (pt-PT) translations use pt-BR phrasing; update the
value for the "Create Your First Group" key from "Crie seu primeiro grupo" to a
pt-PT phrasing such as "Crie o seu primeiro grupo", and scan the nearby keys
around the ones at lines 3028-3029 to replace Brazilian terms like "busca" and
bare "seu" with pt-PT equivalents (e.g., "pesquisa" or "procura" and "o seu") so
the locale reads consistently; adjust the string values for those keys
accordingly.
---
Nitpick comments:
In `@src/app/groups/groups.component.html`:
- Line 29: Remove the redundant null check on dataSource?.records$ (records$ is
an Observable<number> initialized to 0) and consolidate the multiple async
bindings by creating a single ng-container that subscribes once using the "as"
syntax (e.g., bind dataSource?.records$ | async to a local template variable
like records) and then replace subsequent uses with that local variable for the
checks and displays; update template references to use records === 0 for the
empty-state logic and eliminate the repeated (dataSource?.records$ | async)
subscriptions in the component template.
In `@src/app/home/dashboard/dashboard.component.scss`:
- Around line 10-28: The styles hardcode colors and non-8px spacing in
.dashboard-card, .summary-bar, and .summary-tile; replace color literals (`#fff`,
`#f2f7fb`, `#555`, `#2b5b9c`, `#999`) with the corresponding SCSS theme variables from
main.scss / mifosx-theme.scss (e.g., $color-background, $color-surface,
$color-text, $color-primary, $color-muted) and convert all pixel spacing (30px,
20px, 25px, 15px, 40px, 10px) to multiples of the 8px grid (8, 16, 24, 32, 40,
etc.) using spacing variables (e.g., $space-8, $space-16) or calc-based helpers;
ensure box-shadow and border-radius use theme tokens (e.g., $shadow-none,
$radius-lg) and update the rules inside .dashboard-card, .summary-bar,
.summary-tile (and similar blocks noted at lines 41-54, 75-85, 89-102) to
reference these variables so the dashboard conforms to the shared theme and 8px
spacing system.
In `@src/app/shared/components/empty-state/empty-state.component.ts`:
- Around line 11-12: The icon Input in EmptyStateComponent is currently typed as
string which prevents passing FontAwesome tuples or IconDefinition; update the
Input type(s) named icon (and any other similar Input like secondaryIcon if
present) to use IconProp from '@fortawesome/fontawesome-svg-core' and update the
import (ensure IconProp is imported) so the fa-icon binding accepts
lookups/definitions and restores template type safety.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8a233ed1-dcc4-4581-8a5f-bc50a627ad3b
📒 Files selected for processing (23)
src/app/groups/groups.component.htmlsrc/app/groups/groups.component.scsssrc/app/groups/groups.component.tssrc/app/home/dashboard/dashboard.component.htmlsrc/app/home/dashboard/dashboard.component.scsssrc/app/home/dashboard/dashboard.component.tssrc/app/shared/components/empty-state/empty-state.component.htmlsrc/app/shared/components/empty-state/empty-state.component.scsssrc/app/shared/components/empty-state/empty-state.component.tssrc/app/shared/shared.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
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/app/shared/components/empty-state/empty-state.component.scss (1)
13-13:⚠️ Potential issue | 🟡 MinorAlign vertical padding with the 8px grid.
Line 13 uses
60px, which breaks the spacing grid. Please use an 8px multiple (64px).🎯 Suggested fix
- padding: 60px 24px; + padding: 64px 24px;As per coding guidelines
src/**/*.{scss,html}: Stick to the 8px grid system for visual design and spacing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/shared/components/empty-state/empty-state.component.scss` at line 13, The vertical padding in empty-state.component.scss uses 60px which violates the 8px grid; update the padding declaration (the line with "padding: 60px 24px;") to use an 8px-multiple (replace 60px with 64px) so it reads padding: 64px 24px; ensuring the EmptyState component adheres to the spacing guidelines.src/assets/translations/cs-CS.json (1)
353-353:⚠️ Potential issue | 🟡 MinorTranslate new
cs-CSentries instead of keeping English placeholders.Lines 353, 3029, and 3030 are still English, which leads to mixed-language UI in Czech locale.
💡 Suggested fix
- "Create Your First Group": "Create Your First Group", + "Create Your First Group": "Vytvořte svou první skupinu", - "No Groups Found": "No Groups Found", - "No Groups Found Description": "We couldn't find any groups. Try a different search or create one now.", + "No Groups Found": "Nebyly nalezeny žádné skupiny", + "No Groups Found Description": "Nenašli jsme žádné skupiny. Zkuste jiné vyhledávání nebo nyní vytvořte novou."Also applies to: 3029-3030
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/cs-CS.json` at line 353, The cs-CS locale file contains untranslated English placeholders (notably the JSON entry with key/value "Create Your First Group" and two other English strings in the same file) that must be replaced with proper Czech translations; update the value(s) for the "Create Your First Group" key (and the other English entries in cs-CS) with their Czech translations, preserving the exact JSON keys, proper quoting/escaping, and trailing commas/format so the file remains valid JSON, then run the i18n/localization lint or tests to ensure no missing keys or formatting errors.
🧹 Nitpick comments (5)
src/app/home/dashboard/dashboard.component.html (3)
19-19: Magic number and fragile formatting.The division by
1000000and the hardcodedMsuffix create tightly coupled formatting logic. If the data scale changes, both need updating.Consider extracting this to a method or pipe that handles large number formatting consistently, or at minimum define the divisor as a named constant.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/dashboard/dashboard.component.html` at line 19, The template currently divides totalCollections by 1000000 and appends a hardcoded "M", which is brittle; update the DashboardComponent to encapsulate this logic (e.g., add a method formatLargeNumber(value: number) or create a LargeNumberPipe) and replace the inline expression in the template (the {{ totalCollections ... }} binding) to call that method/pipe; ensure the divisor (1_000_000) is a named constant (e.g., MILLION) inside the component/pipe and return a properly localized currency string with the suffix handled by the method/pipe so formatting and scale changes are centralized.
41-45: Use a stable unique key for tracking.
track activitytracks by object reference. SincefilteredActivitiesemits a new array on each filter operation, all options may be re-rendered unnecessarily.Track by a unique property instead:
♻️ Proposed fix
- `@for` (activity of filteredActivities | async; track activity) { + `@for` (activity of filteredActivities | async; track activity.path) {As per coding guidelines for Angular code: "verify ... trackBy on *ngFor" (applies to
@foras well).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/dashboard/dashboard.component.html` around lines 41 - 45, The template uses "track activity" which tracks by object reference and causes unnecessary re-renders; update the `@for` track expression to use a stable unique property (e.g., change to "track activity.id" or "track activity.path") referencing the unique field on each activity in filteredActivities, or add a stable id property to the activity objects if missing; ensure you update the template's `@for` track clause (and the activity model if needed) so ngFor/@for tracks by that unique property instead of the whole object.
43-43: Activity names are not translated.The activity labels (e.g., "client", "groups", "accounting") are displayed without the translate pipe. For full i18n support, these should be localized.
This may require mapping each activity to a translation key. If out of scope for this PR, consider tracking it as follow-up work.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/dashboard/dashboard.component.html` at line 43, The activity labels are currently rendered raw via the binding "{{ activity.activity }}" in the DashboardComponent template; update the span to use the translate pipe (or a lookup map) so labels are localized (e.g., replace the raw binding with a translation key lookup like activity.activity | translate or map activity.activity to a key and pass that into the translate pipe). Ensure the change targets the template element that renders activity.activity and, if needed, add or reference a mapping in the DashboardComponent (e.g., an activitiesToTranslationKey map) so all values like "client", "groups", "accounting" resolve to i18n keys.src/app/home/dashboard/dashboard.component.scss (2)
22-72: Duplicate style definitions and hardcoded colors.
Duplicate definitions:
.tile-labeland.tile-numberare defined twice — once nested inside.summary-tile(lines 40-55) and again directly under.summary-bar(lines 58-72). Only one set is needed; remove the redundant block.Hardcoded colors: Values like
#f2f7fb,#e0e6ed,#555, and#2b5b9cshould ideally use theme variables or CSS custom properties for consistency with the design system.♻️ Remove duplicate definitions
.tile-number { margin: 0; font-size: 36px; font-weight: 700; color: `#2b5b9c`; line-height: 1; } } - - .tile-label { - display: block; - color: `#555`; - font-size: 15px; - font-weight: 500; - margin-bottom: 10px; - text-transform: capitalize; - } - - .tile-number { - margin: 0; - font-size: 36px; - font-weight: 700; - color: `#2b5b9c`; - } }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 the current code and only fix it if needed. In `@src/app/home/dashboard/dashboard.component.scss` around lines 22 - 72, Remove the duplicated style blocks by keeping only the nested .tile-label and .tile-number inside .summary-tile (or conversely keep the global ones and remove the nested ones) so there is a single source of truth for .tile-label and .tile-number; replace hardcoded color and background values (`#f2f7fb`, `#e0e6ed`, `#555`, `#2b5b9c`) and any repeated sizing where appropriate with the SCSS theme variables or CSS custom properties from src/main.scss or src/theme/mifosx-theme.scss (e.g., use the theme color vars for background, border, text and primary number color) and ensure .summary-tile retains its layout rules (flex, padding, border-radius, transition) while referencing those variables.
10-14: Consider using SCSS variables or CSS custom properties for colors.Hardcoded hex values (
#fff) bypass the theme system. The project guidelines recommend leveraging SCSS variables fromsrc/theme/mifosx-theme.scssor CSS custom properties likevar(--md-sys-color-surface).The
!importanton line 13 is also a code smell — if specificity is the issue, consider restructuring selectors instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/dashboard/dashboard.component.scss` around lines 10 - 14, Replace the hardcoded color and remove the !important on the .dashboard-card rule: change background: `#fff` to use the shared theme variable (e.g., background: $surface-color or background: var(--md-sys-color-surface) depending on your theme) and remove box-shadow: none !important; if specificity is required, increase selector specificity (for example scope with :host .dashboard-card or a more specific selector) or adjust the component stylesheet order so the box-shadow override is applied without !important; update the .dashboard-card rule accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/app/shared/components/empty-state/empty-state.component.scss`:
- Line 13: The vertical padding in empty-state.component.scss uses 60px which
violates the 8px grid; update the padding declaration (the line with "padding:
60px 24px;") to use an 8px-multiple (replace 60px with 64px) so it reads
padding: 64px 24px; ensuring the EmptyState component adheres to the spacing
guidelines.
In `@src/assets/translations/cs-CS.json`:
- Line 353: The cs-CS locale file contains untranslated English placeholders
(notably the JSON entry with key/value "Create Your First Group" and two other
English strings in the same file) that must be replaced with proper Czech
translations; update the value(s) for the "Create Your First Group" key (and the
other English entries in cs-CS) with their Czech translations, preserving the
exact JSON keys, proper quoting/escaping, and trailing commas/format so the file
remains valid JSON, then run the i18n/localization lint or tests to ensure no
missing keys or formatting errors.
---
Nitpick comments:
In `@src/app/home/dashboard/dashboard.component.html`:
- Line 19: The template currently divides totalCollections by 1000000 and
appends a hardcoded "M", which is brittle; update the DashboardComponent to
encapsulate this logic (e.g., add a method formatLargeNumber(value: number) or
create a LargeNumberPipe) and replace the inline expression in the template (the
{{ totalCollections ... }} binding) to call that method/pipe; ensure the divisor
(1_000_000) is a named constant (e.g., MILLION) inside the component/pipe and
return a properly localized currency string with the suffix handled by the
method/pipe so formatting and scale changes are centralized.
- Around line 41-45: The template uses "track activity" which tracks by object
reference and causes unnecessary re-renders; update the `@for` track expression to
use a stable unique property (e.g., change to "track activity.id" or "track
activity.path") referencing the unique field on each activity in
filteredActivities, or add a stable id property to the activity objects if
missing; ensure you update the template's `@for` track clause (and the activity
model if needed) so ngFor/@for tracks by that unique property instead of the
whole object.
- Line 43: The activity labels are currently rendered raw via the binding "{{
activity.activity }}" in the DashboardComponent template; update the span to use
the translate pipe (or a lookup map) so labels are localized (e.g., replace the
raw binding with a translation key lookup like activity.activity | translate or
map activity.activity to a key and pass that into the translate pipe). Ensure
the change targets the template element that renders activity.activity and, if
needed, add or reference a mapping in the DashboardComponent (e.g., an
activitiesToTranslationKey map) so all values like "client", "groups",
"accounting" resolve to i18n keys.
In `@src/app/home/dashboard/dashboard.component.scss`:
- Around line 22-72: Remove the duplicated style blocks by keeping only the
nested .tile-label and .tile-number inside .summary-tile (or conversely keep the
global ones and remove the nested ones) so there is a single source of truth for
.tile-label and .tile-number; replace hardcoded color and background values
(`#f2f7fb`, `#e0e6ed`, `#555`, `#2b5b9c`) and any repeated sizing where appropriate with
the SCSS theme variables or CSS custom properties from src/main.scss or
src/theme/mifosx-theme.scss (e.g., use the theme color vars for background,
border, text and primary number color) and ensure .summary-tile retains its
layout rules (flex, padding, border-radius, transition) while referencing those
variables.
- Around line 10-14: Replace the hardcoded color and remove the !important on
the .dashboard-card rule: change background: `#fff` to use the shared theme
variable (e.g., background: $surface-color or background:
var(--md-sys-color-surface) depending on your theme) and remove box-shadow: none
!important; if specificity is required, increase selector specificity (for
example scope with :host .dashboard-card or a more specific selector) or adjust
the component stylesheet order so the box-shadow override is applied without
!important; update the .dashboard-card rule accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 368b4a91-8bdb-44d7-b9f6-b409f7115b36
📒 Files selected for processing (23)
src/app/groups/groups.component.htmlsrc/app/groups/groups.component.scsssrc/app/groups/groups.component.tssrc/app/home/dashboard/dashboard.component.htmlsrc/app/home/dashboard/dashboard.component.scsssrc/app/home/dashboard/dashboard.component.tssrc/app/shared/components/empty-state/empty-state.component.htmlsrc/app/shared/components/empty-state/empty-state.component.scsssrc/app/shared/components/empty-state/empty-state.component.tssrc/app/shared/shared.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 as they are similar to previous changes (17)
- src/assets/translations/en-US.json
- src/app/shared/components/empty-state/empty-state.component.html
- src/assets/translations/es-CL.json
- src/app/groups/groups.component.scss
- src/assets/translations/es-MX.json
- src/assets/translations/pt-PT.json
- src/app/home/dashboard/dashboard.component.ts
- src/assets/translations/sw-SW.json
- src/assets/translations/ko-KO.json
- src/app/groups/groups.component.html
- src/assets/translations/lv-LV.json
- src/assets/translations/fr-FR.json
- src/assets/translations/ne-NE.json
- src/assets/translations/lt-LT.json
- src/assets/translations/it-IT.json
- src/app/shared/components/empty-state/empty-state.component.ts
- src/app/groups/groups.component.ts
HarshDeep-dev
force-pushed
the
feat/dashboard-metrics
branch
2 times, most recently
from
e470c9b to
7338472
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/assets/translations/pt-PT.json (1)
353-353:⚠️ Potential issue | 🟡 MinorUse pt-PT phrasing for the new empty-state strings.
Line 353 and Line 3029 still use pt-BR-leaning wording (
"seu","busca"). Please switch to pt-PT equivalents for locale consistency.💬 Suggested wording
- "Create Your First Group": "Crie seu primeiro grupo", + "Create Your First Group": "Crie o seu primeiro grupo", - "No Groups Found Description": "Não conseguimos encontrar nenhum grupo. Tente uma busca diferente ou crie um agora.", + "No Groups Found Description": "Não conseguimos encontrar nenhum grupo. Tente uma pesquisa diferente ou crie um agora.",Also applies to: 3029-3029
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/pt-PT.json` at line 353, The pt-PT translation uses pt-BR wording—update the empty-state strings to pt-PT phrasing by replacing the value for the "Create Your First Group" key from "Crie seu primeiro grupo" to a pt-PT form (e.g., "Crie o seu primeiro grupo" or "Crie o primeiro grupo") and also find and replace the other empty-state value that uses "busca" (line with "busca") to use the pt-PT term "pesquisa"; ensure you update the exact JSON values for the matching keys so locale consistency is preserved.
🧹 Nitpick comments (2)
src/app/home/dashboard/dashboard.component.ts (1)
87-97: Replaceanywith concrete types in frequency computation.This block can be fully typed (
Record<string, number>and typed entry tuples) for stricter compile-time safety.♻️ Typed version
- getFrequentActivities() { - const frequencyCounts: any = {}; + getFrequentActivities(): string[] { + const frequencyCounts: Record<string, number> = {}; let index = this.userActivity?.length; while (index) { const activity = this.userActivity[--index]; frequencyCounts[activity] = (frequencyCounts[activity] || 0) + 1; } return Object.entries(frequencyCounts) - .sort((a: any, b: any) => b[1] - a[1]) - .map((entry: any[]) => entry[0]) + .sort(([, countA], [, countB]) => countB - countA) + .map(([activity]) => activity)As per coding guidelines
src/app/**/*.ts: Use TypeScript for all application code with strict typing conventions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/dashboard/dashboard.component.ts` around lines 87 - 97, The frequency computation in getFrequentActivities uses loose any types; replace frequencyCounts: any with frequencyCounts: Record<string, number>, type index as number, and ensure the Object.entries result and downstream variables use typed tuples like [string, number][] (and map/filter callbacks use typed parameters such as (entry: [string, number]) and (a: [string, number], b: [string, number]) ) so the sort/map/filter operate with concrete types instead of any.src/app/home/dashboard/dashboard.component.scss (1)
40-72: Deduplicate repeated tile typography rules.
.tile-labeland.tile-numberare declared twice in the same scope chain. Consolidating these avoids accidental divergence later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/dashboard/dashboard.component.scss` around lines 40 - 72, The CSS repeats .tile-label and .tile-number rules twice; consolidate them into single declarations to avoid divergence by merging all unique properties (for .tile-label include color, font-size, font-weight, margin-bottom, text-align if needed, line-height:1.3 and text-transform:capitalize as applicable; for .tile-number include margin, font-size, font-weight, color, and line-height:1) and remove the duplicate blocks so only one .tile-label and one .tile-number remain (locate occurrences of .tile-label and .tile-number in dashboard.component.scss and combine their properties).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/home/dashboard/dashboard.component.scss`:
- Around line 10-104: Update spacing values to the 8px grid: change
.dashboard-card padding 30px -> 32px and .summary-bar gap 20px -> 24px and
margin-bottom 30px -> 32px; inside .summary-tile change padding 25px 15px ->
24px 16px, border-radius 12px -> 16px, min-height 125px -> 128px, and tile
spacing .tile-label margin-bottom 15px -> 16px and .tile-number font-size 36px
-> 32px; adjust the outer .tile-label margin-bottom 10px -> 8px and font-size
15px -> 16px, and .tile-number font-size 36px -> 32px; in .search-container
change .search-icon margin-right 10px -> 8px; in .charts-grid change gap 30px ->
32px; ensure all modified selectors are .dashboard-card, .summary-bar,
.summary-tile, .tile-label, .tile-number, .search-icon, and .charts-grid
.chart-item so spacing conforms to the 8px grid.
In `@src/app/home/dashboard/dashboard.component.ts`:
- Around line 57-59: The code assigns userActivity with
JSON.parse(localStorage.getItem('mifosXLocation') || '[]') which can still throw
or produce a non-array; wrap the parse in a try/catch and validate the shape
(e.g., try { const v = JSON.parse(raw); this.userActivity = Array.isArray(v) ? v
: []; } catch { this.userActivity = []; }) to ensure a safe default, and apply
the same defensive parsing to the corresponding read in SidenavComponent
(replace the direct localStorage.getItem('mifosXLocation') parse with the same
try/catch + Array.isArray validation or extract a small helper like
safeParseArray(key) and use it from the DashboardComponent constructor and the
SidenavComponent initialization).
---
Duplicate comments:
In `@src/assets/translations/pt-PT.json`:
- Line 353: The pt-PT translation uses pt-BR wording—update the empty-state
strings to pt-PT phrasing by replacing the value for the "Create Your First
Group" key from "Crie seu primeiro grupo" to a pt-PT form (e.g., "Crie o seu
primeiro grupo" or "Crie o primeiro grupo") and also find and replace the other
empty-state value that uses "busca" (line with "busca") to use the pt-PT term
"pesquisa"; ensure you update the exact JSON values for the matching keys so
locale consistency is preserved.
---
Nitpick comments:
In `@src/app/home/dashboard/dashboard.component.scss`:
- Around line 40-72: The CSS repeats .tile-label and .tile-number rules twice;
consolidate them into single declarations to avoid divergence by merging all
unique properties (for .tile-label include color, font-size, font-weight,
margin-bottom, text-align if needed, line-height:1.3 and
text-transform:capitalize as applicable; for .tile-number include margin,
font-size, font-weight, color, and line-height:1) and remove the duplicate
blocks so only one .tile-label and one .tile-number remain (locate occurrences
of .tile-label and .tile-number in dashboard.component.scss and combine their
properties).
In `@src/app/home/dashboard/dashboard.component.ts`:
- Around line 87-97: The frequency computation in getFrequentActivities uses
loose any types; replace frequencyCounts: any with frequencyCounts:
Record<string, number>, type index as number, and ensure the Object.entries
result and downstream variables use typed tuples like [string, number][] (and
map/filter callbacks use typed parameters such as (entry: [string, number]) and
(a: [string, number], b: [string, number]) ) so the sort/map/filter operate with
concrete types instead of any.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d1c602e7-1e68-49ee-b9c0-a3ef5e290446
📒 Files selected for processing (19)
src/app/groups/groups.component.htmlsrc/app/groups/groups.component.scsssrc/app/home/dashboard/dashboard.component.htmlsrc/app/home/dashboard/dashboard.component.scsssrc/app/home/dashboard/dashboard.component.tssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.jsonsrc/environments/.env.ts
💤 Files with no reviewable changes (1)
- src/environments/.env.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- src/app/home/dashboard/dashboard.component.html
- src/assets/translations/es-MX.json
- src/assets/translations/lv-LV.json
- src/assets/translations/fr-FR.json
- src/assets/translations/cs-CS.json
- src/assets/translations/ne-NE.json
- src/app/groups/groups.component.scss
- src/assets/translations/en-US.json
- src/assets/translations/sw-SW.json
- src/app/groups/groups.component.html
- src/assets/translations/de-DE.json
HarshDeep-dev
force-pushed
the
feat/dashboard-metrics
branch
from
7338472 to
cce9bb9
Compare
IOhacker
requested changes
Mar 15, 2026
Contributor
IOhacker
left a comment
IOhacker
left a comment
There was a problem hiding this comment.
Use thir Jira ticket for this PR, make sure to include it in the title and description
HarshDeep-dev
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/app/home/dashboard/dashboard.component.scss (2)
27-27: Consider aligningborder-radiusto the 8px grid.The
border-radius: 12pxvalue is off the 8px grid. Consider using8pxfor subtle rounding or16pxfor more pronounced corners.- border-radius: 12px; + border-radius: 16px;As per coding guidelines "Stick to the 8px grid system for visual design and spacing".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/dashboard/dashboard.component.scss` at line 27, Replace the non-conforming border-radius value in dashboard.component.scss (the rule setting border-radius: 12px) with an 8px value to align with the 8px grid system (or 16px if you prefer stronger rounding); update the border-radius declaration in the same selector where border-radius: 12px is defined so it uses 8px (or 16px) instead.
12-13: Consider extracting hardcoded colors to theme variables.Multiple hardcoded color values are used throughout:
#fff,#f2f7fb,#e0e6edfor backgrounds/borders#555,#999for muted text#2b5b9cfor accent numbersWhile the theme files may not have exact equivalents for all these colors, consider using CSS custom properties like
var(--md-sys-color-surface)orvar(--md-sys-color-on-surface)where applicable, or defining new SCSS variables in the theme for dashboard-specific colors to improve maintainability.Also applies to: 25-26, 41-41, 53-53, 60-60, 71-71, 84-84
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/dashboard/dashboard.component.scss` around lines 12 - 13, Replace hardcoded hex colors in src/app/home/dashboard/dashboard.component.scss (e.g., `#fff`, `#f2f7fb`, `#e0e6ed`, `#555`, `#999`, `#2b5b9c` used in selectors in this file) with theme-aware variables: use existing CSS custom properties like var(--md-sys-color-surface) / var(--md-sys-color-on-surface) where appropriate, or introduce new SCSS variables (e.g., $dashboard-surface, $dashboard-muted, $dashboard-accent) in your theme and reference them here; update all occurrences called out in the review (lines around the file where those hex values appear) so the styles read background: var(--...) or background: $dashboard-surface and remove hardcoded hex literals. Ensure any new SCSS variables are added to the shared theme file and documented so other components can reuse them.src/app/home/dashboard/dashboard.component.html (1)
19-19: Hardcoded currency and suffix may not display correctly for all locales.The
'USD'currency code andMsuffix are hardcoded. For international users, consider:
- Using a configurable currency based on the tenant/organization settings
- Applying locale-aware formatting for large numbers instead of hardcoded "M" suffix
💡 Example using locale-aware compact notation
If the target browsers support it, you could use a custom pipe leveraging
Intl.NumberFormatwithnotation: 'compact', or keep displaying the full number and let the currency pipe handle it:<!-- Option 1: Full number with currency --> <h2 class="tile-number">{{ totalCollections | currency: organizationCurrency : 'symbol' : '1.0-0' }}</h2> <!-- Option 2: Custom compact pipe (would need implementation) --> <h2 class="tile-number">{{ totalCollections | compactCurrency: organizationCurrency }}</h2>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/dashboard/dashboard.component.html` at line 19, The template currently hardcodes 'USD' and a trailing "M" in the expression using totalCollections; update the dashboard view to use a configurable currency and locale-aware formatting instead: expose a tenant/organization currency property (e.g., organizationCurrency) on DashboardComponent and replace the hardcoded currency argument with that property, remove the manual "M" suffix and either use the built-in currency pipe to show the full localized value ({{ totalCollections | currency: organizationCurrency : 'symbol' : '1.0-0' }}) or implement a compactCurrency pipe that uses Intl.NumberFormat({ notation: 'compact' }) and reference it as {{ totalCollections | compactCurrency: organizationCurrency }} so formatting is locale-aware and driven by organization settings; ensure DashboardComponent supplies organizationCurrency and any locale if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/home/dashboard/dashboard.component.ts`:
- Line 120: Remove the debug console.log('Dashboard Updated with Live Data!') in
DashboardComponent (the statement shown in dashboard.component.ts) before
merging; either delete the line or replace it with a call to the application's
logging facility (e.g., use the shared Logger/LoggingService or an
Angular-injected logger and log at an appropriate level such as info) so
production code does not contain raw console.debug/console.log statements.
- Around line 79-123: fetchDashboardData() currently subscribes to
dashboardService.getActiveLoansSummary() with only a next handler, so API errors
are swallowed; add an error handler (and optional complete/finally behavior) to
the subscribe call to handle failures: inside the subscribe callback object add
error: (err) => { log the error (console.error or this.logger), set safe
default/clear values for this.activeLoans, this.newClients,
this.totalCollections and this.parRatio, and surface user feedback (e.g., show a
toast/snackbar or set an error flag used by the template) } and consider using
the complete or finalize operator if any cleanup is required; reference
fetchDashboardData, dashboardService.getActiveLoansSummary, and the existing
subscribe(...) call to locate where to add this.
In `@src/app/home/dashboard/dashboard.service.ts`:
- Around line 19-36: Remove the manual HttpHeaders from getActiveLoansSummary
(do not construct new HttpHeaders or hardcode
'Fineract-Platform-TenantId'/'Content-Type'); rely on existing interceptors for
those headers and keep only the HttpParams construction (symbols:
getActiveLoansSummary, HttpParams). Eliminate the unused reportName variable or,
if keeping it, use it in the GET call by passing `/runreports/${reportName}`
(symbol: reportName) instead of calling encodeURIComponent inline; ensure the
request uses this.http.get with the params only (symbol: this.http.get). Also
remove any hardcoded tenant IDs so the dynamic tenant from SettingsService via
interceptors is used.
---
Nitpick comments:
In `@src/app/home/dashboard/dashboard.component.html`:
- Line 19: The template currently hardcodes 'USD' and a trailing "M" in the
expression using totalCollections; update the dashboard view to use a
configurable currency and locale-aware formatting instead: expose a
tenant/organization currency property (e.g., organizationCurrency) on
DashboardComponent and replace the hardcoded currency argument with that
property, remove the manual "M" suffix and either use the built-in currency pipe
to show the full localized value ({{ totalCollections | currency:
organizationCurrency : 'symbol' : '1.0-0' }}) or implement a compactCurrency
pipe that uses Intl.NumberFormat({ notation: 'compact' }) and reference it as {{
totalCollections | compactCurrency: organizationCurrency }} so formatting is
locale-aware and driven by organization settings; ensure DashboardComponent
supplies organizationCurrency and any locale if needed.
In `@src/app/home/dashboard/dashboard.component.scss`:
- Line 27: Replace the non-conforming border-radius value in
dashboard.component.scss (the rule setting border-radius: 12px) with an 8px
value to align with the 8px grid system (or 16px if you prefer stronger
rounding); update the border-radius declaration in the same selector where
border-radius: 12px is defined so it uses 8px (or 16px) instead.
- Around line 12-13: Replace hardcoded hex colors in
src/app/home/dashboard/dashboard.component.scss (e.g., `#fff`, `#f2f7fb`, `#e0e6ed`,
`#555`, `#999`, `#2b5b9c` used in selectors in this file) with theme-aware variables:
use existing CSS custom properties like var(--md-sys-color-surface) /
var(--md-sys-color-on-surface) where appropriate, or introduce new SCSS
variables (e.g., $dashboard-surface, $dashboard-muted, $dashboard-accent) in
your theme and reference them here; update all occurrences called out in the
review (lines around the file where those hex values appear) so the styles read
background: var(--...) or background: $dashboard-surface and remove hardcoded
hex literals. Ensure any new SCSS variables are added to the shared theme file
and documented so other components can reuse them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ec125d15-1c69-47e1-8cbd-584c90e06d1b
📒 Files selected for processing (7)
src/app/groups/groups.component.htmlsrc/app/groups/groups.component.scsssrc/app/home/dashboard/dashboard.component.htmlsrc/app/home/dashboard/dashboard.component.scsssrc/app/home/dashboard/dashboard.component.tssrc/app/home/dashboard/dashboard.service.tssrc/environments/.env.ts
💤 Files with no reviewable changes (1)
- src/environments/.env.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app/groups/groups.component.html
- src/app/groups/groups.component.scss
HarshDeep-dev
force-pushed
the
feat/dashboard-metrics
branch
from
fe7ba06 to
6cd5e42
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/app/home/dashboard/dashboard.component.ts (1)
79-90: Consider tracking API response typing as a separate enhancement.The dashboard parsing currently uses
anytypes (response: any, column iteration withc: any). While strict typing conventions apply to this file path, the project treats comprehensive API response layer typing as a cross-cutting refactoring tracked separately rather than addressed within individual PRs. Either include typed interfaces for the dashboard API response shape in this PR if scope permits, or defer to the coordinated API typing initiative.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/dashboard/dashboard.component.ts` around lines 79 - 90, The code in fetchDashboardData uses untyped any for the API response and column items (response: any, c: any) which conflicts with strict typing rules; either define and use a small local interface (e.g., ActiveLoansResponse with columnHeaders: ColumnHeader[] and data: any[]) and ColumnHeader { columnName: string; ... } and change the getActiveLoansSummary subscription signature to next: (response: ActiveLoansResponse) => { ... } and replace c: any with c: ColumnHeader, or if this PR should not add cross-cutting API types, explicitly annotate response and column items with a narrow TODO-typed alias (e.g., type ActiveLoansResponse = unknown /* tracked elsewhere */) and use type assertions at use sites to satisfy the compiler while leaving full typing to the centralized refactor; update references in fetchDashboardData and where cols.findIndex is used accordingly (function fetchDashboardData, method getActiveLoansSummary, variable response, property columnHeaders).src/app/home/dashboard/dashboard.service.ts (1)
19-33: Defer API response typing to separate cross-cutting enhancement.The hardcoded
R_officeId: '1'should be parameterized to support office-specific dashboard metrics. However, introducingObservable<ActiveLoansSummaryResponse>and associated interfaces is part of a project-wide API response typing initiative that should be tracked separately per established patterns, not as part of this change. Focus this PR on the office-awareness improvement alone.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/home/dashboard/dashboard.service.ts` around lines 19 - 33, The getActiveLoansSummary function currently hardcodes R_officeId='1'; change it to accept an officeId parameter (e.g., getActiveLoansSummary(officeId: string | number)) and use that value when building HttpParams (set('R_officeId', String(officeId))). Keep the return type as Observable<any> for this PR (do not add ActiveLoansSummaryResponse/typing) and ensure callers are updated to pass the appropriate officeId where getActiveLoansSummary is invoked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/home/dashboard/dashboard.component.ts`:
- Around line 84-92: The column-index lookups (activeLoansIdx, clientsIdx,
princRepaidIdx, intRepaidIdx, feesRepaidIdx, penRepaidIdx, princOverdueIdx,
princOutIdx) can be -1 and are then used for aggregation; update the aggregation
in dashboard.component.ts to guard against missing columns by checking each
index >= 0 before reading rows (or defaulting the summed value to 0), skipping
or logging missing columns instead of indexing with -1; apply the same guard to
the other index sets used in the aggregation block around the later rows (the
section noted as also applies to: 99-110).
- Around line 76-77: The PR accidentally made tiles load live data by calling
fetchDashboardData() unconditionally in ngOnInit and the error path sets
this.tiles = 0, breaking the UI-only placeholder behavior. Revert to keeping the
UI-only placeholders by either removing the unconditional call to
fetchDashboardData() from ngOnInit or gating it behind a feature flag/boolean
(e.g., useLiveData) so ngOnInit does not call fetchDashboardData() during this
PR; additionally, update fetchDashboardData()'s error handling so it does not
assign this.tiles = 0 on failure (leave this.tiles untouched) to preserve
placeholders. Target the ngOnInit method, the fetchDashboardData function, and
the this.tiles assignment to implement this fix.
---
Nitpick comments:
In `@src/app/home/dashboard/dashboard.component.ts`:
- Around line 79-90: The code in fetchDashboardData uses untyped any for the API
response and column items (response: any, c: any) which conflicts with strict
typing rules; either define and use a small local interface (e.g.,
ActiveLoansResponse with columnHeaders: ColumnHeader[] and data: any[]) and
ColumnHeader { columnName: string; ... } and change the getActiveLoansSummary
subscription signature to next: (response: ActiveLoansResponse) => { ... } and
replace c: any with c: ColumnHeader, or if this PR should not add cross-cutting
API types, explicitly annotate response and column items with a narrow
TODO-typed alias (e.g., type ActiveLoansResponse = unknown /* tracked elsewhere
*/) and use type assertions at use sites to satisfy the compiler while leaving
full typing to the centralized refactor; update references in fetchDashboardData
and where cols.findIndex is used accordingly (function fetchDashboardData,
method getActiveLoansSummary, variable response, property columnHeaders).
In `@src/app/home/dashboard/dashboard.service.ts`:
- Around line 19-33: The getActiveLoansSummary function currently hardcodes
R_officeId='1'; change it to accept an officeId parameter (e.g.,
getActiveLoansSummary(officeId: string | number)) and use that value when
building HttpParams (set('R_officeId', String(officeId))). Keep the return type
as Observable<any> for this PR (do not add ActiveLoansSummaryResponse/typing)
and ensure callers are updated to pass the appropriate officeId where
getActiveLoansSummary is invoked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d4f83160-0398-42c9-bcc5-fac2f549e73a
📒 Files selected for processing (3)
src/app/home/dashboard/dashboard.component.scsssrc/app/home/dashboard/dashboard.component.tssrc/app/home/dashboard/dashboard.service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/home/dashboard/dashboard.component.scss
HarshDeep-dev
force-pushed
the
feat/dashboard-metrics
branch
from
be74d9c to
21227aa
Compare
Member
Author
|
@IOhacker Ready for your review! |
IOhacker
requested changes
Mar 16, 2026
|
|
||
| activeLoans = 0; | ||
| totalCollections = 0; | ||
| parRatio = 0.038; // Keeping static for now, as it's a separate report |
Contributor
There was a problem hiding this comment.
Still there are fixed values
| activeLoans = 0; | ||
| totalCollections = 0; | ||
| parRatio = 0.038; // Keeping static for now, as it's a separate report | ||
| newClients = 85; |
Contributor
There was a problem hiding this comment.
Still there are fixed values
HarshDeep-dev
force-pushed
the
feat/dashboard-metrics
branch
from
21227aa to
b2297fb
Compare
Contributor
|
@HarshDeep-dev I will revert this PR seems it has broken the dashboard UI https://demo.mifos.community/#/dashboard 
|
IOhacker
pushed a commit
that referenced
this pull request
Mar 16, 2026
This was referenced Mar 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR addresses WEB-861 : (https://mifosforge.jira.com/browse/WEB-861) by synchronizing the Dashboard metric tiles with live data from the Fineract Active Loans - Summary report API. It also refines the UI layout .
Screenshots :
Changes
Live Data Integration: Connected Dashboard tiles to DashboardService using the runreports API to fetch real-time metrics (Active Loans, Total Collections, PAR Ratio, and Client count).
UI Alignment
Error Handling: Added API subscription error handling to ensure metrics reset to zero/default states gracefully if a network failure occurs.
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
[x] If you have multiple commits please combine them into one commit by squashing them.
[x] Read and understood the contribution guidelines at web-app/.github/CONTRIBUTING.md.