feat(loan): implement loan dashboard screen - #2647
Merged
niyajali merged 3 commits intoJun 30, 2026
Merged
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:
WalkthroughAdds a Loan Dashboard feature (UI, state, viewmodel, routing), new chart composables, icons/colors/design tokens, many loan strings, DI/navigation wiring for dashboard, and two optional date-array fields on loan entities ( Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Nav as Navigation
participant Screen as LoanDashboardScreen
participant VM as LoanDashboardViewModel
participant Repo as LoanAccountSummaryRepository
User->>Nav: navigateToLoanDashboardScreen(loanId)
Nav->>Screen: instantiate with loanId
Screen->>VM: request state for loanId
VM->>Repo: loadLoanAccountSummary(loanId)
Repo-->>VM: DataState (Loading / Success / Error)
VM->>Screen: emit ViewState (Loading / Success / Error)
Screen->>User: render UI (hero, graphs, timeline, transactions)
User->>Screen: interact (toggle legend, tap graph, open transaction)
Screen->>VM: dispatch LoanDashboardAction
VM->>Nav: emit navigation event (NavigateToTransactions / NavigateBack / other)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreen.kt (1)
359-368: Prefer string resources for badge labels.Deriving the badge copy from
status.namehard-codes English and can drift from product copy. ALoanStatus -> StringResourcemapping keeps these labels localizable like the rest of this screen.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreen.kt` around lines 359 - 368, The badge currently builds its label from status.name (statusText) which hard-codes English; replace that logic with a mapping from the domain enum (LoanStatus) to a string resource and use stringResource(...) inside LoanDashboardScreen where Badge/Text are rendered. Add a function or when-expression (e.g., loanStatusToStringRes(status: LoanStatus): Int) returning R.string keys for each status, then replace the statusText usage with stringResource(loanStatusToStringRes(status)) so the badge copy is localizable and consistent with the rest of the screen.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/loans/LoanWithAssociationsEntity.kt`:
- Line 188: Update the Room database version and add a migration that adds the
new nullable column for LoanWithAssociationsEntity: increment the VERSION
constant in MifosDatabase (all platform targets) and implement a Room Migration
from the old version to the new version that executes an ALTER TABLE to add the
nullable integer column corresponding to overpaidOnDate; register this Migration
in the database builder (or autoMigrations) so existing installations can open
the upgraded schema without errors and run your DB tests to verify migration
success.
In
`@core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosDonutGraph.kt`:
- Around line 31-37: The MifosDonutGraph composable uses the progress parameter
directly when computing the arc sweep angle, which can produce invalid angles
for overpaid loans; clamp progress into the 0f..1f range before using it (e.g.,
call progress.coerceIn(0f, 1f)) and use that clamped value when computing
sweepAngle and any dependent values in the drawing logic inside MifosDonutGraph
so arcs are always between 0° and 360°.
In
`@core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosStackedBarChart.kt`:
- Line 91: ySteps and the subsequent per-segment height computation use
graphMaxY directly, which causes NaN/Infinity when graphMaxY is 0; update
MifosStackedBarChart to guard against zero/negative graphMaxY by using a safe
divisor or early-zero path: compute ySteps using a max(graphMaxY, 0f) and when
computing segment.value / graphMaxY (the block around segment.value / graphMaxY,
lines ~156-163) use a conditional (e.g., ratio = if (graphMaxY > 0f)
segment.value / graphMaxY else 0f) or substitute 1f as the divisor only for the
division while keeping the rendered height 0 when graphMaxY <= 0; apply the same
guard wherever graphMaxY is used for division so animated heights remain valid.
- Around line 277-281: formatGraphValue currently uses Float.toString() which
can produce scientific notation or precision artifacts; replace that with a
deterministic decimal formatter: convert the Float to a BigDecimal via
BigDecimal(value.toDouble()), call setScale(2, RoundingMode.HALF_UP) and use
toPlainString() to get a stable "12345.67" style string, then split that string
into integer/decimal parts and insert thousands separators into the integer part
(as the existing reversed().chunked(3)... logic does). Update the function
formatGraphValue to use this approach so labels always show a plain decimal with
two fractional digits and proper grouping.
In `@feature/loan/src/commonMain/composeResources/values/strings.xml`:
- Around line 391-392: There is a duplicate string resource named
feature_loan_loan_officer which conflicts with an existing resource; remove or
rename the duplicate entry (the string with name "feature_loan_loan_officer") in
strings.xml and, if you choose to rename it, update all dashboard/consumer code
references to the new resource key so callers reference the unique name; ensure
the final resources file only defines one "feature_loan_loan_officer" key (or a
single new key) and keep "feature_loan_loan_account_number" unchanged.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreen.kt`:
- Around line 762-775: The Spacer inside LoanDashboardScreen is incorrectly
reusing the external `modifier` (causing caller-supplied modifiers to leak into
the child); replace the reused `modifier.width(...)` with a fresh
`Modifier.width(...)` for the spacer so the layout between `MifosDonutGraph` and
the following `Column` is not affected by parent modifiers or test tags.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardViewModel.kt`:
- Around line 84-115: loadLoanDetails currently starts a new collector each call
causing multiple concurrent collectors; before calling
repository.getLoanById(...).onEach(...).launchIn(viewModelScope) cancel any
previous collector/job (e.g., keep a nullable Job property like loadLoanJob),
then assign the returned Job to it; alternatively replace manual job handling by
using a single upstream with flatMapLatest/switchMap to ensure only the latest
getLoanById is collected. Apply the same fix for the other instance mentioned
(lines 584-586).
- Around line 267-269: The current recent-transactions sorting uses
transactions.sortedByDescending { it.date.joinToString("") } which
lexicographically orders concatenated strings and misorders dates; replace this
with a proper date comparison in the sorting step (e.g., build a comparable date
object or compare numeric components) so sortedByDescending uses year/month/day
numerically. Locate the transactions collection and the sortedByDescending call
in LoanDashboardViewModel (the lambda referencing it.date) and change it to sort
by a numeric/temporal key (e.g., LocalDate.of(year, month, day) or
compareByDescending(year, month, day)) before calling take(3).
- Around line 118-151: loanDetails.currency.code and .decimalPlaces are nullable
and must be accessed safely before passing into CurrencyFormatter.format; update
the extraction of currencyCode and maxDigits in LoanDashboardViewModel (where
currencyCode and maxDigits are defined) to use safe navigation and reasonable
fallbacks (e.g., loanDetails.currency?.code ?: "<defaultCode>" and
loanDetails.currency?.decimalPlaces ?: <defaultDigits>) or mirror the fallback
logic from LoanAccountProfileScreen.kt, then ensure all calls to
CurrencyFormatter.format(...) (including heroValue formatting and other uses
like getNextRepaymentInfo, getRecentTransactions, getRepaymentProgressData) use
these safe values so Currency.getInstance is never invoked with null.
- Around line 103-112: The Elvis RHS currently returns a lambda instead of
executing it, so when state.data is null the empty-state update never runs;
update the DataState.Success branch so that when state.data is null you execute
the update (e.g., replace the lambda literal with an executed block such as
using run { ... } or directly call mutableStateFlow.update) to set viewState =
LoanDashboardState.ViewState.Empty; modify the branch around
populateLoanDashboardState, referencing DataState.Success, state.data,
populateLoanDashboardState(...), and mutableStateFlow.update to ensure the empty
state is applied.
---
Nitpick comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreen.kt`:
- Around line 359-368: The badge currently builds its label from status.name
(statusText) which hard-codes English; replace that logic with a mapping from
the domain enum (LoanStatus) to a string resource and use stringResource(...)
inside LoanDashboardScreen where Badge/Text are rendered. Add a function or
when-expression (e.g., loanStatusToStringRes(status: LoanStatus): Int) returning
R.string keys for each status, then replace the statusText usage with
stringResource(loanStatusToStringRes(status)) so the badge copy is localizable
and consistent with the rest of the screen.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 62b06779-cf31-4833-90d9-76ba10a2fb6c
📒 Files selected for processing (15)
core/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/loans/LoanWithAssociationsEntity.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosDonutGraph.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosStackedBarChart.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/icon/MifosIcons.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/DesignToken.ktfeature/loan/src/commonMain/composeResources/values/strings.xmlfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/di/LoanModule.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileNavigation.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreenRoute.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardState.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardViewModel.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/navigation/LoanNavigation.kt
Contributor
|
@sahilshivekar Please fix the merge conflicts. Also, is this graph interactive as in the web-app? 
|
sahilshivekar
force-pushed
the
feat/MIFOSAC-668-Loan-dashboard
branch
2 times, most recently
from
March 16, 2026 11:41
0fd5428 to
64934bb
Compare
Contributor
Author
@biplab1 Merge conflicts are resolved now. |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.kt (1)
264-267: Consider aligning status token naming.
loanClosedRescheduledis the only new status color without theStatussuffix, which makes the API slightly inconsistent.Optional naming alignment
- val loanClosedRescheduled = Color(0xFF000AAD) + val loanClosedRescheduledStatus = Color(0xFF000AAD)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.kt` around lines 264 - 267, The status color naming is inconsistent: update the identifier loanClosedRescheduled to follow the existing Status suffix convention (e.g., loanClosedRescheduledStatus) so it matches other status tokens; locate the Color declaration for loanClosedRescheduled in Color.kt and rename the symbol and all references/usages to the new name to keep the API consistent.core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosStackedBarChart.kt (2)
193-206: Consider avoiding force unwrap on mutable state.While
isTooltipActiveensuresactiveTooltipis non-null at this point, using!!on a mutablevaris a code smell since Kotlin cannot smart-cast it. Capturing the value or usingletwould be cleaner.♻️ Suggested refactor
- if (isTooltipActive && segment.value > 0f) { + val tooltip = activeTooltip + if (tooltip != null && + tooltip.barIndex == barIndex && + tooltip.segmentIndex == segmentIndex && + segment.value > 0f + ) { val density = LocalDensity.current val offsetPx = with(density) { DesignToken.spacing.negativeDp20.roundToPx() } Popup( alignment = Alignment.TopCenter, offset = IntOffset(0, offsetPx), properties = PopupProperties( dismissOnClickOutside = true, focusable = true, ), onDismissRequest = { activeTooltip = null }, ) { - ChartTooltip(info = activeTooltip!!) + ChartTooltip(info = tooltip) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosStackedBarChart.kt` around lines 193 - 206, The code force-unwraps the mutable var activeTooltip inside the Popup (ChartTooltip(info = activeTooltip!!)), which is a code smell; capture the current non-null tooltip into an immutable val (e.g., val tooltip = activeTooltip) after checking isTooltipActive, then use that captured tooltip inside the Popup/ChartTooltip, or use activeTooltip?.let { tooltip -> Popup { ChartTooltip(info = tooltip) } } to avoid the !! and ensure a stable non-null reference for ChartTooltip.
241-259: HardcodedColor.Whitemay not adapt to all themes.The tooltip uses hardcoded white text color. While this works with the current dark
AppColors.borderColorbackground, consider using a theme color likeKptTheme.colorScheme.onSurfaceor defining a dedicated tooltip content color for better theme consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosStackedBarChart.kt` around lines 241 - 259, The Text elements inside MifosStackedBarChart (the title Text and the segment Text using Color.White) are hardcoding white; update them to use a theme-aware color (e.g., KptTheme.colorScheme.onSurface or a dedicated tooltip content color) so the tooltip adapts to light/dark themes—replace Color.White in the two Text calls in this component with the chosen theme color reference and ensure the Box background still contrasts appropriately.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosDonutGraph.kt`:
- Around line 88-90: The tap handler in MifosDonutGraph only updates
activeTooltip when tappedSegment is non-null, so tapping on the empty
ring/center doesn't dismiss an existing tooltip; update the handler that
references tappedSegment and activeTooltip (the block currently using
tappedSegment?.let { segmentInfo -> ... }) to explicitly set activeTooltip =
null when tappedSegment is null and otherwise toggle as before (i.e., when
tappedSegment is non-null, set activeTooltip = if (activeTooltip == segmentInfo)
null else segmentInfo).
In
`@core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosStackedBarChart.kt`:
- Around line 280-286: The formatting of negative numbers in formatGraphValue is
broken because the thousands-separator logic runs on the signed whole value;
update formatGraphValue to handle the sign separately: determine sign = if
(value < 0) "-" else "", then operate on the absolute scaled/whole/fraction
values (use value.absoluteValue or whole.absoluteValue) when computing
integerPart (the reversed().chunked(3)... logic), and finally prepend the sign
to the returned string so negative numbers become e.g. "-123.45" instead of
"-,123.45".
In `@feature/loan/src/commonMain/composeResources/values/strings.xml`:
- Line 452: The string resource feature_loan_timeline_disbursed_date currently
reads "Amount Disbursed" but is used to display a date in
LoanDashboardViewModel; update the string value to a date-appropriate label such
as "Date Disbursed" (or "Disbursed Date") so the UI copy matches the data being
shown, and verify LoanDashboardViewModel still references
feature_loan_timeline_disbursed_date.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/navigation/LoanNavigation.kt`:
- Line 96: The current assignment forwards possibly-invalid loan IDs directly
via navigateToDashboard = navController::navigateToLoanDashboardScreen, which
lets a -1 ID reach the dashboard fetch; change this to a small guard wrapper
that checks the loanId (e.g., if loanId != -1) before invoking
navController.navigateToLoanDashboardScreen(loanId) and otherwise no-op (or
route to a safe fallback). Update the binding for navigateToDashboard to use
that lambda wrapper so invalid IDs are filtered out before calling
navigateToLoanDashboardScreen.
---
Nitpick comments:
In
`@core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosStackedBarChart.kt`:
- Around line 193-206: The code force-unwraps the mutable var activeTooltip
inside the Popup (ChartTooltip(info = activeTooltip!!)), which is a code smell;
capture the current non-null tooltip into an immutable val (e.g., val tooltip =
activeTooltip) after checking isTooltipActive, then use that captured tooltip
inside the Popup/ChartTooltip, or use activeTooltip?.let { tooltip -> Popup {
ChartTooltip(info = tooltip) } } to avoid the !! and ensure a stable non-null
reference for ChartTooltip.
- Around line 241-259: The Text elements inside MifosStackedBarChart (the title
Text and the segment Text using Color.White) are hardcoding white; update them
to use a theme-aware color (e.g., KptTheme.colorScheme.onSurface or a dedicated
tooltip content color) so the tooltip adapts to light/dark themes—replace
Color.White in the two Text calls in this component with the chosen theme color
reference and ensure the Box background still contrasts appropriately.
In
`@core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.kt`:
- Around line 264-267: The status color naming is inconsistent: update the
identifier loanClosedRescheduled to follow the existing Status suffix convention
(e.g., loanClosedRescheduledStatus) so it matches other status tokens; locate
the Color declaration for loanClosedRescheduled in Color.kt and rename the
symbol and all references/usages to the new name to keep the API consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 88829cf7-fd15-4467-8b3c-6f1dd62d619a
📒 Files selected for processing (15)
core/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/loans/LoanWithAssociationsEntity.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosDonutGraph.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosStackedBarChart.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/icon/MifosIcons.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/DesignToken.ktfeature/loan/src/commonMain/composeResources/values/strings.xmlfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/di/LoanModule.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileNavigation.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreenRoute.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardState.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardViewModel.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/navigation/LoanNavigation.kt
🚧 Files skipped from review as they are similar to previous changes (4)
- feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/di/LoanModule.kt
- feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileScreen.kt
- feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileNavigation.kt
- feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreen.kt
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/loan/src/commonMain/composeResources/values/strings.xml (1)
322-322:⚠️ Potential issue | 🔴 CriticalFix malformed XML - stray characters will break resource compilation.
Line 322 has extra
">characters at the end which will cause XML parsing failure:<string name="principle_paid_off">Principle Paid Off</string>">🐛 Proposed fix
- <string name="principle_paid_off">Principle Paid Off</string>"> + <string name="principle_paid_off">Principle Paid Off</string>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/loan/src/commonMain/composeResources/values/strings.xml` at line 322, Remove the stray trailing characters in the string resource named "principle_paid_off" so the element closes correctly (i.e., ensure it ends with </string> and not </string>">); edit the <string name="principle_paid_off">Principle Paid Off</string> entry to remove the extra '">' characters and save, then rebuild resources to verify compilation succeeds.
♻️ Duplicate comments (1)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/navigation/LoanNavigation.kt (1)
100-100:⚠️ Potential issue | 🟡 MinorGuard dashboard navigation against invalid loan IDs.
This still forwards any
loanIddirectly. If the profile flow emits its sentinel/default value before account data is ready, the dashboard route is pushed and the fetch fails immediately.🛡️ Small guard
- navigateToDashboard = navController::navigateToLoanDashboardScreen, + navigateToDashboard = { loanId -> + if (loanId > 0) { + navController.navigateToLoanDashboardScreen(loanId) + } + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/navigation/LoanNavigation.kt` at line 100, The navigation assignment currently forwards loan IDs unguarded via navController::navigateToLoanDashboardScreen; replace that direct method reference with a small lambda that checks the incoming loanId against the profile flow sentinel/default value and only calls navController.navigateToLoanDashboardScreen(loanId) when the id is valid (otherwise no-op or log/handle). Locate the navigateToDashboard assignment and change it to use a wrapper lambda that validates the loanId before delegating to navigateToLoanDashboardScreen to prevent pushing the dashboard route with an invalid id.
🧹 Nitpick comments (1)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreen.kt (1)
179-197: Extract the status visibility rules.These status lists are already duplicated and slightly different. Moving them into named sets/helpers will make the dashboard rules much easier to audit and keep in sync as statuses evolve.
Also applies to: 239-246
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreen.kt` around lines 179 - 197, Extract the duplicated status lists into well-named constants or predicate helpers and use them instead of repeating conditions: create e.g. val SHOW_NEXT_REPAYMENT = setOf(LoanStatus.ACTIVE, LoanStatus.WAITING_FOR_DISBURSAL) and val SHOW_OTHER_SECTIONS = setOf(LoanStatus.ACTIVE, LoanStatus.OVERPAID, LoanStatus.CLOSED_OBLIGATIONS_MET, LoanStatus.CLOSED_WRITTEN_OFF, LoanStatus.CLOSED_RESCHEDULED) or extension predicates like LoanStatus.isVisibleForNextRepayment() / LoanStatus.isVisibleForDetails(), then replace the inline checks around NextRepaymentCard and the other duplicated block to use membership (state.loanStatus in SHOW_...) or the predicate methods so the visibility rules are defined once and reused.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreen.kt`:
- Around line 359-362: The badge label currently builds val statusText from
status.name which hardcodes English; replace this with a mapping from the
LoanStatus enum to localized string resources (e.g. add a when(status:
LoanStatus) -> stringResource(R.string.loan_status_active) /
context.getString(...) for each enum case) and use that resource-backed label in
place of statusText; update any helper or composable that renders the badge to
call the resource mapper so all statuses use localization rather than derived
names.
---
Outside diff comments:
In `@feature/loan/src/commonMain/composeResources/values/strings.xml`:
- Line 322: Remove the stray trailing characters in the string resource named
"principle_paid_off" so the element closes correctly (i.e., ensure it ends with
</string> and not </string>">); edit the <string
name="principle_paid_off">Principle Paid Off</string> entry to remove the extra
'">' characters and save, then rebuild resources to verify compilation succeeds.
---
Duplicate comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/navigation/LoanNavigation.kt`:
- Line 100: The navigation assignment currently forwards loan IDs unguarded via
navController::navigateToLoanDashboardScreen; replace that direct method
reference with a small lambda that checks the incoming loanId against the
profile flow sentinel/default value and only calls
navController.navigateToLoanDashboardScreen(loanId) when the id is valid
(otherwise no-op or log/handle). Locate the navigateToDashboard assignment and
change it to use a wrapper lambda that validates the loanId before delegating to
navigateToLoanDashboardScreen to prevent pushing the dashboard route with an
invalid id.
---
Nitpick comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreen.kt`:
- Around line 179-197: Extract the duplicated status lists into well-named
constants or predicate helpers and use them instead of repeating conditions:
create e.g. val SHOW_NEXT_REPAYMENT = setOf(LoanStatus.ACTIVE,
LoanStatus.WAITING_FOR_DISBURSAL) and val SHOW_OTHER_SECTIONS =
setOf(LoanStatus.ACTIVE, LoanStatus.OVERPAID, LoanStatus.CLOSED_OBLIGATIONS_MET,
LoanStatus.CLOSED_WRITTEN_OFF, LoanStatus.CLOSED_RESCHEDULED) or extension
predicates like LoanStatus.isVisibleForNextRepayment() /
LoanStatus.isVisibleForDetails(), then replace the inline checks around
NextRepaymentCard and the other duplicated block to use membership
(state.loanStatus in SHOW_...) or the predicate methods so the visibility rules
are defined once and reused.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9f1dcd2a-d5f8-4a35-9188-e01a6aa0bce2
📒 Files selected for processing (15)
core/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/loans/LoanWithAssociationsEntity.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosDonutGraph.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosStackedBarChart.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/icon/MifosIcons.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/DesignToken.ktfeature/loan/src/commonMain/composeResources/values/strings.xmlfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/di/LoanModule.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileNavigation.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreenRoute.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardState.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardViewModel.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/navigation/LoanNavigation.kt
🚧 Files skipped from review as they are similar to previous changes (5)
- core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/DesignToken.kt
- feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardState.kt
- feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/di/LoanModule.kt
- feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileNavigation.kt
- core/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/loans/LoanWithAssociationsEntity.kt
sahilshivekar
force-pushed
the
feat/MIFOSAC-668-Loan-dashboard
branch
from
March 16, 2026 12:52
64934bb to
d72f7bd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
feature/loan/src/commonMain/composeResources/values/strings.xml (2)
349-349: Polish the section comment wording for maintainability.This comment is a bit hard to scan; consider standardizing it to a concise style (e.g.,
<!-- Apply new loan preview screen -->).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/loan/src/commonMain/composeResources/values/strings.xml` at line 349, The XML comment " <!-- Apply new loan Preview screen-->" is poorly capitalized and spaced; update the comment text to a concise, standardized form such as "<!-- Apply new loan preview screen -->" by replacing the existing comment in the strings.xml where the comment appears (search for the exact existing comment string) to improve readability and maintain consistent comment style.
438-440: Consider consistent title casing for adjacent dashboard labels.These values are sentence case while nearby labels are title case, which causes minor visual inconsistency.
Suggested copy tweak
- <string name="feature_loan_total_interest">Total interest</string> - <string name="feature_loan_remaining_balance">Remaining balance</string> + <string name="feature_loan_total_interest">Total Interest</string> + <string name="feature_loan_remaining_balance">Remaining Balance</string>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/loan/src/commonMain/composeResources/values/strings.xml` around lines 438 - 440, The two string resources feature_loan_total_interest and feature_loan_remaining_balance use sentence case and should be changed to title case to match adjacent labels; update their values to "Total Interest" and "Remaining Balance" respectively by editing the string entries for feature_loan_total_interest and feature_loan_remaining_balance so casing is consistent across the dashboard.core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosStackedBarChart.kt (1)
205-205: Consider avoiding force unwrap.While
activeTooltip!!is safe here (guarded byisTooltipActivecheck), consider usingactiveTooltip?.letfor consistency and to avoid the!!operator.♻️ Suggested change
- ChartTooltip(info = activeTooltip!!) + activeTooltip?.let { ChartTooltip(info = it) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosStackedBarChart.kt` at line 205, The code is force-unwrapping activeTooltip when rendering ChartTooltip (ChartTooltip(info = activeTooltip!!)); replace the force unwrap with a safe call and scoped use to avoid !!: use activeTooltip?.let { ChartTooltip(info = it) } and keep the existing isTooltipActive guard (or remove redundant guard) so ChartTooltip is only created when activeTooltip is non-null; update the call sites in MifosStackedBarChart where activeTooltip and isTooltipActive are used to follow this pattern.core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/icon/MifosIcons.kt (1)
58-58: Unused import.
Icons.Filled.TrendingUpis imported but not used—onlyIcons.AutoMirrored.Filled.TrendingUp(line 267) is referenced. Consider removing the unused import.♻️ Suggested removal
-import androidx.compose.material.icons.filled.TrendingUp🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/icon/MifosIcons.kt` at line 58, Remove the unused import "import androidx.compose.material.icons.filled.TrendingUp" from MifosIcons.kt; the code uses Icons.AutoMirrored.Filled.TrendingUp (not Icons.Filled.TrendingUp), so delete that import line to resolve the unused-import warning and keep only the necessary imports for MifosIcons.feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardViewModel.kt (1)
182-193: Use the defined constant instead of hardcoded string.Line 190 uses a hardcoded string
"loanStatusType.withdrawn.by.client"while the same value is already defined as a constantWITHDRAWN_BY_APPLICANT_CODEat line 79. Use the constant for consistency and maintainability.♻️ Suggested fix
- loanDetails.status.code == "loanStatusType.withdrawn.by.client" -> LoanStatus.WITHDRAWN_BY_APPLICANT + loanDetails.status.code == WITHDRAWN_BY_APPLICANT_CODE -> LoanStatus.WITHDRAWN_BY_APPLICANT🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardViewModel.kt` around lines 182 - 193, Replace the hardcoded withdrawn status string in getLoanStatus with the existing constant: update the comparison loanDetails.status.code == "loanStatusType.withdrawn.by.client" in the getLoanStatus function to use WITHDRAWN_BY_APPLICANT_CODE instead, ensuring you reference the constant defined earlier (WITHDRAWN_BY_APPLICANT_CODE) so the code is consistent and maintainable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/loans/LoanTimelineEntity.kt`:
- Around line 84-85: The schema change adding withdrawnOnDate to
LoanTimelineEntity requires bumping the Room database version and adding a
migration that alters the Timeline table to add the new column; update the
database version constant (where the RoomDatabase is built) and register a
Migration from the previous version that executes "ALTER TABLE Timeline ADD
COLUMN withdrawnOnDate ..." with the correct SQL type and nullability to match
LoanTimelineEntity (nullable List storage strategy), and include the related
change for overpaidOnDate in LoanWithAssociationsEntity within the same
migration so both columns are added atomically and existing databases can open
successfully.
---
Nitpick comments:
In
`@core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosStackedBarChart.kt`:
- Line 205: The code is force-unwrapping activeTooltip when rendering
ChartTooltip (ChartTooltip(info = activeTooltip!!)); replace the force unwrap
with a safe call and scoped use to avoid !!: use activeTooltip?.let {
ChartTooltip(info = it) } and keep the existing isTooltipActive guard (or remove
redundant guard) so ChartTooltip is only created when activeTooltip is non-null;
update the call sites in MifosStackedBarChart where activeTooltip and
isTooltipActive are used to follow this pattern.
In
`@core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/icon/MifosIcons.kt`:
- Line 58: Remove the unused import "import
androidx.compose.material.icons.filled.TrendingUp" from MifosIcons.kt; the code
uses Icons.AutoMirrored.Filled.TrendingUp (not Icons.Filled.TrendingUp), so
delete that import line to resolve the unused-import warning and keep only the
necessary imports for MifosIcons.
In `@feature/loan/src/commonMain/composeResources/values/strings.xml`:
- Line 349: The XML comment " <!-- Apply new loan Preview screen-->" is
poorly capitalized and spaced; update the comment text to a concise,
standardized form such as "<!-- Apply new loan preview screen -->" by replacing
the existing comment in the strings.xml where the comment appears (search for
the exact existing comment string) to improve readability and maintain
consistent comment style.
- Around line 438-440: The two string resources feature_loan_total_interest and
feature_loan_remaining_balance use sentence case and should be changed to title
case to match adjacent labels; update their values to "Total Interest" and
"Remaining Balance" respectively by editing the string entries for
feature_loan_total_interest and feature_loan_remaining_balance so casing is
consistent across the dashboard.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardViewModel.kt`:
- Around line 182-193: Replace the hardcoded withdrawn status string in
getLoanStatus with the existing constant: update the comparison
loanDetails.status.code == "loanStatusType.withdrawn.by.client" in the
getLoanStatus function to use WITHDRAWN_BY_APPLICANT_CODE instead, ensuring you
reference the constant defined earlier (WITHDRAWN_BY_APPLICANT_CODE) so the code
is consistent and maintainable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 99bddf2c-3c30-4807-a511-1106e8ca2f42
📒 Files selected for processing (16)
core/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/loans/LoanTimelineEntity.ktcore/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/loans/LoanWithAssociationsEntity.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosDonutGraph.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosStackedBarChart.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/icon/MifosIcons.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/DesignToken.ktfeature/loan/src/commonMain/composeResources/values/strings.xmlfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/di/LoanModule.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileNavigation.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreenRoute.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardState.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardViewModel.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/navigation/LoanNavigation.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileNavigation.kt
- feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/di/LoanModule.kt
Contributor
|
@sahilshivekar The typography, color scheme, icons (colors, sizes), and other UI components don't look aligned with the UI of the Client Flow. Please review. |
sahilshivekar
force-pushed
the
feat/MIFOSAC-668-Loan-dashboard
branch
2 times, most recently
from
March 17, 2026 19:06
c774784 to
6e66978
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreen.kt (1)
321-336: Forward the caller'smodifierinto these section roots.These composables expose a
modifier, but each one builds its root from a freshModifier. That drops caller padding, test tags, and semantics, which makes the API harder to reuse and test.♻️ Suggested direction
internal fun LoanHeroSummaryCard( productName: String, accountNumber: String, status: LoanStatus?, heroLabel: String, heroValue: String, modifier: Modifier = Modifier, ) { Column( - modifier = Modifier + modifier = modifier .fillMaxWidth() .border( BorderStroke(DesignToken.strokes.dpPoint5, KptTheme.colorScheme.outlineVariant), DesignToken.shapes.medium, ) .padding(DesignToken.padding.medium), ) {For
RepaymentProgressCard,LoanTimelineCard, andLoanRepaymentGraphSection, wrap the multiple top-level children in a containerColumnand applymodifierto that container.Also applies to: 795-815, 884-905, 1023-1102
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreen.kt` around lines 321 - 336, The root composables (e.g., LoanHeroSummaryCard) currently ignore the caller-supplied modifier by constructing their root with a fresh Modifier; change each to apply the passed-in modifier to the root container instead of starting with Modifier—e.g., in LoanHeroSummaryCard replace the root Column's modifier = Modifier... with modifier = modifier.fillMaxWidth()... so caller padding, semantics and test tags are preserved; for RepaymentProgressCard, LoanTimelineCard and LoanRepaymentGraphSection wrap their multiple top-level children in a single parent Column (or other suitable container) and apply the incoming modifier to that container, then move the existing border/padding/styling onto that container.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosStackedBarChart.kt`:
- Around line 109-115: The Y-axis labels in MifosStackedBarChart are losing
fractional precision because the Text uses
formatGraphValue(step).substringBefore('.') which truncates decimals; update the
label formatting in the ySteps.reversed().forEach block to preserve decimals for
small scales — e.g., call formatGraphValue(step) directly or conditionally keep
decimals when step < 1 (or when the fractional part is non-zero) so values like
0.25 and 0.50 render correctly; locate the Text that currently uses
substringBefore('.') and replace it with a conditional/format that preserves
meaningful fractional digits.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardViewModel.kt`:
- Around line 283-299: The recent-transactions logic currently leaves write-offs
neutral; update the isIncreasingDebt computation inside LoanDashboardViewModel
(where getTransactionTypeResource and isIncreasingDebt are used) to treat
transaction.type.writeOff == true as debt-reducing by returning false in the
when branches so write-off transactions get the repayment-style icon/color and
negative sign; locate the nullable transaction.type handling and add writeOff
checks alongside repayment/recovery/waive cases to return false.
- Around line 257-266: The next payment selection uses periods?.firstOrNull and
can pick the disbursement row; change the lookup to skip the
non-installment/disbursement entry like getPeriodsGraphValues() does — e.g. use
loanDetails.repaymentSchedule.periods?.drop(1)?.firstOrNull { it.complete ==
false } (or otherwise filter out the disbursement/installmentNumber==0 row) when
computing nextPeriod in LoanDashboardViewModel so the dueDate/amount reflect the
first actual installment.
---
Nitpick comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreen.kt`:
- Around line 321-336: The root composables (e.g., LoanHeroSummaryCard)
currently ignore the caller-supplied modifier by constructing their root with a
fresh Modifier; change each to apply the passed-in modifier to the root
container instead of starting with Modifier—e.g., in LoanHeroSummaryCard replace
the root Column's modifier = Modifier... with modifier =
modifier.fillMaxWidth()... so caller padding, semantics and test tags are
preserved; for RepaymentProgressCard, LoanTimelineCard and
LoanRepaymentGraphSection wrap their multiple top-level children in a single
parent Column (or other suitable container) and apply the incoming modifier to
that container, then move the existing border/padding/styling onto that
container.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2059b4a8-d4f6-46e1-8f8f-3ae035cd27f5
📒 Files selected for processing (16)
core/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/loans/LoanTimelineEntity.ktcore/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/loans/LoanWithAssociationsEntity.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosDonutGraph.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosStackedBarChart.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/icon/MifosIcons.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/DesignToken.ktfeature/loan/src/commonMain/composeResources/values/strings.xmlfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/di/LoanModule.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileNavigation.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardScreenRoute.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardState.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardViewModel.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/navigation/LoanNavigation.kt
🚧 Files skipped from review as they are similar to previous changes (9)
- core/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/loans/LoanTimelineEntity.kt
- core/database/src/commonMain/kotlin/com/mifos/room/entities/accounts/loans/LoanWithAssociationsEntity.kt
- core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/icon/MifosIcons.kt
- feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileNavigation.kt
- feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/di/LoanModule.kt
- core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.kt
- core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosDonutGraph.kt
- feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/navigation/LoanNavigation.kt
- core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/DesignToken.kt
Contributor
Author
@biplab1 I have made changes, give it a look when you get time |
biplab1
reviewed
Mar 24, 2026
biplab1
reviewed
Mar 24, 2026
biplab1
reviewed
Mar 24, 2026
Contributor
I was talking about this kind of interactivity shown in the screen recording from web-app where one can toggle the graph sections like in the bar graph: Dashboard.Active.Account.mp4 |
Contributor
Author
@biplab1 I wasn't aware of this kind of interactivity on the donut graph, in current implementation we can't toggle it. |
sahilshivekar
force-pushed
the
feat/MIFOSAC-668-Loan-dashboard
branch
from
April 3, 2026 13:16
8928e02 to
cc0a002
Compare
niyajali
requested changes
Apr 3, 2026
niyajali
left a comment
niyajali
left a comment
Collaborator
There was a problem hiding this comment.
same issues are present in many files consider checking all and update accordingly
sahilshivekar
force-pushed
the
feat/MIFOSAC-668-Loan-dashboard
branch
from
April 3, 2026 19:07
cc0a002 to
45e5d11
Compare
biplab1
reviewed
Apr 10, 2026
biplab1
changed the title
sahilshivekar
force-pushed
the
feat/MIFOSAC-668-Loan-dashboard
branch
from
April 19, 2026 06:39
d47197f to
855484d
Compare
sahilshivekar
force-pushed
the
feat/MIFOSAC-668-Loan-dashboard
branch
3 times, most recently
from
April 19, 2026 07:13
1a88d2a to
fbfd69a
Compare
Contributor
Author
|
@coderabbitai config |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainThe PR description images show three screenshots of the Mifos loan dashboard implementation:
🛠 Debug Information Gathered
📋 Repository:
|
| Setting | Value |
|---|---|
| Profile | chill |
| Auto Review | ✅ Enabled |
| Incremental Review | ✅ Enabled |
| Auto Pause After | 5 reviewed commits |
| Request Changes Workflow | ❌ Disabled |
| Draft PRs | ❌ Skipped |
| High-level Summary | ✅ Enabled |
| Sequence Diagrams | ✅ Enabled |
| Path Filters | (none) |
| Path Instructions | (none) |
| Ignore Title Keywords | (none) |
🛠️ Tools (all enabled by default)
Detekt, AST-grep, LanguageTool, GitHub Checks, Gitleaks, TruffleHog, actionlint, markdownlint, yamllint, and many more.
🧠 Knowledge Base
| Setting | Value |
|---|---|
| Learnings Scope | auto (local for public repos) |
| Web Search | ✅ Enabled |
| Code Guidelines | ✅ Enabled |
| Jira / Linear | auto (disabled for public repos) |
✨ Finishing Touches
| Feature | Status |
|---|---|
| Docstrings generation | ✅ Enabled |
| Unit test generation | ✅ Enabled |
| Simplify | ❌ Disabled |
If you'd like to customize the configuration (e.g., add path-specific review instructions, filter out certain files, change the review profile to assertive, or add custom pre-merge checks), you can create a .coderabbit.yaml file at the root of the repository. See the CodeRabbit configuration docs for more details.
🧠 Learnings used
Learnt from: sahilshivekar
Repo: openMF/mifos-x-field-officer-app PR: 2647
File: core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosDonutGraph.kt:31-37
Timestamp: 2026-03-16T11:08:58.622Z
Learning: In PR `#2647` (openMF/mifos-x-field-officer-app), the `MifosDonutGraph` composable in `core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosDonutGraph.kt` intentionally receives a `progress` value already capped at `1.0f` (100%) for overpaid loans. The ViewModel/state layer handles capping; for overpaid status the surplus balance is displayed separately on the right side of the donut graph rather than rendering a >360° arc. The composable itself does not clamp the value, as the caller guarantees it is in [0, 1].
Learnt from: biplab1
Repo: openMF/mifos-x-field-officer-app PR: 2647
File: feature/loan/src/commonMain/composeResources/values/strings.xml:460-460
Timestamp: 2026-03-24T00:43:11.613Z
Learning: In PR `#2647` (openMF/mifos-x-field-officer-app), the timeline string resource keys in `feature/loan/src/commonMain/composeResources/values/strings.xml` should follow the `_on_date` suffix naming pattern for date-type events (e.g., `feature_loan_timeline_amount_disbursed_on_date`, `feature_loan_timeline_application_submitted_on_date`, `feature_loan_timeline_application_approved_on_date`, `feature_loan_timeline_expected_disbursement_on_date`, `feature_loan_timeline_expected_maturity_on_date`, `feature_loan_timeline_maturity_on_date`). This was a naming convention decision by maintainer biplab1 to make date-carrying keys clearly distinguishable from state-only keys.
Learnt from: sahilshivekar
Repo: openMF/mifos-x-field-officer-app PR: 2647
File: feature/loan/src/commonMain/composeResources/values/strings.xml:452-452
Timestamp: 2026-03-16T11:54:08.124Z
Learning: In PR `#2647` (openMF/mifos-x-field-officer-app), the string resource `feature_loan_timeline_disbursed_date` in `feature/loan/src/commonMain/composeResources/values/strings.xml` intentionally uses the value "Amount Disbursed" as a timeline step label. It conveys that the loan amount was disbursed on the date shown below it in the timeline, not just a bare date field label. This is a deliberate UX copy choice.
Learnt from: sahilshivekar
Repo: openMF/mifos-x-field-officer-app PR: 2647
File: feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/navigation/LoanNavigation.kt:96-96
Timestamp: 2026-03-16T11:58:36.951Z
Learning: In `feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/navigation/LoanNavigation.kt` (Kotlin/Compose Multiplatform, mifos-x-field-officer-app), `navigateToDashboard` uses a direct method reference `navController::navigateToLoanDashboardScreen` without an ID guard. This is intentional: `LoanAccountProfileScreen` always receives a valid `loanId`, and `LoanDashboardScreen` already handles invalid/missing data by displaying a "Failed to fetch Loan details" error state. Do not flag the absence of an ID guard here in future reviews.
Learnt from: sahilshivekar
Repo: openMF/mifos-x-field-officer-app PR: 2647
File: feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardViewModel.kt:283-299
Timestamp: 2026-03-17T19:33:37.858Z
Learning: In `feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDashboard/LoanDashboardViewModel.kt` (Kotlin/Compose Multiplatform, mifos-x-field-officer-app), the `isIncreasingDebt` flag in `getRecentTransactions` intentionally leaves `type.writeOff == true` in the neutral branch (`null`). Write-off transactions are neither debt-increasing nor debt-reducing from the UI perspective (no cash flow occurs), so they are displayed with a neutral icon/color. Do not flag `writeOff` being absent from the debt-reducing branch as an issue.
Learnt from: kartikey004
Repo: openMF/mifos-x-field-officer-app PR: 2651
File: core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/LoanReschedulesRepositoryImpl.kt:58-61
Timestamp: 2026-03-15T13:01:51.059Z
Learning: In the Loan Reschedules feature (PR `#2651`, openMF/mifos-x-field-officer-app), the `deleteReschedule` method in `LoanReschedulesRepositoryImpl` intentionally delegates to `dataManagerLoan.rejectLoanReschedule()`, which calls the backend `POST ...?command=reject`. The "delete" UI action is designed to reject the reschedule request (marking it as REJECTED), not to physically remove it from the database. This is the intended product behavior confirmed by the author.
|
Contributor
|
@sahilshivekar Please fix the conflicts and pr checks failures. |
sahilshivekar
force-pushed
the
feat/MIFOSAC-668-Loan-dashboard
branch
from
June 25, 2026 09:23
fbfd69a to
0470e66
Compare
revanthkumarJ
approved these changes
Jun 29, 2026
niyajali
approved these changes
Jun 30, 2026
niyajali
left a comment
niyajali
left a comment
Collaborator
There was a problem hiding this comment.
@sahilshivekar This is great work!
|
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
4 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.
Fixes - Jira-#668
Before
before_mifosac_668.mp4
After
As the content displayed on screen changes based on loan status, separate screen recording is added for each loan status
Active
mifosac_668_active.mp4
Overpaid
mifosac_668_overpaid.mp4
Waiting for disbursal
mifosac_668_waiting_for_disbursal.mp4
Pending approval
Rejected
Withdrawn by applicant
Closed obligations met
mifosac_668_closed_obligations_met.mp4
For following statuses no loan account found in the app, previews was failing with rendering issues
Closed written off
Closed rescheduled
Summary by CodeRabbit
New Features
UI / Visual
Theme
Localization
Navigation
Chores