Skip to content

Commit c67a30a

Browse files
committed
fixup! fixup! fixup! fixup! fixup! fixup! fixup! TF-4363 Add collapse threads support for search email presentation and tests
Signed-off-by: dab246 <tdvu@linagora.com>
1 parent ab50508 commit c67a30a

18 files changed

Lines changed: 197 additions & 75 deletions
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
# 85. Riverpod State Management Pilot — Local Settings & Collapsed Thread in Search
2+
3+
Date: 2026-04-23
4+
5+
## Status
6+
7+
Accepted
8+
9+
## Related ADRs
10+
11+
- [ADR-0071](./0071-collapse-threads-in-email-query.md) — collapseThreads in Email/query
12+
13+
## Context
14+
15+
As part of implementing the collapsed thread feature in search email (TF-4363), two controllers need to read the `collapseThreads` local setting:
16+
17+
- `ThreadController` — must react when the setting changes to re-trigger search
18+
- `SearchEmailController` — must read the current value at query time
19+
20+
The setting is written by `PreferencesController` via `UpdateLocalSettingsInteractor` when the user toggles it in `PreferencesView`, and must also be reset on logout to prevent stale cross-account data.
21+
22+
Rather than routing this through a GetX-based `LocalSettingsService` with a reactive `Rx` field and `ever(...)` listeners — which would require manual equality tracking and expose mutable state broadly — we chose to use Riverpod's `StateNotifier` as the single source of truth from the start.
23+
24+
This is also a **pilot** to validate the GetX + Riverpod coexistence pattern before applying it more broadly across the codebase.
25+
26+
## Decision
27+
28+
Introduce `localSettingsNotifierProvider` (`StateNotifier<PreferencesSetting>`) backed by a global `ProviderContainer` (`appProviderContainer`). All reads and writes go through this provider directly.
29+
30+
### Architecture
31+
32+
```text
33+
┌─────────────────────────────────────────────────────┐
34+
│ appProviderContainer (global ProviderContainer) │
35+
│ │
36+
│ localSettingsNotifierProvider │
37+
│ StateNotifier<PreferencesSetting> │
38+
│ ▲ write ▲ write read ▼ │
39+
│ │ │ │ │
40+
│ LocalSettings Preferences Thread / │
41+
│ Service Controller Search │
42+
│ (initial load) (on toggle) Controllers │
43+
└─────────────────────────────────────────────────────┘
44+
```
45+
46+
### Write paths
47+
48+
**On app start / settings route reload:**
49+
`LocalSettingsService.onInit()``_loadInitialSettings()` → cache read → `localSettingsNotifierProvider.notifier.update()`
50+
51+
**On user toggle in PreferencesView:**
52+
`PreferencesController.handleSuccessViewState(UpdateLocalSettingsSuccess)``localSettingsNotifierProvider.notifier.update()`
53+
54+
**On logout:**
55+
`BaseController.clearAllData()``localSettingsNotifierProvider.notifier.update(PreferencesSetting.initial())`
56+
57+
### Read paths
58+
59+
**`ThreadController`** — subscribes via `appProviderContainer.listen(localSettingsNotifierProvider, ...)`. Riverpod skips notification when `PreferencesSetting` equality is unchanged (`EquatableMixin`), so no manual dedup is needed.
60+
61+
**`SearchEmailController`** — reads on demand via `appProviderContainer.read(localSettingsNotifierProvider)`.
62+
63+
### `LocalSettingsService` role
64+
65+
`LocalSettingsService` is a `GetxService` acting as a **temporary bridge** between the GetX DI graph and Riverpod. It remains a `GetxService` because all DI wiring in the project uses GetX bindings — introducing a plain class would be inconsistent with the existing layer. Its sole responsibility is loading the initial value from cache into the provider on startup via `onInit`.
66+
67+
It will be removed entirely once `ManageAccountRepository` and its datasources are migrated to Riverpod providers, at which point `LocalSettingsNotifier` can inject and call `GetLocalSettingsInteractor` directly.
68+
69+
### Settings route reload on web
70+
71+
When the user reloads the browser directly on the settings route, `MailboxDashBoardBindings` does not run. `ManageAccountDashBoardBindings` guards against this:
72+
73+
```dart
74+
if (!Get.isRegistered<LocalSettingsService>()) {
75+
Get.put(LocalSettingsService(Get.find<GetLocalSettingsInteractor>()));
76+
}
77+
```
78+
79+
## Consequences
80+
81+
**Positive**
82+
- No redundant cache round-trip — provider is updated directly from `UpdateLocalSettingsSuccess` result
83+
- No manual equality tracking needed — Riverpod + `EquatableMixin` handles it
84+
- Single write surface for `PreferencesSetting`; no mutable `Rx` exposed across the codebase
85+
- Validates the GetX + Riverpod coexistence pattern for future migrations
86+
87+
**Negative**
88+
- `appProviderContainer` is a global singleton — controllers depend directly on a concrete implementation rather than an abstraction, which is a known DIP violation. The correct solution is to wrap it behind an interface (e.g. `LocalSettingsRepository`), but doing so now would add complexity before the pattern is proven at scale. This should be addressed when the migration reaches the repository layer.
89+
- Two DI systems (GetX + Riverpod) coexist during the migration period; developers must know which layer owns which state
90+
- `LocalSettingsService` is a temporary class that must be removed when the migration reaches the repository layer

integration_test/scenarios/labels/create_label_from_choose_label_modal_with_existing_labels_scenario.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ class CreateLabelFromChooseLabelModalWithExistingLabelsScenario
4242

4343
await threadRobot.openMailbox();
4444
await expectViewVisible($(LabelListView));
45-
await $.native.pressBack();
45+
await $.platformAutomator.android.pressBack();
4646

4747
await threadRobot.longPressEmailWithSubject(
4848
'Email 1 subject ${existingLabel.safeDisplayName}',

lib/features/base/base_controller.dart

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ import 'package:tmail_ui_user/main/exceptions/remote/method_level_exception.dart
6262
import 'package:tmail_ui_user/main/exceptions/remote/network_exception.dart';
6363
import 'package:tmail_ui_user/main/localizations/app_localizations.dart';
6464
import 'package:tmail_ui_user/main/routes/app_routes.dart';
65+
import 'package:tmail_ui_user/features/manage_account/domain/model/preferences/preferences_setting.dart';
66+
import 'package:tmail_ui_user/features/manage_account/presentation/providers/local_settings_notifier.dart';
67+
import 'package:tmail_ui_user/main/providers/app_provider_container.dart';
6568
import 'package:tmail_ui_user/main/routes/route_navigation.dart';
6669
import 'package:tmail_ui_user/main/utils/app_config.dart';
6770
import 'package:tmail_ui_user/main/universal_import/html_stub.dart' as html;
@@ -601,6 +604,11 @@ abstract class BaseController extends GetxController
601604
logWarning('BaseController::clearAllData: Cannot clear all data: $e');
602605
} finally {
603606
AttachmentKeywordConfigManager().clearCache();
607+
// Reset local settings to the default value so that stale account data
608+
// is not visible if another account logs in before the next reload completes.
609+
appProviderContainer
610+
.read(localSettingsNotifierProvider.notifier)
611+
.update(PreferencesSetting.initial());
604612
}
605613
}
606614

lib/features/mailbox_dashboard/presentation/bindings/mailbox_dashboard_bindings.dart

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,6 @@ class MailboxDashBoardBindings extends BaseBindings {
176176
void dependencies() {
177177
CleanupBindings().dependencies();
178178
super.dependencies();
179-
Get.put(LocalSettingsService(
180-
Get.find<GetLocalSettingsInteractor>(),
181-
));
182179
SendingQueueBindings().dependencies();
183180
MailboxBindings().dependencies();
184181
ThreadBindings().dependencies();
@@ -190,6 +187,10 @@ class MailboxDashBoardBindings extends BaseBindings {
190187

191188
@override
192189
void bindingsController() {
190+
// Must be registered first so [localSettingsNotifierProvider] is populated
191+
// before any controller reads local settings.
192+
Get.put(LocalSettingsService(Get.find<GetLocalSettingsInteractor>()));
193+
193194
Get.put(AppGridDashboardController(
194195
Get.find<GetAppDashboardConfigurationInteractor>(),
195196
Get.find<GetAppGridLinagraEcosystemInteractor>(),
Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
1-
import 'package:get/get.dart';
21
import 'package:tmail_ui_user/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart';
3-
import 'package:tmail_ui_user/features/manage_account/presentation/services/local_settings_service.dart';
42
import 'package:tmail_ui_user/features/thread_detail/presentation/action/thread_detail_ui_action.dart';
53

64
extension NotifyThreadDetailSettingUpdated on MailboxDashBoardController {
7-
Future<void> notifyThreadDetailSettingUpdated() async {
8-
await Get.find<LocalSettingsService>().reload();
5+
void notifyThreadDetailSettingUpdated() {
96
dispatchThreadDetailUIAction(UpdatedThreadDetailSettingAction());
107
}
118
}

lib/features/manage_account/presentation/manage_account_dashboard_bindings.dart

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import 'package:get/get.dart';
2+
import 'package:tmail_ui_user/features/manage_account/domain/usecases/get_local_settings_interactor.dart';
23
import 'package:tmail_ui_user/features/manage_account/presentation/bindings/setting_interactor_bindings.dart';
34
import 'package:tmail_ui_user/features/manage_account/presentation/identities/identity_bindings.dart';
45
import 'package:tmail_ui_user/features/manage_account/presentation/manage_account_dashboard_controller.dart';
56
import 'package:tmail_ui_user/features/manage_account/presentation/menu/manage_account_menu_bindings.dart';
67
import 'package:tmail_ui_user/features/manage_account/presentation/menu/settings/settings_bindings.dart';
8+
import 'package:tmail_ui_user/features/manage_account/presentation/services/local_settings_service.dart';
79
import 'package:tmail_ui_user/features/paywall/presentation/paywall_bindings.dart';
810

911
class ManageAccountDashBoardBindings extends Bindings {
@@ -12,6 +14,12 @@ class ManageAccountDashBoardBindings extends Bindings {
1214
void dependencies() {
1315
SettingInteractorBindings().dependencies();
1416
PaywallBindings().dependencies();
17+
// Ensure [LocalSettingsService] is available when the user navigates
18+
// directly to the settings route (e.g. web page reload), bypassing
19+
// [MailboxDashBoardBindings] which also registers it during a normal session.
20+
if (!Get.isRegistered<LocalSettingsService>()) {
21+
Get.put(LocalSettingsService(Get.find<GetLocalSettingsInteractor>()));
22+
}
1523
Get.put(ManageAccountDashBoardController());
1624
SettingsBindings().dependencies();
1725
ManageAccountMenuBindings().dependencies();

lib/features/manage_account/presentation/preferences/preferences_controller.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import 'package:tmail_ui_user/features/manage_account/domain/state/update_local_
1818
import 'package:tmail_ui_user/features/manage_account/domain/usecases/get_local_settings_interactor.dart';
1919
import 'package:tmail_ui_user/features/manage_account/domain/usecases/update_local_settings_interactor.dart';
2020
import 'package:tmail_ui_user/features/manage_account/presentation/manage_account_dashboard_controller.dart';
21+
import 'package:tmail_ui_user/features/manage_account/presentation/providers/local_settings_notifier.dart';
22+
import 'package:tmail_ui_user/main/providers/app_provider_container.dart';
2123
import 'package:tmail_ui_user/features/manage_account/presentation/model/preferences_option_type.dart';
2224
import 'package:tmail_ui_user/features/server_settings/domain/state/get_server_setting_state.dart';
2325
import 'package:tmail_ui_user/features/server_settings/domain/state/update_server_setting_state.dart';
@@ -85,6 +87,9 @@ class PreferencesController extends BaseController {
8587
_updateLocalSettingOptionValue(success.preferencesSetting);
8688
} else if (success is UpdateLocalSettingsSuccess) {
8789
_updateLocalSettingOptionValue(success.preferencesSetting);
90+
appProviderContainer
91+
.read(localSettingsNotifierProvider.notifier)
92+
.update(success.preferencesSetting);
8893
} else if (success is GettingLocalSettingsState) {
8994
_localSettingLoaderStatus = LoaderStatus.loading;
9095
} else {
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import 'package:flutter_riverpod/flutter_riverpod.dart';
2+
import 'package:tmail_ui_user/features/manage_account/domain/model/preferences/preferences_setting.dart';
3+
4+
class LocalSettingsNotifier extends StateNotifier<PreferencesSetting> {
5+
LocalSettingsNotifier() : super(PreferencesSetting.initial());
6+
7+
void update(PreferencesSetting settings) {
8+
state = settings;
9+
}
10+
}
11+
12+
final localSettingsNotifierProvider =
13+
StateNotifierProvider<LocalSettingsNotifier, PreferencesSetting>(
14+
(ref) => LocalSettingsNotifier(),
15+
);

lib/features/manage_account/presentation/services/local_settings_service.dart

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,41 @@
11
import 'package:core/utils/app_logger.dart';
22
import 'package:get/get.dart';
3-
import 'package:tmail_ui_user/features/manage_account/domain/model/preferences/preferences_setting.dart';
43
import 'package:tmail_ui_user/features/manage_account/domain/state/get_local_settings_state.dart';
54
import 'package:tmail_ui_user/features/manage_account/domain/usecases/get_local_settings_interactor.dart';
5+
import 'package:tmail_ui_user/features/manage_account/presentation/providers/local_settings_notifier.dart';
6+
import 'package:tmail_ui_user/main/providers/app_provider_container.dart';
67

8+
/// Responsible for loading local settings from cache on startup and pushing
9+
/// the result into [localSettingsNotifierProvider] so all Riverpod consumers
10+
/// receive the initial value automatically.
711
class LocalSettingsService extends GetxService {
812
final GetLocalSettingsInteractor _getLocalSettingsInteractor;
913

1014
LocalSettingsService(this._getLocalSettingsInteractor);
1115

12-
final localSettings = PreferencesSetting.initial().obs;
13-
14-
Future<void>? _pendingLoad;
15-
1616
@override
1717
void onInit() {
1818
super.onInit();
19-
reload();
19+
_loadInitialSettings();
2020
}
2121

22-
Future<void> reload() => _pendingLoad ??= _loadLocalSettings()
23-
.whenComplete(() => _pendingLoad = null);
24-
25-
Future<void> _loadLocalSettings() {
22+
Future<void> _loadInitialSettings() {
2623
return _getLocalSettingsInteractor.execute().last.then(
2724
(result) => result.fold(
2825
(failure) => logWarning(
29-
'LocalSettingsService::_loadLocalSettings:failure: $failure',
26+
'LocalSettingsService::_loadInitialSettings:failure: $failure',
3027
),
3128
(success) {
3229
if (success is GetLocalSettingsSuccess) {
33-
localSettings.value = success.preferencesSetting;
30+
appProviderContainer
31+
.read(localSettingsNotifierProvider.notifier)
32+
.update(success.preferencesSetting);
3433
}
3534
},
3635
),
3736
).catchError((error, stackTrace) {
3837
logWarning(
39-
'LocalSettingsService::_loadLocalSettings:error: $error | stackTrace: $stackTrace',
38+
'LocalSettingsService::_loadInitialSettings:error: $error | stackTrace: $stackTrace',
4039
);
4140
});
4241
}

lib/features/search/email/presentation/search_email_controller.dart

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ import 'package:tmail_ui_user/features/mailbox_dashboard/domain/usecases/quick_s
5353
import 'package:tmail_ui_user/features/mailbox_dashboard/domain/usecases/save_recent_search_interactor.dart';
5454
import 'package:tmail_ui_user/features/mailbox_dashboard/presentation/action/dashboard_action.dart';
5555
import 'package:tmail_ui_user/features/mailbox_dashboard/presentation/extensions/handle_store_email_sort_order_extension.dart';
56-
import 'package:tmail_ui_user/features/manage_account/presentation/services/local_settings_service.dart';
56+
import 'package:tmail_ui_user/features/manage_account/presentation/providers/local_settings_notifier.dart';
57+
import 'package:tmail_ui_user/main/providers/app_provider_container.dart';
5758
import 'package:tmail_ui_user/features/mailbox_dashboard/presentation/extensions/update_emails_with_new_mailbox_id_extension.dart';
5859
import 'package:tmail_ui_user/features/mailbox_dashboard/presentation/model/dashboard_routes.dart';
5960
import 'package:tmail_ui_user/features/mailbox_dashboard/presentation/model/search/email_receive_time_type.dart';
@@ -162,10 +163,8 @@ class SearchEmailController extends BaseController
162163

163164
Set<String> get listHasKeywordFiltered => searchEmailFilter.value.hasKeyword;
164165

165-
late final LocalSettingsService _localSettingsService = Get.find<LocalSettingsService>();
166-
167166
bool get _isCollapseThreadsEnabled =>
168-
_localSettingsService.localSettings.value.threadConfig.isEnabled;
167+
appProviderContainer.read(localSettingsNotifierProvider).threadConfig.isEnabled;
169168

170169
SearchEmailController(
171170
this._quickSearchEmailInteractor,

0 commit comments

Comments
 (0)