fix/account-switch-in-logout #618
Conversation
WalkthroughConsolidates account/profile loading into a unified fetch in GeneralSettingsScreen, replaces per-account provider caching with _loadAccountsProfileData, updates account-switch and logout flows to use profile lists, and adds ActiveAccountTile to render the active account; imports and legacy metadata providers removed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Screen as GeneralSettingsScreen
participant Accounts as AccountsStore
participant Profiles as ProfileDataLoader
participant UI as ActiveAccountTile
User->>Screen: Open settings
Screen->>Accounts: getAccounts()
Accounts-->>Screen: List<Account>
Screen->>Profiles: loadProfileData concurrently for accounts
Profiles-->>Screen: List<ContactModel> (_accountsProfileData)
Screen-->>UI: provide active account context
UI-->>User: render active account tile
sequenceDiagram
autonumber
actor User
participant Screen as GeneralSettingsScreen
participant Switcher as AccountSwitcherSheet
participant Active as ActiveAccountProvider
User->>Screen: Tap "Switch Account"
Screen->>Switcher: _showAccountSwitcher(_accountsProfileData)
Switcher-->>User: display profiles
User->>Switcher: select profile
Switcher-->>Screen: selectedProfile.publicKey
Screen->>Active: _switchAccount(pubkey)
Active-->>Screen: active pubkey set
sequenceDiagram
autonumber
actor User
participant Screen as GeneralSettingsScreen
participant Auth as AuthService
participant Switcher as AccountSwitcherSheet
User->>Screen: Sign out
Screen->>Auth: signOut()
alt multiple accounts remain
Screen->>Switcher: _showAccountSwitcher()
Switcher-->>Screen: optional selected pubkey
Screen->>Screen: _loadAccountsProfileData()
else single/no account
Screen-->>User: navigate home
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (5 passed)✅ Passed checks (5 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing touches🧪 Generate unit tests
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 |
josefinalliende
force-pushed
the
refactor/settings-account-loading
branch
5 times, most recently
from
72a0b9e to
73b6401
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
lib/ui/settings/widgets/active_account_tile.dart (5)
25-25: Explicitly type local AsyncValue.Declare the exact type to match our guidelines.
- final activeAccountState = ref.watch(activeAccountProvider); + final AsyncValue<ActiveAccountState> activeAccountState = ref.watch(activeAccountProvider);
16-21: Avoid silent empty pubkey; assert non-null when used.This helper is only called when
state.account != null, so use a non-null assertion and drop the empty fallback.- ContactModel _activeAccountProfileData(ActiveAccountState state) { - return ContactModel.fromMetadata( - pubkey: state.account?.pubkey ?? '', - metadata: state.metadata, - ); - } + ContactModel _activeAccountProfileData(ActiveAccountState state) { + assert(state.account != null, 'Active account is null when building profile data'); + return ContactModel.fromMetadata( + pubkey: state.account!.pubkey, + metadata: state.metadata, + ); + }
40-45: Localize user-facing strings and don’t surface raw errors.Replace hardcoded text with AppLocalizations and show a friendly message on error.
+import 'package:flutter_gen/gen_l10n/app_localizations.dart'; ... - return const Center(child: Text('No accounts found')); + return Center(child: Text(AppLocalizations.of(context)!.settingsNoAccountsFound)); ... - error: (error, stack) => Center(child: Text('Error: $error')), + error: (error, stack) => + Center(child: Text(AppLocalizations.of(context)!.genericError)),Note: adjust keys to your l10n ARB.
24-27: Remove blank line inside function.Our Dart guidelines disallow blank lines within functions.
- final AsyncValue<ActiveAccountState> activeAccountState = ref.watch(activeAccountProvider); - + final AsyncValue<ActiveAccountState> activeAccountState = ref.watch(activeAccountProvider);
37-37: Prefer a dedicated route constant.If available, use a named route (e.g.,
Routes.settingsShareProfile) instead of string concatenation.lib/ui/settings/general_settings_screen.dart (4)
47-50: Reload only when the active pubkey actually changes.The current listener reloads profiles on any
AsyncDataemission, which can be frequent (e.g., metadata changes). Guard by pubkey to avoid redundant work.- (previous, next) { - if (next is AsyncData) { - _loadAccountsProfileData(); - } - }, + (previous, next) { + if (next is AsyncData) { + final String? prevKey = + (previous is AsyncData) ? previous.value?.account?.pubkey : null; + final String? nextKey = next.value?.account?.pubkey; + if (prevKey != nextKey) { + _loadAccountsProfileData(); + } + } + },
77-79: Localize toast message.Use AppLocalizations instead of hardcoded strings.
- ref.showErrorToast('Failed to load accounts'); + ref.showErrorToast(AppLocalizations.of(context)!.settingsAccountsLoadFailed);Note: import AppLocalizations and use your actual key.
108-127: Return the bottom-sheet Future so callers can truly await it.
_showAccountSwitcherisasyncbut doesn’t await/return the sheet’s Future;await _showAccountSwitcher()will resolve immediately.Future<void> _showAccountSwitcher({ bool isDismissible = true, bool showSuccessToast = false, }) async { if (_accounts.isEmpty) { await _loadAccountsProfileData(); } if (!mounted) return; - SwitchProfileBottomSheet.show( + return SwitchProfileBottomSheet.show( context: context, profiles: _accountsProfileData, isDismissible: isDismissible, showSuccessToast: showSuccessToast, onProfileSelected: (selectedProfile) async { await _switchAccount(selectedProfile.publicKey); }, ); }Also, remove blank lines within this function to match our style guide.
94-101: No-op switch when selecting the current account.Avoid writing the same pubkey and showing a “switched” toast if the selected account is already active.
Future<void> _switchAccount(String accountPubkey) async { try { - await ref.read(activePubkeyProvider.notifier).setActivePubkey(accountPubkey); + final String? current = ref.read(activePubkeyProvider); + if (current == accountPubkey) { + if (mounted) ref.showSuccessToast('This account is already active'); + return; + } + await ref.read(activePubkeyProvider.notifier).setActivePubkey(accountPubkey); if (mounted) { ref.showSuccessToast('Account switched successfully'); }Note: adjust the read of
activePubkeyProviderif its value type differs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
lib/ui/settings/general_settings_screen.dart(7 hunks)lib/ui/settings/widgets/active_account_tile.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
lib/ui/settings/widgets/active_account_tile.dartlib/ui/settings/general_settings_screen.dart
🧠 Learnings (4)
📚 Learning: 2025-09-07T02:15:31.931Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/src/rust/api/utils.dart:12-16
Timestamp: 2025-09-07T02:15:31.931Z
Learning: The lingering PublicKey references in lib/config/providers/auth_provider.dart, lib/config/providers/group_provider.dart, and rust_builder/cargokit/build_tool/lib/src/options.dart identified during the utils.dart API cleanup were fixed in PR #597 in the whitenoise_flutter codebase.
Applied to files:
lib/ui/settings/general_settings_screen.dart
📚 Learning: 2025-09-07T13:10:16.542Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#597
File: lib/config/providers/group_provider.dart:311-314
Timestamp: 2025-09-07T13:10:16.542Z
Learning: In the whitenoise_flutter codebase, the User class used in group_provider.dart (and similar contexts) is presentational only, not the actual user class from the Rust API. There are plans to remove this User class and replace it with UserProfileData, similar to the planned consolidation with ContactModel.
Applied to files:
lib/ui/settings/general_settings_screen.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use the whitenoise rust crate (via flutter_rust_bridge) as the source of all data and only way to trigger changes in our data model.
Applied to files:
lib/ui/settings/general_settings_screen.dart
📚 Learning: 2025-08-23T11:02:28.308Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#527
File: lib/ui/core/ui/wn_avatar.dart:1-6
Timestamp: 2025-08-23T11:02:28.308Z
Learning: In the whitenoise_flutter codebase, flutter_screenutil is re-exported through lib/ui/core/themes/src/app_theme.dart, so files that import app_theme.dart do not need to directly import flutter_screenutil to use .w, .h, .sp extensions.
Applied to files:
lib/ui/settings/general_settings_screen.dart
🔇 Additional comments (1)
lib/ui/settings/general_settings_screen.dart (1)
195-202: Flow after logout looks good.Reloading accounts and showing the switcher when more than one remain is consistent with the PR’s intent.
josefinalliende
force-pushed
the
refactor/settings-account-loading
branch
from
4509d49 to
f1f17f7
Compare
josefinalliende
changed the title
josefinalliende
force-pushed
the
refactor/settings-account-loading
branch
from
612e987 to
46b26be
Compare
Description
This PR simplifies a bit the general settings screen account loading and account switching logic. By making this changes, I realized that there was a bug in the switcher for the logout case #623 , where account that is about to be logged out is still shown.
Just changing the active pubkey is enough now, there is no need to reload groups of follows manually from the settings screen cause those providers are listening to the active pubkey provider.
This PR:
Type of Change
Checklist
just precommitto ensure that formatting and linting are correctjust check-flutter-coverageto ensure that flutter coverage rules are passingCHANGELOG.mdfile with your changes (if they affect the user experience)Summary by CodeRabbit
New Features
Enhancements
UX