Fixed login Issue using mifos/password(Login credentials)#3071
Fixed login Issue using mifos/password(Login credentials)#3071Aditya002500 wants to merge 17 commits intoopenMF:developmentfrom
Conversation
|
Warning Rate limit exceeded
⌛ 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. 📝 WalkthroughWalkthroughThe changes introduce localization support for login error messages via Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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: 1
🤖 Fix all issues with AI agents
In
`@feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/login/LoginViewModel.kt`:
- Around line 112-122: Replace the hardcoded error message in LoginViewModel
(inside the user.clients.isEmpty() branch) with a string resource: add
feature_sign_in_no_linked_client_error to your resources and pass a Res.string
reference (consistent with other messages that use Res.string.*) into
DialogState.Error instead of the literal String; if DialogState.Error currently
only accepts String, update its constructor/type to accept a StringResource/Res
or resolve the resource before calling updateState, and also remove the extra
leading whitespace/indentation in that block to match surrounding formatting.
🧹 Nitpick comments (2)
core/datastore/src/commonMain/kotlin/org/mifos/mobile/core/datastore/UserPreferencesDataSource.kt (1)
160-170: Correct fix - now aligns with the update pattern used elsewhere in the class.The addition of
_userInfo.value = UserData.DEFAULTensures the in-memory state is synchronized with persistent storage, matching the pattern used inupdateUserInfo,updateToken,updateClientId, etc. This is the key fix for the auth state inconsistency.Minor: The indentation on line 163 appears to have extra leading whitespace compared to the surrounding code.
🔧 Suggested indentation fix
suspend fun clearInfo() { withContext(dispatcher) { settings.putUserPreference(UserData.DEFAULT) - _userInfo.value = UserData.DEFAULT + _userInfo.value = UserData.DEFAULT val cleared = settings.getSettingsPreference().copy( isAuthenticated = false, )feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/login/LoginViewModel.kt (1)
123-135: Simplify redundantisNotEmpty()check.Since the early return at line 121-122 guarantees
user.clientsis not empty at this point, the conditional on lines 126-130 is redundant. You can safely useuser.clients[0]directly.♻️ Suggested simplification
val userData = UserData( userId = user.userId, userName = user.username.orEmpty(), - clientId = if (user.clients.isNotEmpty()) { - user.clients[0] - } else { - user.userId - }, + clientId = user.clients[0], isAuthenticated = user.isAuthenticated, base64EncodedAuthenticationKey = user.base64EncodedAuthenticationKey.orEmpty(), officeName = user.officeName.orEmpty(), password = state.password, )
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cmp-android/src/main/AndroidManifest.xmlcore/datastore/src/commonMain/kotlin/org/mifos/mobile/core/datastore/UserPreferencesDataSource.ktfeature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/login/LoginViewModel.kt
🧰 Additional context used
🧬 Code graph analysis (1)
feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/login/LoginViewModel.kt (10)
feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeViewModel.kt (1)
updateState(211-213)feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/registration/RegistrationViewModel.kt (1)
updateState(61-63)feature/beneficiary/src/commonMain/kotlin/org/mifos/mobile/feature/beneficiary/beneficiaryDetail/BeneficiaryDetailViewModel.kt (1)
updateState(100-102)feature/accounts/src/commonMain/kotlin/org/mifos/mobile/feature/accounts/accountTransactions/TransactionViewModel.kt (1)
updateState(116-118)feature/savings-application/src/commonMain/kotlin/org/mifos/mobile/feature/savings/application/savingsApplication/SavingsApplyViewModel.kt (1)
updateState(167-169)feature/beneficiary/src/commonMain/kotlin/org/mifos/mobile/feature/beneficiary/beneficiaryApplication/BeneficiaryApplicationViewModel.kt (1)
updateState(96-98)feature/beneficiary/src/commonMain/kotlin/org/mifos/mobile/feature/beneficiary/beneficiaryList/BeneficiaryListViewModel.kt (1)
updateState(131-133)feature/qr/src/commonMain/kotlin/org/mifos/mobile/feature/qr/qr/QrCodeReaderViewModel.kt (1)
updateState(63-65)feature/qr/src/commonMain/kotlin/org/mifos/mobile/feature/qr/qrCodeDisplay/QrCodeDisplayViewModel.kt (1)
updateState(91-93)feature/qr/src/commonMain/kotlin/org/mifos/mobile/feature/qr/qrCodeImport/QrCodeImportViewModel.kt (1)
updateState(59-61)
🔇 Additional comments (1)
cmp-android/src/main/AndroidManifest.xml (1)
37-37: Good security and reliability improvement.Disabling
android:allowBackupprevents Android from restoring stale authentication credentials after app reinstall or device migration, which aligns with the PR's goal of fixing auth state inconsistencies. This is a best practice for apps handling sensitive user credentials.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| if (user.clients.isEmpty()) { | ||
| updateState { | ||
| it.copy( | ||
| isError = true, | ||
| dialogState = LoginState.DialogState.Error( | ||
| "This account is not linked to any client. Please contact your administrator.", | ||
| ), | ||
| ) | ||
| } | ||
| return | ||
| } |
There was a problem hiding this comment.
Hardcoded string should be a string resource for localization.
The error message is hardcoded while other error strings in this file use Res.string.* resources (see lines 99-100). For consistency and proper i18n support, extract this to a string resource.
Also, the indentation on these lines appears to have extra leading whitespace.
🔧 Suggested fix
Add a string resource in the appropriate resources file:
<string name="feature_sign_in_no_linked_client_error">This account is not linked to any client. Please contact your administrator.</string>Then update the code:
- if (user.clients.isEmpty()) {
- updateState {
- it.copy(
- isError = true,
- dialogState = LoginState.DialogState.Error(
- "This account is not linked to any client. Please contact your administrator.",
- ),
- )
- }
- return
- }
+ if (user.clients.isEmpty()) {
+ updateState {
+ it.copy(
+ isError = true,
+ dialogState = LoginState.DialogState.Error(
+ Res.string.feature_sign_in_no_linked_client_error,
+ ),
+ )
+ }
+ return
+ }Note: This would require changing DialogState.Error to accept a StringResource instead of String, or resolving the resource at this point.
🤖 Prompt for AI Agents
In
`@feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/login/LoginViewModel.kt`
around lines 112 - 122, Replace the hardcoded error message in LoginViewModel
(inside the user.clients.isEmpty() branch) with a string resource: add
feature_sign_in_no_linked_client_error to your resources and pass a Res.string
reference (consistent with other messages that use Res.string.*) into
DialogState.Error instead of the literal String; if DialogState.Error currently
only accepts String, update its constructor/type to accept a StringResource/Res
or resolve the resource before calling updateState, and also remove the extra
leading whitespace/indentation in that block to match surrounding formatting.
There was a problem hiding this comment.
@Aditya002500 Please address the suggested changes requested by coderabbitai
There was a problem hiding this comment.
@amanna13, thank you for following up on this!
To summarize what needs to be addressed in this code block:
-
Add a string resource for the error message in the appropriate resources file (likely
feature/auth/src/commonMain/composeResources/values/strings.xml):<string name="feature_sign_in_no_linked_client_error">This account is not linked to any client. Please contact your administrator.</string>
-
Update the code to use the string resource and fix the indentation:
- Remove the extra leading whitespace (3 spaces) before
ifon line 112 - Replace the hardcoded string with a reference to the string resource
- Remove the extra leading whitespace (3 spaces) before
-
Handle the type mismatch: Since
DialogState.Errorcurrently acceptsString, you'll need to either:- Change
DialogState.Errorto acceptStringResource(recommended for consistency), OR - Resolve the string resource before passing it to
DialogState.Error
- Change
The first approach would maintain better type safety and consistency with other error handling patterns in the codebase, where string resources are resolved at the UI layer rather than in the ViewModel.
Aditya002500
left a comment
Aditya002500
left a comment
There was a problem hiding this comment.
Fixed indentation
|
@Aditya002500 The ticket number seems to be incorrect. Please check. |
|
Please update the ticket number and its corresponding URL in the PR description. |
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 (4)
feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/di/SettingsModule.kt (1)
27-35: Duplicate ViewModel registration:UpdatePasscodeViewModelis registered twice.
viewModelOf(::UpdatePasscodeViewModel)appears at both line 29 and line 31. Remove the duplicate registration.🐛 Proposed fix
val SettingsModule = module { viewModelOf(::SettingsViewModel) viewModelOf(::UpdatePasscodeViewModel) viewModelOf(::ChangePasswordViewModel) - viewModelOf(::UpdatePasscodeViewModel) viewModelOf(::FaqViewModel) viewModelOf(::LanguageViewModel) viewModelOf(::ChangeThemeViewModel) viewModelOf(::SelectedServicesViewModel) }core/datastore/src/commonMain/kotlin/org/mifos/mobile/core/datastore/UserPreferencesRepositoryImpl.kt (1)
139-146: Pre-existing null safety issue:clientId!!will throw NPE if null is passed.The method signature accepts
Long?but immediately force-unwraps it. Consider handling the null case gracefully or changing the parameter type to non-nullableLong.Suggested fix
override suspend fun updateClientId(clientId: Long?): DataState<Unit> { return try { - val result = preferenceManager.updateClientId(clientId!!) + val id = clientId ?: return DataState.Error(IllegalArgumentException("clientId cannot be null")) + val result = preferenceManager.updateClientId(id) DataState.Success(result) } catch (e: Exception) { DataState.Error(e) } }feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeViewModel.kt (1)
317-329: Pre-existing null safety issue: force unwrap on nullable parameter.
savingAccountListis typed asList<SavingAccount>?but is force-unwrapped with!!at line 319. While the current call site at line 257 guards this withisNotEmpty(), the function signature doesn't guarantee a non-null list.Consider making the parameter non-nullable or using safe iteration
- private fun getSavingAccountDetails(savingAccountList: List<SavingAccount>?) { + private fun getSavingAccountDetails(savingAccountList: List<SavingAccount>) { var totalAmount = 0.0 - for (savingAccount in savingAccountList!!) { + for (savingAccount in savingAccountList) { totalAmount += savingAccount.accountBalance }Or if the nullable type must be retained:
- for (savingAccount in savingAccountList!!) { + for (savingAccount in savingAccountList.orEmpty()) {core/datastore/src/commonMain/kotlin/org/mifos/mobile/core/datastore/UserPreferencesDataSource.kt (1)
160-170:clearInfo()should also reset selected services to defaults.When a user logs out, the
_selectedServicesstate flow and the persistedSELECTED_SERVICESpreference retain their values. Since selected services are user-specific (as shown by their use in HomeViewModel to filter service cards per user), the next user logging in would initially see the previous user's service selections. Reset both_selectedServicesand the stored preference toDEFAULT_SELECTED_SERVICESinclearInfo()to maintain user data separation.
🤖 Fix all issues with AI agents
In
`@core/datastore/src/commonMain/kotlin/org/mifos/mobile/core/datastore/UserPreferencesDataSource.kt`:
- Around line 239-246: getSelectedServices() currently treats an empty stored
string the same as "not configured" because
settings.getString(SELECTED_SERVICES, "") returns "" for both cases; change the
logic to distinguish absent vs explicit empty by either (A) checking presence
via settings.contains(SELECTED_SERVICES) (or using getString with a nullable
default) and returning DEFAULT_SELECTED_SERVICES only when the key is missing,
while returning emptySet() when the stored value is "" — or (B) introduce a
separate boolean flag (e.g., SELECTED_SERVICES_CONFIGURED) that you set when the
user saves selections and check in getSelectedServices(); update the code paths
that write the preference (the setter that calls settings.putString or writes
SELECTED_SERVICES_CONFIGURED) and modify getSelectedServices() accordingly so
DEFAULT_SELECTED_SERVICES is returned only for "not yet configured" and an
explicit empty string yields an empty set.
🧹 Nitpick comments (3)
feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/componenets/SettingsItems.kt (1)
108-116: Verify icon choice:MifosIcons.AppRecentis also used byAppInfo.The
SelectedServicesitem usesMifosIcons.AppRecent, which is the same icon used byAppInfo(line 168). Consider using a distinct icon for better visual differentiation in the settings list.feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/selectedservices/SelectedServicesViewModel.kt (1)
57-64: Use a stable snapshot for apply; verify failure handling.
CapturingpendingSelectedServicesbefore launching avoids stale reads. Also, if persistence can fail, consider surfacing that instead of navigating back.♻️ Suggested adjustment
private fun handleApplyChanges() { - viewModelScope.launch { - userPreferencesRepository.setSelectedServices(state.pendingSelectedServices) - mutableStateFlow.update { - it.copy(currentSelectedServices = state.pendingSelectedServices) - } + val pending = state.pendingSelectedServices + viewModelScope.launch { + userPreferencesRepository.setSelectedServices(pending) + mutableStateFlow.update { + it.copy(currentSelectedServices = pending) + } sendEvent(SelectedServicesEvent.NavigateBack) } }feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/selectedservices/SelectedServicesScreen.kt (1)
98-129: Apply themodifierto the scaffold instead of the button.
Right now the caller’smodifieronly affects the bottom button, which is surprising for a screen-level modifier.♻️ Suggested adjustment
MifosElevatedScaffold( + modifier = modifier, onNavigateBack = { onAction(SelectedServicesAction.OnNavigateBack) }, topBarTitle = stringResource(Res.string.feature_settings_selected_services_topbar), bottomBar = { MifosButton( - modifier = modifier + modifier = Modifier .fillMaxWidth() .height(DesignToken.sizes.buttonHeight) .padding(horizontal = KptTheme.spacing.md), shape = KptTheme.shapes.medium,
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
core/common/src/commonMain/kotlin/org/mifos/mobile/core/common/Constants.ktcore/datastore/src/commonMain/kotlin/org/mifos/mobile/core/datastore/UserPreferencesDataSource.ktcore/datastore/src/commonMain/kotlin/org/mifos/mobile/core/datastore/UserPreferencesRepository.ktcore/datastore/src/commonMain/kotlin/org/mifos/mobile/core/datastore/UserPreferencesRepositoryImpl.ktfeature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeViewModel.ktfeature/settings/src/commonMain/composeResources/values/strings.xmlfeature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/componenets/SettingsItems.ktfeature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/di/SettingsModule.ktfeature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/navigation/SettingsNavGraphRoute.ktfeature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/selectedservices/SelectedServicesRoute.ktfeature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/selectedservices/SelectedServicesScreen.ktfeature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/selectedservices/SelectedServicesViewModel.kt
🧰 Additional context used
📓 Path-based instructions (1)
**/{src,}/**/di/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
Define Koin modules in the
di/package for each module
Files:
feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/di/SettingsModule.kt
🧠 Learnings (2)
📚 Learning: 2026-01-07T10:05:07.700Z
Learnt from: CR
Repo: openMF/mifos-mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T10:05:07.700Z
Learning: Use ViewModels with StateFlow/SharedFlow for state management in feature modules
Applied to files:
feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/selectedservices/SelectedServicesViewModel.ktfeature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/di/SettingsModule.ktfeature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeViewModel.kt
📚 Learning: 2026-01-07T10:05:07.700Z
Learnt from: CR
Repo: openMF/mifos-mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T10:05:07.700Z
Learning: Use the `ScreenState<T>` pattern for loading/success/error states in feature modules
Applied to files:
feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeViewModel.kt
🧬 Code graph analysis (2)
feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/selectedservices/SelectedServicesScreen.kt (2)
core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/component/MifosScaffold.kt (1)
MifosElevatedScaffold(209-284)core/designsystem/src/commonMain/kotlin/org/mifos/mobile/core/designsystem/theme/Theme.kt (1)
MifosMobileTheme(267-291)
feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/navigation/SettingsNavGraphRoute.kt (1)
feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/selectedservices/SelectedServicesRoute.kt (1)
selectedServicesDestination(22-30)
🔇 Additional comments (23)
core/common/src/commonMain/kotlin/org/mifos/mobile/core/common/Constants.kt (1)
111-111: LGTM!The constant follows the existing naming conventions and is appropriately placed in the settings constants section.
feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/componenets/SettingsItems.kt (2)
36-37: LGTM!Imports follow the established pattern for other settings items.
192-192: LGTM!The new item is properly added to the settings list in a logical position.
feature/settings/src/commonMain/composeResources/values/strings.xml (1)
71-85: LGTM!String resources follow the established naming conventions and are well-organized. The "A/c" abbreviation for "Account" is used consistently across related entries.
feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/navigation/SettingsNavGraphRoute.kt (2)
25-25: LGTM!Import follows the established pattern for other destination imports.
90-92: LGTM!The
selectedServicesDestinationis wired consistently with other destinations in the settings graph, usingpopBackStackfor the back navigation action.core/datastore/src/commonMain/kotlin/org/mifos/mobile/core/datastore/UserPreferencesRepository.kt (1)
78-80: LGTM!The new interface members for observing and setting selected services are well-defined and consistent with the existing API patterns in this repository interface.
feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/selectedservices/SelectedServicesRoute.kt (1)
18-30: LGTM!The navigation helpers follow the established pattern for settings routes. Using
composableWithPushTransitionsensures consistent transition animations, and the internal visibility is appropriate for feature-module navigation.core/datastore/src/commonMain/kotlin/org/mifos/mobile/core/datastore/UserPreferencesRepositoryImpl.kt (1)
190-195: LGTM!The implementation correctly delegates to
preferenceManagerfor both observing and setting selected services, following the established delegation pattern used throughout this repository.core/datastore/src/commonMain/kotlin/org/mifos/mobile/core/datastore/UserPreferencesDataSource.kt (2)
160-170: Critical fix: Resetting in-memory state after clearing persistent storage.This change correctly addresses the root cause of MM-352. Previously, only the persistent storage was cleared but the in-memory
_userInfostate retained stale values, causing authentication checks to fail and show "unexpected error occurred".
265-276: LGTM on the default services configuration.The default set of services is well-defined and covers the expected feature set. Using a companion object constant makes it reusable and testable.
feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeViewModel.kt (1)
66-77: LGTM!The observer correctly:
- Uses
distinctUntilChanged()to prevent redundant state updates- Filters from the original
serviceCardslist (not the current state) ensuring consistent behavior- Converts to
PersistentListfor Compose immutability requirementsBased on learnings, this follows the StateFlow/SharedFlow pattern recommended for ViewModels.
feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/selectedservices/SelectedServicesViewModel.kt (6)
22-35: Clean initial state + observation wiring.
Initialization seeds defaults and streams repository updates into state actions clearly.
37-43: Action routing is straightforward.
The handler cleanly dispatches all action types without gaps.
46-55: Toggle logic is clear and immutable.
Set updates are concise and safe.
67-73: Load handler keeps current/pending in sync.
This makes state hydration predictable.
77-83: State model andhasChangesderivation look good.
Clear and minimal.
85-96: Event/action modeling is tidy.
Sealed interfaces keep the action space explicit.feature/settings/src/commonMain/kotlin/org/mifos/mobile/feature/settings/selectedservices/SelectedServicesScreen.kt (5)
57-73: Service catalog structure is clear.
The model and list are straightforward and readable.
75-96: Screen wiring to state + events looks solid.
The event bridge is clean and concise.
131-149: List rendering is straightforward.
The list composition reads cleanly.
151-179: Checkbox row composition looks good.
Layout and styling choices are consistent with the design system.
182-193: Preview mirrors default state nicely.
Helpful for quick UI checks.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| fun getSelectedServices(): Set<String> { | ||
| val stored = settings.getString(SELECTED_SERVICES, "") | ||
| return if (stored.isEmpty()) { | ||
| DEFAULT_SELECTED_SERVICES | ||
| } else { | ||
| stored.split(",").toSet() | ||
| } | ||
| } |
There was a problem hiding this comment.
Empty set cannot be distinguished from "not yet configured".
If a user explicitly deselects all services, getSelectedServices() returns DEFAULT_SELECTED_SERVICES instead of an empty set. This may not be the intended behavior.
Consider using a sentinel value or separate "configured" flag
fun getSelectedServices(): Set<String> {
- val stored = settings.getString(SELECTED_SERVICES, "")
- return if (stored.isEmpty()) {
+ val stored = settings.getStringOrNull(SELECTED_SERVICES)
+ return if (stored == null) {
DEFAULT_SELECTED_SERVICES
} else {
- stored.split(",").toSet()
+ if (stored.isEmpty()) emptySet() else stored.split(",").toSet()
}
}This allows distinguishing between "never configured" (null → defaults) and "explicitly empty" (empty string → empty set).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun getSelectedServices(): Set<String> { | |
| val stored = settings.getString(SELECTED_SERVICES, "") | |
| return if (stored.isEmpty()) { | |
| DEFAULT_SELECTED_SERVICES | |
| } else { | |
| stored.split(",").toSet() | |
| } | |
| } | |
| fun getSelectedServices(): Set<String> { | |
| val stored = settings.getStringOrNull(SELECTED_SERVICES) | |
| return if (stored == null) { | |
| DEFAULT_SELECTED_SERVICES | |
| } else { | |
| if (stored.isEmpty()) emptySet() else stored.split(",").toSet() | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@core/datastore/src/commonMain/kotlin/org/mifos/mobile/core/datastore/UserPreferencesDataSource.kt`
around lines 239 - 246, getSelectedServices() currently treats an empty stored
string the same as "not configured" because
settings.getString(SELECTED_SERVICES, "") returns "" for both cases; change the
logic to distinguish absent vs explicit empty by either (A) checking presence
via settings.contains(SELECTED_SERVICES) (or using getString with a nullable
default) and returning DEFAULT_SELECTED_SERVICES only when the key is missing,
while returning emptySet() when the stored value is "" — or (B) introduce a
separate boolean flag (e.g., SELECTED_SERVICES_CONFIGURED) that you set when the
user saves selections and check in getSelectedServices(); update the code paths
that write the preference (the setter that calls settings.putString or writes
SELECTED_SERVICES_CONFIGURED) and modify getSelectedServices() accordingly so
DEFAULT_SELECTED_SERVICES is returned only for "not yet configured" and an
explicit empty string yields an empty set.
feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeViewModel.kt
Show resolved
Hide resolved
feature/home/src/commonMain/kotlin/org/mifos/mobile/feature/home/HomeViewModel.kt
Show resolved
Hide resolved
|
@Aditya002500 Is the screenshot after the changes in this PR? |
No sir but, I have updated now |
|
@Aditya002500 Can you please upload the screen recording before and after flow? |
|
Looks good to me |
|
@Aditya002500 Please fix the spotless issue by running |
| <application | ||
| android:name=".AndroidApp" | ||
| android:allowBackup="true" | ||
| android:allowBackup="false" |
There was a problem hiding this comment.
@Aditya002500 If this is set to false then no backups or restore will be available for this app. Was this the cause of the issue you are trying to fix in this PR?
Ref: https://developer.android.com/reference/android/R.attr.html#allowBackup
There was a problem hiding this comment.
It didnt fixed it, initially it was guided by the Rajan sir to start with this. And sir I asked some stuff to ai it seems that it is safer to keep it false for a banking app and keep things at server side, kindly let me know if needed to be changed.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/login/LoginViewModel.kt`:
- Around line 109-122: When handling the DataState.Success branch in
LoginViewModel where you currently early-return on user.clients.isEmpty(),
revoke/cleanup the server session and local credentials before returning: call
the appropriate logout/revoke method on the auth repository (e.g.,
authRepository.logout() or authRepository.revokeToken()) and clear any stored
auth state via userPreferencesRepositoryImpl (e.g., clear saved token/user info)
so no orphaned session remains; place these calls just before the existing
updateState error block’s return inside the is DataState.Success branch that
checks user.clients.isEmpty().
🧹 Nitpick comments (1)
feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/login/LoginViewModel.kt (1)
123-131: Dead else-branch:user.clients.isNotEmpty()is always true here.The early return on line 121 guarantees
clientsis non-empty at this point. The conditional and its fallback touser.userIdare now dead code.Suggested simplification
val userData = UserData( userId = user.userId, userName = user.username.orEmpty(), - clientId = if (user.clients.isNotEmpty()) { - user.clients[0] - } else { - user.userId - }, + clientId = user.clients[0], isAuthenticated = user.isAuthenticated,
feature/auth/src/commonMain/kotlin/org/mifos/mobile/feature/auth/login/LoginViewModel.kt
Show resolved
Hide resolved
4 participants
Fixes - MM-352
Shows "unexpected error occurred" because client data fetches fail from the server
Both storage AND in-memory state are cleared immediately, so auth check fails correctly
Login behavior
BeforeFix.mp4
|
WhatsApp.Video.2026-02-02.at.23.21.12.mp4
| |
Changes in -
cmp-android\src\main\AndroidManifest.xml
core\datastore\src\commonMain\kotlin\org\mifos\mobile\core\datastore\UserPreferencesDataSource.kt
feature\auth\src\commonMain\kotlin\org\mifos\mobile\feature\auth\login\LogInNavigation.kt
Summary by CodeRabbit