-
Notifications
You must be signed in to change notification settings - Fork 0
flutter: Transactions Phase B1 — grouping persistence + unit test #67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…nce.share/shareXFiles in QR + ShareService; remove local stub
…hare_plus directly
…g and DeleteFamilyDialog
…or) + fix const-eval in common widgets
…pture and fix duplicates
…ignores; fix template onCancel and batch dialogs snackbars
…vigator capture and mounted guards
…rs (accounts/dashboard types, const-eval, family settings void, email Address stub); wire A/B/C entrances
…oup toggle in TransactionList; optional filter icon in RecentTransactions
…sactions; finalize TransactionList search bar impl
…onFilter); prune TransactionStats fields
…roup controls (Phase B1 UI)
…s; add controller persistence unit test
…on-date as flat for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds SharedPreferences-backed persistence for transaction controller grouping functionality and includes unit tests. It introduces the foundation for transaction grouping by date/category/account with the ability to persist collapsed group states. This is part of Phase B1 of the transaction filtering and grouping feature implementation.
- Adds
TransactionGroupingenum and persistence methods toTransactionController - Implements unit tests for grouping persistence functionality using SharedPreferences
- Adds new user assets screen with routing and navigation entry points
Reviewed Changes
Copilot reviewed 51 out of 55 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/transactions/transaction_controller_grouping_test.dart | New unit test file covering transaction controller grouping and collapse persistence |
| lib/providers/transaction_provider.dart | Adds TransactionGrouping enum, state fields, and SharedPreferences persistence methods |
| lib/screens/accounts/user_assets_screen.dart | New screen for displaying user asset overview with net worth calculations |
| lib/core/router/app_router.dart | Adds routing configuration for user assets screen |
| Various UI files | Multiple code quality improvements including context capture before async operations and constant value fixes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| final prefs = await SharedPreferences.getInstance(); | ||
| final groupingStr = prefs.getString("tx_grouping"); |
Copilot
AI
Sep 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The persistence keys 'tx_grouping' and 'tx_group_collapse' are not namespaced per ledger as mentioned in the PR description. Consider adding ledger context to these keys to support multiple ledgers properly.
| final messenger = ScaffoldMessenger.of(context); | ||
| final router = GoRouter.of(context); | ||
|
|
||
| setState(() { _isLoading = true; |
Copilot
AI
Sep 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The setState call appears to have a formatting issue with the opening brace placement. It should be properly formatted as setState(() {\n_isLoading = true;\n});
| setState(() { _isLoading = true; | |
| setState(() { | |
| _isLoading = true; |
| ? '超支 ${remaining.abs().toStringAsFixed(2)}' | ||
| : '剩余 ${remaining.toStringAsFixed(2)}', |
Copilot
AI
Sep 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The currency formatting has been simplified to use toStringAsFixed(2) which removes proper currency formatting with symbols and locale-specific number formatting. This may impact user experience in different locales.
| final body = [if (text != null && text.isNotEmpty) text, file.path].whereType<String>().join('\n\n'); | ||
| await Share.share(body); |
Copilot
AI
Sep 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The share functionality has been simplified to text-only sharing, which may not provide the same user experience as the original file sharing implementation. Consider documenting this limitation or implementing proper file sharing.
Resolved conflicts in: - transaction_provider.dart: removed duplicate imports, enum, and methods - family_activity_log_screen.dart: use direct stats assignment - theme_management_screen.dart: use pre-captured messenger - family_settings_service.dart: use method signatures with parameters
Adds SharedPreferences-backed persistence for TransactionController grouping and collapsed groups (tx_grouping, tx_group_collapse). Includes unit test covering persistence. No UI changes in this PR; follow-ups will wire UI toggles.\n\nNotes:\n- Scoped to controller; keys not yet namespaced per ledger (planned follow-up).\n- Ran targeted test: test/transactions/transaction_controller_grouping_test.dart — passed.\n