Feature/manage pocket screen#2057
Open
Kartikey15dem wants to merge 4 commits into
Open
Conversation
Kartikey15dem
marked this pull request as draft
July 21, 2026 16:39
Contributor
📝 WalkthroughWalkthroughAdds Manage Pocket account linking and delinking, currency-aware pocket data and dashboard balances, reusable Compose dialog and pager components, updated pocket navigation, and dashboard presentation refinements. ChangesPocket management
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ManagePocketScreen
participant ManagePocketViewModel
participant PocketRepository
User->>ManagePocketScreen: Select accounts or request delink
ManagePocketScreen->>ManagePocketViewModel: Dispatch ManagePocketAction
ManagePocketViewModel->>PocketRepository: Link or delink accounts
PocketRepository-->>ManagePocketViewModel: Return DataState
ManagePocketViewModel-->>ManagePocketScreen: Update account and dialog state
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
feat(feature/pocket):implement pocket dashboard screen feat(feature/pocket):implement pocket dashboard screen feat(feature/pocket):implement pocket dashboard screen # Conflicts: # feature/pocket/src/commonMain/kotlin/org/mifospay/feature/pocket/viewmodels/PocketDashboardViewModel.kt
fix(feature/pocket):fix static checks
Kartikey15dem
force-pushed
the
feature/manage-pocket-screen
branch
from
July 22, 2026 07:13
e398d63 to
11e49ae
Compare
feat(feature/pocket):implement manage pocket screen feat(feature/pocket):implement manage pocket screen feat(feature/pocket):implement manage pocket screen feat(feature/pocket):implement manage pocket screen
Kartikey15dem
force-pushed
the
feature/manage-pocket-screen
branch
from
July 25, 2026 11:10
11e49ae to
73b7fa0
Compare
Kartikey15dem
marked this pull request as ready for review
July 25, 2026 12:01
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
feature/pocket/src/commonMain/kotlin/org/mifospay/feature/pocket/screens/ManagePocketScreen.kt (2)
769-773: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the non-clickable
Cardoverload.
onClick = {}withenabled = falseonly to get a static container also applies disabled content colors. PreferCard(modifier, shape, colors)withoutonClick.🤖 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/pocket/src/commonMain/kotlin/org/mifospay/feature/pocket/screens/ManagePocketScreen.kt` around lines 769 - 773, Update the Card invocation in ManagePocketScreen to use the non-clickable overload by removing the empty onClick and enabled arguments, while preserving the existing modifier and any shape or color configuration.
540-588: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated account list.
The search overlay repeats the
LazyColumn+SelectablePocketAccountCard+AccountSelectionChangedblock from Lines 513-535 verbatim, including the identifier string built in three places ("${accountId}_${accountType.name}", also in the ViewModel). Extract a privateAvailableAccountsList(accounts, selectedIds, onSelectedChange)composable and a shared identifier helper.🤖 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/pocket/src/commonMain/kotlin/org/mifospay/feature/pocket/screens/ManagePocketScreen.kt` around lines 540 - 588, Extract the repeated account-list UI into a private AvailableAccountsList composable accepting accounts, selectedIds, and an onSelectedChange callback, then reuse it in both the main list and search overlay instead of duplicating LazyColumn and SelectablePocketAccountCard logic. Add and use a shared account identifier helper for the "${accountId}_${accountType.name}" value in the composable and ViewModel, preserving selection behavior and item keys.feature/pocket/src/commonMain/kotlin/org/mifospay/feature/pocket/viewmodels/ManagePocketViewModel.kt (1)
337-342: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the unused
ErrorStringvariant and importStringResource.
ManagePocketUiState.ErrorStringis never produced by the ViewModel, so the screen carries a dead branch. Also prefer an import over the fully-qualifiedorg.jetbrains.compose.resources.StringResourcein both sealed interfaces.Also applies to: 364-369
🤖 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/pocket/src/commonMain/kotlin/org/mifospay/feature/pocket/viewmodels/ManagePocketViewModel.kt` around lines 337 - 342, Remove the unused ManagePocketUiState.ErrorString variant, add an import for StringResource, and replace the fully qualified type in ManagePocketUiState.Error; apply the same import/type cleanup to the other sealed interface referenced by the comment.core/data/src/commonMain/kotlin/org/mifospay/core/data/repositoryImpl/PocketRepositoryImp.kt (1)
226-227: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueUse a
Setfor the already-linked lookup.
alreadyLinkedAccountsis aList<Pair<Long, AccountType>>queried with!inonce per loan/savings/share account, giving O(n·m) scans. ASetmakes each check O(1) and expresses intent better.♻️ Proposed change
- val alreadyLinkedAccounts = (detailedPocketCache.value as? DataState.Success) - ?.data?.map { Pair(it.pocket.accountId, it.pocket.accountType) } ?: emptyList() + val alreadyLinkedAccounts = (detailedPocketCache.value as? DataState.Success) + ?.data?.mapTo(mutableSetOf()) { Pair(it.pocket.accountId, it.pocket.accountType) } + ?: emptySet()🤖 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/data/src/commonMain/kotlin/org/mifospay/core/data/repositoryImpl/PocketRepositoryImp.kt` around lines 226 - 227, Update alreadyLinkedAccounts in the detailedPocketCache lookup to collect the accountId/accountType pairs into a Set instead of a List, preserving the existing empty fallback and Pair values so subsequent !in checks use constant-time membership lookup.feature/pocket/src/commonMain/composeResources/values/strings.xml (1)
35-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse or remove the unused delink title string.
feature_pocket_delink_account_titleis defined infeature/pocket/src/commonMain/composeResources/values/strings.xml, but no code references it. If it is intended for the delink confirmation sheet, wire it in; otherwise remove the resource.🤖 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/pocket/src/commonMain/composeResources/values/strings.xml` at line 35, Resolve the unused feature_pocket_delink_account_title resource by either wiring it into the delink confirmation sheet’s title or removing the string resource if that sheet does not use it. Keep the resource and its usage consistent.
🤖 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
`@core/designsystem/src/commonMain/kotlin/org/mifospay/core/designsystem/component/MifosTabPager.kt`:
- Around line 45-52: Update the indicator lambda in MifosTabPager to retrieve
the current tab position with getOrNull(currentPage), and render
SecondaryIndicator only when a position exists; preserve the existing modifier,
padding, and color for valid indices while safely skipping the indicator for
empty or out-of-range tabPositions.
- Around line 69-74: Update MifosTabPager so TabRow selection and setCurrentPage
use the same pagerState.currentPage that HorizontalPager updates during swipes,
rather than the stale currentPage parameter. Keep tab taps and pager swipes
synchronized through this single source of truth.
In
`@feature/pocket/src/commonMain/kotlin/org/mifospay/feature/pocket/screens/ManagePocketScreen.kt`:
- Around line 464-467: Update the search hint construction in ManagePocketScreen
around feature_pocket_search_accounts_hint so it interpolates a localized string
resource mapped from the selected AccountType, rather than
tabs[selectedTabIndex].name.lowercase(). Apply the same mapping to the
additional occurrence, preserving the existing tab-selection behavior and using
the appropriate SAVINGS, LOAN, and SHARE resources from strings.xml.
- Around line 709-718: Update the clear-search Icon in the trailingIcon block of
ManagePocketScreen to use a localized string resource for contentDescription
instead of the hardcoded “Close Icon” literal. Add the resource to the
appropriate strings.xml with wording that describes clearing the search, such as
“Clear search.”
In
`@feature/pocket/src/commonMain/kotlin/org/mifospay/feature/pocket/viewmodels/ManagePocketViewModel.kt`:
- Around line 12-20: Remove the duplicated license header after the package
declaration in ManagePocketViewModel.kt and ManagePocketScreen.kt; retain only
each file’s header at the top and remove the incorrect openMF/mobile-mobile URL
block.
In
`@feature/pocket/src/commonMain/kotlin/org/mifospay/feature/pocket/viewmodels/PocketDashboardViewModel.kt`:
- Around line 157-169: Update the empty-account fallback in the formattedTotal
calculation to avoid the hardcoded "$0.00" USD value. Use CurrencyFormatter to
format zero without assuming a currency symbol, or reuse an existing
empty/placeholder string resource consistent with
PocketDashboardState.totalBalance.
- Around line 91-93: Remove the per-emission coroutine launches in
PocketDashboardViewModel.handleReceivedAccounts and
ManagePocketViewModel.handleLinkedAccounts/handleAvailableAccounts by resolving
getString fallback values once in the collector coroutine or mapping within the
single loadJob collector, so each DataState is handled synchronously and
emissions cannot complete out of order. Apply the corresponding change in
feature/pocket/src/commonMain/kotlin/org/mifospay/feature/pocket/viewmodels/PocketDashboardViewModel.kt
lines 91-93 and
feature/pocket/src/commonMain/kotlin/org/mifospay/feature/pocket/viewmodels/ManagePocketViewModel.kt
lines 223-228; both sites require the same update.
---
Nitpick comments:
In
`@core/data/src/commonMain/kotlin/org/mifospay/core/data/repositoryImpl/PocketRepositoryImp.kt`:
- Around line 226-227: Update alreadyLinkedAccounts in the detailedPocketCache
lookup to collect the accountId/accountType pairs into a Set instead of a List,
preserving the existing empty fallback and Pair values so subsequent !in checks
use constant-time membership lookup.
In `@feature/pocket/src/commonMain/composeResources/values/strings.xml`:
- Line 35: Resolve the unused feature_pocket_delink_account_title resource by
either wiring it into the delink confirmation sheet’s title or removing the
string resource if that sheet does not use it. Keep the resource and its usage
consistent.
In
`@feature/pocket/src/commonMain/kotlin/org/mifospay/feature/pocket/screens/ManagePocketScreen.kt`:
- Around line 769-773: Update the Card invocation in ManagePocketScreen to use
the non-clickable overload by removing the empty onClick and enabled arguments,
while preserving the existing modifier and any shape or color configuration.
- Around line 540-588: Extract the repeated account-list UI into a private
AvailableAccountsList composable accepting accounts, selectedIds, and an
onSelectedChange callback, then reuse it in both the main list and search
overlay instead of duplicating LazyColumn and SelectablePocketAccountCard logic.
Add and use a shared account identifier helper for the
"${accountId}_${accountType.name}" value in the composable and ViewModel,
preserving selection behavior and item keys.
In
`@feature/pocket/src/commonMain/kotlin/org/mifospay/feature/pocket/viewmodels/ManagePocketViewModel.kt`:
- Around line 337-342: Remove the unused ManagePocketUiState.ErrorString
variant, add an import for StringResource, and replace the fully qualified type
in ManagePocketUiState.Error; apply the same import/type cleanup to the other
sealed interface referenced by the comment.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5007abe0-2aeb-4871-891f-689c382b9aa9
📒 Files selected for processing (13)
cmp-shared/src/commonMain/kotlin/org/mifospay/shared/navigation/MifosNavHost.ktcore/data/src/commonMain/kotlin/org/mifospay/core/data/repositoryImpl/PocketRepositoryImp.ktcore/designsystem/src/commonMain/kotlin/org/mifospay/core/designsystem/component/MifosAccountCard.ktcore/designsystem/src/commonMain/kotlin/org/mifospay/core/designsystem/component/MifosAlertDialog.ktcore/designsystem/src/commonMain/kotlin/org/mifospay/core/designsystem/component/MifosTabPager.ktcore/model/src/commonMain/kotlin/org/mifospay/core/model/pocket/PocketAccount.ktfeature/pocket/src/commonMain/composeResources/values/strings.xmlfeature/pocket/src/commonMain/kotlin/org/mifospay/feature/pocket/di/PocketModule.ktfeature/pocket/src/commonMain/kotlin/org/mifospay/feature/pocket/navigation/ManagePocketRoute.ktfeature/pocket/src/commonMain/kotlin/org/mifospay/feature/pocket/screens/ManagePocketScreen.ktfeature/pocket/src/commonMain/kotlin/org/mifospay/feature/pocket/screens/PocketDashboardScreen.ktfeature/pocket/src/commonMain/kotlin/org/mifospay/feature/pocket/viewmodels/ManagePocketViewModel.ktfeature/pocket/src/commonMain/kotlin/org/mifospay/feature/pocket/viewmodels/PocketDashboardViewModel.kt
fix(feature/pocket):fix MifosTabPager
Nagarjuna0033
approved these changes
Jul 25, 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
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-#MW-380
Summary
This PR adds the Manage Pocket screen, which allows users to view, add, and remove accounts that they want to keep track of in their "Pocket".
Features Added in Manage Pocket Screen:
Here are the cool things you can do on this screen:
View Linked Accounts:
Link More Accounts:
Search for Accounts:
Select & Link:
Remove (Delink) Accounts:
Summary by CodeRabbit