AI scribe toggle #4208
AI scribe toggle #4208
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR adds comprehensive AI Scribe configuration support across the application. It introduces a new AIScribeConfig model extending PreferencesConfig with an isEnabled flag, along with supporting state classes and a GetAIScribeConfigInteractor for configuration retrieval. The composer controller is updated to cache and expose aiScribeConfig, with initialization occurring during setup. The preferences layer gains storage management for AI Scribe settings through PreferencesSettingManager. The UI layer is enhanced to conditionally render AI Scribe options based on both configuration availability and capability support. Localization strings are added for French, Russian, and Vietnamese languages. The manage account dashboard adds capability detection, and preferences UI components filter display based on runtime AI Scribe availability. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
|
This PR has been deployed to https://linagora.github.io/tmail-flutter/4208. |
|
(Deleted an unrelated comment) |
dab246
force-pushed
the
ai-scribe-jmap
branch
from
5289b0d to
86d9551
Compare
hoangdat
force-pushed
the
ai-scribe-settings
branch
from
17d84cf to
e17af18
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
lib/features/composer/presentation/composer_view.dart (1)
473-475: AIScribe enablement check is correct; consider centralizing itUsing
controller.aiScribeConfig.isEnabled && controller.isAIScribeAvailablehere correctly ties the AI assistant button to both server capability and the local toggle.Given the same condition is now needed in multiple composer views (and already drifted on web tablet), consider exposing a single
controller.canOpenAIScribeModal(or similar) and using it everywhere to keep behavior consistent.test/features/composer/presentation/composer_controller_test.dart (1)
57-57: Test setup looks correct, but consider adding tests for AIScribe config behavior.The mock setup for GetAIScribeConfigInteractor correctly mirrors the production code changes (import, mock spec, field declaration, initialization, and constructor injection). However, there are no tests that verify the AIScribe config loading behavior or how the controller uses the config.
Consider adding tests to verify:
- AIScribe config is loaded when ComposerController is initialized
- The cached AIScribe config is accessible via the aiScribeConfig getter
- UI behavior changes based on AIScribe config availability
Example test structure:
test('Should load AIScribe config on initialization', () async { // arrange when(mockGetAIScribeConfigInteractor.execute()) .thenAnswer((_) => Stream.value(Right(GetAIScribeConfigSuccess(AIScribeConfig.initial())))); // act await composerController?.initializeView(); // assert verify(mockGetAIScribeConfigInteractor.execute()); expect(composerController?.aiScribeConfig, isNotNull); });Also applies to: 184-184, 230-230, 302-302, 320-320
lib/features/manage_account/domain/model/preferences/ai_scribe_config.dart (1)
1-34: AIScribeConfig model is sound; consider a const constructor if allowedThe model cleanly represents a single
isEnabledpreference with JSON and equality support; copyWith is correct. IfPreferencesConfighas a const default constructor, you could makeAIScribeConfig’s constructorconstto improve immutability and enable const usage in more places, but this is purely optional.lib/features/composer/presentation/composer_controller.dart (1)
302-320: Consider handling GetAIScribeConfigFailure explicitly to avoid noisy errors
_loadAIScribeConfig()runs on every composer init and its success path simply updates the cached config, which is fine. However, anyGetAIScribeConfigFailurecurrently falls through tosuper.handleFailureViewState(failure), which may surface a generic error toast even though the feature can safely continue with the defaultAIScribeConfig.initial()value.If BaseController does show user‑visible errors for generic
FeatureFailures, you might want to special‑case AIScribe config failures here, e.g. log and ignore while keeping the default config, so opening the composer doesn’t randomly show an error when AIScribe is unavailable.Possible adjustment (if BaseController shows a toast for this failure)
@override void handleFailureViewState(Failure failure) { - if (failure is LocalFilePickerFailure) { + if (failure is LocalFilePickerFailure) { _handlePickFileFailure(failure); } else if (failure is LocalImagePickerFailure) { _handlePickImageFailure(failure); + } else if (failure is GetAIScribeConfigFailure) { + // Optional: silently ignore AIScribe config failures here, + // relying on the default AIScribeConfig.initial(). + logError('ComposerController::_loadAIScribeConfig failed: ${failure.exception}'); } else { super.handleFailureViewState(failure); } }Also applies to: 322-324, 408-429
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
lib/features/composer/presentation/composer_bindings.dart(2 hunks)lib/features/composer/presentation/composer_controller.dart(7 hunks)lib/features/composer/presentation/composer_view.dart(1 hunks)lib/features/composer/presentation/composer_view_web.dart(1 hunks)lib/features/manage_account/data/datasource_impl/manage_account_datasource_impl.dart(2 hunks)lib/features/manage_account/data/local/preferences_setting_manager.dart(5 hunks)lib/features/manage_account/domain/model/preferences/ai_scribe_config.dart(1 hunks)lib/features/manage_account/domain/model/preferences/preferences_setting.dart(3 hunks)lib/features/manage_account/domain/state/get_ai_scribe_config_state.dart(1 hunks)lib/features/manage_account/domain/usecases/get_ai_scribe_config_interactor.dart(1 hunks)lib/features/manage_account/presentation/manage_account_dashboard_controller.dart(2 hunks)lib/features/manage_account/presentation/model/preferences_option_type.dart(5 hunks)lib/features/manage_account/presentation/preferences/bindings/preferences_interactors_bindings.dart(3 hunks)lib/features/manage_account/presentation/preferences/preferences_controller.dart(3 hunks)lib/features/manage_account/presentation/preferences/preferences_view.dart(1 hunks)lib/l10n/intl_fr.arb(1 hunks)lib/l10n/intl_ru.arb(1 hunks)lib/l10n/intl_vi.arb(1 hunks)lib/main/localizations/app_localizations.dart(1 hunks)test/features/composer/presentation/composer_controller_test.dart(5 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-12-15T06:24:50.369Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4137
File: lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart:32-32
Timestamp: 2025-12-15T06:24:50.369Z
Learning: In lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart and lib/features/manage_account/presentation/manage_account_dashboard_controller.dart, SentryManager.instance.setUser() intentionally sends PII (account ID, display name, username, email) to Sentry for error tracking. The team plans to document this in the app's privacy policy for transparency and compliance.
Applied to files:
lib/features/manage_account/presentation/manage_account_dashboard_controller.dartlib/features/manage_account/presentation/preferences/preferences_controller.darttest/features/composer/presentation/composer_controller_test.dartlib/features/manage_account/presentation/preferences/bindings/preferences_interactors_bindings.dartlib/features/manage_account/data/datasource_impl/manage_account_datasource_impl.dartlib/features/composer/presentation/composer_bindings.dartlib/features/composer/presentation/composer_controller.dartlib/features/manage_account/data/local/preferences_setting_manager.dart
📚 Learning: 2025-12-09T09:36:45.349Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4194
File: lib/features/manage_account/presentation/manage_account_dashboard_controller.dart:174-176
Timestamp: 2025-12-09T09:36:45.349Z
Learning: In Dart/Flutter projects using GetX, do not wrap getBinding<T>() calls in try/catch since they return null when not found. Only wrap Get.find<T>() calls in try/catch because they throw if a dependency is unavailable. When a binding is optional, consider checking Get.isRegistered<T>() or handling the null/exception path gracefully instead of blindly catching, and document the expectation for failure modes where a dependency may not be registered.
Applied to files:
lib/features/manage_account/presentation/manage_account_dashboard_controller.dartlib/features/manage_account/domain/model/preferences/preferences_setting.dartlib/features/manage_account/presentation/preferences/preferences_controller.darttest/features/composer/presentation/composer_controller_test.dartlib/features/manage_account/presentation/preferences/bindings/preferences_interactors_bindings.dartlib/features/manage_account/data/datasource_impl/manage_account_datasource_impl.dartlib/features/manage_account/domain/usecases/get_ai_scribe_config_interactor.dartlib/features/manage_account/domain/state/get_ai_scribe_config_state.dartlib/features/composer/presentation/composer_view_web.dartlib/features/composer/presentation/composer_bindings.dartlib/features/manage_account/presentation/model/preferences_option_type.dartlib/features/manage_account/presentation/preferences/preferences_view.dartlib/features/composer/presentation/composer_controller.dartlib/features/manage_account/domain/model/preferences/ai_scribe_config.dartlib/features/manage_account/data/local/preferences_setting_manager.dartlib/main/localizations/app_localizations.dartlib/features/composer/presentation/composer_view.dart
📚 Learning: 2025-12-09T12:47:45.861Z
Learnt from: zatteo
Repo: linagora/tmail-flutter PR: 4196
File: scribe/lib/scribe/ai/data/datasource_impl/ai_datasource_impl.dart:34-35
Timestamp: 2025-12-09T12:47:45.861Z
Learning: In the tmail-flutter repository, avoid suggesting replacing DioError with DioException unless there is a codebase-wide migration. This pattern applies to all Dart files; when reviewing, only propose a DioError-safe change (e.g., compatible error handling or conversion) unless a global migration is in place. Ensure consistency of DioError usage across modules and flag any deviations for a repo-wide decision.
Applied to files:
lib/features/manage_account/presentation/manage_account_dashboard_controller.dartlib/features/manage_account/domain/model/preferences/preferences_setting.dartlib/features/manage_account/presentation/preferences/preferences_controller.darttest/features/composer/presentation/composer_controller_test.dartlib/features/manage_account/presentation/preferences/bindings/preferences_interactors_bindings.dartlib/features/manage_account/data/datasource_impl/manage_account_datasource_impl.dartlib/features/manage_account/domain/usecases/get_ai_scribe_config_interactor.dartlib/features/manage_account/domain/state/get_ai_scribe_config_state.dartlib/features/composer/presentation/composer_view_web.dartlib/features/composer/presentation/composer_bindings.dartlib/features/manage_account/presentation/model/preferences_option_type.dartlib/features/manage_account/presentation/preferences/preferences_view.dartlib/features/composer/presentation/composer_controller.dartlib/features/manage_account/domain/model/preferences/ai_scribe_config.dartlib/features/manage_account/data/local/preferences_setting_manager.dartlib/main/localizations/app_localizations.dartlib/features/composer/presentation/composer_view.dart
📚 Learning: 2025-12-12T04:54:11.121Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4191
File: lib/features/email/presentation/extensions/handle_email_action_extension.dart:37-80
Timestamp: 2025-12-12T04:54:11.121Z
Learning: In lib/features/email/presentation/extensions/handle_email_action_extension.dart, the mailboxDashBoardController.selectedEmail should only be synchronized when isMobileThreadDisabled is true. This is intentional behavior and should not be changed to update selectedEmail in non-mobile or thread-enabled contexts.
Applied to files:
test/features/composer/presentation/composer_controller_test.dartlib/features/composer/presentation/composer_controller.dart
📚 Learning: 2025-12-08T08:11:08.985Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4189
File: lib/features/login/presentation/extensions/handle_company_server_login_info_extension.dart:50-56
Timestamp: 2025-12-08T08:11:08.985Z
Learning: In the tmail-flutter codebase, prefer descriptive variable names that include full context rather than abbreviated names. For example, `removeLoginInfoInteractor` is preferred over shorter alternatives like `removeInteractor` or `interactor`, as it clearly conveys both the action (remove) and the specific data being operated on (loginInfo).
Applied to files:
test/features/composer/presentation/composer_controller_test.dartlib/features/manage_account/presentation/preferences/bindings/preferences_interactors_bindings.dartlib/features/composer/presentation/composer_bindings.dart
📚 Learning: 2025-12-12T07:43:26.643Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4204
File: lib/features/email/presentation/extensions/handle_label_for_email_extension.dart:94-148
Timestamp: 2025-12-12T07:43:26.643Z
Learning: In lib/features/email/presentation/extensions/handle_label_for_email_extension.dart and similar keyword synchronization code, the addKeyword() function is the preferred method for adding keywords to emails. The resyncKeywords() pattern is being phased out and will be replaced in favor of addKeyword().
Applied to files:
lib/features/composer/presentation/composer_controller.dart
🔇 Additional comments (19)
lib/features/manage_account/domain/model/preferences/preferences_setting.dart (1)
3-3: LGTM!The AIScribeConfig integration follows the established pattern used for other preference configs in this file. The getter implementation correctly uses
firstWhereOrNullwith a type check and provides a sensible fallback toAIScribeConfig.initial().Also applies to: 19-19, 53-61
lib/features/manage_account/presentation/manage_account_dashboard_controller.dart (1)
15-15: LGTM!The
isAICapabilitySupportedgetter is implemented consistently with other capability support checks in this controller. The pattern of null-checkingaccountId.valueandsessionCurrentbefore callingisSupported()matches the existing codebase conventions.Also applies to: 336-342
lib/l10n/intl_vi.arb (1)
5215-5232: AI Scribe localization keys look consistent with usageThe new
aiScribe*keys and metadata follow the existing ARB structure and match the accessors inPreferencesOptionType/AppLocalizations. No issues from a localization/intl tooling perspective.lib/features/manage_account/presentation/preferences/bindings/preferences_interactors_bindings.dart (1)
9-9: GetAIScribeConfigInteractor DI wiring is consistentThe interactor is bound and disposed in line with existing patterns (
SaveLanguageInteractor, server settings interactors) and uses the same taggedManageAccountRepository. No functional or lifecycle issues spotted.Also applies to: 66-69, 96-110
lib/features/manage_account/data/datasource_impl/manage_account_datasource_impl.dart (1)
6-6: AIScribeConfig integration into local settings toggling is soundHandling
AIScribeConfigas a dedicated branch, delegating to_preferencesSettingManager.updateAIScribeand then reloading preferences, is consistent with howThreadDetailConfigandSpamReportConfigare treated. No correctness or error‑handling issues observed.Also applies to: 48-51
lib/features/manage_account/presentation/preferences/preferences_view.dart (1)
73-80: Correctly gating AI Scribe preference on capabilityFiltering
PreferencesOptionType.aiScribebycontroller.isAIScribeAvailableensures the AI Scribe toggle only appears when the capability is supported, without impacting other local options. This matches the intended behavior.lib/features/manage_account/presentation/model/preferences_option_type.dart (1)
7-13: PreferencesOptionType extended cleanly for AI ScribeThe
aiScribeoption is integrated consistently:
- Marked as
isLocal: true.- Title/explanation/toggle description wired to the new localization keys.
isEnableddelegates topreferencesSetting.aiScribeConfig.isEnabled.This aligns with the existing pattern for other local options (thread, spamReport).
Also applies to: 18-31, 33-46, 48-61, 63-79
lib/features/manage_account/presentation/preferences/preferences_controller.dart (1)
10-10: PreferencesController wiring for AI Scribe is consistent with existing local options
isAIScribeAvailablecleanly proxies the capability flag fromManageAccountDashBoardController.- The new
aiScribebranch in_updateLocalPreferencesSettingmirrors the pattern forthreadandspamReport, flippingisEnabledand delegating toUpdateLocalSettingsInteractorvia anAIScribeConfig.No issues with flow or state management here.
Also applies to: 51-55, 146-160
lib/features/composer/presentation/composer_bindings.dart (2)
57-57: LGTM!The import for GetAIScribeConfigInteractor is correctly added to support the new AI Scribe configuration feature.
342-342: The GetAIScribeConfigInteractor is properly registered by PreferencesInteractorsBindings with the correct composerId tag, and cleanup is correctly implemented. No action needed.lib/main/localizations/app_localizations.dart (1)
5142-5161: LGTM!The three new localization getters (aiScribe, aiScribeSettingExplanation, aiScribeToggleDescription) follow the established Intl.message pattern and provide appropriate user-facing strings for the AI Scribe feature.
lib/features/manage_account/data/local/preferences_setting_manager.dart (4)
4-4: LGTM!The import and constant key follow the established naming conventions for preferences management.
Also applies to: 21-22
47-48: LGTM!The case for AIScribeConfig in the loadPreferences switch statement correctly uses AIScribeConfig.fromJson for deserialization, consistent with other config types.
79-83: LGTM!The savePreferences method correctly handles AIScribeConfig persistence using the new key and JSON serialization, following the pattern used for other config types.
152-168: LGTM!The getAIScribeConfig() and updateAIScribe() methods follow the established patterns in this class:
- getAIScribeConfig() reloads preferences, retrieves the JSON string, and returns either initial or deserialized config
- updateAIScribe() uses the get-modify-save pattern with copyWith
Both implementations are consistent with similar methods for ThreadConfig, SpamReportConfig, and TextFormattingMenuConfig.
lib/features/manage_account/domain/usecases/get_ai_scribe_config_interactor.dart (1)
1-21: Implementation is correct and all dependencies are properly implemented.The interactor correctly follows the standard use case pattern with proper state emission flow (loading → success/failure). The ManageAccountRepository.getLocalSettings() method is implemented in the repository layer, returns PreferencesSetting, and the model includes the aiScribeConfig property with proper null-safe fallback to AIScribeConfig.initial().
lib/l10n/intl_ru.arb (1)
5127-5144: AI Scribe RU localization entries look consistent and well‑formedKeys, metadata blocks, and Russian wording are correct and match surrounding settings conventions. No changes needed.
lib/features/manage_account/domain/state/get_ai_scribe_config_state.dart (1)
1-18: AIScribe config state classes follow existing state patternLoading/success/failure types are correctly wired, expose AIScribeConfig via props, and integrate with the shared Failure/Success base classes. Looks ready to use with the existing state machinery.
lib/features/composer/presentation/composer_controller.dart (1)
110-114: AIScribeConfig injection and caching in ComposerController look consistentThe new AIScribeConfig Rx cache, getter, and injected
GetAIScribeConfigInteractorare wired in the same style as other interactors/controllers in this file. Defaulting toAIScribeConfig.initial()ensures a safe, enabled-by-default state until the real config loads.Also applies to: 167-168, 186-187, 274-275
dab246
force-pushed
the
ai-scribe-settings
branch
from
159d09a to
9603bd1
Compare
…sing all three AI Scribe keys
Add a toggle to enable or disable AI scribe. Toggle is displayed only if JMAP capability is present. It is then stored locally.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.