refactor(loan): centralize loan status mapping across screens - #2697
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.kt⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (2)📚 Learning: 2026-02-06T13:15:16.968ZApplied to files:
📚 Learning: 2026-04-01T05:03:14.323ZApplied to files:
🔇 Additional comments (2)
Summary by CodeRabbit
WalkthroughLoan status handling now uses a shared ChangesLoan status normalization Loan status handling is centralized across the loan feature. Review checkpoint tableThe send-money icon change is separate. Send money drawable
Sequence Diagram(s)The shared status mapper now feeds profile, summary, and client loan account rendering. Loan status flowsequenceDiagram
participant LoanStatusEntity
participant LoanAccountProfileViewModel
participant LoanAccountProfileScreen
participant LoanAccountSummaryScreen
participant ClientLoanAccountsScreen
LoanStatusEntity->>LoanAccountProfileViewModel: loan.status.getLoanStatus()
LoanAccountProfileViewModel->>LoanAccountProfileScreen: currentStatus and nextActionButtonRes
LoanStatusEntity->>LoanAccountSummaryScreen: loanWithAssociations.status.getLoanStatus()
LoanStatusEntity->>ClientLoanAccountsScreen: loan.status.getLoanStatus().label
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ 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 |
…use in xml file
sahilshivekar
marked this pull request as ready for review
June 26, 2026 13:38
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.kt (1)
259-267: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueShared foundational module touched + naming inconsistency on the rescheduled token.
Heads up:
Color.ktlives in thecoredesign-system module, so these token additions are high-impact and ripple into every consumer — worth an extra careful pass to confirm the new tokens are minimal and necessary.Separately,
loanClosedRescheduledbreaks theloan*Statusnaming pattern used by its siblings (loanClosedOverpaidStatus,loanClosedObligationsMetStatus,loanClosedWrittenOffStatus). Rename for consistency.♻️ Suggested rename
- val loanClosedRescheduled = Color(0xFF000AAD) + val loanClosedRescheduledStatus = Color(0xFF000AAD)Update the reference in
LoanStatus.ktaccordingly (AppColors.loanClosedRescheduled→AppColors.loanClosedRescheduledStatus).As per path instructions: "core-base is a shared foundational module and requires extra review attention" (applies to
core/**here), and Kotlin naming should "Ensure consistency".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.kt` around lines 259 - 267, The new color token in AppColors/Color.kt should follow the existing loan*Status naming pattern, so rename loanClosedRescheduled to loanClosedRescheduledStatus for consistency with the other loan status tokens. Update any consumer references, especially in LoanStatus.kt, to use the new AppColors.loanClosedRescheduledStatus name, and keep the shared core design-system additions minimal and necessary.Source: Path instructions
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileScreen.kt (1)
289-304: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueCompute the status once instead of mapping twice.
loanAccount.status.getLoanStatus()is evaluated twice here (color on Line 292, label on Line 300), recomputed on each recomposition. Hoist it to a singleval.♻️ Suggested change
+ val loanStatus = loanAccount.status.getLoanStatus() Box( modifier = Modifier .clip(KptTheme.shapes.large) - .background(loanAccount.status.getLoanStatus().color) + .background(loanStatus.color) .padding( horizontal = DesignToken.padding.medium, vertical = KptTheme.spacing.xs, ), contentAlignment = Alignment.Center, ) { Text( - text = stringResource(loanAccount.status.getLoanStatus().label).uppercase(), + text = stringResource(loanStatus.label).uppercase(),As per path instructions: "Prefer lifting state up instead of recomputing in child composables".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileScreen.kt` around lines 289 - 304, The loan status is being mapped twice in the status badge block inside LoanAccountProfileScreen, once for the color and once for the label, which recomputes it on each recomposition. Hoist loanAccount.status.getLoanStatus() into a single local val in that composable section and reuse it for both the background color and the stringResource label lookup to keep the status mapping computed once.Source: Path instructions
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileViewModel.kt (1)
108-142: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTwo parallel status→action maps can silently diverge.
calculateNextActionResource(button label) andhandleNextAction(navigation target) both switch over the sameLoanStatus, but as separatewhenblocks. If one is updated without the other, the button could display "Make Repayment" while navigating elsewhere. Consider a single source of truth mappingLoanStatus→ (label resource, action) to keep them in lockstep.Not blocking, but it's a real maintenance hazard given the two lists already enumerate the same three cases.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileViewModel.kt` around lines 108 - 142, The button-label mapping in calculateNextActionResource and the navigation mapping in handleNextAction can drift because they duplicate the same LoanStatus cases in separate when blocks. Refactor LoanAccountProfileViewModel to use a single source of truth for status-to-next-action behavior, so both the StringResource returned for the CTA and the LoanProfileAction sent by NavigateToAction come from the same mapping. Keep the existing symbols calculateNextActionResource, handleNextAction, and LoanStatus as the main lookup points while consolidating the three shared cases and the fallback branch.feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountSummary/LoanAccountSummaryScreen.kt (1)
273-283: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueReuse a single
getLoanStatus()result for label and color.
getLoanStatus()runs on Line 273 (label) and again on Line 282 (circle color) inside the composable. Hoist it to onevalbefore theCanvas.♻️ Suggested change
- val statusDescription = stringResource(loanWithAssociations.status.getLoanStatus().label) + val loanStatus = loanWithAssociations.status.getLoanStatus() + val statusDescription = stringResource(loanStatus.label) Canvas( modifier = Modifier .size(DesignToken.sizes.iconMedium) .semantics { contentDescription = "Loan status: $statusDescription" }, onDraw = { drawCircle( - color = loanWithAssociations.status.getLoanStatus().color, + color = loanStatus.color, ) }, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountSummary/LoanAccountSummaryScreen.kt` around lines 273 - 283, Reuse a single LoanStatus value in LoanAccountSummaryScreen: the current code calls loanWithAssociations.status.getLoanStatus() twice for the label and circle color inside the composable. Hoist the result into one val before the Canvas, then use that same value for both stringResource(...label) and drawCircle(...color) to avoid duplicate computation.feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientLoanAccounts/ClientLoanAccountsScreen.kt (1)
78-78: 📐 Maintainability & Code Quality | 🔵 Trivial
feature:clientdirectly depends onfeature:loanto accessgetLoanStatusThe Gradle configuration in
feature/client/build.gradle.ktsexplicitly includesimplementation(projects.feature.loan), making the import valid.However, coupling feature modules together is generally an anti-pattern. Shared utilities like getLoanStatus should reside in a common core module to prevent circular or meshed dependencies between features.
Dependency Analysis
feature/clientcurrently depends on multiple feature modules:
feature.savingsfeature.loan(used for getLoanStatus)feature.documentfeature.recurringDepositfeature.groupsfeature.dataTablefeature.noteThis mesh of inter-feature dependencies simplifies refactoring if shared logic moves to a central
coremodule.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientLoanAccounts/ClientLoanAccountsScreen.kt` at line 78, Move the getLoanStatus dependency out of feature:client and into a shared core/common module so ClientLoanAccountsScreen no longer imports from feature:loan. Update the LoanStatus utility location and expose it from a neutral shared module, then change the import in ClientLoanAccountsScreen to the new shared symbol. Keep the feature module boundaries clean by avoiding direct feature-to-feature references while preserving the same status mapping behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/utils/LoanStatus.kt`:
- Around line 69-81: The fallback in LoanStatusEntity.getLoanStatus() is
incorrectly mapping every unrecognized state to LoanStatus.REJECTED. Update the
status mapping to use a neutral UNKNOWN outcome for unmatched/unsupported flags,
and only return REJECTED when LoanStatusEntity exposes an explicit rejected
indicator. If the enum currently lacks UNKNOWN, add it and wire it to the
existing loanUnknownStatus color/token so LoanStatusEntity.getLoanStatus() and
the related UI stay aligned.
---
Nitpick comments:
In
`@core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.kt`:
- Around line 259-267: The new color token in AppColors/Color.kt should follow
the existing loan*Status naming pattern, so rename loanClosedRescheduled to
loanClosedRescheduledStatus for consistency with the other loan status tokens.
Update any consumer references, especially in LoanStatus.kt, to use the new
AppColors.loanClosedRescheduledStatus name, and keep the shared core
design-system additions minimal and necessary.
In
`@feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientLoanAccounts/ClientLoanAccountsScreen.kt`:
- Line 78: Move the getLoanStatus dependency out of feature:client and into a
shared core/common module so ClientLoanAccountsScreen no longer imports from
feature:loan. Update the LoanStatus utility location and expose it from a
neutral shared module, then change the import in ClientLoanAccountsScreen to the
new shared symbol. Keep the feature module boundaries clean by avoiding direct
feature-to-feature references while preserving the same status mapping behavior.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileScreen.kt`:
- Around line 289-304: The loan status is being mapped twice in the status badge
block inside LoanAccountProfileScreen, once for the color and once for the
label, which recomputes it on each recomposition. Hoist
loanAccount.status.getLoanStatus() into a single local val in that composable
section and reuse it for both the background color and the stringResource label
lookup to keep the status mapping computed once.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileViewModel.kt`:
- Around line 108-142: The button-label mapping in calculateNextActionResource
and the navigation mapping in handleNextAction can drift because they duplicate
the same LoanStatus cases in separate when blocks. Refactor
LoanAccountProfileViewModel to use a single source of truth for
status-to-next-action behavior, so both the StringResource returned for the CTA
and the LoanProfileAction sent by NavigateToAction come from the same mapping.
Keep the existing symbols calculateNextActionResource, handleNextAction, and
LoanStatus as the main lookup points while consolidating the three shared cases
and the fallback branch.
In
`@feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountSummary/LoanAccountSummaryScreen.kt`:
- Around line 273-283: Reuse a single LoanStatus value in
LoanAccountSummaryScreen: the current code calls
loanWithAssociations.status.getLoanStatus() twice for the label and circle color
inside the composable. Hoist the result into one val before the Canvas, then use
that same value for both stringResource(...label) and drawCircle(...color) to
avoid duplicate computation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 91fa9885-17e0-4ad8-b7cd-907ae0e9db79
📒 Files selected for processing (8)
core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.ktcore/ui/src/commonMain/composeResources/drawable/send_money.xmlfeature/client/src/commonMain/kotlin/com/mifos/feature/client/clientLoanAccounts/ClientLoanAccountsScreen.ktfeature/loan/src/commonMain/composeResources/values/strings.xmlfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileViewModel.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountSummary/LoanAccountSummaryScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/utils/LoanStatus.kt
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: PR Checks / Build Web Application
🧰 Additional context used
📓 Path-based instructions (5)
**/*.kt
⚙️ CodeRabbit configuration file
**/*.kt: Additional Code Review Guidelines:
- Null Safety & Stability
- Avoid using
!!operator- Handle null cases explicitly using safe calls or proper state handling
- Do not assume values are always non-null without guarantees
- Architecture Boundaries
- ViewModel must not depend on specific network/library implementations
- Ensure proper separation between data, domain, and presentation layers
- Do not format data (currency, dates, calculations) inside the UI layer
- All formatting must be handled in the ViewModel and exposed via state (e.g., StateFlow)
- Performance Considerations
- Avoid unnecessary recompositions in Compose
- Do not attach heavy logic to frequently changing states (e.g., scrollState)
- Prefer lifting state up instead of recomputing in child composables
- Compose & Navigation Best Practices
- NEVER trigger navigation functions or side-effects directly during composition
- Always wrap navigation calls inside
LaunchedEffectorEventsEffectto avoid repeated execution on recomposition- Avoid triggering intensive side-effects during recomposition
- Navigation routes must be type-safe.
- Ensure all route classes or objects used for navigation are annotated with
@Serializable.
- UI Structure
- Dialogs must be separated into their own composables
- Do not embed dialogs inline within complex main screens
- Localization Consistency
- Ensure all supported languages are updated consistently across modules
- Verify translations exist for newly added UI strings
- Code Cleanliness
- Avoid unnecessary inline comments unless critical
- Remove leftover debug or commented code
- Focus on correctness, readability, and maintainability over cosmetic nitpicks.
- Avoid reviewing README, config, or asset files.
- Prioritize identifying bugs, performance issues, and architectural concerns.
- Naming & Intent Rules:
- Follow the official Kotlin Coding Conventions:
https://kotlinlang.org/docs/coding-conventions.html- Use self-explanatory ...
Files:
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/utils/LoanStatus.ktfeature/client/src/commonMain/kotlin/com/mifos/feature/client/clientLoanAccounts/ClientLoanAccountsScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountSummary/LoanAccountSummaryScreen.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileViewModel.kt
**/*Screen.kt
⚙️ CodeRabbit configuration file
**/*Screen.kt: Screen architecture rules:Each screen must follow a 2-layer structure:
Layer 1 (Entry/Route Composable):
- Function Name: Typically
*ScreenRouteor the entry-point composable.- Responsibilities: Inject ViewModel (e.g.,
koinViewModel), take navigation lambdas, collect theStateFlow, and handle ViewModel events.- Logic: Should only handle ViewModel interaction, state collection, and triggering navigation in response to ViewModel events.
Layer 2 (Stateful/Content Composable —
*Screenor*ScreenContent):
- Parameters: MUST only take
state(the UI state object) and a singleonActionlambda (e.g.,onAction: (FeatureAction) -> Unit).- Responsibilities: Render the UI based strictly on the provided
state.- Rule: MUST NOT pass multiple separate lambda functions for different UI interactions; consolidate them into the single
onAction.- Constraint: Must NOT contain any business logic or ViewModel/Navigation references.
Internal/private helper composables (e.g., dialogs, sections, sub-components):
- These are NOT subject to the single
onActionrule.- They may accept specific, focused lambdas (e.g.,
onRetry: () -> Unit) or a narrowedonActionas appropriate.- However, they must NOT be passed the ViewModel or navigation controllers directly.
UI consistency:
- Avoid hardcoded values (dp, sp, padding, fontSize, colors)
- Use DesignToken, KptTheme, AppColors, MifosTypography for spacing, typography, and colors
Code quality:
- Keep Composables small and readable
- Avoid deeply nested UI
Flag:
- Missing Layer 2 (
*Screen/*ScreenContent) separation from the entry-point composable- Layer 2 (
*Screen/*ScreenContent) receiving multiple separate lambdas instead of a singleonAction- UI logic inside the entry-point composable
- Business logic inside any Composable
- Hardcoded strings instead of using string resources
- Hardcoded dp/sp values
- Direct styling instead of using DesignToken or KptTheme
Files:
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientLoanAccounts/ClientLoanAccountsScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountSummary/LoanAccountSummaryScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileScreen.kt
**/{core-base,core}/**/*.kt
⚙️ CodeRabbit configuration file
**/{core-base,core}/**/*.kt: Critical Module Change Detection:
- Changes in
core-basemodule must be treated as high-impact.- Flag any PR that modifies files inside
core-basefor careful review.- Verify that modifications in
core-baseare necessary and minimal.
Output:- Clearly highlight that
core-baseis a shared foundational module and requires extra review attention.
Files:
core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.kt
**/composeResources/values*/strings.xml
⚙️ CodeRabbit configuration file
**/composeResources/values*/strings.xml: String resource conventions:Naming:
- All keys must follow:
feature_{feature_name}_{ui_text_in_snake_case}Examples:
"In Advance" → feature_loan_in_advance
"Outstanding" → feature_loan_outstanding
- Avoid generic names like: title, text1, label
- The suffix should be a short, readable representation of the UI text
- Avoid multiple keys representing the same UI text
- Keys must be lowercase and use snake_case
Flag:
- Incorrect naming pattern
- Generic or unclear key names
- Duplicate keys for same UI text
Files:
feature/loan/src/commonMain/composeResources/values/strings.xml
**/*ViewModel.kt
⚙️ CodeRabbit configuration file
**/*ViewModel.kt: MVI architecture rules:
- All new features must follow MVI
- ViewModel must extend BaseViewModel
- The ViewModel MUST maintain a single UI state (e.g., a single StateFlow)
instead of multiple separate state variables.Required:
- Use *State, *Event, *Action
- Naming must be consistent (FeatureViewModel, FeatureState, FeatureEvent, FeatureAction)
- Follow unidirectional flow:
Action → ViewModel → State → UIInternal reducer/action architecture rules:
Async operations MUST NOT directly mutate UI state repeatedly inside
Flow collectors, suspend callbacks, or repository result handlers.Repository/network/database results MUST be converted into internal
actions usingsendAction(...).
handleAction(...)must act as the primary reducer responsible for:
- state mutation
- reducer-style state transitions
- triggering follow-up actions
Large async methods must be split into:
- async collection layer
- internal action dispatching
- reducer/state handling
Avoid directly calling another business/data-loading method from
repository collectors or async callbacks.
Prefer dispatching follow-up internal actions instead.Preferred pattern:
repository result
-> sendAction(...)
-> handleAction(...)
-> mutableStateFlow.update { ... }Anti-pattern examples:
mutableStateFlow.update { ... } inside collect { }
fetchX() -> directly calls fetchY() inside async collector
large methods mixing:
- collection
- state mutation
- navigation
- business chainingFlag:
- Multiple
mutableStateFlow.update {}calls inside collect { }- Direct state mutation inside async repository callbacks
- Async methods performing both collection and reducer logic
- Direct business-flow chaining from async collectors
- Missing internal reducer actions for async results
- Missing *State / *Event / *Action
- ViewModel not...
Files:
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileViewModel.kt
🧠 Learnings (4)
📚 Learning: 2026-02-06T13:15:16.968Z
Learnt from: sahilshivekar
Repo: openMF/android-client PR: 2603
File: feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanTransaction/LoanTransactionsViewModel.kt:43-106
Timestamp: 2026-02-06T13:15:16.968Z
Learning: Guideline: When a Kotlin function parameter is nullable (e.g., balance: Double?, currencyCode: String?, maximumFractionDigits: Int?) and downstream calls require a non-null value, add null-safety handling in all implementations. Specifically, avoid calling Currency.getInstance(currencyCode) with a possibly null currencyCode; provide a safe default (e.g., currencyCode ?: "$") or validate before use. Ensure all platform targets (Android/Desktop/Native) follow consistent null handling, and consider centralizing currencyCode normalization in the common layer if feasible. Add tests covering null currencyCode to prevent NPEs.
Applied to files:
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/utils/LoanStatus.ktfeature/client/src/commonMain/kotlin/com/mifos/feature/client/clientLoanAccounts/ClientLoanAccountsScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountSummary/LoanAccountSummaryScreen.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileViewModel.kt
📚 Learning: 2026-04-01T05:03:14.323Z
Learnt from: kartikey004
Repo: openMF/mifos-x-field-officer-app PR: 2659
File: feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanDisbursement/LoanAccountDisbursementScreen.kt:190-195
Timestamp: 2026-04-01T05:03:14.323Z
Learning: In this repo, existing `SelectableDates` / `SelectableDates.isSelectableDate(utcTimeMillis: Long)` implementations use `Clock.System.now().toEpochMilliseconds()` (UTC epoch millis) for date-boundary checks. During PR reviews, do not flag these checks for not using a timezone-aware `LocalDate` start-of-day approach; treat it as an established project-wide pattern. If a change is desired, handle it as a coordinated project-wide improvement rather than as a per-PR review issue.
Applied to files:
feature/loan/src/commonMain/kotlin/com/mifos/feature/loan/utils/LoanStatus.ktfeature/client/src/commonMain/kotlin/com/mifos/feature/client/clientLoanAccounts/ClientLoanAccountsScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountSummary/LoanAccountSummaryScreen.ktcore/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileScreen.ktfeature/loan/src/commonMain/kotlin/com/mifos/feature/loan/loanAccountProfile/LoanAccountProfileViewModel.kt
📚 Learning: 2026-02-16T08:37:28.351Z
Learnt from: kartikey004
Repo: openMF/android-client PR: 2610
File: feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientIdentifiersList/ClientIdentifiersListScreen.kt:60-60
Timestamp: 2026-02-16T08:37:28.351Z
Learning: In Kotlin files under the android-client module, replace MaterialTheme references with the established KptTheme import: import template.core.base.designsystem.theme.KptTheme. This should be applied consistently across files where KptTheme is used, replacing any MaterialTheme imports with the correct KptTheme import path.
Applied to files:
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientLoanAccounts/ClientLoanAccountsScreen.kt
📚 Learning: 2026-02-16T08:26:18.398Z
Learnt from: kartikey004
Repo: openMF/android-client PR: 2610
File: core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/DesignToken.kt:328-337
Timestamp: 2026-02-16T08:26:18.398Z
Learning: In core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/DesignToken.kt, ensure size tokens in AppSizes use raw dp-based names like dp5, dp18, dp20, dp42, dp48, dp72, dp100, dp120, dp128, etc., rather than relying solely on semantic names. This naming convention should be accepted for design token definitions in this file and should guide future token definitions without requiring semantic aliases.
Applied to files:
core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/theme/Color.kt
🔇 Additional comments (3)
core/ui/src/commonMain/composeResources/drawable/send_money.xml (1)
19-19: LGTM!feature/loan/src/commonMain/composeResources/values/strings.xml (1)
476-484: LGTM!feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientLoanAccounts/ClientLoanAccountsScreen.kt (1)
256-256: LGTM!
…us type explicitly
|
niyajali
requested changes
Jun 26, 2026
Contributor
|
LGTM |
Nagarjuna0033
approved these changes
Jun 29, 2026
revanthkumarJ
approved these changes
Jun 29, 2026
niyajali
approved these changes
Jun 30, 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
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-#783
Client Loan Accounts Screen
Loan Account Profile Screen
Background color for the loan status is same as it's in the web app now
Pending Approval
Approved
Active
Closed (Overpaid)
Closed (Obligations Met)
Rejected
Loan accounts with following loan status not found
Closed (Written Off)
Closed (Rescheduled)
Withdrawn by Applicant