feat : collateral data screen added #2546
Conversation
|
Warning Rate limit exceeded@revanthkumarJ has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 46 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis PR introduces a new client collateral details feature by adding an API endpoint to fetch collateral items, implementing repository and data manager methods, creating a collateral details screen with navigation and viewmodel, and updating UI components. The feature integrates across multiple layers from network to UI. Changes
Sequence DiagramsequenceDiagram
participant Nav as NavController
participant Route as ClientCollateralDetailRoute
participant Screen as ClientCollateralDetailScreen
participant VM as ClientCollateralDetailViewmodel
participant Repo as ClientDetailsRepository
participant API as ClientService
participant UI as UI State Machine
Nav->>Route: navigateToClientCollateralDetailRoute(clientId)
Route->>Screen: Render with clientId
Screen->>VM: Collect state from Koin
VM->>VM: Init: check NetworkMonitor
alt Network Available
VM->>Repo: getClientCollaterals(clientId)
Repo->>API: clientService.getClientCollateralItems(clientId)
API-->>Repo: List<CollateralItemResult>
Repo-->>VM: DataState.Success(data)
else Network Unavailable
VM->>VM: Emit Error state
end
VM->>UI: Update state (Loading→Success/Error/Empty)
UI->>Screen: Render based on state
rect rgb(200, 220, 240)
Note over Screen: Empty → MifosEmptyCard<br/>Loading → MifosProgressIndicator<br/>Error → MifosErrorComponent<br/>Success → LazyColumn with items
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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: 4
🧹 Nitpick comments (4)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientCollateralDetails/ClientCollateralDetailScreen.kt (3)
61-73: Consider wiring scaffold back-press to navigation
onBackPressed = { }makes the scaffold’s back action a no-op. SincenavControlleris already available and the breadcrumb bar provides back navigation, you may want to keep behavior consistent by delegating back-press to the nav controller as well.For example:
- MifosScaffold( - title = "", - onBackPressed = { }, - modifier = modifier, - ) { paddingValues -> + MifosScaffold( + title = "", + onBackPressed = { navController.popBackStack() }, + modifier = modifier, + ) { paddingValues ->
121-140: Improve item-count text for localization/pluralization
totalItem + " " + stringResource(Res.string.client_savings_item)hardcodes string concatenation and may not localize well for different languages (plural forms, word order).If possible, prefer a dedicated plural/string resource with format args (e.g.,
"Collateral items: %d") and usestringResourcewith parameters instead of manual concatenation.
142-154: Use explicit numeric types/formatting for monetary and percentage values
totalandtotalCollateralare computed with plain*//and thentoString(). Depending on the types ofbasePrice,quantity, andpctToBase, this may:
- Truncate decimals if everything is
Int(integer division).- Display unformatted raw numbers instead of user-friendly currency/amount strings.
Consider:
- Ensuring the calculation uses
Doubleor a decimal type (e.g.total * (item.pctToBase / 100.0)ifpctToBaseis anInt), and- Formatting values via your existing amount/currency formatter (if available) before passing them as
Strings toMifosActionsCollateralDataListingComponent.feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientCollateralDetails/ClientCollateralDetailViewmodel.kt (1)
98-116: Remove or use currently unused state and event types
ClientCollateralDetailsState.networkConnectionand theClientCollateralDetailsEventsealed interface are defined but never used in this screen’s ViewModel/UI flow.To reduce noise:
- Either wire
networkConnectioninto your UI or remove it from the state for now, and- Drop
ClientCollateralDetailsEventuntil you have concrete one-off events to expose (snackbars, navigation, etc.).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
cmp-navigation/src/commonMain/kotlin/cmp/navigation/authenticated/AuthenticatedNavbarNavigationScreen.kt(2 hunks)core/data/src/commonMain/kotlin/com/mifos/core/data/repository/ClientDetailsRepository.kt(2 hunks)core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsRepositoryImp.kt(2 hunks)core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerClient.kt(2 hunks)core/network/src/commonMain/kotlin/com/mifos/core/network/model/CollateralItem.kt(1 hunks)core/network/src/commonMain/kotlin/com/mifos/core/network/services/ClientService.kt(2 hunks)core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosActionsListingCardComponent.kt(3 hunks)feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientCollateralDetails/ClientCollateralDetailRoute.kt(1 hunks)feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientCollateralDetails/ClientCollateralDetailScreen.kt(1 hunks)feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientCollateralDetails/ClientCollateralDetailViewmodel.kt(1 hunks)feature/client/src/commonMain/kotlin/com/mifos/feature/client/di/ClientModule.kt(2 hunks)feature/client/src/commonMain/kotlin/com/mifos/feature/client/navigation/ClientNavigation.kt(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientCollateralDetails/ClientCollateralDetailRoute.kt (1)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientCollateralDetails/ClientCollateralDetailScreen.kt (1)
ClientCollateralDetailScreen(41-52)
core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosActionsListingCardComponent.kt (1)
core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosListingComponent.kt (2)
MifosListingRowItem(103-128)MifosListingRowItem(172-200)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/navigation/ClientNavigation.kt (1)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientCollateralDetails/ClientCollateralDetailRoute.kt (1)
clientCollateralDetailDestination(22-28)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientCollateralDetails/ClientCollateralDetailScreen.kt (5)
core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosBreadCrumb.kt (1)
MifosBreadcrumbNavBar(36-107)core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosEmptyCard.kt (1)
MifosEmptyCard(28-46)core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosErrorComponent.kt (1)
MifosErrorComponent(41-60)core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosProgressIndicator.kt (1)
MifosProgressIndicator(41-68)core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosActionsListingCardComponent.kt (1)
MifosActionsCollateralDataListingComponent(226-309)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: PR Checks / Build Android Application
🔇 Additional comments (20)
cmp-navigation/src/commonMain/kotlin/cmp/navigation/authenticated/AuthenticatedNavbarNavigationScreen.kt (2)
353-353: Clarify whether onSavings should use the new navigation route.The
onSavingsparameter usesnavigateClientDetailsScreenwhileonClient(line 350) uses the newernavigateToClientProfileRoute. This inconsistency suggests either:
- An incomplete migration where
onSavingsshould also usenavigateToClientProfileRoute- Intentional routing where savings navigation should lead to a different screen than client navigation
If this is unintentional and should be updated, the import on line 80 would also become unused.
78-78: No changes needed—the navigation inconsistency is intentional.The PR correctly distinguishes between two different user flows in the
searchNavGraph:
- Line 350 (
onClient): When a client is selected directly from search → navigates tonavigateToClientProfileRoute(new)- Line 353 (
onSavings): When a savings account is selected from search → navigates tonavigateClientDetailsScreen(existing)These parameters serve different semantic purposes and should use different navigation targets. The change is correct as-is.
core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosActionsListingCardComponent.kt (4)
43-43: LGTM! String resource import added for localization.
235-235: LGTM! Collateral cards now start collapsed.Setting
isExpanded = falseensures collateral data cards start in a collapsed state, which is appropriate for this non-interactive component that doesn't have an onClick handler.
241-241: LGTM! Localized string resource usage.Replacing the hardcoded "Name" string with
stringResource(Res.string.name)enables proper internationalization.
267-306: LGTM! Defensive guard prevents rendering empty menu section.Wrapping the menu Surface and Column in an
if (menuList.isNotEmpty())check ensures the menu UI is only rendered when actions are available, avoiding empty visual elements.core/network/src/commonMain/kotlin/com/mifos/core/network/datamanager/DataManagerClient.kt (2)
30-30: LGTM! Import added for new model type.
189-191: LGTM! Data manager method follows established patterns.The new
getClientCollateralItemsmethod correctly delegates to the service layer and follows the same pattern as other data retrieval methods in this class.core/data/src/commonMain/kotlin/com/mifos/core/data/repository/ClientDetailsRepository.kt (2)
16-16: LGTM! Import added for repository method signature.
45-45: LGTM! Repository interface extended with collateral retrieval method.The new
getClientCollateralsmethod signature is consistent with other repository methods and appropriately wraps the response inDataStatefor error handling.core/network/src/commonMain/kotlin/com/mifos/core/network/services/ClientService.kt (1)
25-25: LGTM! Import added for API response type.feature/client/src/commonMain/kotlin/com/mifos/feature/client/navigation/ClientNavigation.kt (3)
36-37: LGTM! Navigation imports added for collateral details screen.
236-236: LGTM! Collateral data navigation now functional.The navigation lambda is now properly implemented, replacing the previous empty placeholder with actual navigation to the client collateral detail route.
244-246: LGTM! Collateral detail destination registered in navigation graph.The destination is properly registered following the same pattern as other client-related destinations.
feature/client/src/commonMain/kotlin/com/mifos/feature/client/di/ClientModule.kt (2)
20-20: LGTM! Import added for DI registration.
89-89: LGTM! ViewModel registered with Koin DI container.The
ClientCollateralDetailViewmodelis properly registered following the same pattern as other ViewModels in the module.core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsRepositoryImp.kt (2)
19-19: LGTM! Import added for implementation.
76-83: LGTM! Repository implementation follows established error handling pattern.The
getClientCollateralsmethod correctly implements the interface, properly wraps the data manager call in try-catch, and returns appropriateDataStatevalues for success and error cases, consistent with other methods in this class.feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientCollateralDetails/ClientCollateralDetailRoute.kt (1)
17-38: Route and navigation helper look consistentThe
ClientCollateralDetailRouteplus theclientCollateralDetailDestinationandnavigateToClientCollateralDetailRoutehelpers are wired in a type-safe way and align with the serialization-based navigation pattern; no issues spotted here.feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientCollateralDetails/ClientCollateralDetailViewmodel.kt (1)
39-88: State transitions cover main cases; consider also tracking network flag in state if neededThe
getCollateralscoroutine handlesLoading,Success(with empty vs non-empty lists), andError(online and offline) cleanly, and usesnetworkMonitorto distinguish connectivity issues.If you intend to use
networkConnectionfromClientCollateralDetailsStatein the UI, you might also want to update that field alongsidestateupdates; otherwise it can likely be dropped to keep the state minimal.
core/network/src/commonMain/kotlin/com/mifos/core/network/model/CollateralItem.kt
Show resolved
Hide resolved
core/network/src/commonMain/kotlin/com/mifos/core/network/services/ClientService.kt
Show resolved
Hide resolved
...Main/kotlin/com/mifos/feature/client/clientCollateralDetails/ClientCollateralDetailScreen.kt
Show resolved
Hide resolved
...n/kotlin/com/mifos/feature/client/clientCollateralDetails/ClientCollateralDetailViewmodel.kt
Outdated
Show resolved
Hide resolved
revanthkumarJ
force-pushed
the
collateral_data
branch
from
69b64d0 to
bcad818
Compare
2 participants
WhatsApp.Video.2025-11-21.at.10.49.51.PM.mp4
Summary by CodeRabbit
New Features
Bug Fixes & Improvements
✏️ Tip: You can customize this high-level summary in your review settings.