feat(recent-transaction): redesign screen and update state management by Nagarjuna0033 · Pull Request #3045 · openMF/mifos-mobile

Nagarjuna0033 · 2025-12-30T14:28:58Z

Redesign RecentTransactionScreen to use MifosElevatedScaffold, MifosPoweredCard, and updated UI components for transactions and filters.
Migrate RecentTransactionViewModel to inherit from BaseViewModel and refactor state management to use ScreenUiState.
Introduce UiTransaction and RecentTransactionUiState within the ViewModel package to replace the deleted RecentTransactionState.kt.
Replace string-based navigation with type-safe serialization-based navigation in RecentTransactionNavGraph.kt.
Add necessary string resources for the new UI layout and filter dialogs.
Remove obsolete RecentTransactionNavigation.kt and RecentTransactionState.kt.

Didn't create a Jira ticket, click here to create new.

Before	After

Summary by CodeRabbit

Release Notes

New Features
- Added transaction filtering with options to filter by account and transaction type (All, Debits, Credits).
- Introduced Clear All filters button and pull-to-refresh functionality for recent transactions.
- Enhanced error messaging with improved user feedback.
Chores
- Added Kotlin serialization support to build configuration.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

- Redesign `RecentTransactionScreen` to use `MifosElevatedScaffold`, `MifosPoweredCard`, and updated UI components for transactions and filters. - Migrate `RecentTransactionViewModel` to inherit from `BaseViewModel` and refactor state management to use `ScreenUiState`. - Introduce `UiTransaction` and `RecentTransactionUiState` within the ViewModel package to replace the deleted `RecentTransactionState.kt`. - Replace string-based navigation with type-safe serialization-based navigation in `RecentTransactionNavGraph.kt`. - Add necessary string resources for the new UI layout and filter dialogs. - Remove obsolete `RecentTransactionNavigation.kt` and `RecentTransactionState.kt`.

coderabbitai · 2025-12-30T14:29:12Z

📝 Walkthrough

Walkthrough

This PR refactors the recent-transaction feature from a legacy state-management pattern to a BaseViewModel-driven architecture with serializable routes. Navigation functions are renamed for consistency (NavGraph→Destination), the RecentTransactionScreen is reorganized into modular composables (content/dialog), and a new filtering system is introduced with corresponding UI strings and state handling. The ViewModel now manages network-aware data fetching, local transaction filtering, and event-driven navigation.

Changes

Cohort / File(s)	Summary
Navigation Layer `cmp-navigation/src/commonMain/kotlin/cmp/navigation/authenticated/AuthenticatedNavigation.kt`	Renamed `recentTransactionNavGraph` to `recentTransactionDestination`; updated signature to pass `navigateBack` callback instead of `NavController` instance.
Navigation Layer `feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/navigation/RecentTransactionNavGraph.kt`	Introduced serializable `RecentTransactionRoute` data object; renamed function from `recentTransactionScreenRoute` to `recentTransactionDestination`; replaced `composable()` with `composableWithSlideTransitions<RecentTransactionRoute>`.
Navigation Layer `feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/navigation/RecentTransactionNavigation.kt`	Removed entire file containing route constants and sealed navigation hierarchy.
Build Configuration `feature/recent-transaction/build.gradle.kts`	Added Kotlin serialization plugin alias to plugins block.
String Resources `feature/recent-transaction/src/commonMain/composeResources/values/strings.xml`	Added 14 new string resources for filter UI (filter titles, account labels, transaction type options, buttons).
Screen Implementation `feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/screen/RecentTransactionScreen.kt`	Refactored public screen function to internal; introduced modular composables (`RecentTransactionScreenContent`, `RecentTransactionScreenDialog`, `TransactionFilterSheetContent`); switched to state-driven UI with `MifosElevatedScaffold`, pull-to-refresh, and LazyColumn-based transaction rendering; replaced hard-coded strings with generated resources.
State Management `feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/viewmodel/RecentTransactionViewModel.kt`	Major refactor to BaseViewModel architecture; added network monitoring via `NetworkMonitor`; introduced new action/event/state types (`RecentTransactionAction`, `RecentTransactionEvent`, `RecentTransactionUiState`); implemented transaction filtering (ALL/DEBIT/CREDIT), grouping by date, and account-aware loading.
State Management `feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/utils/RecentTransactionState.kt`	Removed entire file containing previous state definitions (enum `TransactionFilterType`, data class `RecentTransactionUiState`, sealed interfaces `RecentTransactionAction`, `RecentTransactionEvent`).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Features/recent transaction screen #3007: Updates authenticated navigation to wire the renamed recentTransactionDestination function.
refactor(feature): replace MaterialTheme with KptTheme #3013: Modifies recent-transaction feature components including screen and navigation code in parallel.
feat(accounts): implement transaction details screen #3008: Modifies the recent-transaction navigation API surface with similar destination renaming and callback propagation patterns.

Suggested reviewers

Arinyadav1
therajanmaurya

Poem

🐰 Hops with glee through filters bright,
Routes now serialized, state flows right,
BaseViewModel's grand new dance,
Screens refactored at a glance—
Navigation clean, our rabbits prance! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 36.84% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the primary changes: a screen redesign and state management update to BaseViewModel, which aligns with the main modifications across the codebase.

✨ Finishing touches

📝 Generate docstrings

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 3

🧹 Nitpick comments (5)

feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/viewmodel/RecentTransactionViewModel.kt (2)
182-197: Consider simplifying the transaction click handling.

The transactionId is already a Long but is converted to String at line 190 (transactionId.toString()). The navigation callback in RecentTransactionNavGraph.kt accepts Long for accountId but String for transactionId. This inconsistency may cause confusion. Consider keeping the types consistent across the navigation layer.

340-348: Consider making the extension function private and avoiding hardcoded currency.

The toUiTransaction() extension is only used within this internal class, so it should be marked as private for proper encapsulation.

The hardcoded "USD" fallback (line 347) may not be appropriate for all deployment regions.
🔎 Proposed fix
-    fun Transactions.toUiTransaction() = UiTransaction(
+    private fun Transactions.toUiTransaction() = UiTransaction(
         id = id?.toLong(),
         date = date,
         amount = amount,
         type = transactionType,
         typeValue = transactionType?.value,
         isCredit = transactionType.isCredit(),
-        currency = currency?.code ?: "USD",
+        currency = currency?.code.orEmpty(),
     )
feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/navigation/RecentTransactionNavGraph.kt (1)
16-16: Unused import.

The androidx.navigation.compose.navigation import appears to be unused since the code only uses composableWithSlideTransitions and doesn't create a nested navigation graph.
🔎 Proposed fix
 import androidx.navigation.NavController
 import androidx.navigation.NavGraphBuilder
-import androidx.navigation.compose.navigation
 import kotlinx.serialization.Serializable
feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/screen/RecentTransactionScreen.kt (2)
464-469: Consider using safe call instead of force unwrap.

While the null check at line 466 makes the !! at line 467 safe, using a safe call pattern is more idiomatic and protects against future refactoring errors.
🔎 Proposed fix
         Button(
-            onClick = {
-                if (selectedAccount != null) {
-                    onAction(RecentTransactionAction.ApplyFilter(selectedAccount!!, selectedType))
-                }
-            },
+            onClick = {
+                selectedAccount?.let { account ->
+                    onAction(RecentTransactionAction.ApplyFilter(account, selectedType))
+                }
+            },
489-521: Consider making FilterOptionChip internal.

FilterOptionChip is a public composable in a file where other composables are marked internal. For consistency and proper encapsulation, consider marking it internal unless it's intended to be reused by other modules.
🔎 Proposed fix
 @Composable
-fun FilterOptionChip(
+internal fun FilterOptionChip(
     label: String,
     isSelected: Boolean,
     onClick: () -> Unit,
     modifier: Modifier = Modifier,
 ) {

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 99c8d68 and 3ddb854.

📒 Files selected for processing (8)

cmp-navigation/src/commonMain/kotlin/cmp/navigation/authenticated/AuthenticatedNavigation.kt
feature/recent-transaction/build.gradle.kts
feature/recent-transaction/src/commonMain/composeResources/values/strings.xml
feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/navigation/RecentTransactionNavGraph.kt
feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/navigation/RecentTransactionNavigation.kt
feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/screen/RecentTransactionScreen.kt
feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/utils/RecentTransactionState.kt
feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/viewmodel/RecentTransactionViewModel.kt

💤 Files with no reviewable changes (2)

feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/utils/RecentTransactionState.kt
feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/navigation/RecentTransactionNavigation.kt

🧰 Additional context used

📓 Path-based instructions (3)

**/*.{kt,kts}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{kt,kts}: Use Koin for dependency injection across all platforms
Implement ScreenState pattern for handling loading/success/error states in features

Files:

feature/recent-transaction/build.gradle.kts
cmp-navigation/src/commonMain/kotlin/cmp/navigation/authenticated/AuthenticatedNavigation.kt
feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/navigation/RecentTransactionNavGraph.kt
feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/viewmodel/RecentTransactionViewModel.kt
feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/screen/RecentTransactionScreen.kt

**/{build.gradle.kts,build.gradle}

📄 CodeRabbit inference engine (CLAUDE.md)

**/{build.gradle.kts,build.gradle}: Apply convention plugins from build-logic/convention/ to standardize module configuration across the project
Use JDK 21 for the project

Files:

feature/recent-transaction/build.gradle.kts

**/*ViewModel.kt

📄 CodeRabbit inference engine (CLAUDE.md)

Use ViewModels with StateFlow/SharedFlow for state management

Files:

feature/recent-transaction/src/commonMain/kotlin/org/mifos/mobile/feature/recent/transaction/viewmodel/RecentTransactionViewModel.kt

🧠 Learnings (7)

📓 Common learnings

Learnt from: CR
Repo: openMF/mifos-mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T16:08:08.437Z
Learning: Applies to **/*ViewModel.kt : Use ViewModels with StateFlow/SharedFlow for state management