Profile photo not refreshing after upload in Client Profile #2519
Conversation
trim.1DD19614-3186-4A87-B516-55F18FA6BC55.MOV |
| when (event) { | ||
| ClientProfileEditEvent.NavigateBack -> onNavigateBack() | ||
| ClientProfileEditEvent.OnSaveSuccess -> { | ||
| onNavigateBack() |
There was a problem hiding this comment.
I commented it because it is not needed for the navigation, and it is excess code
The screen navigation works well with or without it
There was a problem hiding this comment.
Remove it if not needed
| MifosBasicDialog( | ||
| title = "Success", | ||
| content = { Text("Your profile image has been updated successfully!") }, | ||
| confirmText = "Done", |
There was a problem hiding this comment.
use Success and Failure Dilaogs instead of Basic Dialog.
There was a problem hiding this comment.
This one
MifosStatusDialog
which accepts success and Failure state and also move strings to string resources
| <string name="feature_client_profile_update_success_message">Your profile image has been updated successfully!</string> | ||
| <string name="feature_client_profile_update_error_message">We couldn’t update your profile image. Please try again.</string> | ||
| <string name="should_refresh">should_refresh</string> |
There was a problem hiding this comment.
This should be added to feature:client's commonMain strings.xml (android-client\feature\client\src\commonMain\composeResources\values\strings.xml)
There was a problem hiding this comment.
Oh okay, I will do that and push it
📝 WalkthroughWalkthroughThe changes introduce a new status dialog UI component ( Changes
Sequence DiagramsequenceDiagram
participant User
participant EditScreen as ClientProfileEditScreen
participant EditVM as ClientProfileEditViewModel
participant Repo as ClientDetailsRepository
participant ProfileVM as ClientProfileViewModel
participant DB as Local Data
User->>EditScreen: Upload image
EditScreen->>EditVM: TriggerImageUpload event
EditVM->>Repo: uploadClientImage(clientId, image)
Repo->>DB: Save image
DB-->>Repo: Success
Repo->>Repo: Emit clientId via clientDataUpdated
Repo-->>EditVM: DataState.Success
EditVM->>Repo: Fetch updated image via getImage(clientId)
Repo->>DB: Retrieve image
DB-->>Repo: Image bytes
Repo-->>EditVM: DataState.Success (image)
EditVM->>EditVM: Set dialogState = Success
EditScreen->>EditScreen: Show MifosStatusDialog (SUCCESS)
User->>EditScreen: Dismiss dialog
EditScreen->>EditVM: DismissModalBottomSheet
EditVM->>EditScreen: Navigate back
note over ProfileVM,Repo: Auto-refresh flow
ProfileVM->>Repo: Listen to clientDataUpdated
Repo-->>ProfileVM: Emit clientId
ProfileVM->>ProfileVM: Filter updates by route.id
ProfileVM->>Repo: loadClientDetailsAndImage(clientId)
Repo-->>ProfileVM: Fresh client data
ProfileVM->>ProfileVM: Update UI state
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested Reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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: 5
♻️ Duplicate comments (1)
cmp-android/src/main/res/values/strings.xml (1)
426-426: This string resource appears unrelated to the PR objective and was previously flagged for relocation.The
numberofrepaymentsstring does not relate to profile photo refresh functionality. Additionally, a previous review comment indicated this should be added tofeature:client's commonMain strings.xml (android-client\feature\client\src\commonMain\composeResources\values\strings.xml) instead.
🧹 Nitpick comments (2)
cmp-android/src/main/kotlin/cmp/android/app/ComponentActivityExtensions.kt (1)
84-85: Remove redundantdistinctUntilChanged()call.The
distinctUntilChanged()operator is applied to the flow at line 84, but the same operator is also applied inside thecombineblock at line 51. This results in duplicate filtering.Consider removing one of the
distinctUntilChanged()calls to simplify the flow chain:} - .distinctUntilChanged() .conflate()feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsScreen.kt (1)
22-22: Remove unused import.The
profile_should_refreshstring resource is imported but never used in this file. The code uses the constantPROFILE_SHOULD_REFRESH_KEYfrom the utils package instead.-import androidclient.feature.client.generated.resources.profile_should_refresh
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
cmp-android/build.gradle.kts(2 hunks)cmp-android/src/main/kotlin/cmp/android/app/ComponentActivityExtensions.kt(3 hunks)cmp-android/src/main/res/values/strings.xml(1 hunks)core/designsystem/src/commonMain/composeResources/values/strings.xml(1 hunks)core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosBasicDialog.kt(3 hunks)core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosBreadCrumb.kt(3 hunks)core/ui/src/commonMain/kotlin/com/mifos/core/ui/util/Constants.kt(1 hunks)feature/client/src/commonMain/composeResources/values/strings.xml(1 hunks)feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsScreen.kt(4 hunks)feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditScreen.kt(3 hunks)feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditViewModel.kt(5 hunks)feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileScreen.kt(3 hunks)feature/client/src/commonMain/kotlin/com/mifos/feature/client/utils/Constants.kt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditScreen.kt (1)
core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosBasicDialog.kt (1)
MifosStatusDialog(217-278)
🔇 Additional comments (6)
cmp-android/build.gradle.kts (1)
10-10: Formatting improvements align with Kotlin/Android conventions.The addition of a blank line after the license header (line 10) and the newline at end-of-file (line 158) follow standard Kotlin style practices and improve visual separation between the header block and code logic.
Also applies to: 158-158
core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosBreadCrumb.kt (2)
85-88: LGTM! Profile refresh signal correctly propagated on breadcrumb navigation.The implementation correctly sets the refresh flag in the previous back stack entry's saved state before navigating, ensuring the target screen can detect and respond to profile updates.
101-106: LGTM! Refresh signal properly set on back navigation.The back button handler correctly propagates the refresh signal to the previous screen before popping the back stack.
cmp-android/src/main/kotlin/cmp/android/app/ComponentActivityExtensions.kt (1)
56-62: LGTM! Formatting improved for readability.The multi-line formatting of the
SystemBarStyle.autocall improves code readability without changing behavior.feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsScreen.kt (1)
95-108: LGTM! Profile refresh mechanism implemented correctly.The implementation properly:
- Observes the refresh flag from saved state
- Triggers a retry action when the flag is set
- Resets the flag after handling
- Provides a fallback for null savedStateHandle
This enables the screen to refresh profile details when navigating back after an update.
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileScreen.kt (1)
67-80: LGTM! Consistent profile refresh implementation.The refresh mechanism is correctly implemented and consistent with the pattern used in ClientProfileDetailsScreen. The implementation properly observes the saved state flag, triggers a retry, and resets the flag after handling.
core/designsystem/src/commonMain/composeResources/values/strings.xml
Outdated
Show resolved
Hide resolved
core/ui/src/commonMain/kotlin/com/mifos/core/ui/util/Constants.kt
Outdated
Show resolved
Hide resolved
.../src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditScreen.kt
Show resolved
Hide resolved
...c/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditViewModel.kt
Show resolved
Hide resolved
feature/client/src/commonMain/kotlin/com/mifos/feature/client/utils/Constants.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsScreen.kt (1)
95-108: Duplicate refresh logic; consider the extraction suggested for ClientProfileScreen.This implementation is nearly identical to the refresh mechanism in
ClientProfileScreen.kt(lines 67-80). As mentioned in the previous file's review, extracting this pattern into a reusable composable function would eliminate the duplication and improve maintainability.
🧹 Nitpick comments (1)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileScreen.kt (1)
67-80: Profile refresh mechanism works correctly; consider extracting duplicate logic.The implementation correctly observes the refresh flag from saved state and triggers a retry action. The fallback to
remember { mutableStateOf(false) }is safe, though it creates an unchangeable local state when savedStateHandle is null.Since this exact pattern is duplicated in
ClientProfileDetailsScreen.kt(lines 95-108), consider extracting it into a reusable composable function to improve maintainability.Example extraction:
@Composable fun rememberProfileRefreshTrigger( navController: NavController, onRefresh: () -> Unit ): Boolean { val currentBackStackEntry = navController.currentBackStackEntry val savedStateHandle = currentBackStackEntry?.savedStateHandle val profileUpdated by savedStateHandle ?.getStateFlow(PROFILE_SHOULD_REFRESH_KEY, false) ?.collectAsStateWithLifecycle(initialValue = false) ?: remember { mutableStateOf(false) } LaunchedEffect(profileUpdated) { if (profileUpdated) { onRefresh() savedStateHandle?.set(PROFILE_SHOULD_REFRESH_KEY, false) } } return profileUpdated }Then use it as:
rememberProfileRefreshTrigger(navController) { viewModel.trySendAction(ClientProfileAction.OnRetry) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
core/ui/src/commonMain/kotlin/com/mifos/core/ui/util/Constants.kt(1 hunks)feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsScreen.kt(3 hunks)feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileScreen.kt(3 hunks)
🔇 Additional comments (3)
core/ui/src/commonMain/kotlin/com/mifos/core/ui/util/Constants.kt (1)
1-3: LGTM! Clean constant definition.The public constant establishes a single source of truth for the profile refresh key, addressing the previous duplication issue effectively.
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileScreen.kt (1)
30-30: LGTM! Appropriate imports for the refresh mechanism.The imports support the inter-screen refresh functionality using Compose lifecycle and saved state.
Also applies to: 32-32, 45-45
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsScreen.kt (1)
47-47: LGTM! Appropriate imports for the refresh mechanism.The imports support the inter-screen refresh functionality using Compose lifecycle and saved state.
Also applies to: 49-49, 68-68
| @@ -0,0 +1,3 @@ | |||
| package com.mifos.core.ui.util | |||
|
|
|||
| const val PROFILE_SHOULD_REFRESH_KEY = "profile_should_refresh" No newline at end of file | |||
There was a problem hiding this comment.
Creating a whole file just for one const is unnecessary. Check core/common, we already have Constants there; if we need it, put it in there and use it.
|
Okay, I will do that
Thank you Pronay
…On Sat, Jan 3, 2026 at 13:56 Pronay Sarker ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In core/ui/src/commonMain/kotlin/com/mifos/core/ui/util/Constants.kt
<#2519 (comment)>
:
> @@ -0,0 +1,3 @@
+package com.mifos.core.ui.util
+
+const val PROFILE_SHOULD_REFRESH_KEY = "profile_should_refresh"
Creating a whole file just for one const is unnecessary. Check core/common,
we already have Constants there; if we need it, put it in there and use it.
—
Reply to this email directly, view it on GitHub
<#2519 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AY3O3OMBVTIUY27LTVLMB534E64BDAVCNFSM6AAAAACINTCI4OVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTMMRUGMZDSOJWGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@ufuomaisaac, the navigation flag approach is fine but i think it depends on the navigation backstack. I thought of using a repository level broadcast instead so the profile screen automatically refreshes whenever the client is updated without relying on navigation data. this should help us keep the screens independent and makes it easier to handle updates if more screens change the client in the future. also it wont require much changes to multiple files. |
|
A repository level board cast? Also I change a few things, which is the constants, added a new dialog because I was given a new dialog to display I would indulge you to explain your approach and what you mean by Repository level board cast, also I would appreciate if you discuss your approach on the group so that people can also contribute to the approach we are to use, moving forward |
|
I mean exposing a signal from the This way the UI does not need to know where the update came from or worry about passing flags back through the backstack. |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
feature/client/src/commonMain/composeResources/values/strings.xmlfeature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileScreen.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- feature/client/src/commonMain/composeResources/values/strings.xml
| import androidx.compose.material3.MaterialTheme | ||
| import androidx.compose.material3.Text | ||
| import androidx.compose.runtime.Composable | ||
| import androidx.compose.runtime.LaunchedEffect |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Locate Constants file and ClientDetailRepository for refactoring verification
# Find Constants file in core/common
echo "=== Searching for Constants file in core/common ==="
fd -t f "Constants" core/common 2>/dev/null || echo "No Constants file found in core/common"
# Find ClientDetailRepository
echo -e "\n=== Searching for ClientDetailRepository ==="
fd -t f "ClientDetailRepository" 2>/dev/null || echo "No ClientDetailRepository found"
# Check current usage of PROFILE_SHOULD_REFRESH_KEY
echo -e "\n=== Current usage of PROFILE_SHOULD_REFRESH_KEY ==="
rg -n "PROFILE_SHOULD_REFRESH_KEY" --type=kotlin 2>/dev/null || echo "No usage of PROFILE_SHOULD_REFRESH_KEY found"
# Check what core/ui/util contains
echo -e "\n=== Files in core/ui/util ==="
fd -t f . core/ui/util 2>/dev/null | head -20
# Check if PROFILE_SHOULD_REFRESH_KEY is defined in the ClientProfileScreen file itself
echo -e "\n=== Checking ClientProfileScreen.kt for PROFILE_SHOULD_REFRESH_KEY definition ==="
fd -t f "ClientProfileScreen.kt" -x cat {} \; 2>/dev/null | grep -n "PROFILE_SHOULD_REFRESH_KEY" | head -5Repository: openMF/android-client
Length of output: 2235
Move PROFILE_SHOULD_REFRESH_KEY constant to core/common/Constants.kt.
The constant is currently defined in core/ui/util/Constants.kt but should be consolidated in the core/common/Constants.kt file for better organization. It is already used across multiple files including MifosBreadCrumb.kt, making a single canonical location important for maintainability.
🤖 Prompt for AI Agents
In
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileScreen.kt
around line 30, the comment indicates PROFILE_SHOULD_REFRESH_KEY constant
currently lives in core/ui/util/Constants.kt but must be moved to
core/common/Constants.kt; move the constant declaration into
core/common/Constants.kt (creating it there if absent), delete the duplicate
from core/ui/util/Constants.kt, and update all files that import or reference it
(e.g., MifosBreadCrumb.kt and this ClientProfileScreen file) to import from
core.common.Constants instead; after changes, run a project build to fix any
import errors and ensure no other references remain to the old location.
| val currentBackStackEntry = navController.currentBackStackEntry | ||
| val savedStateHandle = currentBackStackEntry?.savedStateHandle | ||
|
|
||
| val profileUpdated by savedStateHandle | ||
| ?.getStateFlow(PROFILE_SHOULD_REFRESH_KEY, false) | ||
| ?.collectAsStateWithLifecycle(initialValue = false) | ||
| ?: remember { mutableStateOf(false) } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "ClientProfileScreen.kt" -x cat -n {}Repository: openMF/android-client
Length of output: 11059
🏁 Script executed:
rg -t kotlin "ClientDetailRepository|PROFILE_SHOULD_REFRESH_KEY" --max-count=20Repository: openMF/android-client
Length of output: 1648
🏁 Script executed:
fd -t f "ClientDetail" -o "Client.*Repository" | head -20Repository: openMF/android-client
Length of output: 220
🏁 Script executed:
fd -t f "Repository" -i | grep -i "client\|detail" | head -20Repository: openMF/android-client
Length of output: 1972
🏁 Script executed:
cat -n core/data/src/commonMain/kotlin/com/mifos/core/data/repository/ClientDetailsRepository.kt | head -100Repository: openMF/android-client
Length of output: 3086
🏁 Script executed:
cat -n feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsScreen.kt | head -100Repository: openMF/android-client
Length of output: 6023
🏁 Script executed:
cat -n feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsScreen.kt | sed -n '98,115p'Repository: openMF/android-client
Length of output: 806
🏁 Script executed:
git log --oneline --all | grep -i "profile\|refresh" | head -20Repository: openMF/android-client
Length of output: 1058
🏁 Script executed:
rg -t kotlin "remember.*mutableStateOf\|collectAsStateWithLifecycle" feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ -A 2 -B 2Repository: openMF/android-client
Length of output: 47
🏁 Script executed:
rg -t kotlin "savedStateHandle.*getStateFlow" -A 3 -B 1Repository: openMF/android-client
Length of output: 9105
🏁 Script executed:
rg -t kotlin "PROFILE_SHOULD_REFRESH_KEY.*true\|set.*PROFILE_SHOULD_REFRESH_KEY" -B 3 -A 1Repository: openMF/android-client
Length of output: 47
🏁 Script executed:
rg -t kotlin "PROFILE_SHOULD_REFRESH_KEY" -B 2 -A 2 | head -60Repository: openMF/android-client
Length of output: 6894
🏁 Script executed:
cat -n core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosBreadCrumb.kt | sed -n '1,50p'Repository: openMF/android-client
Length of output: 2470
🏁 Script executed:
cat -n core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosBreadCrumb.kt | sed -n '50,120p'Repository: openMF/android-client
Length of output: 3129
🏁 Script executed:
cat -n feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileScreen.kt | sed -n '66,82p'Repository: openMF/android-client
Length of output: 839
🏁 Script executed:
git log --all --grep="kartikey004\|profile.*refresh\|broadcast" --oneline | head -20Repository: openMF/android-client
Length of output: 495
Navigation backstack coupling limits refresh reliability and duplicates pattern across screens.
Two substantive concerns:
-
Null safety: If
savedStateHandleis null, the fallbackremember { mutableStateOf(false) }creates a state that remains false indefinitely. The refresh mechanism silently fails with no indication. This occurs in navigation scenarios wherecurrentBackStackEntryis null. -
Architectural coupling: This refresh mechanism is tightly coupled to the navigation backstack. The
MifosBreadCrumbcomponent setsPROFILE_SHOULD_REFRESH_KEY = trueonpreviousBackStackEntry?.savedStateHandlewhen navigating back, and the screen reads fromcurrentBackStackEntry?.savedStateHandle. Changes to navigation structure or backstack behavior could break the refresh flow. The same pattern is duplicated acrossClientProfileScreenandClientProfileDetailsScreen.
A repository-level event or state flow (e.g., from ClientDetailsRepository) that emits when client data is updated would be more robust and independent of navigation changes.
🤖 Prompt for AI Agents
In
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileScreen.kt
around lines 68-74, the screen currently reads PROFILE_SHOULD_REFRESH_KEY off
the NavController backstack (which can be null and falls back to a non-updating
mutableStateOf(false)) causing silent failures and tight coupling to navigation;
replace this with a repository-level StateFlow/SharedFlow in
ClientDetailsRepository (or a shared ViewModel) that emits when client data is
updated, have both ClientProfileScreen and ClientProfileDetailsScreen collect
that flow via collectAsStateWithLifecycle (no null fallback to mutableStateOf),
and remove the duplicated backstack key logic so refresh is driven by the
repository event; ensure the repository exposes a hot flow
(StateFlow/SharedFlow) initialized to false and that the component which
performs updates emits to it after successful updates.
| LaunchedEffect(profileUpdated) { | ||
| if (profileUpdated) { | ||
| viewModel.trySendAction(ClientProfileAction.OnRetry) | ||
| savedStateHandle?.set(PROFILE_SHOULD_REFRESH_KEY, false) // reset after refresh | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Recommend repository-level signal over navigation-flag pattern.
While this LaunchedEffect implementation is functionally correct for the navigation-flag approach, it's a workaround that requires manual flag management across navigation flows. As kartikey004 suggested in PR comments, exposing a signal from ClientDetailRepository that emits whenever client data is updated would allow this screen (and any other screen displaying client data) to listen and refresh independently of navigation origin.
Benefits of repository-level approach:
- Decouples refresh logic from navigation structure
- Automatically notifies all observers when client data changes
- Eliminates manual flag reset logic
- More maintainable and scalable
Based on PR discussion feedback about repository-level broadcast approach.
Do you want me to generate a repository-level implementation pattern using Kotlin Flow/StateFlow that emits client update events from ClientDetailRepository? This would replace the navigation-flag mechanism with a more robust observer pattern.
🤖 Prompt for AI Agents
In
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileScreen.kt
around lines 76-81, replace the navigation-flag LaunchedEffect pattern with a
repository-level update signal: have ClientDetailRepository expose a shared
Flow/StateFlow (e.g., SharedFlow<Unit> or StateFlow<Client?>) that emits when
client data is updated, remove savedStateHandle flag set/reset logic, and in the
ViewModel collect that repository flow and map emissions to
ClientProfileAction.OnRetry or a dedicated refresh action; update the screen to
observe ViewModel state (no manual flag management) and ensure the repository
emits updates whenever client data changes, using a thread-safe
MutableSharedFlow or MutableStateFlow with proper scope.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and examine ClientDetailRepository
fd -t f "ClientDetailRepository" --exec cat {}
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Examine ClientProfileViewModel to understand current state management
cat feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileViewModel.kt
Length of output: 6394
🏁 Script executed:
#!/bin/bash
# Examine ClientProfileEditViewModel to see where image upload happens
cat feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditViewModel.kt
Length of output: 8566
🏁 Script executed:
#!/bin/bash
# Search for repository usage patterns to understand data layer structure
rg -n -C3 "class.*Repository" --type=kt -g '!**/test/**' | head -100
Length of output: 26
🏁 Script executed:
#!/bin/bash
# Find ClientDetailsRepository with correct search
fd -t f -e kt "ClientDetailsRepository"
Length of output: 185
🏁 Script executed:
#!/bin/bash
# Search for ClientDetailsRepository class definition
rg -n "class ClientDetailsRepository" --type-add 'kt:*.kt' --type=kt
Length of output: 132
🏁 Script executed:
#!/bin/bash
# Alternative: search for any file containing ClientDetailsRepository
rg -n "ClientDetailsRepository" --type-add 'kt:*.kt' --type=kt -l | head -10
Length of output: 1073
🏁 Script executed:
#!/bin/bash
# Get the actual repository file
rg -n "interface ClientDetailsRepository|class ClientDetailsRepository" --type-add 'kt:*.kt' --type=kt -A20 | head -80
Length of output: 5572
🏁 Script executed:
#!/bin/bash
# Get the full ClientDetailsRepository interface
cat core/data/src/commonMain/kotlin/com/mifos/core/data/repository/ClientDetailsRepository.kt
Length of output: 2496
🏁 Script executed:
#!/bin/bash
# Get the full ClientDetailsRepositoryImp implementation
cat core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsRepositoryImp.kt
Length of output: 7105
🏁 Script executed:
#!/bin/bash
# Look for getImage implementation to understand data flow
rg -n "fun getImage" --type-add 'kt:*.kt' --type=kt -A10 | head -40
Length of output: 2848
@ufuomaisaac Great! I'll generate a complete repository-level implementation using Kotlin Flow. This will replace the navigation-flag pattern with a more robust observer pattern.
Implementation Guide
1. Update ClientDetailsRepository interface
Add a SharedFlow that emits whenever client data is updated:
// core/data/src/commonMain/kotlin/com/mifos/core/data/repository/ClientDetailsRepository.kt
interface ClientDetailsRepository {
// Add this property to expose client update events
val clientDataUpdated: SharedFlow<Int> // Emits clientId when updated
suspend fun uploadClientImage(clientId: Int, image: MultiPartFormDataContent)
suspend fun deleteClientImage(clientId: Int)
// ... rest of existing methods
}2. Update ClientDetailsRepositoryImp implementation
Implement the SharedFlow and emit after data changes:
// core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsRepositoryImp.kt
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.SharedFlow
import kotlinx.coroutines.flow.asSharedFlow
class ClientDetailsRepositoryImp(
private val dataManagerClient: DataManagerClient,
) : ClientDetailsRepository {
// Add this: MutableSharedFlow for internal emissions
private val _clientDataUpdated = MutableSharedFlow<Int>(
replay = 0,
extraBufferCapacity = 1
)
// Expose as read-only SharedFlow
override val clientDataUpdated: SharedFlow<Int> = _clientDataUpdated.asSharedFlow()
override suspend fun uploadClientImage(clientId: Int, image: MultiPartFormDataContent) {
dataManagerClient.uploadClientImage(clientId, image)
// Emit update signal after successful upload
_clientDataUpdated.tryEmit(clientId)
}
override suspend fun deleteClientImage(clientId: Int) {
dataManagerClient.deleteClientImage(clientId)
// Emit update signal after successful delete
_clientDataUpdated.tryEmit(clientId)
}
// ... rest of existing methods remain unchanged
}3. Update ClientProfileViewModel
Observe the repository signal and trigger refresh automatically:
// feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileViewModel.kt
internal class ClientProfileViewModel(
savedStateHandle: SavedStateHandle,
private val getClientDetailsUseCase: GetClientDetailsUseCase,
private val clientDetailsRepo: ClientDetailsRepository,
private val networkMonitor: NetworkMonitor,
) : BaseViewModel<ClientProfileState, ClientProfileEvent, ClientProfileAction>(
initialState = ClientProfileState(),
) {
private val route = savedStateHandle.toRoute<ClientProfileRoute>()
init {
getClientAndObserveNetwork()
observeClientUpdates() // Add this
}
// Add this method to observe repository updates
private fun observeClientUpdates() {
viewModelScope.launch {
clientDetailsRepo.clientDataUpdated
.filter { updatedClientId -> updatedClientId == route.id }
.collect {
// Automatically refresh when this client's data is updated
loadClientDetailsAndImage(route.id)
}
}
}
// ... rest of existing methods remain unchanged
}4. Update ClientProfileScreen
Remove the navigation-flag logic entirely:
// feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileScreen.kt
@Composable
internal fun ClientProfileScreen(
notes: (Int) -> Unit,
navController: NavController,
documents: (Int) -> Unit,
viewAssociatedAccounts: (Int) -> Unit,
identifiers: (Int) -> Unit,
onNavigateBack: () -> Unit,
navigateToClientDetailsScreen: (Int) -> Unit,
viewAddress: (Int) -> Unit,
modifier: Modifier = Modifier,
viewModel: ClientProfileViewModel = koinViewModel(),
pinpointLocation: (Int) -> Unit,
) {
val state by viewModel.stateFlow.collectAsStateWithLifecycle()
// Remove these lines:
// val currentBackStackEntry = navController.currentBackStackEntry
// val savedStateHandle = currentBackStackEntry?.savedStateHandle
// val profileUpdated by savedStateHandle...
// LaunchedEffect(profileUpdated) { ... }
EventsEffect(viewModel.eventFlow) { event ->
// ... existing event handling
}
ClientProfileScaffold(
modifier = modifier,
state = state,
onAction = remember(viewModel) { { viewModel.trySendAction(it) } },
navController = navController,
)
ClientProfileDialogs(
state = state,
onRetry = remember(viewModel) {
{
viewModel.trySendAction(ClientProfileAction.OnRetry)
}
},
)
}5. Apply the same pattern to ClientProfileDetailsScreen
Remove the flag-based logic from ClientProfileDetailsScreen.kt as well, since it will now automatically refresh via the repository signal.
6. Optional: Clean up breadcrumb navigation
If no other features use PROFILE_SHOULD_REFRESH_KEY, you can remove the flag-setting logic from:
core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosBreadCrumb.ktcore/ui/src/commonMain/kotlin/com/mifos/core/ui/util/Constants.kt(delete the constant)
Benefits of this approach:
✅ Decoupled: Refresh logic is independent of navigation structure
✅ Automatic: Any screen displaying client data automatically refreshes when updated
✅ Scalable: Easy to add more observers (e.g., ClientDetailsScreen, ClientProfileDetailsScreen)
✅ Maintainable: No manual flag management or reset logic needed
✅ Type-safe: Emits clientId so observers can filter for their specific client
✅ Thread-safe: SharedFlow handles concurrent emissions safely
Would you like me to:
- Generate a complete diff for all the files mentioned above?
- Help extend this pattern to other client data mutations (e.g., staff assignment, transfer)?
- Create a similar pattern for other entities in the codebase?
🧬 Code Graph Analysis Results
- Filename: core/ui/src/commonMain/kotlin/com/mifos/core/ui/util/EventsEffect.kt, lines 28-43
@Composable
fun <E> EventsEffect(
eventFlow: Flow<E>,
lifecycleOwner: Lifecycle = LocalLifecycleOwner.current.lifecycle,
handler: suspend (E) -> Unit,
) {
LaunchedEffect(key1 = Unit) {
eventFlow
.filter {
it is BackgroundEvent ||
lifecycleOwner.currentState.isAtLeast(Lifecycle.State.RESUMED)
}
.onEach { handler.invoke(it) }
.launchIn(this)
}
}
- Filename: core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosErrorComponent.kt, lines 41-60
@Composable
fun MifosErrorComponent(
modifier: Modifier = Modifier,
isNetworkConnected: Boolean = true,
message: String? = null,
isEmptyData: Boolean = false,
isRetryEnabled: Boolean = false,
onRetry: () -> Unit = {},
) {
when {
!isNetworkConnected -> NoInternetComponent(isRetryEnabled = isRetryEnabled) { onRetry() }
else -> EmptyDataComponent(
modifier = modifier,
isEmptyData = isEmptyData,
message = message,
isRetryEnabled = isRetryEnabled,
onRetry = onRetry,
)
}
}
- Filename: core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosProgressIndicator.kt, lines 41-68
@Composable
fun MifosProgressIndicator(
modifier: Modifier = Modifier.fillMaxSize(),
) {
val composition by rememberLottieComposition {
LottieCompositionSpec.JsonString(
Res.readBytes(LottieConstants.LOADING_ANIMATION).decodeToString(),
)
}
val progress by animateLottieCompositionAsState(
composition,
iterations = Int.MAX_VALUE,
)
Box(
modifier = modifier
.background(AppColors.customWhite),
contentAlignment = Alignment.Center,
) {
Image(
painter = rememberLottiePainter(
composition = composition,
progress = { progress },
),
contentDescription = "Lottie animation",
)
}
}
- Filename: feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/components/ProfileCard.kt, lines 37-94
@Composable
fun ProfileCard(
image: ByteArray?,
name: String,
accountNo: String,
office: String,
onClick: () -> Unit,
) {
MifosCard(
modifier = Modifier.clickable {
onClick()
},
colors = CardColors(
containerColor = MaterialTheme.colorScheme.primary,
contentColor = AppColors.customWhite,
disabledContainerColor = MaterialTheme.colorScheme.primary,
disabledContentColor = AppColors.customWhite,
),
) {
Row(
Modifier.fillMaxWidth()
.padding(DesignToken.padding.largeIncreasedExtra),
verticalAlignment = Alignment.CenterVertically,
) {
MifosUserImage(
bitmap = image,
modifier = Modifier.size(DesignToken.sizes.avatarLarge),
)
Spacer(Modifier.width(DesignToken.padding.medium))
Column(
Modifier.weight(1f),
verticalArrangement = Arrangement.spacedBy(DesignToken.padding.extraExtraSmall),
) {
Text(
text = name,
style = MifosTypography.titleMediumEmphasized,
)
Text(
text = "Acc. No. $accountNo",
style = MifosTypography.bodySmall,
color = MaterialTheme.colorScheme.secondaryContainer,
)
Text(
text = office,
style = MifosTypography.bodySmall,
color = MaterialTheme.colorScheme.secondaryContainer,
)
}
Spacer(Modifier.width(DesignToken.padding.medium))
Icon(
imageVector = MifosIcons.ChevronRight,
contentDescription = null,
modifier = Modifier.size(DesignToken.sizes.iconSmall),
)
}
}
}
- Filename: core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosBreadCrumb.kt, lines 37-114
@Composable
fun MifosBreadcrumbNavBar(
navController: NavController,
modifier: Modifier = Modifier,
) {
val backStack = navController.currentBackStack.value
.mapNotNull { it.destination.route }
.filter { it.isNotBlank() }
.filter { it.contains("Route", ignoreCase = true) }
fun formatRoute(route: String): String {
var simpleName = route.substringBefore('?').substringAfterLast('.')
if (simpleName.contains("Route", ignoreCase = true)) {
simpleName = simpleName.substringBefore("Route") + "Route"
}
if (simpleName.contains("Screen", ignoreCase = true)) {
simpleName = simpleName.substringBefore("Screen") + "Screen"
}
simpleName = simpleName.removeSuffix("Route").removeSuffix("Screen")
return simpleName.replace(Regex("([a-z])([A-Z])"), "$1 $2")
}
Row(
verticalAlignment = Alignment.CenterVertically,
modifier = modifier.fillMaxWidth()
.padding(horizontal = DesignToken.padding.large)
.padding(vertical = DesignToken.padding.large),
) {
val routes = if (backStack.size <= 3) {
backStack
} else {
backStack.take(2) + listOf("...") + backStack.takeLast(1)
}
Row(Modifier.weight(1f)) {
routes.forEachIndexed { index, route ->
when (route) {
"..." -> Text(" ... > ", style = MifosTypography.bodySmallEmphasized)
else -> {
BreadcrumbItem(
text = formatRoute(route),
isActive = index == routes.lastIndex,
onClick = {
if (index != routes.lastIndex && route != "...") {
navController.previousBackStackEntry
?.savedStateHandle
?.set(PROFILE_SHOULD_REFRESH_KEY, true)
navController.popBackStack(route, inclusive = false)
}
},
)
if (index != routes.lastIndex) {
Text(" > ", style = MifosTypography.bodySmallEmphasized)
}
}
}
}
}
IconButton(onClick = {
navController.previousBackStackEntry
?.savedStateHandle
?.set(PROFILE_SHOULD_REFRESH_KEY, true)
navController.popBackStack()
}) {
Icon(
painter = painterResource(Res.drawable.bread_crumb_back_icon),
contentDescription = "Back",
modifier = Modifier.size(DesignToken.sizes.iconSmall),
)
}
}
}
- Filename: core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosRowCard.kt, lines 42-90
@OptIn(ExperimentalMaterial3Api::class)
@Composable
fun MifosRowCard(
modifier: Modifier = Modifier,
title: String,
leftValues: List<TextUtil>,
rightValues: List<TextUtil>,
onClick: () -> Unit,
) {
MifosListingComponentOutline {
Row(
modifier
.fillMaxWidth()
.clickable { onClick() },
horizontalArrangement = Arrangement.SpaceBetween,
verticalAlignment = Alignment.CenterVertically,
) {
Column(
verticalArrangement = Arrangement.spacedBy(DesignToken.padding.extraExtraSmall),
) {
PrintTextUtil(
TextUtil(
text = title,
style = MifosTypography.titleSmallEmphasized,
),
)
leftValues.forEach {
PrintTextUtil(it)
}
}
Row(
verticalAlignment = Alignment.CenterVertically,
) {
Column(
verticalArrangement = Arrangement.spacedBy(DesignToken.padding.extraExtraSmall),
) {
rightValues.forEach {
PrintTextUtil(it)
}
}
Icon(
imageVector = MifosIcons.ChevronRight,
contentDescription = null,
modifier = Modifier.size(DesignToken.sizes.iconSmall),
)
}
}
}
}
- Filename: core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosRowCard.kt, lines 92-163
@Composable
fun MifosRowCard(
title: String,
leftValues: List<TextUtil>,
rightValues: List<TextUtil>,
modifier: Modifier = Modifier,
byteArray: ByteArray? = null,
imageVector: DrawableResource? = null,
) {
Row(
modifier
.fillMaxWidth(),
verticalAlignment = Alignment.CenterVertically,
) {
if (imageVector == null) {
MifosUserImage(
bitmap = byteArray,
modifier = Modifier.size(DesignToken.sizes.iconExtraLarge),
)
} else {
Icon(
painter = painterResource(imageVector),
modifier = Modifier
.size(DesignToken.sizes.iconExtraLarge)
.background(
color = MaterialTheme.colorScheme.surfaceBright,
shape = CircleShape,
)
.padding(DesignToken.padding.small),
contentDescription = null,
)
}
Spacer(Modifier.width(DesignToken.padding.medium))
Column(
modifier = Modifier.weight(1f),
verticalArrangement = Arrangement.spacedBy(DesignToken.padding.extraExtraSmall),
) {
PrintTextUtil(
TextUtil(
text = title,
style = MifosTypography.titleSmallEmphasized,
color = MaterialTheme.colorScheme.onSurface,
),
)
leftValues.forEach {
PrintTextUtil(
it,
)
}
}
Spacer(Modifier.width(DesignToken.padding.medium))
Row(
verticalAlignment = Alignment.CenterVertically,
) {
Column(
verticalArrangement = Arrangement.spacedBy(DesignToken.padding.extraExtraSmall),
horizontalAlignment = Alignment.End,
) {
rightValues.forEach {
PrintTextUtil(
it,
)
}
}
Icon(
imageVector = MifosIcons.ChevronRight,
contentDescription = null,
modifier = Modifier.size(DesignToken.sizes.iconMiny),
)
}
}
}
There was a problem hiding this comment.
Also solve for ClientProfileEditScreen and ClientProfileDetailsScreen
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Examine ClientProfileDetailsScreen to see current flag-based logic
cat feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsScreen.kt
Length of output: 18647
🏁 Script executed:
#!/bin/bash
# Examine ClientProfileEditScreen to understand current implementation
cat feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditScreen.kt
Length of output: 12534
🏁 Script executed:
#!/bin/bash
# Check ClientProfileDetailsViewModel for refresh logic
cat feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsViewModel.kt
Length of output: 12791
@ufuomaisaac Perfect! I'll provide the complete implementation for both screens. The repository changes are already done, so we just need to update the ViewModels and Screens.
Complete Implementation for All Three Screens
1. ClientProfileDetailsViewModel - Add Repository Observer
Update ClientProfileDetailsViewModel to automatically observe client data updates:
// feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsViewModel.kt
import kotlinx.coroutines.flow.filter
internal class ClientProfileDetailsViewModel(
savedStateHandle: SavedStateHandle,
private val getClientDetailsUseCase: GetClientDetailsUseCase,
private val clientDetailsRepo: ClientDetailsRepository,
private val networkMonitor: NetworkMonitor,
) : BaseViewModel<ClientProfileDetailsState, ClientProfileDetailsEvent, ClientProfileDetailsAction>(
initialState = ClientProfileDetailsState(),
) {
private val route = savedStateHandle.toRoute<ClientProfileDetailsRoute>()
init {
getClientAndObserveNetwork()
observeClientUpdates() // Add this line
}
// Add this method
private fun observeClientUpdates() {
viewModelScope.launch {
clientDetailsRepo.clientDataUpdated
.filter { updatedClientId -> updatedClientId == route.id }
.collect {
// Automatically refresh when this client's data is updated
loadClientDetailsAndImage(route.id)
}
}
}
private fun getClientAndObserveNetwork() {
observeNetwork()
loadClientDetailsAndImage(route.id)
}
// ... rest of the ViewModel remains unchanged
}2. ClientProfileDetailsScreen - Remove Flag Logic
Remove the navigation-flag pattern entirely:
// feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsScreen.kt
@Composable
internal fun ClientProfileDetailsScreen(
navController: NavController,
onNavigateBack: () -> Unit,
navigateToUpdatePhoto: (Int, String, String) -> Unit,
navigateToAssignStaff: (Int) -> Unit,
navigateToUpdateDetails: (Int) -> Unit,
navigateToClientTransfer: (Int) -> Unit,
navigateToClientClosure: (Int) -> Unit,
navigateToApplyNewApplication: (Int, String) -> Unit,
navigateToUpdateDefaultAccount: (Int) -> Unit,
navigateToCollateral: (Int) -> Unit,
navigateToUpdateSignature: (Int, String, String) -> Unit,
modifier: Modifier = Modifier,
viewModel: ClientProfileDetailsViewModel = koinViewModel(),
navigateToAddCharge: (Int) -> Unit,
) {
val state by viewModel.stateFlow.collectAsStateWithLifecycle()
// REMOVE these lines (lines 95-106):
// val currentBackStackEntry = navController.currentBackStackEntry
// val savedStateHandle = currentBackStackEntry?.savedStateHandle
//
// val profileUpdated by savedStateHandle
// ?.getStateFlow(PROFILE_SHOULD_REFRESH_KEY, false)
// ?.collectAsStateWithLifecycle(initialValue = false)
// ?: remember { mutableStateOf(false) }
//
// LaunchedEffect(profileUpdated) {
// if (profileUpdated) {
// viewModel.trySendAction(ClientProfileDetailsAction.OnRetry)
// savedStateHandle?.set(PROFILE_SHOULD_REFRESH_KEY, false)
// }
// }
EventsEffect(viewModel.eventFlow) { event ->
when (event) {
ClientProfileDetailsEvent.NavigateBack -> {
onNavigateBack.invoke()
}
// ... rest of event handling remains unchanged
}
}
ClientProfileDetailsScaffold(
navController = navController,
modifier = modifier,
state = state,
onAction = remember(viewModel) { { viewModel.trySendAction(it) } },
)
}Also remove the import at the top of the file:
// Remove this import:
// import com.mifos.core.ui.util.PROFILE_SHOULD_REFRESH_KEY3. ClientProfileEditScreen - Navigation After Success
The screen already handles success/error dialogs properly. Just ensure the dialogs navigate back after dismissal:
// feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditScreen.kt
@Composable
internal fun ClientProfileEditScreen(
onNavigateBack: () -> Unit,
navController: NavController,
modifier: Modifier = Modifier,
viewModel: ClientProfileEditViewModel = koinViewModel(),
) {
val state by viewModel.stateFlow.collectAsStateWithLifecycle()
ClientProfileEditScaffold(
modifier = modifier,
state = state,
onAction = remember(viewModel) { { viewModel.trySendAction(it) } },
navController = navController,
)
ClientProfileEditDialogs(
state = state,
onRetry = remember(viewModel) {
{ viewModel.trySendAction(ClientProfileEditAction.OnRetry) }
},
onAction = remember(viewModel) { { viewModel.trySendAction(it) } },
onNavigateBack = onNavigateBack, // Add this parameter
)
}
@OptIn(ExperimentalMaterial3Api::class)
@Composable
private fun ClientProfileEditDialogs(
state: ClientProfileEditState,
onRetry: () -> Unit,
onAction: (ClientProfileEditAction) -> Unit,
onNavigateBack: () -> Unit, // Add this parameter
) {
when (state.dialogState) {
is ClientProfileEditState.DialogState.Loading -> MifosProgressIndicator()
is ClientProfileEditState.DialogState.Error -> {
MifosStatusDialog(
status = MifosDialogStatus.FAILURE,
message = stringResource(Res.string.feature_client_error),
onDismissRequest = {
onAction(ClientProfileEditAction.DismissModalBottomSheet)
// Don't navigate back on error, let user retry
}
)
}
is ClientProfileEditState.DialogState.Success -> {
MifosStatusDialog(
status = MifosDialogStatus.SUCCESS,
message = stringResource(Res.string.feature_client_Image_Upload_Successful),
onDismissRequest = {
onAction(ClientProfileEditAction.DismissModalBottomSheet)
onNavigateBack() // Navigate back after successful upload
}
)
}
// ... rest remains unchanged
}
}4. Clean Up Breadcrumb Navigation (Optional but Recommended)
Remove the flag-setting logic from MifosBreadCrumb.kt:
// core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosBreadCrumb.kt
@Composable
fun MifosBreadcrumbNavBar(
navController: NavController,
modifier: Modifier = Modifier,
) {
// ... existing code ...
Row(Modifier.weight(1f)) {
routes.forEachIndexed { index, route ->
when (route) {
"..." -> Text(" ... > ", style = MifosTypography.bodySmallEmphasized)
else -> {
BreadcrumbItem(
text = formatRoute(route),
isActive = index == routes.lastIndex,
onClick = {
if (index != routes.lastIndex && route != "...") {
// REMOVE these lines:
// navController.previousBackStackEntry
// ?.savedStateHandle
// ?.set(PROFILE_SHOULD_REFRESH_KEY, true)
navController.popBackStack(route, inclusive = false)
}
},
)
if (index != routes.lastIndex) {
Text(" > ", style = MifosTypography.bodySmallEmphasized)
}
}
}
}
}
IconButton(onClick = {
// REMOVE these lines:
// navController.previousBackStackEntry
// ?.savedStateHandle
// ?.set(PROFILE_SHOULD_REFRESH_KEY, true)
navController.popBackStack()
}) {
Icon(
painter = painterResource(Res.drawable.bread_crumb_back_icon),
contentDescription = "Back",
modifier = Modifier.size(DesignToken.sizes.iconSmall),
)
}
}Remove the import:
// Remove this import:
// import com.mifos.core.ui.util.PROFILE_SHOULD_REFRESH_KEY5. Delete the Constants File (Optional but Recommended)
Since PROFILE_SHOULD_REFRESH_KEY is no longer used anywhere, you can delete it:
// DELETE THIS FILE:
// core/ui/src/commonMain/kotlin/com/mifos/core/ui/util/Constants.ktSummary of Changes by Screen
✅ ClientProfileScreen (Already provided earlier)
- ViewModel: Added
observeClientUpdates() - Screen: Removed flag logic from lines 68-78
✅ ClientProfileDetailsScreen (New)
- ViewModel: Added
observeClientUpdates() - Screen: Removed flag logic from lines 95-106
✅ ClientProfileEditScreen (New)
- ViewModel: Already emits signals (done in PR)
- Screen: Navigate back after successful upload
- Dialogs: Updated to navigate back on success
✅ MifosBreadCrumb (Cleanup)
- Removed all
PROFILE_SHOULD_REFRESH_KEYflag-setting logic
✅ Constants.kt (Cleanup)
- Delete the entire file since it only contained the now-unused constant
How It Works Now
-
Upload Flow:
- User uploads photo in
ClientProfileEditScreen ClientProfileEditViewModeluploads via repository- Repository emits
clientDataUpdatedsignal with clientId - All three screens automatically refresh if displaying that client
- User uploads photo in
-
Navigation Flow:
- User navigates: ClientProfile → ClientProfileDetails → ClientProfileEdit
- After upload success → navigates back to ClientProfileDetails
- ClientProfileDetails automatically refreshes via repository signal
- User navigates back → ClientProfile automatically refreshes via repository signal
-
No manual flag management needed!
Migration Checklist
- Update
ClientProfileViewModel(add observeClientUpdates) - Update
ClientProfileScreen(remove flag logic) - Update
ClientProfileDetailsViewModel(add observeClientUpdates) - Update
ClientProfileDetailsScreen(remove flag logic) - Update
ClientProfileEditScreen(add onNavigateBack to dialogs) - Update
MifosBreadCrumb.kt(remove flag-setting logic) - Delete
core/ui/src/commonMain/kotlin/com/mifos/core/ui/util/Constants.kt - Test the full flow: Profile → Details → Edit → Upload → Back → Verify refresh
Would you like me to generate complete file diffs for all these changes?
🧬 Code Graph Analysis Results
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileScreen.kt
[Relevant code snippets that could assist understanding ClientProfileScreen.kt and related screens (ClientProfileEditScreen and ClientProfileDetailsScreen). Each entry includes the source filename and the snippet's line range.]
- core/ui/src/commonMain/kotlin/com/mifos/core/ui/util/EventsEffect.kt
Lines 28-43
@Composable
fun <E> EventsEffect(
eventFlow: Flow<E>,
lifecycleOwner: Lifecycle = LocalLifecycleOwner.current.lifecycle,
handler: suspend (E) -> Unit,
) {
LaunchedEffect(key1 = Unit) {
eventFlow
.filter {
it is BackgroundEvent ||
lifecycleOwner.currentState.isAtLeast(Lifecycle.State.RESUMED)
}
.onEach { handler.invoke(it) }
.launchIn(this)
}
}
- core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosErrorComponent.kt
Lines 41-60
@Composable
fun MifosErrorComponent(
modifier: Modifier = Modifier,
isNetworkConnected: Boolean = true,
message: String? = null,
isEmptyData: Boolean = false,
isRetryEnabled: Boolean = false,
onRetry: () -> Unit = {},
) {
when {
!isNetworkConnected -> NoInternetComponent(isRetryEnabled = isRetryEnabled) { onRetry() }
else -> EmptyDataComponent(
modifier = modifier,
isEmptyData = isEmptyData,
message = message,
isRetryEnabled = isRetryEnabled,
onRetry = onRetry,
)
}
}
- core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosProgressIndicator.kt
Lines 41-68
@Composable
fun MifosProgressIndicator(
modifier: Modifier = Modifier.fillMaxSize(),
) {
val composition by rememberLottieComposition {
LottieCompositionSpec.JsonString(
Res.readBytes(LottieConstants.LOADING_ANIMATION).decodeToString(),
)
}
val progress by animateLottieCompositionAsState(
composition,
iterations = Int.MAX_VALUE,
)
Box(
modifier = modifier
.background(AppColors.customWhite),
contentAlignment = Alignment.Center,
) {
Image(
painter = rememberLottiePainter(
composition = composition,
progress = { progress },
),
contentDescription = "Lottie animation",
)
}
}
- feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/components/ProfileCard.kt
Lines 37-94
@Composable
fun ProfileCard(
image: ByteArray?,
name: String,
accountNo: String,
office: String,
onClick: () -> Unit,
) {
MifosCard(
modifier = Modifier.clickable {
onClick()
},
colors = CardColors(
containerColor = MaterialTheme.colorScheme.primary,
contentColor = AppColors.customWhite,
disabledContainerColor = MaterialTheme.colorScheme.primary,
disabledContentColor = AppColors.customWhite,
),
) {
Row(
Modifier.fillMaxWidth()
.padding(DesignToken.padding.largeIncreasedExtra),
verticalAlignment = Alignment.CenterVertically,
) {
MifosUserImage(
bitmap = image,
modifier = Modifier.size(DesignToken.sizes.avatarLarge),
)
Spacer(Modifier.width(DesignToken.padding.medium))
Column(
Modifier.weight(1f),
verticalArrangement = Arrangement.spacedBy(DesignToken.padding.extraExtraSmall),
) {
Text(
text = name,
style = MifosTypography.titleMediumEmphasized,
)
Text(
text = "Acc. No. $accountNo",
style = MifosTypography.bodySmall,
color = MaterialTheme.colorScheme.secondaryContainer,
)
Text(
text = office,
style = MifosTypography.bodySmall,
color = MaterialTheme.colorScheme.secondaryContainer,
)
}
Spacer(Modifier.width(DesignToken.padding.medium))
Icon(
imageVector = MifosIcons.ChevronRight,
contentDescription = null,
modifier = Modifier.size(DesignToken.sizes.iconSmall),
)
}
}
}
- core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosBreadCrumb.kt
Lines 37-114
@Composable
fun MifosBreadcrumbNavBar(
navController: NavController,
modifier: Modifier = Modifier,
) {
val backStack = navController.currentBackStack.value
.mapNotNull { it.destination.route }
.filter { it.isNotBlank() }
.filter { it.contains("Route", ignoreCase = true) }
fun formatRoute(route: String): String {
var simpleName = route.substringBefore('?').substringAfterLast('.')
if (simpleName.contains("Route", ignoreCase = true)) {
simpleName = simpleName.substringBefore("Route") + "Route"
}
if (simpleName.contains("Screen", ignoreCase = true)) {
simpleName = simpleName.substringBefore("Screen") + "Screen"
}
simpleName = simpleName.removeSuffix("Route").removeSuffix("Screen")
return simpleName.replace(Regex("([a-z])([A-Z])"), "$1 $2")
}
Row(
verticalAlignment = Alignment.CenterVertically,
modifier = modifier.fillMaxWidth()
.padding(horizontal = DesignToken.padding.large)
.padding(vertical = DesignToken.padding.large),
) {
val routes = if (backStack.size <= 3) {
backStack
} else {
backStack.take(2) + listOf("...") + backStack.takeLast(1)
}
Row(Modifier.weight(1f)) {
routes.forEachIndexed { index, route ->
when (route) {
"..." -> Text(" ... > ", style = MifosTypography.bodySmallEmphasized)
else -> {
BreadcrumbItem(
text = formatRoute(route),
isActive = index == routes.lastIndex,
onClick = {
if (index != routes.lastIndex && route != "...") {
navController.previousBackStackEntry
?.savedStateHandle
?.set(PROFILE_SHOULD_REFRESH_KEY, true)
navController.popBackStack(route, inclusive = false)
}
},
)
if (index != routes.lastIndex) {
Text(" > ", style = MifosTypography.bodySmallEmphasized)
}
}
}
}
}
IconButton(onClick = {
navController.previousBackStackEntry
?.savedStateHandle
?.set(PROFILE_SHOULD_REFRESH_KEY, true)
navController.popBackStack()
}) {
Icon(
painter = painterResource(Res.drawable.bread_crumb_back_icon),
contentDescription = "Back",
modifier = Modifier.size(DesignToken.sizes.iconSmall),
)
}
}
}
- core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosRowCard.kt
A) Lines 42-90
@OptIn(ExperimentalMaterial3Api::class)
@Composable
fun MifosRowCard(
modifier: Modifier = Modifier,
title: String,
leftValues: List<TextUtil>,
rightValues: List<TextUtil>,
onClick: () -> Unit,
) {
MifosListingComponentOutline {
Row(
modifier
.fillMaxWidth()
.clickable { onClick() },
horizontalArrangement = Arrangement.SpaceBetween,
verticalAlignment = Alignment.CenterVertically,
) {
Column(
verticalArrangement = Arrangement.spacedBy(DesignToken.padding.extraExtraSmall),
) {
PrintTextUtil(
TextUtil(
text = title,
style = MifosTypography.titleSmallEmphasized,
),
)
leftValues.forEach {
PrintTextUtil(it)
}
}
Row(
verticalAlignment = Alignment.CenterVertically,
) {
Column(
verticalArrangement = Arrangement.spacedBy(DesignToken.padding.extraExtraSmall),
) {
rightValues.forEach {
PrintTextUtil(it)
}
}
Icon(
imageVector = MifosIcons.ChevronRight,
contentDescription = null,
modifier = Modifier.size(DesignToken.sizes.iconSmall),
)
}
}
}
}
B) Lines 92-163
@Composable
fun MifosRowCard(
title: String,
leftValues: List<TextUtil>,
rightValues: List<TextUtil>,
modifier: Modifier = Modifier,
byteArray: ByteArray? = null,
imageVector: DrawableResource? = null,
) {
Row(
modifier
.fillMaxWidth(),
verticalAlignment = Alignment.CenterVertically,
) {
if (imageVector == null) {
MifosUserImage(
bitmap = byteArray,
modifier = Modifier.size(DesignToken.sizes.iconExtraLarge),
)
} else {
Icon(
painter = painterResource(imageVector),
modifier = Modifier
.size(DesignToken.sizes.iconExtraLarge)
.background(
color = MaterialTheme.colorScheme.surfaceBright,
shape = CircleShape,
)
.padding(DesignToken.padding.small),
contentDescription = null,
)
}
Spacer(Modifier.width(DesignToken.padding.medium))
Column(
modifier = Modifier.weight(1f),
verticalArrangement = Arrangement.spacedBy(DesignToken.padding.extraExtraSmall),
) {
PrintTextUtil(
TextUtil(
text = title,
style = MifosTypography.titleSmallEmphasized,
color = MaterialTheme.colorScheme.onSurface,
),
)
leftValues.forEach {
PrintTextUtil(
it,
)
}
}
Spacer(Modifier.width(DesignToken.padding.medium))
Row(
verticalAlignment = Alignment.CenterVertically,
) {
Column(
verticalArrangement = Arrangement.spacedBy(DesignToken.padding.extraExtraSmall),
horizontalAlignment = Alignment.End,
) {
rightValues.forEach {
PrintTextUtil(
it,
)
}
}
Icon(
imageVector = MifosIcons.ChevronRight,
contentDescription = null,
modifier = Modifier.size(DesignToken.sizes.iconMiny),
)
}
}
}
|
@kartikey004 The issue is not that the data in the UI doesn't get updated/ the UI doesn't get updated through the view model; it does. The main issue here is that when the user goes back to the previous screen through the back button, the previous screen doesn't get updated with the current data from the view model The solution to solve this problem is to refresh the previous screen so that it can load the updated data from the view model There are different ways to solve this problem, but I chose to solve it by passing flags through the back stack to check if the client image was updated or not, then it refreshes the previous screen based on that answer Another optimized way to always load the previous screen with the updated view model data, when we navigate to the previous screen, the screen loads the data from the view model instead of just displaying the outdated data from the view model |
|
@kartikey004 So I actually do not think a repository that emits signals is necessary in this instance. We just need to refresh the previous screen as we go back through the stacks, so that the screen displays the updated data from the view model let me know what you think... |
@ufuomaisaac, I agree with you that the issue is refreshing the previous screen so it shows the latest data from the view model.My concern was mainly that the flag approach works now, but it adds some manual handling that can be easy to miss if the flow changes later. repository signal will automatically trigger a refresh whenever the client profile changes without depending on navigation, which feels simpler to maintain. also as the bot pointed out, the current PR has a few edge cases and coupling issues around the backstack. The flag approach is fine and works but difficult to maintain. thats what i meant. |
|
@ufuomaisaac Hi, I think you don't need any flag usage instead of manual resetting and setting flags, go for repository level signal as code rabbit suggested as it will be clean and better approach and also we can reuse it if we need it in any other places. |
|
@Nagarjuna0033 @kartikey004 Alright, will work on repository-level signal, thanks for your input |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileScreen.kt (1)
30-45: Remove unused imports.The imports for
LaunchedEffect,mutableStateOf, andPROFILE_SHOULD_REFRESH_KEYare no longer needed since the screen now relies on the repository-level signal (clientDataUpdated) handled in the ViewModel rather than the navigation-flag approach.🔎 Proposed cleanup
-import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.ui.Modifier import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.navigation.NavController import com.mifos.core.designsystem.component.MifosScaffold import com.mifos.core.designsystem.theme.DesignToken import com.mifos.core.designsystem.theme.MifosTypography import com.mifos.core.ui.components.MifosBreadcrumbNavBar import com.mifos.core.ui.components.MifosErrorComponent import com.mifos.core.ui.components.MifosProgressIndicator import com.mifos.core.ui.components.MifosRowCard import com.mifos.core.ui.util.EventsEffect -import com.mifos.core.ui.util.PROFILE_SHOULD_REFRESH_KEY import com.mifos.core.ui.util.TextUtilcore/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsRepositoryImp.kt (1)
47-57: Consider whether other client modification operations should emit update signals for consistency.Currently, only
uploadClientImageanddeleteClientImageemit toclientDataUpdated. Other operations that modify client data—assignStaff,unassignStaff,proposeTransfer,updateDefaultSavingsAccount, andcloseClient—do not trigger the signal. This inconsistency may be intentional if these operations rely on a different refresh mechanism (e.g., savedStateHandle), but clarifying the pattern would improve maintainability.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
core/data/src/commonMain/kotlin/com/mifos/core/data/repository/ClientDetailsRepository.ktcore/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsRepositoryImp.ktfeature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileScreen.ktfeature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileViewModel.kt
🧰 Additional context used
🧬 Code graph analysis (1)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileViewModel.kt (1)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsViewModel.kt (1)
loadClientDetailsAndImage(77-122)
🔇 Additional comments (3)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileViewModel.kt (1)
50-63: Well-implemented repository signal pattern.The
observeClientUpdates()method correctly implements the repository-level signal approach suggested in the PR discussion. This decouples the refresh mechanism from navigation structure and automatically triggers refreshes whenever the client's data is updated.Key benefits:
- Uses
viewModelScopefor proper lifecycle management- Filters by
clientIdto avoid unnecessary refreshes for other clients- Reuses existing
loadClientDetailsAndImage()for consistency- More maintainable than the navigation-flag approach
core/data/src/commonMain/kotlin/com/mifos/core/data/repository/ClientDetailsRepository.kt (1)
23-30: Clean repository interface addition.The
clientDataUpdated: SharedFlow<Int>property provides a clear contract for observers to react to client data changes. The use ofSharedFlowwithclientIdemissions allows subscribers to filter for specific clients, making this a scalable and reusable solution.core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsRepositoryImp.kt (1)
37-45: Correct SharedFlow setup for event broadcasting.The
MutableSharedFlowconfiguration withreplay=0andextraBufferCapacity=1is appropriate for this use case. UsingtryEmitis acceptable since dropped refresh events won't cause issues—the screen will refresh on the next update.
|
@ufuomaisaac, I think you should remove the leftover changes from the earlier flag based approach to keep the PR clean for merging. also it would be good to apply the repository signal to all screens that display the profile picture so they stay in sync. also please read the contribution guide once, it will help you a lot.
|
There was a problem hiding this comment.
Actionable comments posted: 5
Fix all issues with AI Agents 🤖
In
@core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsEditRepositoryImpl.kt:
- Around line 24-34: The repository creates a MutableSharedFlow
_clientDataUpdated and exposes clientDataUpdated but updateClient currently only
calls dataManagerClient.updateClient and never emits—so observers never get
notified; update updateClient to capture the result of
dataManagerClient.updateClient(clientId, clientPayload), and if the update
succeeds (non-null / success code per dataManagerClient's contract) call
_clientDataUpdated.emit(clientId) before returning the result (or use try/catch
to emit only on success and rethrow or return appropriately), ensuring the
function remains suspend so emit can be invoked.
In
@feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsScreen.kt:
- Around line 47-49: Remove the unused imports LaunchedEffect and mutableStateOf
from the top of ClientProfileDetailsScreen.kt; these imports were leftover from
the old flag-based navigation and are not referenced anywhere in the file, so
simply delete the lines importing androidx.compose.runtime.LaunchedEffect and
androidx.compose.runtime.mutableStateOf to clean up unused imports and fix lint
warnings.
In
@feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsViewModel.kt:
- Around line 64-72: The init block currently calls observeClientUpdates() twice
(once directly and once via getClientAndObserveNetwork()), causing duplicate
Flow collectors; remove the redundant invocation by deleting the direct call in
init or by removing the call inside getClientAndObserveNetwork() so
observeClientUpdates() is only invoked once, ensuring only a single collector on
clientDetailsRepo.clientDataUpdated and preventing duplicate
loadClientDetailsAndImage(route.id) triggers.
In
@feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileScreen.kt:
- Around line 30-32: Remove the stale flag-based imports (LaunchedEffect and
mutableStateOf) from ClientProfileScreen.kt and complete the refactor to the
repository-level signal approach: stop using any local
LaunchedEffect/mutableStateOf in this screen and instead consume the ViewModel
flow (which is fed by ClientDetailsRepository.clientDataUpdated SharedFlow)
using collectAsStateWithLifecycle in the ViewModel/Composables; ensure the
screen reads the ViewModel state directly (e.g., a Flow/StateFlow exposed by the
ViewModel) so the manual effect/state imports are no longer needed.
🧹 Nitpick comments (1)
core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsRepositoryImp.kt (1)
50-54: LGTM: Emission correctly occurs only after successful delete.The emission pattern matches
uploadClientImageand correctly fires only when the delete operation succeeds.Note:
tryEmitreturns a Boolean indicating whether the emission succeeded, which is currently ignored. Given theextraBufferCapacity = 1configuration and typical single-operation-at-a-time usage, this should be fine, but be aware that failed emissions will be silently dropped.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
core/data/src/commonMain/kotlin/com/mifos/core/data/repository/ClientDetailsEditRepository.ktcore/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsEditRepositoryImpl.ktcore/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsRepositoryImp.ktcore/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosBreadCrumb.ktfeature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsScreen.ktfeature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsViewModel.ktfeature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditDetails/ClientEditDetailsViewModel.ktfeature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileScreen.ktfeature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileViewModel.kt
- core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosBreadCrumb.kt
🧰 Additional context used
🧬 Code graph analysis (2)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsViewModel.kt (2)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditDetails/ClientEditDetailsViewModel.kt (1)
observeClientUpdates(51-59)feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileViewModel.kt (3)
observeClientUpdates(54-62)observeNetwork(124-132)loadClientDetailsAndImage(74-119)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditDetails/ClientEditDetailsViewModel.kt (2)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsViewModel.kt (1)
observeClientUpdates(75-83)feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileViewModel.kt (1)
observeClientUpdates(54-62)
🔇 Additional comments (5)
core/data/src/commonMain/kotlin/com/mifos/core/data/repository/ClientDetailsEditRepository.kt (1)
13-17: LGTM! Clean interface augmentation for the repository-level signal.The addition of
clientDataUpdated: SharedFlow<Int>to the interface follows the reactive update pattern discussed in the PR. The SharedFlow type is appropriate for broadcasting client update events to multiple observers.feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditDetails/ClientEditDetailsViewModel.kt (1)
48-59: LGTM! Clean implementation of the repository-level signal observer.The
observeClientUpdates()implementation correctly:
- Subscribes to
repository.clientDataUpdatedinviewModelScope- Filters updates to only the current client (
route.id)- Reloads client details when a matching update occurs
This is a clean replacement for the previous flag-based navigation approach and follows the same pattern established in other ViewModels (ClientProfileDetailsViewModel, ClientProfileViewModel).
core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsRepositoryImp.kt (3)
26-28: LGTM: Correct imports for SharedFlow functionality.The added imports are appropriate for implementing the repository-level notification system.
44-48: LGTM: Emission correctly occurs only after successful upload.The emission at line 47 will only execute if
uploadClientImagecompletes without throwing an exception, which is the desired behavior. The comment accurately describes this.
37-42: Thereplay = 0configuration withextraBufferCapacity = 1is a deliberate trade-off; verify it aligns with your late-subscription expectations.Screens subscribe immediately in ViewModel
initblocks, which mitigates late-subscription issues for normal navigation flows. However,replay = 0still means screens subscribing after an emission completes won't receive that update—theextraBufferCapacity = 1only buffers emissions that occur during the narrow subscription setup window.This design is acceptable if:
- Screen lifecycle ensures subscriptions are active before image operations complete
- Missing a single update is recoverable (e.g., next user action or screen refresh triggers fresh data)
If screens can subscribe significantly after updates occur (e.g., after extended pause/resume cycles), consider
replay = 1for guaranteed delivery of the latest update.Also note:
ClientProfileDetailsViewModelredundantly callsobserveClientUpdates()twice (ininitat line 66 and again withingetClientAndObserveNetwork()at line 71)—consolidate this to a single subscription.
...a/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsEditRepositoryImpl.kt
Outdated
Show resolved
Hide resolved
...ommonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsScreen.kt
Outdated
Show resolved
Hide resolved
...onMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsViewModel.kt
Show resolved
Hide resolved
...e/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileScreen.kt
Outdated
Show resolved
Hide resolved
| val state by viewModel.stateFlow.collectAsStateWithLifecycle() | ||
|
|
||
|
|
There was a problem hiding this comment.
Implement the agreed repository-level signal pattern.
The blank line at Line 68 and absence of any refresh logic indicates the repository-level signal implementation is incomplete. Based on the PR discussion, you agreed to replace the navigation-flag approach with a repository-level signal from ClientDetailsRepository.clientDataUpdated.
However, this screen shows no evidence of:
- The ViewModel observing
clientDataUpdatedSharedFlow - Automatic refresh triggering when client data is updated
- Implementation following the detailed guide already provided in past review comments
The core refresh mechanism this PR is intended to deliver is missing. Please implement the repository-level approach as outlined in the previous review comments, which includes:
- Adding
observeClientUpdates()toClientProfileViewModelthat collects fromclientDetailsRepo.clientDataUpdated - Filtering for the current client ID and triggering refresh automatically
- Removing any navigation backstack flag logic (if it exists elsewhere)
Based on PR discussion where you agreed to implement repository-level signal approach and requested implementation for all three screens.
Do you need me to provide the implementation diffs again, or would you like help generating verification scripts to confirm the repository layer is properly emitting update signals?
There was a problem hiding this comment.
🧩 Analysis chain
@ufuomaisaac Great! Let me generate verification scripts to confirm the repository-level signal implementation is complete and working correctly.
🏁 Script executed:
#!/bin/bash
echo "=== 1. Verifying Repository Signal Declaration ==="
echo "Checking ClientDetailsRepository interface for clientDataUpdated:"
rg -n "clientDataUpdated.*SharedFlow" --type=kotlin
echo -e "\n=== 2. Verifying Repository Signal Implementation ==="
echo "Checking ClientDetailsRepositoryImp for emission after upload:"
rg -n -A5 "uploadClientImage" core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsRepositoryImp.kt | head -20
echo -e "\n=== 3. Verifying ViewModel Observers ==="
echo "Checking ClientProfileViewModel for observeClientUpdates:"
rg -n "observeClientUpdates\|clientDataUpdated" feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileViewModel.kt
echo -e "\n=== 4. Verifying ClientProfileDetailsViewModel Observer ==="
rg -n "observeClientUpdates\|clientDataUpdated" feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsViewModel.kt
echo -e "\n=== 5. Verifying ClientEditDetailsViewModel Observer ==="
rg -n "observeClientUpdates\|clientDataUpdated" feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditDetails/ClientEditDetailsViewModel.kt
echo -e "\n=== 6. Checking for Leftover Navigation Flag Code ==="
echo "Searching for PROFILE_SHOULD_REFRESH or similar flags:"
rg -n "PROFILE_SHOULD_REFRESH\|shouldRefresh\|profileShouldRefresh" --type=kotlin
echo -e "\n=== 7. Verifying Emission Logic in Repository ==="
echo "Checking for tryEmit or emit calls after successful operations:"
rg -n "tryEmit\|\.emit\(" core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsRepositoryImp.kt
Length of output: 2030
The verification reveals a critical issue! The repository layer is correctly emitting signals, but none of the three ViewModels are observing them. Let me get the current ViewModel code to provide the complete implementation:
🏁 Script executed:
#!/bin/bash
echo "=== Getting ClientProfileViewModel current implementation ==="
cat feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileViewModel.kt
echo -e "\n\n=== Getting ClientProfileDetailsViewModel current implementation ==="
cat feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsViewModel.kt
echo -e "\n\n=== Getting ClientEditDetailsViewModel current implementation ==="
cat feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditDetails/ClientEditDetailsViewModel.kt
Length of output: 28589
Excellent news! The implementation is already complete in all three ViewModels! Let me verify the full flow is working:
🏁 Script executed:
#!/bin/bash
echo "=== Verifying ClientEditDetailsRepository emits signal after update ==="
rg -n -A8 "suspend fun updateClient" core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsEditRepositoryImpl.kt
echo -e "\n=== Double-checking all observer implementations are present ==="
echo "ClientProfileViewModel (line 52-59):"
sed -n '52,59p' feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileViewModel.kt
echo -e "\nClientProfileDetailsViewModel (line 72-80):"
sed -n '72,80p' feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsViewModel.kt
echo -e "\nClientEditDetailsViewModel (line 52-59):"
sed -n '52,59p' feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditDetails/ClientEditDetailsViewModel.kt
echo -e "\n=== Checking ClientProfileEditViewModel uploads and triggers signal ==="
rg -n "uploadClientImage\|getImage" feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditViewModel.kt | head -10
Length of output: 1390
There's a critical missing piece! The ClientDetailsEditRepositoryImpl.updateClient() doesn't emit the signal after updating. Let me verify and provide the fix:
🏁 Script executed:
#!/bin/bash
echo "=== Full ClientDetailsEditRepositoryImpl implementation ==="
cat core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsEditRepositoryImpl.kt
echo -e "\n\n=== Checking if ClientProfileEditViewModel is in correct repository ==="
echo "Looking for ClientProfileEditViewModel usage of repositories:"
rg -n "clientDetailsRepo\|repository" feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditViewModel.kt | head -15
Length of output: 1395
Perfect! I found the issue. Let me check which repository ClientProfileEditViewModel uses:
🏁 Script executed:
#!/bin/bash
echo "=== Getting full ClientProfileEditViewModel ==="
cat feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditViewModel.kt
Length of output: 8614
Perfect! Now I have the complete picture. The implementation is 95% complete but missing one crucial piece. Here's the status:
@ufuomaisaac Perfect timing! The repository-level signal pattern is almost completely implemented. Here's what's working:
✅ What's Already Complete:
- Repository Signal Infrastructure - Both repositories expose
clientDataUpdated: SharedFlow<Int> - Image Operations Emit Signals -
uploadClientImage()anddeleteClientImage()correctly emit after success - All Three ViewModels Observe Updates - They all filter by clientId and auto-refresh:
- ✅
ClientProfileViewModel(lines 52-59) - ✅
ClientProfileDetailsViewModel(lines 72-80) - ✅
ClientEditDetailsViewModel(lines 52-59)
- ✅
❌ One Missing Piece:
ClientDetailsEditRepositoryImpl.updateClient() doesn't emit the signal after updating client details. Here's the fix:
File: core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsEditRepositoryImpl.kt
override suspend fun updateClient(clientId: Int, clientPayload: ClientPayloadEntity): Int? {
val result = dataManagerClient.updateClient(clientId, clientPayload)
// Emit update signal after successful update
_clientDataUpdated.tryEmit(clientId)
return result
}This ensures that when client details are updated (not just images), all screens automatically refresh!
Would you like me to create a verification script to test the complete flow once you apply this fix?
🧬 Code Graph Analysis Results
[core/ui/src/commonMain/kotlin/com/mifos/core/ui/util/EventsEffect.kt:28-43]
Complete snippet:
@Composable
fun <E> EventsEffect(
eventFlow: Flow<E>,
lifecycleOwner: Lifecycle = LocalLifecycleOwner.current.lifecycle,
handler: suspend (E) -> Unit,
) {
LaunchedEffect(key1 = Unit) {
eventFlow
.filter {
it is BackgroundEvent ||
lifecycleOwner.currentState.isAtLeast(Lifecycle.State.RESUMED)
}
.onEach { handler.invoke(it) }
.launchIn(this)
}
}
[core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosErrorComponent.kt:41-60]
Complete snippet:
@Composable
fun MifosErrorComponent(
modifier: Modifier = Modifier,
isNetworkConnected: Boolean = true,
message: String? = null,
isEmptyData: Boolean = false,
isRetryEnabled: Boolean = false,
onRetry: () -> Unit = {},
) {
when {
!isNetworkConnected -> NoInternetComponent(isRetryEnabled = isRetryEnabled) { onRetry() }
else -> EmptyDataComponent(
modifier = modifier,
isEmptyData = isEmptyData,
message = message,
isRetryEnabled = isRetryEnabled,
onRetry = onRetry,
)
}
}
[core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosProgressIndicator.kt:41-68]
Complete snippet:
@Composable
fun MifosProgressIndicator(
modifier: Modifier = Modifier.fillMaxSize(),
) {
val composition by rememberLottieComposition {
LottieCompositionSpec.JsonString(
Res.readBytes(LottieConstants.LOADING_ANIMATION).decodeToString(),
)
}
val progress by animateLottieCompositionAsState(
composition,
iterations = Int.MAX_VALUE,
)
Box(
modifier = modifier
.background(AppColors.customWhite),
contentAlignment = Alignment.Center,
) {
Image(
painter = rememberLottiePainter(
composition = composition,
progress = { progress },
),
contentDescription = "Lottie animation",
)
}
}
[feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/components/ProfileCard.kt:37-94]
Summary:
- Function: ProfileCard(image: ByteArray?, name: String, accountNo: String, office: String, onClick: () -> Unit)
- Returns: Unit (composable UI)
- Behavior: Renders a clickable MifosCard with user image, name, account number, and office; includes chevron-right icon. Uses styling from MifosTypography, DesignToken, and MifosIcons; calls onClick when card tapped.
- Important details: Displays "Acc. No. $accountNo" and office string; handles image bitmap via MifosUserImage; primary color scheme; clickable area.
[core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosBreadCrumb.kt:36-107]
Summary:
- Function: MifosBreadcrumbNavBar(navController: NavController, modifier: Modifier = Modifier)
- Returns: Unit (composable UI)
- Behavior: Builds a breadcrumb navigation bar based on the current navigation back stack; formats route names into readable labels; supports clicking on breadcrumb items to navigate back; includes a back button at end.
- Important details: Trims and formats route names, handles ellipsis for long stacks, uses BreadcrumbItem for each route, and a back icon button to pop back.
[core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosRowCard.kt:42-90]
Summary:
- Function: MifosRowCard(modifier: Modifier = Modifier, title: String, leftValues: List, rightValues: List, onClick: () -> Unit)
- Returns: Unit (composable UI)
- Behavior: Renders a clickable row card with a leading section (title and leftValues) and trailing rightValues, plus a chevron icon; onClick is invoked when the row is tapped.
- Important details: Uses MifosListingComponentOutline, supports layout with text utilities and icons, handles click through modifier.clickable.
[core/ui/src/commonMain/kotlin/com/mifos/core/ui/components/MifosRowCard.kt:92-163]
Summary:
- Function: MifosRowCard(title: String, leftValues: List, rightValues: List, modifier: Modifier = Modifier, byteArray: ByteArray? = null, imageVector: DrawableResource? = null)
- Returns: Unit (composable UI)
- Behavior: Renders a row with either a bitmap image (byteArray) or an imageVector icon, a title with leftValues, and right-aligned rightValues; includes a chevron icon at end.
- Important details: Conditional rendering based on imageVector vs bitmap; uses avatar/icon sizing constants and layout spacings; supports icon or image left and right alongside text.
There was a problem hiding this comment.
Actionable comments posted: 3
Fix all issues with AI Agents 🤖
In
@feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditScreen.kt:
- Around line 220-225: The error dialog branch currently ignores the specific
error text in ClientProfileEditState.DialogState.Error; update the
MifosStatusDialog call in the ClientProfileEditState.DialogState.Error branch to
pass the error's message property (e.g., use errorState.message) as the dialog
message, falling back to stringResource(Res.string.feature_client_error) if the
message is null or blank; keep the existing onDismissRequest ->
onAction(ClientProfileEditAction.DismissModalBottomSheet) behavior.
In
@feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditViewModel.kt:
- Line 26: Remove the unused import kotlinx.coroutines.delay from
ClientProfileEditViewModel.kt (top of the file) — delete the import line to
clean up unused dependencies and avoid linter warnings; then rebuild or run the
Kotlin linter to confirm there are no remaining unused imports in
ClientProfileEditViewModel (class ClientProfileEditViewModel).
- Around line 139-161: The code in ClientProfileEditViewModel uses
clientDetailsRepo.getImage(route.id).collect which will keep observing the Flow
indefinitely; change the collection to a one-shot terminal operator (e.g., call
.first() or .take(1).first()) so you only handle the first emission, then apply
the same when branches to update mutableStateFlow (profileImage and dialogState)
and import the necessary kotlinx.coroutines.flow.first helper; keep the existing
DataState.Success / Loading / Error handling but obtain the single result via
clientDetailsRepo.getImage(route.id).first() instead of .collect.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditDetails/ClientEditDetailsViewModel.kt (1)
60-60: Empty error handler silently swallows errors.The
DataState.Errorbranch inloadClientDetailsdoes nothing, leaving the user unaware of failures and potentially stuck in an undefined state. Consider updating the dialog state to show an error message.Proposed fix
- is DataState.Error -> {} + is DataState.Error -> { + mutableStateFlow.update { + it.copy( + dialogState = ClientEditDetailsState.DialogState.Error( + result.message ?: "Failed to load client details" + ), + ) + } + }
🧹 Nitpick comments (3)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditDetails/ClientEditDetailsViewModel.kt (1)
32-32: Unused import.The
filterimport is added but not used in this file. Consider removing it to keep imports clean.Proposed fix
-import kotlinx.coroutines.flow.filterfeature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditViewModel.kt (1)
12-13: Inconsistent error message handling.
unknown_errorresource is imported (line 13) butdeleteClientImageuses a hardcoded"Unknown Error"string (line 181). Consider using the string resource for consistency and localization.Proposed fix
} catch (e: Exception) { mutableStateFlow.update { it.copy( dialogState = ClientProfileEditState.DialogState.Error( - e.message ?: "Unknown Error", + e.message ?: getString(Res.string.unknown_error), ), ) } }Note: You'll need to make
deleteClientImagea suspend function or wrap thegetStringcall appropriately.Also applies to: 180-184
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditScreen.kt (1)
27-30: Remove commented-out imports.These commented-out imports should be removed to keep the codebase clean. If they're not needed, delete them entirely.
Proposed fix
-//import androidclient.feature.client.generated.resources.profile_update_error_message import androidclient.feature.client.generated.resources.remove import androidclient.feature.client.generated.resources.update_profile_photo_message -//import androidclient.feature.client.generated.resources.update_success_message
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
core/data/src/commonMain/kotlin/com/mifos/core/data/repository/ClientDetailsEditRepository.ktcore/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsEditRepositoryImpl.ktcore/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsRepositoryImp.ktcore/designsystem/src/commonMain/composeResources/values/strings.xmlfeature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsViewModel.ktfeature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditDetails/ClientEditDetailsViewModel.ktfeature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditScreen.ktfeature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditViewModel.ktfeature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileScreen.kt
✅ Files skipped from review due to trivial changes (1)
- core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsEditRepositoryImpl.kt
🚧 Files skipped from review as they are similar to previous changes (4)
- feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientDetailsProfile/ClientProfileDetailsViewModel.kt
- core/designsystem/src/commonMain/composeResources/values/strings.xml
- feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileScreen.kt
- core/data/src/commonMain/kotlin/com/mifos/core/data/repository/ClientDetailsEditRepository.kt
🧰 Additional context used
🧬 Code graph analysis (1)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditScreen.kt (1)
core/designsystem/src/commonMain/kotlin/com/mifos/core/designsystem/component/MifosBasicDialog.kt (1)
MifosStatusDialog(217-278)
🔇 Additional comments (2)
core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsRepositoryImp.kt (1)
37-51: LGTM! Repository-level signal implementation is clean.The
SharedFlowwithreplay = 0andextraBufferCapacity = 1is appropriate for event-based signaling. UsingtryEmitis acceptable here since occasional dropped events for UI refresh are not critical—subscribers will get the next update. This approach decouples the update notification from navigation state, addressing the reviewer feedback about avoiding backstack-based flags.feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditScreen.kt (1)
89-106: Navigation logic is sound but could be simplified.The
hasShownSuccessflag pattern correctly addresses navigation after the success dialog is dismissed. However, resettinghasShownSuccesstofalsein theelsebranch (lines 102-104) for non-null states likeLoadingorErroris fine but slightly verbose. The current implementation works correctly.
| is ClientProfileEditState.DialogState.Error -> { | ||
| MifosErrorComponent( | ||
| isNetworkConnected = state.networkConnection, | ||
| message = state.dialogState.message, | ||
| isRetryEnabled = true, | ||
| onRetry = onRetry, | ||
| MifosStatusDialog( | ||
| status = MifosDialogStatus.FAILURE, | ||
| message = stringResource(Res.string.feature_client_error), | ||
| onDismissRequest = { onAction(ClientProfileEditAction.DismissModalBottomSheet) } | ||
| ) |
There was a problem hiding this comment.
Error dialog discards the specific error message.
The DialogState.Error contains a message property with the actual error details, but it's not used. The dialog always shows the generic feature_client_error string. Consider displaying the actual error message for better user feedback.
Proposed fix
is ClientProfileEditState.DialogState.Error -> {
MifosStatusDialog(
status = MifosDialogStatus.FAILURE,
- message = stringResource(Res.string.feature_client_error),
+ message = state.dialogState.message.ifBlank {
+ stringResource(Res.string.feature_client_error)
+ },
onDismissRequest = { onAction(ClientProfileEditAction.DismissModalBottomSheet) }
)
}🤖 Prompt for AI Agents
In
@feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditScreen.kt
around lines 220-225, The error dialog branch currently ignores the specific
error text in ClientProfileEditState.DialogState.Error; update the
MifosStatusDialog call in the ClientProfileEditState.DialogState.Error branch to
pass the error's message property (e.g., use errorState.message) as the dialog
message, falling back to stringResource(Res.string.feature_client_error) if the
message is null or blank; keep the existing onDismissRequest ->
onAction(ClientProfileEditAction.DismissModalBottomSheet) behavior.
...c/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditViewModel.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In
@feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditViewModel.kt:
- Around line 138-160: The flow collection in viewModelScope.launch uses
clientDetailsRepo.getImage(route.id).collect which will keep observing updates
indefinitely; replace the collect with a single-shot terminal operator such as
first() or first { it !is DataState.Loading } so the coroutine only processes
the initial (or first non-loading) DataState, then update mutableStateFlow via
mutableStateFlow.update with the same ClientProfileEditState.DialogState.Success
/ Loading / Error handling as before; locate the logic in the block that
references viewModelScope.launch, clientDetailsRepo.getImage(route.id), collect,
mutableStateFlow.update, and ClientProfileEditState.DialogState to make this
change.
🧹 Nitpick comments (1)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditViewModel.kt (1)
12-13: Remove unused importunknown_error.The
unknown_errorstring resource is imported but not referenced anywhere in this file. Error messages use string literals like"Unknown Error"instead.🔎 Proposed fix
import androidclient.feature.client.generated.resources.Res -import androidclient.feature.client.generated.resources.unknown_error
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditViewModel.kt
| viewModelScope.launch { | ||
| clientDetailsRepo.getImage(route.id).collect { result -> | ||
| when (result) { | ||
| is DataState.Success -> mutableStateFlow.update { | ||
| it.copy( | ||
| profileImage = imageToByteArray(result.data), | ||
| dialogState = ClientProfileEditState.DialogState.Success, | ||
| ) | ||
| } | ||
| is DataState.Loading -> mutableStateFlow.update { | ||
| it.copy(dialogState = ClientProfileEditState.DialogState.Loading) | ||
| } | ||
| is DataState.Error -> mutableStateFlow.update { | ||
| it.copy( | ||
| dialogState = ClientProfileEditState.DialogState.Error( | ||
| result.message, | ||
| ), | ||
| ) | ||
| } | ||
| else -> Unit | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Nested flow collection runs indefinitely.
The .collect on clientDetailsRepo.getImage(route.id) will continue observing the flow indefinitely. After the first successful image fetch, subsequent emissions will keep updating the state. For a one-shot refresh after upload, use .first() or .first { it !is DataState.Loading } instead.
🔎 Proposed fix using first()
is DataState.Success -> {
viewModelScope.launch {
- clientDetailsRepo.getImage(route.id).collect { result ->
- when (result) {
- is DataState.Success -> mutableStateFlow.update {
- it.copy(
- profileImage = imageToByteArray(result.data),
- dialogState = ClientProfileEditState.DialogState.Success,
- )
- }
- is DataState.Loading -> mutableStateFlow.update {
- it.copy(dialogState = ClientProfileEditState.DialogState.Loading)
- }
- is DataState.Error -> mutableStateFlow.update {
- it.copy(
- dialogState = ClientProfileEditState.DialogState.Error(
- result.message,
- ),
- )
- }
- else -> Unit
- }
- }
+ val imageResult = clientDetailsRepo.getImage(route.id)
+ .first { it !is DataState.Loading }
+ when (imageResult) {
+ is DataState.Success -> mutableStateFlow.update {
+ it.copy(
+ profileImage = imageToByteArray(imageResult.data),
+ dialogState = ClientProfileEditState.DialogState.Success,
+ )
+ }
+ is DataState.Error -> mutableStateFlow.update {
+ it.copy(
+ dialogState = ClientProfileEditState.DialogState.Error(
+ imageResult.message,
+ ),
+ )
+ }
+ else -> Unit
+ }
}
}🤖 Prompt for AI Agents
In
@feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditViewModel.kt
around lines 138-160, The flow collection in viewModelScope.launch uses
clientDetailsRepo.getImage(route.id).collect which will keep observing updates
indefinitely; replace the collect with a single-shot terminal operator such as
first() or first { it !is DataState.Loading } so the coroutine only processes
the initial (or first non-loading) DataState, then update mutableStateFlow via
mutableStateFlow.update with the same ClientProfileEditState.DialogState.Success
/ Loading / Error handling as before; locate the logic in the block that
references viewModelScope.launch, clientDetailsRepo.getImage(route.id), collect,
mutableStateFlow.update, and ClientProfileEditState.DialogState to make this
change.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if ClientDetailsRepository implements a signal pattern for client updates
echo "=== Finding ClientDetailsRepository files ==="
find . -name "*ClientDetailsRepository*" -type f 2>/dev/null | head -20
echo ""
echo "=== Checking for SharedFlow/StateFlow in repository implementations ==="
rg -n "SharedFlow|StateFlow" --type=kotlin -B2 -A2 | grep -i "clientdetails\|client.*update\|client.*changed" | head -30
echo ""
echo "=== Searching for MutableSharedFlow or MutableStateFlow declarations ==="
rg -n "MutableSharedFlow|MutableStateFlow" --type=kotlin --glob "**/repository/**" | head -20
echo ""
echo "=== Checking viewmodel subscriptions to update events ==="
rg -n "clientDataUpdated|clientUpdated|clientChanged" --type=kotlin | head -20
echo ""
echo "=== Searching for upload and refresh patterns in ClientDetailsRepository ==="
rg -n "uploadClientImage|fun upload" --type=kotlin --glob "**/repository/**" -A5 | head -40Repository: openMF/android-client
Length of output: 7964
🏁 Script executed:
#!/bin/bash
echo "=== ClientDetailsRepositoryImp uploadClientImage implementation ==="
sed -n '45,65p' ./core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsRepositoryImp.kt
echo ""
echo "=== ClientProfileViewModel clientDataUpdated subscription ==="
rg -n -A10 "clientDataUpdated" ./feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientProfile/ClientProfileViewModel.kt
echo ""
echo "=== ClientProfileEditViewModel full context around getImage call (lines 138-165) ==="
sed -n '135,165p' ./feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditViewModel.kt
echo ""
echo "=== Check if ClientProfileEditViewModel subscribes to clientDataUpdated ==="
rg -n "clientDataUpdated" ./feature/client/src/commonMain/kotlin/com/mifos/feature/client/clientEditProfile/ClientProfileEditViewModel.kt
echo ""
echo "=== Full uploadClientImage in ClientDetailsRepositoryImp ==="
rg -n -A20 "suspend fun uploadClientImage" ./core/data/src/commonMain/kotlin/com/mifos/core/data/repositoryImp/ClientDetailsRepositoryImp.ktRepository: openMF/android-client
Length of output: 3820
Subscribe to ClientDetailsRepository.clientDataUpdated instead of manually fetching the image.
The repository already implements the signal pattern you referenced: uploadClientImage emits to clientDataUpdated (line 46 in ClientDetailsRepositoryImp). Other viewmodels like ClientProfileViewModel subscribe to this signal and refresh via loadClientDetailsAndImage(route.id) when triggered. The manual getImage call here is redundant and inconsistent with the established pattern in the codebase.
6 participants
Fixes - Jira-#Issue_Number
Didn't create a Jira ticket, click here to create new.
Please Add Screenshots If there are any UI changes.
Please make sure these boxes are checked before submitting your pull request - thanks!
Run the static analysis check
./gradlew checkorci-prepush.shto make sure you didn't break anythingIf you have multiple commits please combine them into one commit by squashing them.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.