TF-4141 Expected actions #4210
TF-4141 Expected actions #4210
Conversation
WalkthroughAdds an "Action Required" virtual mailbox and AI "needs-action" capability spanning models, settings, controllers, filters, UI, resources, and tests. Introduces a needs-action keyword and mailbox role, PresentationEmail.hasNeedAction, server setting aiNeedsActionEnabled and capability checks, mailbox/dashboard extensions to auto-create/remove the folder, a centralized MailboxFilterBuilder, TagWidget and AiActionTagWidget, localization and image/color assets, updated preference wiring, and multiple unit/widget tests and fixtures. Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks✅ 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 |
![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 (4)
lib/features/mailbox_dashboard/presentation/extensions/handle_ai_action_extension.dart (1)
20-27: Consider using non-null assertions for consistency.Since
isAiCapabilitySupportedalready validates thatsessionCurrentandaccountId.valueare non-null, the method could use!assertions for clarity. However, the current implementation is safe becausegetCountEmailslikely handles nullable parameters.void autoRefreshCountEmailsInActionRequiredFolder() { if (isAiCapabilitySupported) { actionRequiredFolderController.getCountEmails( - session: sessionCurrent, - accountId: accountId.value, + session: sessionCurrent!, + accountId: accountId.value!, ); } }lib/features/mailbox/presentation/extensions/list_mailbox_node_extension.dart (1)
8-14: Return value discarded ininsertAfterInbox.The method delegates to
insertAfterByPrioritywhich returns abool, butinsertAfterInboxhas avoidreturn type and discards the result. If callers need to know whether insertion occurred, this should return the value.- /// Insert [newNode] after Inbox if present, otherwise at the beginning. - void insertAfterInbox(MailboxNode newNode) { - insertAfterByPriority( + /// Insert [newNode] after Inbox if present, otherwise at the beginning. + /// Returns true if insertion occurred, false if node already exists. + bool insertAfterInbox(MailboxNode newNode) { + return insertAfterByPriority( newNode, [_isInbox], ); }lib/features/base/widget/labels/tag_widget.dart (1)
57-64: Truncation logic may produce strings longer than maxLength.The current implementation subtracts 1 from
maxLengthbut adds a 3-character ellipsis (...), resulting in strings up tomaxLength + 2characters. Consider usingmaxLength - 3or the Unicode ellipsis character (…).String _truncateName(String name, {int maxLength = 16}) { try { if (name.length <= maxLength) return name; - return '${name.substring(0, maxLength - 1)}...'; + return '${name.substring(0, maxLength - 3)}…'; } catch (_) { return name; } }lib/features/mailbox/presentation/extensions/handle_action_required_tab_extension.dart (1)
50-63: Consider clarifying the mutation semantics.The
insertAfterStarredOrInboxmethod mutateschildrenin-place and returns a boolean indicating whether insertion occurred. The current logic correctly handles both cases (insert vs. update count), but the side-effect-based control flow could be clearer with a comment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
assets/images/ic_mailbox_action_required.svgis excluded by!**/*.svg
📒 Files selected for processing (41)
core/lib/presentation/extensions/color_extension.dart(1 hunks)core/lib/presentation/resources/image_paths.dart(1 hunks)lib/features/base/widget/labels/ai_action_tag_widget.dart(1 hunks)lib/features/base/widget/labels/tag_widget.dart(1 hunks)lib/features/email/presentation/utils/email_action_reactor/email_action_reactor.dart(1 hunks)lib/features/home/domain/extensions/session_extensions.dart(1 hunks)lib/features/mailbox/presentation/extensions/handle_action_required_tab_extension.dart(1 hunks)lib/features/mailbox/presentation/extensions/list_mailbox_node_extension.dart(1 hunks)lib/features/mailbox/presentation/extensions/presentation_mailbox_extension.dart(3 hunks)lib/features/mailbox/presentation/mailbox_controller.dart(5 hunks)lib/features/mailbox/presentation/mixin/mailbox_widget_mixin.dart(1 hunks)lib/features/mailbox_dashboard/presentation/bindings/action_required_interactor_bindings.dart(1 hunks)lib/features/mailbox_dashboard/presentation/bindings/mailbox_dashboard_bindings.dart(2 hunks)lib/features/mailbox_dashboard/presentation/controller/action_required_folder_controller.dart(1 hunks)lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart(3 hunks)lib/features/mailbox_dashboard/presentation/extensions/handle_ai_action_extension.dart(1 hunks)lib/features/mailbox_dashboard/presentation/mailbox_dashboard_view_web.dart(1 hunks)lib/features/search/email/presentation/search_email_view.dart(2 hunks)lib/features/search/mailbox/presentation/search_mailbox_controller.dart(2 hunks)lib/features/thread/data/repository/thread_repository_impl.dart(2 hunks)lib/features/thread/domain/exceptions/thread_exceptions.dart(1 hunks)lib/features/thread/domain/repository/thread_repository.dart(1 hunks)lib/features/thread/domain/state/get_count_emails_in_folder_state.dart(1 hunks)lib/features/thread/domain/usecases/get_count_unread_emails_in_folder_interactor.dart(1 hunks)lib/features/thread/presentation/extensions/handle_pull_to_refresh_list_email_extension.dart(2 hunks)lib/features/thread/presentation/extensions/handle_select_message_filter_extension.dart(1 hunks)lib/features/thread/presentation/thread_controller.dart(11 hunks)lib/features/thread/presentation/thread_view.dart(5 hunks)lib/features/thread/presentation/widgets/email_tile_builder.dart(4 hunks)lib/features/thread/presentation/widgets/email_tile_web_builder.dart(7 hunks)lib/features/thread/presentation/widgets/web_tablet_body_email_item_widget.dart(4 hunks)lib/l10n/intl_messages.arb(2 hunks)lib/main/localizations/app_localizations.dart(1 hunks)model/lib/email/presentation_email.dart(1 hunks)model/lib/extensions/keyword_identifier_extension.dart(1 hunks)model/lib/extensions/presentation_mailbox_extension.dart(1 hunks)model/lib/mailbox/presentation_mailbox.dart(2 hunks)test/features/mailbox/presentation/extensions/list_mailbox_node_extension_test.dart(1 hunks)test/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller_test.dart(4 hunks)test/features/mailbox_dashboard/presentation/view/mailbox_dashboard_view_widget_test.dart(4 hunks)test/features/search/verify_before_time_in_search_email_filter_test.dart(4 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4204
File: lib/features/email/presentation/model/popup_menu_item_email_action.dart:34-35
Timestamp: 2025-12-12T09:03:05.962Z
Learning: In the tmail-flutter codebase, popup menu items intentionally use a consistent hoverIcon (imagePaths.icThumbsUp) across all popup item types (email actions, mailbox actions, profile settings, etc.), while actionIcon varies per action type. This is a deliberate UI/UX design choice.
📚 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:
model/lib/extensions/presentation_mailbox_extension.dartmodel/lib/email/presentation_email.dartlib/features/thread/presentation/extensions/handle_select_message_filter_extension.dartlib/features/mailbox_dashboard/presentation/extensions/handle_ai_action_extension.dartlib/features/search/mailbox/presentation/search_mailbox_controller.dartlib/features/mailbox/presentation/extensions/presentation_mailbox_extension.dartlib/features/thread/presentation/widgets/web_tablet_body_email_item_widget.dartlib/features/email/presentation/utils/email_action_reactor/email_action_reactor.dartlib/features/mailbox_dashboard/presentation/mailbox_dashboard_view_web.dartlib/features/mailbox/presentation/extensions/handle_action_required_tab_extension.dartlib/features/mailbox_dashboard/presentation/controller/action_required_folder_controller.dartlib/features/thread/presentation/widgets/email_tile_builder.dartlib/features/mailbox_dashboard/presentation/bindings/mailbox_dashboard_bindings.dartlib/features/thread/presentation/widgets/email_tile_web_builder.dartlib/features/thread/presentation/extensions/handle_pull_to_refresh_list_email_extension.dartlib/features/thread/presentation/thread_controller.dartlib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.darttest/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller_test.dartlib/features/thread/presentation/thread_view.dartlib/features/search/email/presentation/search_email_view.dartmodel/lib/mailbox/presentation_mailbox.dartlib/features/mailbox/presentation/mixin/mailbox_widget_mixin.darttest/features/search/verify_before_time_in_search_email_filter_test.dartlib/features/mailbox_dashboard/presentation/bindings/action_required_interactor_bindings.darttest/features/mailbox_dashboard/presentation/view/mailbox_dashboard_view_widget_test.dartlib/features/mailbox/presentation/mailbox_controller.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:
model/lib/extensions/presentation_mailbox_extension.dartmodel/lib/extensions/keyword_identifier_extension.dartlib/features/thread/domain/repository/thread_repository.dartlib/features/thread/data/repository/thread_repository_impl.dartcore/lib/presentation/extensions/color_extension.dartcore/lib/presentation/resources/image_paths.dartmodel/lib/email/presentation_email.dartlib/features/thread/presentation/extensions/handle_select_message_filter_extension.dartlib/features/home/domain/extensions/session_extensions.dartlib/features/mailbox_dashboard/presentation/extensions/handle_ai_action_extension.dartlib/features/search/mailbox/presentation/search_mailbox_controller.dartlib/features/mailbox/presentation/extensions/presentation_mailbox_extension.dartlib/main/localizations/app_localizations.darttest/features/mailbox/presentation/extensions/list_mailbox_node_extension_test.dartlib/features/thread/presentation/widgets/web_tablet_body_email_item_widget.dartlib/features/email/presentation/utils/email_action_reactor/email_action_reactor.dartlib/features/mailbox_dashboard/presentation/mailbox_dashboard_view_web.dartlib/features/base/widget/labels/ai_action_tag_widget.dartlib/features/mailbox/presentation/extensions/handle_action_required_tab_extension.dartlib/features/mailbox_dashboard/presentation/controller/action_required_folder_controller.dartlib/features/thread/presentation/widgets/email_tile_builder.dartlib/features/base/widget/labels/tag_widget.dartlib/features/mailbox_dashboard/presentation/bindings/mailbox_dashboard_bindings.dartlib/features/thread/presentation/widgets/email_tile_web_builder.dartlib/features/thread/presentation/extensions/handle_pull_to_refresh_list_email_extension.dartlib/features/thread/domain/usecases/get_count_unread_emails_in_folder_interactor.dartlib/features/thread/presentation/thread_controller.dartlib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.darttest/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller_test.dartlib/features/thread/presentation/thread_view.dartlib/features/search/email/presentation/search_email_view.dartmodel/lib/mailbox/presentation_mailbox.dartlib/features/mailbox/presentation/mixin/mailbox_widget_mixin.darttest/features/search/verify_before_time_in_search_email_filter_test.dartlib/features/thread/domain/state/get_count_emails_in_folder_state.dartlib/features/mailbox_dashboard/presentation/bindings/action_required_interactor_bindings.darttest/features/mailbox_dashboard/presentation/view/mailbox_dashboard_view_widget_test.dartlib/features/mailbox/presentation/mailbox_controller.dartlib/features/thread/domain/exceptions/thread_exceptions.dartlib/features/mailbox/presentation/extensions/list_mailbox_node_extension.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:
model/lib/extensions/presentation_mailbox_extension.dartmodel/lib/extensions/keyword_identifier_extension.dartlib/features/thread/domain/repository/thread_repository.dartlib/features/thread/data/repository/thread_repository_impl.dartcore/lib/presentation/extensions/color_extension.dartcore/lib/presentation/resources/image_paths.dartmodel/lib/email/presentation_email.dartlib/features/thread/presentation/extensions/handle_select_message_filter_extension.dartlib/features/home/domain/extensions/session_extensions.dartlib/features/mailbox_dashboard/presentation/extensions/handle_ai_action_extension.dartlib/features/search/mailbox/presentation/search_mailbox_controller.dartlib/features/mailbox/presentation/extensions/presentation_mailbox_extension.dartlib/main/localizations/app_localizations.darttest/features/mailbox/presentation/extensions/list_mailbox_node_extension_test.dartlib/features/thread/presentation/widgets/web_tablet_body_email_item_widget.dartlib/features/email/presentation/utils/email_action_reactor/email_action_reactor.dartlib/features/mailbox_dashboard/presentation/mailbox_dashboard_view_web.dartlib/features/base/widget/labels/ai_action_tag_widget.dartlib/features/mailbox/presentation/extensions/handle_action_required_tab_extension.dartlib/features/mailbox_dashboard/presentation/controller/action_required_folder_controller.dartlib/features/thread/presentation/widgets/email_tile_builder.dartlib/features/base/widget/labels/tag_widget.dartlib/features/mailbox_dashboard/presentation/bindings/mailbox_dashboard_bindings.dartlib/features/thread/presentation/widgets/email_tile_web_builder.dartlib/features/thread/presentation/extensions/handle_pull_to_refresh_list_email_extension.dartlib/features/thread/domain/usecases/get_count_unread_emails_in_folder_interactor.dartlib/features/thread/presentation/thread_controller.dartlib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.darttest/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller_test.dartlib/features/thread/presentation/thread_view.dartlib/features/search/email/presentation/search_email_view.dartmodel/lib/mailbox/presentation_mailbox.dartlib/features/mailbox/presentation/mixin/mailbox_widget_mixin.darttest/features/search/verify_before_time_in_search_email_filter_test.dartlib/features/thread/domain/state/get_count_emails_in_folder_state.dartlib/features/mailbox_dashboard/presentation/bindings/action_required_interactor_bindings.darttest/features/mailbox_dashboard/presentation/view/mailbox_dashboard_view_widget_test.dartlib/features/mailbox/presentation/mailbox_controller.dartlib/features/thread/domain/exceptions/thread_exceptions.dartlib/features/mailbox/presentation/extensions/list_mailbox_node_extension.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:
model/lib/extensions/keyword_identifier_extension.dartlib/features/thread/data/repository/thread_repository_impl.dartmodel/lib/email/presentation_email.dartlib/features/thread/presentation/widgets/web_tablet_body_email_item_widget.dartlib/features/thread/presentation/widgets/email_tile_builder.dartlib/features/thread/presentation/widgets/email_tile_web_builder.dartlib/features/thread/presentation/extensions/handle_pull_to_refresh_list_email_extension.dartlib/features/thread/presentation/thread_controller.dartlib/features/thread/presentation/thread_view.dartlib/features/search/email/presentation/search_email_view.dart
📚 Learning: 2025-12-12T07:56:31.877Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4204
File: lib/features/email/data/datasource_impl/email_hive_cache_datasource_impl.dart:579-582
Timestamp: 2025-12-12T07:56:31.877Z
Learning: In lib/features/email/data/datasource_impl/email_hive_cache_datasource_impl.dart, the addLabelToEmail method intentionally throws UnimplementedError because label synchronization with the email cache is handled separately via extension methods (e.g., _autoSyncLabelToSelectedEmailOnMemory in handle_label_for_email_extension.dart). Implementing it in the cache datasource would cause data conflicts. This differs from other keyword operations like markAsRead, markAsStar, markAsAnswered, and markAsForwarded, which are implemented directly in the cache datasource.
Applied to files:
lib/features/thread/data/repository/thread_repository_impl.dartlib/features/thread/presentation/widgets/web_tablet_body_email_item_widget.dartlib/features/thread/presentation/widgets/email_tile_builder.dartlib/features/thread/presentation/widgets/email_tile_web_builder.dartlib/features/thread/presentation/extensions/handle_pull_to_refresh_list_email_extension.dartlib/features/thread/presentation/thread_view.dartlib/features/search/email/presentation/search_email_view.dartlib/features/mailbox_dashboard/presentation/bindings/action_required_interactor_bindings.dart
📚 Learning: 2025-12-12T09:03:05.962Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4204
File: lib/features/email/presentation/model/popup_menu_item_email_action.dart:34-35
Timestamp: 2025-12-12T09:03:05.962Z
Learning: In the tmail-flutter codebase, popup menu items intentionally use a consistent hoverIcon (imagePaths.icThumbsUp) across all popup item types (email actions, mailbox actions, profile settings, etc.), while actionIcon varies per action type. This is a deliberate UI/UX design choice.
Applied to files:
core/lib/presentation/resources/image_paths.dartlib/features/thread/presentation/extensions/handle_select_message_filter_extension.dartlib/features/mailbox/presentation/extensions/presentation_mailbox_extension.dartlib/main/localizations/app_localizations.dartlib/features/thread/presentation/widgets/web_tablet_body_email_item_widget.dartlib/features/mailbox_dashboard/presentation/mailbox_dashboard_view_web.dartlib/features/thread/presentation/widgets/email_tile_builder.dartlib/features/thread/presentation/widgets/email_tile_web_builder.dartlib/features/thread/presentation/thread_view.dartlib/features/search/email/presentation/search_email_view.dart
📚 Learning: 2025-12-15T06:24:38.823Z
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:38.823Z
Learning: In lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart and lib/features/manage_account/presentation/manage_account_dashboard_controller.dart, the code calls SentryManager.instance.setUser() with PII fields (account ID, display name, username, email). This is privacy-sensitive. Review and ensure PII handling complies with policy: avoid sending unnecessary PII to Sentry, redact or hash sensitive fields if possible, or document explicit privacy policy and user consent for such data. If PII must be sent, ensure minimal data, secure handling, and add notes to the privacy policy. Consider adding tests or a lint rule to flag setUser calls that include PII.
Applied to files:
lib/features/mailbox_dashboard/presentation/controller/action_required_folder_controller.dartlib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart
📚 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/mailbox_dashboard/presentation/bindings/mailbox_dashboard_bindings.dartlib/features/thread/presentation/extensions/handle_pull_to_refresh_list_email_extension.darttest/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller_test.darttest/features/mailbox_dashboard/presentation/view/mailbox_dashboard_view_widget_test.dartlib/features/mailbox/presentation/mailbox_controller.dart
📚 Learning: 2025-12-12T11:00:57.625Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4205
File: core/lib/utils/app_logger.dart:80-86
Timestamp: 2025-12-12T11:00:57.625Z
Learning: In the tmail-flutter codebase, logging philosophy differs by platform: web applications use the `webConsoleEnabled` parameter in `app_logger.dart` to selectively display logs in production console, while mobile applications intentionally suppress all logs in release builds (enforced by `BuildUtils.isDebugMode` check in `_debugPrint`).
Applied to files:
lib/features/thread/presentation/extensions/handle_pull_to_refresh_list_email_extension.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/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller_test.darttest/features/mailbox_dashboard/presentation/view/mailbox_dashboard_view_widget_test.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Build web version and deploy
- GitHub Check: analyze-test (model)
- GitHub Check: analyze-test (contact)
- GitHub Check: analyze-test (fcm)
- GitHub Check: analyze-test (email_recovery)
- GitHub Check: analyze-test (core)
- GitHub Check: analyze-test (server_settings)
- GitHub Check: analyze-test (default)
- GitHub Check: analyze-test (rule_filter)
- GitHub Check: analyze-test (forward)
🔇 Additional comments (63)
lib/features/mailbox_dashboard/presentation/mailbox_dashboard_view_web.dart (1)
547-548: TheisActionRequiredproperty is properly defined and the filtering logic is consistently applied.The property is correctly defined in
model/lib/extensions/presentation_mailbox_extension.dartas a non-nullable bool getter that compares the mailbox role. The conditional exclusion of the unread filter option follows the same pattern as the existingisFavoritecheck and is consistently applied across both the web view and thread view components, using the null-safe!= truecomparison. The changes align with the "Action Required" mailbox feature design.core/lib/presentation/extensions/color_extension.dart (1)
289-289: LGTM: New color constant for AI action tag.The
aiActionTagcolor constant is properly defined and follows the existing naming convention in this file.model/lib/mailbox/presentation_mailbox.dart (3)
22-22: LGTM: Action-required role constant added.The
actionRequiredRoleconstant follows the same pattern as other mailbox role constants in this class.
30-34: LGTM: Action-required folder definition.The
actionRequiredFolderstatic field is properly initialized with appropriateMailboxId,MailboxName, androle, following the same pattern asfavoriteFolderon lines 25-29.
47-47: LGTM: Action-required role object.The
roleActionRequiredstatic field follows the same pattern as other role definitions in this class.lib/features/thread/domain/repository/thread_repository.dart (1)
97-100: LGTM: New repository method for counting unread emails.The method signature is clear and follows the existing repository pattern. The return type
Future<int>is appropriate for a count operation.core/lib/presentation/resources/image_paths.dart (1)
48-48: LGTM: New image path accessor for action-required mailbox.The
icMailboxActionRequiredgetter follows the existing pattern and naming convention used throughout this file.lib/features/thread/presentation/extensions/handle_pull_to_refresh_list_email_extension.dart (2)
41-41:limitEmailFetchedInFolderis properly defined in ThreadController.The property exists as a getter that dynamically returns
nullfor action-required mailboxes andThreadConstants.defaultLimitotherwise, enabling flexible email fetch limits per mailbox state. The change improves upon the hardcoded constant by supporting this context-aware behavior.
50-50:shouldBypassCacheis properly defined in ThreadController.The property is correctly defined as a getter on line 579-580 (
bool get shouldBypassCache => selectedMailbox?.isVirtualFolder ?? false;) and is accessible within the extension, abstracting the caching logic appropriately for virtual folder handling.lib/features/thread/presentation/extensions/handle_select_message_filter_extension.dart (1)
16-17: isActionRequired property is properly defined and consistently used.The conditional logic correctly excludes the unread filter option when viewing an action-required mailbox. The property is accessible via the presentation_mailbox_extension import and follows the same safe navigation pattern as the isFavorite check on line 18.
lib/features/search/mailbox/presentation/search_mailbox_controller.dart (1)
258-262: Code changes verified successfully.The reactive listener pattern is correctly implemented.
actionRequiredFolderControlleris properly accessible viadashboardController, andonActionRequiredFolderCountChangedis defined via theMailboxActionHandlerMixin. The listener follows the established pattern used throughout the codebase.lib/features/mailbox_dashboard/presentation/bindings/mailbox_dashboard_bindings.dart (1)
211-211: The controller instantiation is correct.
ActionRequiredFolderControllerhas no explicit constructor and uses Dart's implicit default constructor. Dependencies are intentionally injected after instantiation via theinjectBinding()method (called inmailbox_dashboard_controller.dartline 868), following a lazy binding pattern. No constructor parameters are required or expected.model/lib/email/presentation_email.dart (1)
141-143:hasNeedActiongetter is consistent with existing keyword flagsImplementation matches the existing
hasRead/hasStarredpattern (null‑safecontainsKeycheck) and looks correct.lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart (1)
119-120: EnsureActionRequiredFolderControlleris always registered beforeMailboxDashBoardControlleris built
actionRequiredFolderControlleris resolved eagerly withGet.find<ActionRequiredFolderController>()and later used only whenisAiCapabilitySupportedis true, which is fine logically. However, this assumes the binding forActionRequiredFolderControlleris in place for every code path (app + tests) that constructsMailboxDashBoardController; otherwiseGet.findwill throw at construction time, even when AI is not supported.Please double‑check that:
MailboxDashBoardBindings(and any test setup) unconditionally registersActionRequiredFolderControllerbefore this controller is instantiated, and- there is no alternative construction path that bypasses those bindings.
If there are flows where the AI feature (and its controller) is optional, consider making this dependency conditional (e.g., via
Get.isRegistered<ActionRequiredFolderController>()) while still avoiding wrappinggetBinding<T>()in try/catch, as per existing GetX usage in this repo.[ suggest_optional_refactor ]
As per coding guidelines, …Also applies to: 126-126, 236-237, 867-869
lib/features/home/domain/extensions/session_extensions.dart (1)
25-37: AI capability identifier addition is consistent
linagoraAICapabilityfollows the same pattern as the other Linagora capability identifiers and is safe to expose forisSupportedchecks; no issues here.lib/main/localizations/app_localizations.dart (1)
5440-5452: New localization keys for “Action Required” look correct; confirm capitalizationBoth
actionRequiredandactionRequiredMailboxDisplayNameare wired correctly viaIntl.messageand follow existing naming conventions. The only nuance is the different casing ('Action Required'vs'Action required'); if mailbox names are generally Title Case (e.g. Inbox, Starred), you may want to confirm that this mixed‑case variant is intentional for the mailbox display name.lib/features/mailbox/presentation/mixin/mailbox_widget_mixin.dart (1)
24-56: Behavior change: default virtual folders now expose only “Open in new tab” (web) and no actions on mobileSwitching the early guard from
mailbox.isFavoritetomailbox.isVirtualFoldermeans all virtual default folders (Starred, Action Required, etc.) now:
- On web: only show
openInNewTab.- On mobile: have an empty actions list.
This is sensible if virtual folders are meant to be read‑only aggregations with no folder‑level operations, but it does change the mobile UX (no context/popup actions at all for those folders). Please confirm this is the intended behavior for all virtual folders and that the UI correctly handles an empty actions list (e.g., no broken or empty menus).
lib/features/email/presentation/utils/email_action_reactor/email_action_reactor.dart (1)
716-723: LGTM – Virtual folder abstraction properly applied.The change from
isFavoritetoisVirtualFoldercorrectly extends the existing favorites logic to also cover the new action-required mailbox. When viewing virtual folders (favorites or action-required), the code now properly falls back to finding the actual mailbox that contains the email viapresentationEmail.findMailboxContain(mapMailbox).model/lib/extensions/keyword_identifier_extension.dart (1)
8-8: LGTM – Keyword identifier follows JMAP conventions.The new
needActionMailkeyword is properly defined following the JMAP standard format with the$prefix, consistent with the existingunsubscribeMailpattern.lib/features/thread/domain/exceptions/thread_exceptions.dart (1)
1-3: LGTM – Exception declarations are appropriate.The exception classes follow Dart conventions for simple exception types.
InteractorIsNullExceptionappears to be used for validating interactor dependencies.lib/features/thread/data/repository/thread_repository_impl.dart (1)
528-544: LGTM – Efficient implementation for counting unread action-required emails.The method correctly filters emails with the
needActionMailkeyword that are not seen, requesting only theidproperty to minimize data transfer. The implementation efficiently returns the count with a safe null check.lib/features/mailbox/presentation/extensions/presentation_mailbox_extension.dart (1)
20-21: LGTM – Action-required mailbox role properly integrated.The action-required role is consistently added to all three display methods (
getDisplayName,getDisplayNameWithoutContext, andgetMailboxIcon), following the existing pattern for other mailbox roles.Also applies to: 51-52, 82-83
model/lib/extensions/presentation_mailbox_extension.dart (1)
39-41: LGTM – Virtual folder abstraction is well-designed.The
isActionRequiredgetter follows the established pattern for role checks, andisVirtualFolderprovides a clean abstraction for grouping virtual mailboxes (favorites and action-required). This enables consistent handling of virtual folders throughout the codebase.lib/l10n/intl_messages.arb (1)
5133-5144: LGTM – Localization entries properly defined.The new translation keys follow the ARB format correctly. The different capitalization between
actionRequired("Action Required") andactionRequiredMailboxDisplayName("Action required") appears intentional for their respective UI contexts (tag label vs. mailbox name).lib/features/base/widget/labels/ai_action_tag_widget.dart (1)
7-23: LGTM – Clean, focused widget implementation.The
AiActionTagWidgetis well-designed with:
- Proper const constructor for performance
- Composition pattern using
TagWidget- Platform-specific tooltip behavior (web only)
- Clear single responsibility
test/features/mailbox_dashboard/presentation/view/mailbox_dashboard_view_widget_test.dart (1)
57-57: LGTM! Proper test setup for ActionRequiredFolderController.The mock registration follows the established pattern used for other controllers in this test file (e.g.,
SpamReportController,DownloadController). ThefallbackGeneratorsare correctly specified to handle GetX lifecycle callbacks.Also applies to: 174-174, 241-241, 329-329
lib/features/mailbox/presentation/mailbox_controller.dart (4)
72-72: LGTM! Import added for the new action-required tab extension.The import follows the existing pattern for similar extensions (e.g.,
handle_favorite_tab_extension.darton line 73).
266-287: LGTM! Consistent setup of action-required folder after mailbox operations.The
setUpActionRequiredFoldercalls are correctly placed alongsideaddFavoriteFolderToMailboxList()in all three branches (failure, success, and create-default success), ensuring the action-required folder is consistently initialized after mailbox data is fetched.
441-445: LGTM! Reactive subscription for action-required folder count changes.The
ever()subscription correctly observes count changes from theactionRequiredFolderControllerand delegates toonActionRequiredFolderCountChanged. This follows the established reactive pattern used elsewhere in this controller.
614-614: LGTM! Action-required folder setup after mailbox refresh.Correctly placed after
addFavoriteFolderToMailboxList()to maintain consistency with other mailbox initialization flows.lib/features/thread/presentation/widgets/email_tile_builder.dart (2)
23-23: LGTM! Clean addition of AI-enabled flag.The
isAIEnabledparameter with a default value oftrueallows callers to opt-out when AI capability is not supported, while maintaining backward compatibility.Also applies to: 35-35
134-134: LGTM! Conditional AI action tag rendering.The
_shouldShowAIActiongetter correctly combines the feature flag (isAIEnabled) with the email's action state (hasNeedAction), ensuring the tag only appears when both conditions are met. The placement in the email partial content row is appropriate.Also applies to: 142-144
lib/features/thread/presentation/widgets/web_tablet_body_email_item_widget.dart (2)
171-172: LGTM! Consistent AI action tag rendering.The conditional rendering follows the same pattern as
EmailTileBuilder, placing theAiActionTagWidgetin the email content row when the condition is met.
26-26: Default values are consistent in the web/tablet context.Both
WebTabletBodyEmailItemWidget.shouldShowAIActionand the webEmailTileBuilder.isAIEnableddefault tofalse. The caller correctly passes the computed_shouldShowAIActionvalue (which combinesisAIEnabled && hasNeedAction), so there is no inconsistency requiring verification.Likely an incorrect or invalid review comment.
lib/features/mailbox_dashboard/presentation/extensions/handle_ai_action_extension.dart (1)
6-18: LGTM! Clean capability check implementation.The
isAiCapabilitySupportedgetter properly validates bothaccountIdandsessionCurrentbefore delegating to the capability validation. The early return pattern is clear and defensive.lib/features/mailbox/presentation/extensions/list_mailbox_node_extension.dart (2)
28-46: LGTM! Well-designed generic insertion method.The
insertAfterByPrioritymethod is cleanly implemented:
- Prevents duplicates via
_containsMailboxcheck- Iterates through priorities in order
- Falls back to inserting at the beginning
- Returns a boolean to indicate success
This generalization reduces code duplication and makes the insertion logic extensible.
48-64: LGTM! Robust mailbox matching predicates.The helper methods correctly check both
roleandname(case-insensitive) for mailbox identification, which handles edge cases where the role might not be set but the name is "Inbox" or "Starred".lib/features/mailbox_dashboard/presentation/bindings/action_required_interactor_bindings.dart (1)
21-78: LGTM! Dependency structure is well-organized and correctly sequenced.The
ActionRequiredInteractorBindingsclass correctly follows the establishedInteractorsBindingspattern. All required dependencies—ThreadAPI,EmailCacheManager,StateCacheManager, andIOSSharingManager—are registered upfront inmain_bindingsviaLocalBindingsandNetworkBindings, before this binding is invoked conditionally during dashboard initialization. The layered dependency chain (data source implementations → abstractions → repository implementation → abstraction → interactor) is properly structured usingGet.lazyPutfor lazy initialization.lib/features/thread/presentation/widgets/email_tile_web_builder.dart (3)
12-12: LGTM! Clean addition of AI action support flag.The new
isAIEnabledproperty is well-structured with a safe default offalse, and the import is properly added.Also applies to: 30-30, 45-45
175-175: Consistent AI action tag rendering across all responsive layouts.The
AiActionTagWidgetis correctly rendered conditionally using_shouldShowAIActionacross mobile, tablet, and desktop layouts. The margin adjustment for desktop layout ensures proper spacing.Also applies to: 192-192, 376-377
425-427: Well-encapsulated condition for AI action visibility.The
_shouldShowAIActiongetter cleanly combines the AI capability check with the email's action requirement state.test/features/search/verify_before_time_in_search_email_filter_test.dart (1)
57-57: Correct test infrastructure update for ActionRequiredFolderController.The mock specification and DI registration follow the established patterns in this test file, ensuring the new controller dependency is properly wired for tests.
Also applies to: 144-144, 212-212, 359-359
lib/features/base/widget/labels/tag_widget.dart (1)
28-35: Consider if manual truncation is redundant with TextOverflow.ellipsis.The
Textwidget already hasoverflow: TextOverflow.ellipsisandmaxLines: 1. The manual_truncateNametruncation may be intended for tooltip display purposes (showing full text in tooltip while truncating visible text), but if not needed, it adds complexity.lib/features/mailbox_dashboard/presentation/controller/action_required_folder_controller.dart (3)
15-26: Well-structured controller with proper dependency injection pattern.The use of
getBinding<T>()for retrieving the interactor is correct (returns null when not found), and the nullable field with lazy injection viainjectBinding()follows established patterns in this codebase. Based on learnings, no try/catch is needed forgetBinding<T>().
28-57: Defensive null checks provide clear failure states.The method properly validates all required parameters before execution and emits appropriate failure states with specific exceptions, enabling callers to distinguish between different failure modes.
59-81: Appropriate state handling and cleanup.Success/failure handlers correctly update the observable count, and
onClose()properly cleans up the interactor reference to prevent memory leaks.test/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller_test.dart (1)
60-60: Consistent test infrastructure update.The mock specification and DI registration for
ActionRequiredFolderControllercorrectly follow the established patterns in this test file.Also applies to: 173-173, 250-250, 333-333
test/features/mailbox/presentation/extensions/list_mailbox_node_extension_test.dart (2)
98-180: Comprehensive test coverage for insertAfterStarredOrInbox.Tests cover all key scenarios: insertion after starred (by role and name), fallback to inbox, fallback to beginning, duplicate prevention, and case-insensitive matching.
182-288: Thorough test coverage for insertAfterByPriority.The tests effectively validate priority-based insertion with multiple predicates, fallback behavior, duplicate prevention, and empty list handling. The local predicate helpers (
isInbox,isStarred,isSent) make the tests readable and maintainable.lib/features/search/email/presentation/search_email_view.dart (3)
669-710: Clean consolidation of mobile/web list rendering.The refactoring to use
ListView.separatedwith a unified_buildEmailItemmethod improves code maintainability by eliminating duplicate branches. The separator logic correctly handles platform-specific styling.
713-759: Well-structured email item builder with proper reactive state.The
Obxwrapper correctly reads reactive state from the dashboard controller, and theisAIEnabledflag is properly passed toEmailTileBuilder. The extraction of this logic into a dedicated method improves readability.
21-21: The import is necessary and correctly used in the file.The
handle_ai_action_extension.dartextension provides theisAiCapabilitySupportedgetter onMailboxDashBoardController, which is accessed at lines 727–728 (dashboardController.isAiCapabilitySupported) and passed to theEmailTileBuilderat line 737. The import is required for this extension method to be available.Likely an incorrect or invalid review comment.
lib/features/thread/domain/state/get_count_emails_in_folder_state.dart (1)
1-18: LGTM!The state classes follow the established pattern in the codebase with proper
LoadingState,UIState, andFeatureFailureimplementations. Thepropsoverride for equatable comparison is correctly implemented.lib/features/thread/domain/usecases/get_count_unread_emails_in_folder_interactor.dart (1)
9-28: LGTM!The interactor follows the standard stream-based pattern with proper loading → success/failure state emissions. The repository call is correctly awaited and exceptions are properly wrapped in the typed failure class.
lib/features/thread/presentation/thread_controller.dart (5)
579-585: Verify unbounded fetch for action-required mailbox.When
isActionRequiredis true,limitEmailFetchedInFolderreturnsnull, which means no limit is applied. Ensure this is intentional and the server/backend can handle potentially large result sets efficiently.
625-651: LGTM on the filter condition refactor.The new
getFilterConditionForLoadMailboxmethod cleanly separates filter building logic for favorite, action-required, and regular mailboxes. The delegation to specialized builder methods improves readability.
783-802: LGTM on WebSocket handling for virtual folders.The handler correctly routes to
_refreshChangeListEmailsInVirtualFolderfor virtual folders and triggers the auto-refresh for action-required folder count after processing messages.
951-954: Verify virtual folder email validation logic.For virtual folders,
_validatePresentationEmailbypasses the_belongToCurrentMailboxIdcheck, allowing emails from any mailbox. This aligns with the virtual folder concept (favorites/action-required span multiple mailboxes), but ensure duplicate detection via_notDuplicatedInCurrentListis sufficient.
680-715: ConfirmFilterMessageOption.unreadexclusion is intentional for action-required mailbox.The
_buildActionRequiredMailboxFilterlacks a case forFilterMessageOption.unread—it falls through todefault. This is intentional. The UI filter menu explicitly excludes unread as an option for action-required mailboxes (if (selectedMailbox?.isActionRequired != true) FilterMessageOption.unread), since the default case already ensures all action-required emails are filtered for unread status (vianotKeyword: emailSeenandhasKeyword: needActionMail).lib/features/thread/presentation/thread_view.dart (4)
498-529: LGTM on reactive refactor for dragging state.The
Obxwrapper correctly encapsulates all reactive reads (currentSelectMode,isSenderImportantFlagEnabled,selectedEmail,isAiCapabilitySupported), ensuring the widget rebuilds appropriately when these values change. TheisAIEnabledparameter is properly wired.
532-594: Good extraction of swipe background widgets.The background and secondary background widgets are cleanly extracted. Note that
presentationEmail.hasReadis read outside theObxblock, but this is acceptable since the parent list rebuilds when email state changes.
596-641: LGTM on the Dismissible refactor.The reactive values are correctly read inside
Obx, and theEmailTileBuildernow receives theisAIEnabledflag. The separation of background widget definitions improves readability.
781-784: LGTM on virtual folder check for empty state.The change from
isFavoritetoisVirtualFolderaligns with the broader abstraction introduced in this PR, correctly handling both favorite and action-required mailboxes.
lib/features/mailbox/presentation/extensions/handle_action_required_tab_extension.dart
Outdated
Show resolved
Hide resolved
|
This PR has been deployed to https://linagora.github.io/tmail-flutter/4210. |
|
Stupid question: How do we get the count of It is either...
Personal proposal: Like Thanks. |
We haven't had any discussions on this issue yet, so for now I'm displaying the |
Re-move it ! |
|
Functionally, the idea is to provide the user with information so they know whether or not there are actions expected of them (and how many). Ultimately, I would like to have this counter. However, if this presents technical problems, we can imagine starting without it for the time being. |
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
lib/features/mailbox_dashboard/presentation/extensions/handle_ai_action_extension.dart (1)
3-3: Unused import.The
capability_validator.dartimport doesn't appear to be used in this file. Consider removing it unless it's required transitively.import 'package:tmail_ui_user/features/home/domain/extensions/session_extensions.dart'; import 'package:tmail_ui_user/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart'; -import 'package:tmail_ui_user/main/error/capability_validator.dart';lib/features/thread/presentation/widgets/empty_emails_widget.dart (1)
110-116: Consider adding a sub-message for action-required folders.When
isActionRequiredFolderis true,_getSubMessageEmptyEmailfalls through to the else branch and returnsstartToComposeEmails, which may not be contextually appropriate for an action-required folder. Consider adding a dedicated message or returning an empty string.String _getSubMessageEmptyEmail(AppLocalizations appLocalizations) { if (isFavoriteFolder) { return appLocalizations.startToAddFavoritesEmails; + } else if (isActionRequiredFolder) { + return ''; // Or a more appropriate message like "AI will flag emails requiring your action" } else { return appLocalizations.startToComposeEmails; } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
lib/features/mailbox/presentation/extensions/handle_action_required_tab_extension.dart(1 hunks)lib/features/mailbox/presentation/extensions/list_mailbox_node_extension.dart(1 hunks)lib/features/mailbox/presentation/mailbox_controller.dart(6 hunks)lib/features/mailbox_dashboard/presentation/extensions/handle_ai_action_extension.dart(1 hunks)lib/features/search/mailbox/presentation/search_mailbox_controller.dart(4 hunks)lib/features/thread/presentation/extensions/handle_open_context_menu_filter_email_action_extension.dart(1 hunks)lib/features/thread/presentation/extensions/handle_pull_to_refresh_list_email_extension.dart(1 hunks)lib/features/thread/presentation/thread_controller.dart(8 hunks)lib/features/thread/presentation/thread_view.dart(6 hunks)lib/features/thread/presentation/widgets/empty_emails_widget.dart(4 hunks)lib/l10n/intl_messages.arb(2 hunks)lib/main/localizations/app_localizations.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- lib/features/thread/presentation/extensions/handle_pull_to_refresh_list_email_extension.dart
- lib/features/mailbox/presentation/extensions/list_mailbox_node_extension.dart
- lib/features/mailbox/presentation/extensions/handle_action_required_tab_extension.dart
- lib/main/localizations/app_localizations.dart
- lib/l10n/intl_messages.arb
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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.
📚 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:
lib/features/mailbox_dashboard/presentation/extensions/handle_ai_action_extension.dartlib/features/thread/presentation/widgets/empty_emails_widget.dartlib/features/search/mailbox/presentation/search_mailbox_controller.dartlib/features/mailbox/presentation/mailbox_controller.dartlib/features/thread/presentation/extensions/handle_open_context_menu_filter_email_action_extension.dartlib/features/thread/presentation/thread_controller.dartlib/features/thread/presentation/thread_view.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/mailbox_dashboard/presentation/extensions/handle_ai_action_extension.dartlib/features/thread/presentation/widgets/empty_emails_widget.dartlib/features/search/mailbox/presentation/search_mailbox_controller.dartlib/features/mailbox/presentation/mailbox_controller.dartlib/features/thread/presentation/extensions/handle_open_context_menu_filter_email_action_extension.dartlib/features/thread/presentation/thread_controller.dartlib/features/thread/presentation/thread_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/mailbox_dashboard/presentation/extensions/handle_ai_action_extension.dartlib/features/thread/presentation/widgets/empty_emails_widget.dartlib/features/search/mailbox/presentation/search_mailbox_controller.dartlib/features/mailbox/presentation/mailbox_controller.dartlib/features/thread/presentation/extensions/handle_open_context_menu_filter_email_action_extension.dartlib/features/thread/presentation/thread_controller.dartlib/features/thread/presentation/thread_view.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/mailbox/presentation/mailbox_controller.dartlib/features/thread/presentation/thread_view.dart
📚 Learning: 2025-12-12T09:03:05.962Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4204
File: lib/features/email/presentation/model/popup_menu_item_email_action.dart:34-35
Timestamp: 2025-12-12T09:03:05.962Z
Learning: In the tmail-flutter codebase, popup menu items intentionally use a consistent hoverIcon (imagePaths.icThumbsUp) across all popup item types (email actions, mailbox actions, profile settings, etc.), while actionIcon varies per action type. This is a deliberate UI/UX design choice.
Applied to files:
lib/features/thread/presentation/extensions/handle_open_context_menu_filter_email_action_extension.dartlib/features/thread/presentation/thread_view.dart
📚 Learning: 2025-12-12T07:56:31.877Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4204
File: lib/features/email/data/datasource_impl/email_hive_cache_datasource_impl.dart:579-582
Timestamp: 2025-12-12T07:56:31.877Z
Learning: In lib/features/email/data/datasource_impl/email_hive_cache_datasource_impl.dart, the addLabelToEmail method intentionally throws UnimplementedError because label synchronization with the email cache is handled separately via extension methods (e.g., _autoSyncLabelToSelectedEmailOnMemory in handle_label_for_email_extension.dart). Implementing it in the cache datasource would cause data conflicts. This differs from other keyword operations like markAsRead, markAsStar, markAsAnswered, and markAsForwarded, which are implemented directly in the cache datasource.
Applied to files:
lib/features/thread/presentation/thread_view.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Build web version and deploy
- GitHub Check: analyze-test (server_settings)
- GitHub Check: analyze-test (email_recovery)
- GitHub Check: analyze-test (rule_filter)
- GitHub Check: analyze-test (default)
- GitHub Check: analyze-test (model)
- GitHub Check: analyze-test (core)
- GitHub Check: analyze-test (forward)
- GitHub Check: analyze-test (contact)
- GitHub Check: analyze-test (fcm)
🔇 Additional comments (22)
lib/features/mailbox_dashboard/presentation/extensions/handle_ai_action_extension.dart (1)
5-18: LGTM!The extension is well-structured with proper null safety checks before calling the capability check. The early return pattern for null values prevents potential null pointer exceptions.
lib/features/thread/presentation/widgets/empty_emails_widget.dart (3)
17-17: LGTM!The new
isActionRequiredFolderparameter is properly added with a sensible default value, maintaining backward compatibility.Also applies to: 27-28
83-89: LGTM!The
isVirtualFolderlogic correctly combines both folder types for determining sub-message visibility.
103-104: LGTM!The action-required branch is correctly placed after favorites, with appropriate localization.
lib/features/thread/presentation/extensions/handle_open_context_menu_filter_email_action_extension.dart (1)
16-21: LGTM!The conditional exclusion of the unread filter option for action-required mailboxes follows the existing pattern used for starred filter in favorites, and the null-safe comparison
!= trueis correctly applied.lib/features/search/mailbox/presentation/search_mailbox_controller.dart (3)
61-61: LGTM!The new imports for action-required and AI action extensions are correctly added to support the new functionality.
Also applies to: 72-72
216-224: LGTM!The replacement of
addFavoriteFolderToMailboxList()withautoCreateVirtualFolder()in both failure and success branches ofonDone()ensures consistent behavior and follows the DRY principle.
903-908: LGTM!The
autoCreateVirtualFolder()method is well-structured, correctly gating the action-required folder creation behind the AI capability check. The implementation mirrors the pattern inMailboxController, ensuring consistency across the codebase.lib/features/mailbox/presentation/mailbox_controller.dart (4)
72-72: LGTM!The new imports for
handle_action_required_tab_extension.dartandhandle_ai_action_extension.dartare correctly added to enable the new virtual folder functionality.Also applies to: 89-89
267-286: LGTM!The
autoCreateVirtualFolder()call is correctly placed in all three branches (GetAllMailboxFailure,GetAllMailboxSuccess,CreateDefaultMailboxAllSuccess), ensuring virtual folders are created regardless of the mailbox fetch outcome.
606-606: LGTM!The
autoCreateVirtualFolder()call in_handleRefreshChangeMailboxSuccessensures virtual folders are properly maintained after mailbox refresh operations.
1564-1570: LGTM!The
autoCreateVirtualFolder()implementation is clean and follows the same pattern as inSearchMailboxController. The AI capability check properly gates the action-required folder creation while always ensuring the favorite folder is added.lib/features/thread/presentation/thread_view.dart (5)
23-23: LGTM!The import for
handle_ai_action_extension.dartenables access toisAiCapabilitySupportedfor AI-enabled rendering.
498-529: LGTM!The refactored
_buildEmailItemWhenDraggingproperly wraps reactive state reads inObxand passesisAIEnabledtoEmailTileBuilder. The reactive variables are correctly accessed within theObxclosure.
531-594: Consider moving background widgets inside Obx if they depend on reactive state.The
backgroundWidgetandsecondaryBackgroundWidgetare created outside theObxclosure but depend onpresentationEmail.hasRead. SincepresentationEmailis passed as a parameter and not reactive, this is fine. However, if the read status could change while the widget is displayed, the background wouldn't update until a rebuild.This is likely acceptable for swipe actions, but verify this matches the expected UX behavior.
596-641: LGTM!The
Obxwrapper correctly encapsulates all reactive state access, andisAIEnabledis properly propagated toEmailTileBuilder. TheDismissiblewidget configuration correctly uses the reactiveselectModeAllfor swipe direction determination.
781-800: LGTM!The empty-state logic correctly uses
isVirtualFolder(which encompasses both favorites and action-required) for the mailbox ID comparison check, andisActionRequiredFolderis properly passed toEmptyEmailsWidget.lib/features/thread/presentation/thread_controller.dart (5)
13-14: LGTM - Required imports for new filter logic.The new imports for filter operators and UTC date are necessary to support the virtual folder filtering logic introduced in
_buildFavoriteMailboxFilterand_buildActionRequiredMailboxFilter.Also applies to: 18-18
507-516: LGTM - Correct generalization from favorite to virtual folder.The refactoring from
isFavoritetoisVirtualFoldercorrectly generalizes the logic to support multiple virtual folder types (favorites and action-required). The early-return condition properly handles virtual folders by bypassing the mailbox ID check, which is appropriate since virtual folders aggregate emails across mailboxes based on keywords rather than mailbox membership.Also applies to: 552-560
621-711: Verify virtual folder type coverage and mutual exclusivity.The new filter builder methods are well-structured and the logic appears correct. However, there are two items worth verifying:
Fallback handling (lines 643-646): If
isVirtualFolder == truebut neitherisFavoritenorisActionRequiredis true, the code falls through togetFilterCondition, which includes a mailbox ID constraint (inMailbox: mailboxIdSelected). This would be incorrect for virtual folders, which aggregate across mailboxes. Please verify that the model guaranteesisVirtualFolderis true if and only if at least one ofisFavoriteorisActionRequiredis true, making this fallback case unreachable for virtual folders.Multiple virtual folder types (lines 629-634 vs 636-641): The two separate
ifchecks (notif-else) mean that if a mailbox is bothisFavoriteandisActionRequired, the favorite filter takes precedence. Please verify these properties are mutually exclusive by design, or document the intended precedence order if overlap is possible.Consider adding an assertion or throwing an error in the fallback case if
isVirtualFolder == trueto make the invariant explicit and catch potential bugs:+ if (mailbox?.isVirtualFolder == true) { + throw StateError( + 'Virtual folder must be either favorite or action-required. ' + 'Mailbox: ${mailbox?.name?.name}' + ); + } + return getFilterCondition( mailboxIdSelected: selectedMailboxId, oldestEmail: oldestEmail, );
783-784: LGTM - Consistent virtual folder handling in WebSocket updates.The method rename from
_refreshChangeListEmailsInFavoriteFolderto_refreshChangeListEmailsInVirtualFoldercorrectly reflects the broader scope of supporting multiple virtual folder types. The WebSocket message handling properly routes to this method whenisVirtualFolder == true, ensuring all virtual folder types receive fresh data on state changes.Also applies to: 889-921
600-600: LGTM - Appropriate cache and validation handling for virtual folders.The changes correctly disable caching for virtual folders (lines 600, 940) and adjust email validation logic (line 947):
Cache disabled for virtual folders: Ensures fresh results since virtual folders aggregate emails by keywords that can change dynamically. This prevents stale data without complex cache invalidation logic.
Validation logic updated: Line 947 correctly allows emails in virtual folders to bypass the mailbox ID containment check (
_belongToCurrentMailboxId), since virtual folders aggregate across multiple mailboxes based on keyword criteria rather than mailbox membership.These changes align with the PR discussion about avoiding inefficient query iterations while maintaining data accuracy.
Also applies to: 940-940, 947-947
|
|
|
Also please be aware that we introduced a user setting to enable "needs action" Disabled by default it needs explicit user content to be turned on. Please add the corresponding option into the user settings. |
|
ah got it, a server setting for this |
Indeed. We want to skip processing if the user disabled the feature. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
lib/features/thread/presentation/thread_controller.dart (1)
552-560: Same early-return concern applies here.This mirrors the logic in
_getAllEmailSuccess. Verify that virtual folders with acurrentMailboxIdshould indeed skip refresh processing.
🧹 Nitpick comments (1)
lib/features/thread/presentation/thread_controller.dart (1)
621-647: Add defensive handling for unexpected virtual folder types.The method checks
isFavorite(line 629) andisActionRequired(line 636), but ifisVirtualFolderis true and neither condition matches, the code falls through to the default case (line 643), which may not handle virtual folders correctly.Apply this diff to add defensive handling:
Filter getFilterConditionForLoadMailbox({ PresentationEmail? oldestEmail, }) { final mailbox = selectedMailbox; final filterOption = mailboxDashBoardController.filterMessageOption.value; final before = oldestEmail?.receivedAt; if (mailbox?.isFavorite == true) { return _buildFavoriteMailboxFilter( filterOption: filterOption, before: before, ); } if (mailbox?.isActionRequired == true) { return _buildActionRequiredMailboxFilter( filterOption: filterOption, before: before, ); } + + // Virtual folders should be handled above; log if we reach here unexpectedly + if (mailbox?.isVirtualFolder == true) { + log('ThreadController::getFilterConditionForLoadMailbox: Unhandled virtual folder type'); + } return getFilterCondition( mailboxIdSelected: selectedMailboxId, oldestEmail: oldestEmail, ); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/features/thread/presentation/thread_controller.dart(8 hunks)model/lib/email/presentation_email.dart(1 hunks)model/lib/extensions/keyword_identifier_extension.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- model/lib/extensions/keyword_identifier_extension.dart
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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.
📚 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:
lib/features/thread/presentation/thread_controller.dartmodel/lib/email/presentation_email.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/thread/presentation/thread_controller.dartmodel/lib/email/presentation_email.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/thread/presentation/thread_controller.dartmodel/lib/email/presentation_email.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:
model/lib/email/presentation_email.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Build web version and deploy
- GitHub Check: analyze-test (fcm)
- GitHub Check: analyze-test (contact)
- GitHub Check: analyze-test (server_settings)
- GitHub Check: analyze-test (core)
- GitHub Check: analyze-test (rule_filter)
- GitHub Check: analyze-test (email_recovery)
- GitHub Check: analyze-test (default)
- GitHub Check: analyze-test (model)
- GitHub Check: analyze-test (forward)
🔇 Additional comments (11)
model/lib/email/presentation_email.dart (1)
141-142: LGTM!The
hasNeedActiongetter follows the established pattern for keyword checks and integrates cleanly with the action-required workflow introduced in this PR.lib/features/thread/presentation/thread_controller.dart (10)
13-14: LGTM!The new imports support the enhanced filter logic for action-required and virtual folders.
Also applies to: 18-18
600-600: LGTM!Skipping cache for virtual folders is correct, as they aggregate emails from multiple sources rather than a single mailbox.
607-619: LGTM!The switch to
isVirtualFoldercorrectly routes both starred and action-required mailboxes to the virtual-folder filter path.
779-797: LGTM!The switch to
isVirtualFoldercorrectly routes WebSocket refresh for both starred and action-required folders.
889-921: LGTM!The method rename from
_refreshChangeListEmailsInFavoriteFolderto_refreshChangeListEmailsInVirtualFolderbetter reflects its expanded scope to include action-required folders.
940-940: LGTM!Consistent with the cache strategy for virtual folders.
946-949: LGTM!Bypassing the mailbox ID check for virtual folders is correct, as they aggregate emails across multiple mailboxes.
507-516: Verify the early-return logic for virtual folders.The condition on line 514 includes
isVirtualFolder, which causes an early return whencurrentMailboxIdis non-null and the mailbox is virtual. This differs from the previousisFavoritelogic. Ensure this correctly handles scenarios where action-required or starred virtual folders need to process email success callbacks.Run the following script to understand how virtual folders are distinguished and whether they should have a currentMailboxId:
#!/bin/bash # Check how isVirtualFolder, isFavorite, and isActionRequired are defined rg -nP -A5 'bool get isVirtualFolder' --type=dart # Check if virtual folders have associated mailbox IDs rg -nP -C3 'isVirtualFolder.*mailboxId|mailboxId.*isVirtualFolder' --type=dart
681-694: No simplification possible. TheLogicFilterOperator.ANDpattern is necessary here because the starred filter requires two distincthasKeywordvalues (emailFlaggedandneedsActionMail), andEmailFilterConditionsupports only single keyword values per parameter. Multiple keywords require separate conditions combined with AND.Likely an incorrect or invalid review comment.
676-711: ThenotKeyword: emailSeenfilter in all three cases is intentional and part of the feature design. The commit that introduced this code (cddc606) explicitly states "TF-4141 Perform load all unread action required emails when open Action required folder." This constraint is by design, not a bug. The feature deliberately filters to show only unread emails that require action, making this filter a core requirement rather than unnecessary logic.Likely an incorrect or invalid review comment.
dab246
force-pushed
the
feature/tf-4141-expected-actions
branch
from
b17afdd to
2835ab7
Compare
Fixed demo-drag.mov
Added Screen.Recording.2025-12-23.at.01.08.48.mov |
…r into other folder
dab246
force-pushed
the
feature/tf-4141-expected-actions
branch
from
942acf8 to
468ccb3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
test/features/search/verify_before_time_in_search_email_filter_test.dart (2)
1024-1024: Consistent use of centralized filter accessor.
1133-1133: Consistent use of centralized filter accessor.test/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller_test.dart (1)
545-545: Consistent use of centralized filter accessor.lib/features/search/email/presentation/search_email_view.dart (1)
678-701: Separator still not reactive to selection mode changes.The refactored separator builder removed the dead code (good!), but still accesses
controller.selectionMode.valueat line 681 without anObxwrapper. This means divider colors won't update when entering/exiting selection mode—the list will show checkboxes (viaObxin_buildEmailItem), but dividers will retain their original color until the list rebuilds for another reason.🔎 Proposed fix
Wrap the separator content in
Obxto react to selection mode changes:separatorBuilder: (context, index) { - final isMobile = PlatformInfo.isMobile; - final isInactive = - controller.selectionMode.value == SelectMode.INACTIVE; - - final dividerColor = isMobile - ? null - : (isInactive ? null : Colors.white); - - final padding = isMobile - ? SearchEmailUtils.getPaddingItemListMobile( - context, - controller.responsiveUtils, - ) - : ItemEmailTileStyles.getPaddingDividerWeb( - context, - controller.responsiveUtils, - ); - - return Padding( - padding: padding, - child: Divider(color: dividerColor), - ); + return Obx(() { + final isMobile = PlatformInfo.isMobile; + final isInactive = + controller.selectionMode.value == SelectMode.INACTIVE; + + final dividerColor = isMobile + ? null + : (isInactive ? null : Colors.white); + + final padding = isMobile + ? SearchEmailUtils.getPaddingItemListMobile( + context, + controller.responsiveUtils, + ) + : ItemEmailTileStyles.getPaddingDividerWeb( + context, + controller.responsiveUtils, + ); + + return Padding( + padding: padding, + child: Divider(color: dividerColor), + ); + }); },
🧹 Nitpick comments (5)
lib/features/base/mixin/ai_scribe_mixin.dart (1)
18-24: LGTM! Consider adding documentation.The new method correctly checks AI Scribe endpoint availability with proper null safety. The non-nullable
boolreturn type (defaulting tofalse) is a good choice.Optional: Add documentation for the public API
+ /// Returns whether the AI Scribe endpoint is available for the given session and account. + /// + /// Returns `true` if the account has AI capability with an available Scribe endpoint, + /// `false` otherwise (including when session or accountId are null). bool isAIScribeEndpointAvailable({Session? session, AccountId? accountId}) { final aiCapability = getAICapability( session: session, accountId: accountId, ); return aiCapability?.isScribeEndpointAvailable == true; }lib/features/mailbox/presentation/widgets/mailbox_item_widget.dart (1)
326-340: Consider adding inline comments for clarity.The background color logic correctly suppresses highlighting for action-required mailboxes during drag operations (lines 333-335), preventing visual feedback for an invalid drop target. However, the special handling for
isDraggingMailbox && isActionRequiredmight not be immediately obvious to future maintainers.Consider adding a brief inline comment explaining why action-required folders need distinct treatment during drag operations:
// Dragging action-required mailboxes must not show highlight // because they're virtual folders that don't accept email drops if (widget.isDraggingMailbox && widget.mailboxNode.item.isActionRequired) { return AppColor.colorBgDesktop; }lib/features/mailbox/presentation/extensions/list_mailbox_node_extension.dart (1)
28-45: Well-designed generalized insertion logic.The implementation correctly handles duplicates, priority-based positioning, and edge cases. The early-return pattern and fallback to position 0 work well.
Optional: Clarify empty priorities behavior in documentation
Consider making the empty-priorities case more explicit in the documentation:
- /// Insert [newNode] after the first mailbox matching [priorities]. - /// If none match, inserts at the beginning. + /// Insert [newNode] after the first mailbox matching [priorities]. + /// If no priorities match (or [priorities] is empty), inserts at the beginning.lib/features/mailbox/presentation/extensions/handle_action_required_tab_extension.dart (1)
10-64: Action-required virtual folder add/remove helpers look consistentThe extension cleanly reuses
PresentationMailbox.actionRequiredFolder, injects it into the default tree viainsertAfterStarredOrInbox, and keepsallMailboxesin sync with id-based add/remove._addToAllMailboxesis already idempotent; ifaddActionRequiredFolder()may be called multiple times across flows, consider mirroring that guard in_addToDefaultMailboxTreeto avoid accidental duplicates in the root’s children list.lib/features/base/widget/labels/tag_widget.dart (1)
4-65: Reusable TagWidget implementation is solidThe widget cleanly encapsulates tag styling (padding, radius, optional maxWidth/margin) with optional truncation and tooltip support. The
_truncateNamehelper is defensive and, combined withmaxLines: 1andTextOverflow.ellipsis, should behave well in constrained layouts. If you later notice “double ellipsis” on very shortmaxLengthvalues, you could drop the manual'...'and rely solely onTextOverflow.ellipsis, but that’s purely cosmetic.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
assets/images/ic_mailbox_action_required.svgis excluded by!**/*.svg
📒 Files selected for processing (56)
core/lib/presentation/extensions/color_extension.dartcore/lib/presentation/resources/image_paths.dartlib/features/base/base_mailbox_controller.dartlib/features/base/mixin/ai_scribe_mixin.dartlib/features/base/widget/labels/ai_action_tag_widget.dartlib/features/base/widget/labels/tag_widget.dartlib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dartlib/features/email/presentation/utils/email_action_reactor/email_action_reactor.dartlib/features/home/domain/extensions/session_extensions.dartlib/features/mailbox/presentation/action/mailbox_ui_action.dartlib/features/mailbox/presentation/base_mailbox_view.dartlib/features/mailbox/presentation/extensions/handle_action_required_tab_extension.dartlib/features/mailbox/presentation/extensions/list_mailbox_node_extension.dartlib/features/mailbox/presentation/extensions/presentation_mailbox_extension.dartlib/features/mailbox/presentation/mailbox_controller.dartlib/features/mailbox/presentation/mixin/mailbox_widget_mixin.dartlib/features/mailbox/presentation/widgets/mailbox_item_widget.dartlib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dartlib/features/mailbox_dashboard/presentation/extensions/ai_scribe/setup_ai_needs_action_setting_extension.dartlib/features/mailbox_dashboard/presentation/extensions/handle_action_type_for_email_selection.dartlib/features/mailbox_dashboard/presentation/extensions/handle_ai_needs_action_extension.dartlib/features/mailbox_dashboard/presentation/extensions/move_emails_to_mailbox_extension.dartlib/features/mailbox_dashboard/presentation/mailbox_dashboard_view_web.dartlib/features/manage_account/presentation/manage_account_dashboard_controller.dartlib/features/manage_account/presentation/model/preferences_option_type.dartlib/features/manage_account/presentation/preferences/preferences_controller.dartlib/features/manage_account/presentation/preferences/preferences_view.dartlib/features/search/email/presentation/search_email_view.dartlib/features/search/mailbox/presentation/search_mailbox_controller.dartlib/features/server_settings/domain/extensions/tmail_server_settings_extension.dartlib/features/thread/domain/exceptions/thread_exceptions.dartlib/features/thread/domain/model/email_filter.dartlib/features/thread/presentation/extensions/handle_email_filter_extension.dartlib/features/thread/presentation/extensions/handle_open_context_menu_filter_email_action_extension.dartlib/features/thread/presentation/extensions/handle_pull_to_refresh_list_email_extension.dartlib/features/thread/presentation/extensions/handle_select_message_filter_extension.dartlib/features/thread/presentation/filters/mailbox_filter_builder.dartlib/features/thread/presentation/thread_controller.dartlib/features/thread/presentation/thread_view.dartlib/features/thread/presentation/widgets/email_tile_builder.dartlib/features/thread/presentation/widgets/email_tile_web_builder.dartlib/features/thread/presentation/widgets/empty_emails_widget.dartlib/features/thread/presentation/widgets/web_tablet_body_email_item_widget.dartlib/l10n/intl_messages.arblib/main/localizations/app_localizations.dartmodel/lib/email/presentation_email.dartmodel/lib/extensions/keyword_identifier_extension.dartmodel/lib/extensions/presentation_mailbox_extension.dartmodel/lib/mailbox/presentation_mailbox.dartserver_settings/lib/server_settings/tmail_server_settings.dartserver_settings/lib/server_settings/tmail_server_settings_extension.darttest/features/composer/presentation/composer_controller_test.darttest/features/mailbox/presentation/extensions/list_mailbox_node_extension_test.darttest/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller_test.darttest/features/search/verify_before_time_in_search_email_filter_test.darttest/features/thread/presentation/filters/mailbox_filter_builder_test.dart
✅ Files skipped from review due to trivial changes (1)
- test/features/thread/presentation/filters/mailbox_filter_builder_test.dart
🚧 Files skipped from review as they are similar to previous changes (17)
- core/lib/presentation/resources/image_paths.dart
- lib/features/mailbox_dashboard/presentation/extensions/move_emails_to_mailbox_extension.dart
- lib/features/base/base_mailbox_controller.dart
- lib/features/server_settings/domain/extensions/tmail_server_settings_extension.dart
- lib/features/home/domain/extensions/session_extensions.dart
- lib/features/thread/presentation/extensions/handle_open_context_menu_filter_email_action_extension.dart
- lib/features/manage_account/presentation/model/preferences_option_type.dart
- lib/main/localizations/app_localizations.dart
- lib/features/mailbox_dashboard/presentation/extensions/handle_action_type_for_email_selection.dart
- lib/features/mailbox/presentation/base_mailbox_view.dart
- lib/features/thread/presentation/widgets/email_tile_builder.dart
- test/features/composer/presentation/composer_controller_test.dart
- model/lib/extensions/presentation_mailbox_extension.dart
- lib/features/mailbox_dashboard/presentation/extensions/handle_ai_needs_action_extension.dart
- test/features/mailbox/presentation/extensions/list_mailbox_node_extension_test.dart
- lib/features/thread/domain/exceptions/thread_exceptions.dart
- lib/features/manage_account/presentation/manage_account_dashboard_controller.dart
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4210
File: model/lib/mailbox/presentation_mailbox.dart:22-34
Timestamp: 2025-12-23T05:27:10.887Z
Learning: In the tmail-flutter codebase, the Action Required folder (actionRequiredFolder) is a virtual folder that is AI-curated and contains emails with the keyword "needs-action". Unlike the Favorites folder, users should NOT be able to drag emails into the Action Required folder. The UI correctly prevents this by not wrapping the Action Required folder item in a DragTarget (see mailbox_item_widget.dart).
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.
📚 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:
lib/features/thread/presentation/extensions/handle_pull_to_refresh_list_email_extension.dartlib/features/mailbox_dashboard/presentation/mailbox_dashboard_view_web.dartlib/features/thread/presentation/widgets/web_tablet_body_email_item_widget.dartlib/features/mailbox/presentation/extensions/list_mailbox_node_extension.dartserver_settings/lib/server_settings/tmail_server_settings_extension.dartlib/features/mailbox_dashboard/presentation/extensions/ai_scribe/setup_ai_needs_action_setting_extension.dartlib/features/thread/presentation/widgets/empty_emails_widget.dartlib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dartlib/features/thread/presentation/extensions/handle_email_filter_extension.dartlib/features/thread/presentation/extensions/handle_select_message_filter_extension.dartlib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dartlib/features/email/presentation/utils/email_action_reactor/email_action_reactor.dartlib/features/mailbox/presentation/extensions/handle_action_required_tab_extension.dartlib/features/search/mailbox/presentation/search_mailbox_controller.dartlib/features/mailbox/presentation/widgets/mailbox_item_widget.dartlib/features/thread/presentation/thread_view.dartmodel/lib/mailbox/presentation_mailbox.dartlib/features/thread/presentation/thread_controller.dartlib/features/thread/presentation/filters/mailbox_filter_builder.dartlib/features/mailbox/presentation/mailbox_controller.dartlib/features/thread/presentation/widgets/email_tile_web_builder.dartmodel/lib/email/presentation_email.dartlib/l10n/intl_messages.arblib/features/mailbox/presentation/action/mailbox_ui_action.dartlib/features/manage_account/presentation/preferences/preferences_controller.dartlib/features/mailbox/presentation/extensions/presentation_mailbox_extension.darttest/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller_test.dartlib/features/search/email/presentation/search_email_view.darttest/features/search/verify_before_time_in_search_email_filter_test.dartlib/features/mailbox/presentation/mixin/mailbox_widget_mixin.dart
📚 Learning: 2025-12-23T05:27:10.887Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4210
File: model/lib/mailbox/presentation_mailbox.dart:22-34
Timestamp: 2025-12-23T05:27:10.887Z
Learning: In the tmail-flutter codebase, the Action Required folder (actionRequiredFolder) is a virtual folder that is AI-curated and contains emails with the keyword "needs-action". Unlike the Favorites folder, users should NOT be able to drag emails into the Action Required folder. The UI correctly prevents this by not wrapping the Action Required folder item in a DragTarget (see mailbox_item_widget.dart).
Applied to files:
lib/features/thread/presentation/extensions/handle_pull_to_refresh_list_email_extension.dartmodel/lib/extensions/keyword_identifier_extension.dartlib/features/mailbox_dashboard/presentation/mailbox_dashboard_view_web.dartlib/features/thread/presentation/widgets/web_tablet_body_email_item_widget.dartserver_settings/lib/server_settings/tmail_server_settings_extension.dartlib/features/mailbox_dashboard/presentation/extensions/ai_scribe/setup_ai_needs_action_setting_extension.dartlib/features/thread/presentation/widgets/empty_emails_widget.dartlib/features/thread/presentation/extensions/handle_select_message_filter_extension.dartlib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dartlib/features/email/presentation/utils/email_action_reactor/email_action_reactor.dartlib/features/base/widget/labels/ai_action_tag_widget.dartlib/features/mailbox/presentation/extensions/handle_action_required_tab_extension.dartserver_settings/lib/server_settings/tmail_server_settings.dartlib/features/search/mailbox/presentation/search_mailbox_controller.dartlib/features/mailbox/presentation/widgets/mailbox_item_widget.dartlib/features/thread/presentation/thread_view.dartmodel/lib/mailbox/presentation_mailbox.dartlib/features/thread/presentation/thread_controller.dartlib/features/thread/presentation/filters/mailbox_filter_builder.dartlib/features/mailbox/presentation/mailbox_controller.dartlib/features/thread/presentation/widgets/email_tile_web_builder.dartmodel/lib/email/presentation_email.dartlib/l10n/intl_messages.arblib/features/mailbox/presentation/action/mailbox_ui_action.dartlib/features/mailbox/presentation/extensions/presentation_mailbox_extension.darttest/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller_test.dartlib/features/search/email/presentation/search_email_view.darttest/features/search/verify_before_time_in_search_email_filter_test.dartlib/features/mailbox/presentation/mixin/mailbox_widget_mixin.dart
📚 Learning: 2025-12-12T07:56:31.877Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4204
File: lib/features/email/data/datasource_impl/email_hive_cache_datasource_impl.dart:579-582
Timestamp: 2025-12-12T07:56:31.877Z
Learning: In lib/features/email/data/datasource_impl/email_hive_cache_datasource_impl.dart, the addLabelToEmail method intentionally throws UnimplementedError because label synchronization with the email cache is handled separately via extension methods (e.g., _autoSyncLabelToSelectedEmailOnMemory in handle_label_for_email_extension.dart). Implementing it in the cache datasource would cause data conflicts. This differs from other keyword operations like markAsRead, markAsStar, markAsAnswered, and markAsForwarded, which are implemented directly in the cache datasource.
Applied to files:
lib/features/thread/presentation/extensions/handle_pull_to_refresh_list_email_extension.dartlib/features/thread/presentation/widgets/web_tablet_body_email_item_widget.dartlib/features/thread/presentation/thread_view.dartmodel/lib/mailbox/presentation_mailbox.dartlib/features/thread/presentation/thread_controller.dartlib/features/thread/presentation/widgets/email_tile_web_builder.dartlib/features/mailbox/presentation/extensions/presentation_mailbox_extension.darttest/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller_test.darttest/features/search/verify_before_time_in_search_email_filter_test.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/thread/presentation/extensions/handle_pull_to_refresh_list_email_extension.dartmodel/lib/extensions/keyword_identifier_extension.dartlib/features/thread/presentation/widgets/web_tablet_body_email_item_widget.dartlib/features/thread/presentation/extensions/handle_email_filter_extension.dartlib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dartlib/features/thread/presentation/thread_view.dartlib/features/thread/presentation/thread_controller.dartlib/features/mailbox/presentation/mailbox_controller.dartlib/features/thread/presentation/widgets/email_tile_web_builder.dartmodel/lib/email/presentation_email.dartlib/features/mailbox/presentation/extensions/presentation_mailbox_extension.darttest/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller_test.dartlib/features/search/email/presentation/search_email_view.darttest/features/search/verify_before_time_in_search_email_filter_test.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/thread/presentation/extensions/handle_pull_to_refresh_list_email_extension.dartmodel/lib/extensions/keyword_identifier_extension.dartlib/features/mailbox_dashboard/presentation/mailbox_dashboard_view_web.dartlib/features/manage_account/presentation/preferences/preferences_view.dartlib/features/thread/presentation/widgets/web_tablet_body_email_item_widget.dartlib/features/mailbox/presentation/extensions/list_mailbox_node_extension.dartserver_settings/lib/server_settings/tmail_server_settings_extension.dartlib/features/mailbox_dashboard/presentation/extensions/ai_scribe/setup_ai_needs_action_setting_extension.dartlib/features/thread/presentation/widgets/empty_emails_widget.dartlib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dartlib/features/thread/presentation/extensions/handle_email_filter_extension.dartlib/features/thread/presentation/extensions/handle_select_message_filter_extension.dartlib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dartlib/features/email/presentation/utils/email_action_reactor/email_action_reactor.dartlib/features/base/widget/labels/tag_widget.dartlib/features/base/widget/labels/ai_action_tag_widget.dartlib/features/mailbox/presentation/extensions/handle_action_required_tab_extension.dartserver_settings/lib/server_settings/tmail_server_settings.dartlib/features/search/mailbox/presentation/search_mailbox_controller.dartlib/features/mailbox/presentation/widgets/mailbox_item_widget.dartlib/features/thread/presentation/thread_view.dartmodel/lib/mailbox/presentation_mailbox.dartlib/features/thread/presentation/thread_controller.dartlib/features/thread/presentation/filters/mailbox_filter_builder.dartlib/features/mailbox/presentation/mailbox_controller.dartlib/features/thread/presentation/widgets/email_tile_web_builder.dartcore/lib/presentation/extensions/color_extension.dartmodel/lib/email/presentation_email.dartlib/features/thread/domain/model/email_filter.dartlib/features/base/mixin/ai_scribe_mixin.dartlib/features/mailbox/presentation/action/mailbox_ui_action.dartlib/features/manage_account/presentation/preferences/preferences_controller.dartlib/features/mailbox/presentation/extensions/presentation_mailbox_extension.darttest/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller_test.dartlib/features/search/email/presentation/search_email_view.darttest/features/search/verify_before_time_in_search_email_filter_test.dartlib/features/mailbox/presentation/mixin/mailbox_widget_mixin.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/thread/presentation/extensions/handle_pull_to_refresh_list_email_extension.dartmodel/lib/extensions/keyword_identifier_extension.dartlib/features/mailbox_dashboard/presentation/mailbox_dashboard_view_web.dartlib/features/manage_account/presentation/preferences/preferences_view.dartlib/features/thread/presentation/widgets/web_tablet_body_email_item_widget.dartlib/features/mailbox/presentation/extensions/list_mailbox_node_extension.dartserver_settings/lib/server_settings/tmail_server_settings_extension.dartlib/features/mailbox_dashboard/presentation/extensions/ai_scribe/setup_ai_needs_action_setting_extension.dartlib/features/thread/presentation/widgets/empty_emails_widget.dartlib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dartlib/features/thread/presentation/extensions/handle_email_filter_extension.dartlib/features/thread/presentation/extensions/handle_select_message_filter_extension.dartlib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dartlib/features/email/presentation/utils/email_action_reactor/email_action_reactor.dartlib/features/base/widget/labels/tag_widget.dartlib/features/base/widget/labels/ai_action_tag_widget.dartlib/features/mailbox/presentation/extensions/handle_action_required_tab_extension.dartserver_settings/lib/server_settings/tmail_server_settings.dartlib/features/search/mailbox/presentation/search_mailbox_controller.dartlib/features/mailbox/presentation/widgets/mailbox_item_widget.dartlib/features/thread/presentation/thread_view.dartmodel/lib/mailbox/presentation_mailbox.dartlib/features/thread/presentation/thread_controller.dartlib/features/thread/presentation/filters/mailbox_filter_builder.dartlib/features/mailbox/presentation/mailbox_controller.dartlib/features/thread/presentation/widgets/email_tile_web_builder.dartcore/lib/presentation/extensions/color_extension.dartmodel/lib/email/presentation_email.dartlib/features/thread/domain/model/email_filter.dartlib/features/base/mixin/ai_scribe_mixin.dartlib/features/mailbox/presentation/action/mailbox_ui_action.dartlib/features/manage_account/presentation/preferences/preferences_controller.dartlib/features/mailbox/presentation/extensions/presentation_mailbox_extension.darttest/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller_test.dartlib/features/search/email/presentation/search_email_view.darttest/features/search/verify_before_time_in_search_email_filter_test.dartlib/features/mailbox/presentation/mixin/mailbox_widget_mixin.dart
📚 Learning: 2025-12-12T09:03:05.962Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4204
File: lib/features/email/presentation/model/popup_menu_item_email_action.dart:34-35
Timestamp: 2025-12-12T09:03:05.962Z
Learning: In the tmail-flutter codebase, popup menu items intentionally use a consistent hoverIcon (imagePaths.icThumbsUp) across all popup item types (email actions, mailbox actions, profile settings, etc.), while actionIcon varies per action type. This is a deliberate UI/UX design choice.
Applied to files:
lib/features/mailbox_dashboard/presentation/mailbox_dashboard_view_web.dartlib/features/thread/presentation/widgets/web_tablet_body_email_item_widget.dartlib/features/thread/presentation/extensions/handle_select_message_filter_extension.dartlib/features/mailbox/presentation/widgets/mailbox_item_widget.dartlib/features/thread/presentation/thread_view.dartmodel/lib/mailbox/presentation_mailbox.dartlib/features/thread/presentation/widgets/email_tile_web_builder.dartlib/features/mailbox/presentation/extensions/presentation_mailbox_extension.dartlib/features/search/email/presentation/search_email_view.dart
📚 Learning: 2025-12-12T11:00:57.625Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4205
File: core/lib/utils/app_logger.dart:80-86
Timestamp: 2025-12-12T11:00:57.625Z
Learning: In the tmail-flutter codebase, logging philosophy differs by platform: web applications use the `webConsoleEnabled` parameter in `app_logger.dart` to selectively display logs in production console, while mobile applications intentionally suppress all logs in release builds (enforced by `BuildUtils.isDebugMode` check in `_debugPrint`).
Applied to files:
lib/features/manage_account/presentation/preferences/preferences_view.dart
📚 Learning: 2025-12-15T06:24:38.823Z
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:38.823Z
Learning: In lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart and lib/features/manage_account/presentation/manage_account_dashboard_controller.dart, the code calls SentryManager.instance.setUser() with PII fields (account ID, display name, username, email). This is privacy-sensitive. Review and ensure PII handling complies with policy: avoid sending unnecessary PII to Sentry, redact or hash sensitive fields if possible, or document explicit privacy policy and user consent for such data. If PII must be sent, ensure minimal data, secure handling, and add notes to the privacy policy. Consider adding tests or a lint rule to flag setUser calls that include PII.
Applied to files:
lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart
📚 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/mailbox/presentation/extensions/handle_action_required_tab_extension.dartlib/features/manage_account/presentation/preferences/preferences_controller.darttest/features/search/verify_before_time_in_search_email_filter_test.dart
📚 Learning: 2025-12-23T05:27:40.476Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4210
File: lib/l10n/intl_messages.arb:5151-5182
Timestamp: 2025-12-23T05:27:40.476Z
Learning: In lib/l10n, translations for locale .arb files are updated automatically by an external workflow when new English keys are added to intl_messages.arb. Do not manually modify translated locale .arb files in pull requests; rely on the automated workflow to update translations. If manual edits are necessary, clearly reference the automation workflow and ensure translations remain consistent with the source English keys.
Applied to files:
lib/l10n/intl_messages.arb
📚 Learning: 2025-12-19T08:04:06.064Z
Learnt from: hoangdat
Repo: linagora/tmail-flutter PR: 4208
File: lib/l10n/intl_fr.arb:5209-5226
Timestamp: 2025-12-19T08:04:06.064Z
Learning: In all ARB localization files under lib/l10n, AI Scribe translation keys should only be present for the supported locales: en (default), fr, vi, and ru. Do not add AI Scribe keys to other locale files. Ensure fr, vi, and ru ARB files include the same AI Scribe keys as applicable, with en as the source/default. This pattern applies to any ARB file in the lib/l10n directory.
Applied to files:
lib/l10n/intl_messages.arb
📚 Learning: 2025-12-18T09:19:32.437Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4203
File: scribe/pubspec.yaml:37-37
Timestamp: 2025-12-18T09:19:32.437Z
Learning: In the tmail-flutter repository, json_annotation version 4.8.0 is the standardized version across modules and should not be upgraded without considering the standardization practice.
Applied to files:
test/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller_test.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Build web version and deploy
- GitHub Check: analyze-test (default)
- GitHub Check: analyze-test (contact)
- GitHub Check: analyze-test (model)
- GitHub Check: analyze-test (fcm)
- GitHub Check: analyze-test (email_recovery)
- GitHub Check: analyze-test (rule_filter)
- GitHub Check: analyze-test (core)
- GitHub Check: analyze-test (server_settings)
- GitHub Check: analyze-test (forward)
🔇 Additional comments (58)
lib/features/mailbox_dashboard/presentation/mailbox_dashboard_view_web.dart (1)
547-548: LGTM! Correctly hides redundant unread filter for Action Required folder.The condition appropriately excludes the unread filter option when viewing the Action Required virtual folder, since that folder already filters by unseen emails. The implementation follows the established pattern used for the starred filter (lines 549-550).
lib/features/base/mixin/ai_scribe_mixin.dart (1)
28-28: LGTM!The mobile platform guard correctly prevents AI Scribe binding injection on mobile platforms, aligning with the feature scope where AI Scribe is only available on web/desktop.
lib/features/thread/presentation/extensions/handle_select_message_filter_extension.dart (1)
16-17: LGTM! Logic aligns with Action Required folder behavior.The conditional exclusion of the unread filter when the mailbox is Action Required is sound: since the Action Required folder already filters for emails needing action (excluding read/$seen emails), an "unread" filter option would be redundant. The pattern mirrors the existing isFavorite check for the starred filter (lines 18-19). The
isActionRequiredproperty is correctly defined in the PresentationMailbox extension.lib/features/email/presentation/utils/email_action_reactor/email_action_reactor.dart (1)
710-723: Good generalization from Favorites to all virtual folders.The change correctly generalizes the containment logic to handle all virtual folders (including the new Action Required folder) rather than only the Favorites folder. Since
isVirtualFolderis defined asisFavorite || isActionRequired, this ensures that move operations find the real mailbox containing the email, preventing the "invalid UUID" errors reported in PR comments.lib/features/mailbox/presentation/widgets/mailbox_item_widget.dart (3)
34-34: LGTM!The
isDraggingMailboxparameter addition is clean and follows the existing widget pattern. It properly integrates with the background color logic to handle drag state for action-required mailboxes.Also applies to: 49-49
138-148: Correctly prevents drag-into for action-required folders.The conditional DragTarget wrapping properly implements the requirement that users should NOT be able to drag emails into the Action Required virtual folder. By returning
itemWidgetdirectly for action-required mailboxes (lines 138-140), the widget is not wrapped in a DragTarget, preventing drop operations.This aligns with the retrieved learning and addresses the past issue where the server rejected "need-action" as an invalid mailboxId UUID.
Based on learnings, the Action Required folder is a virtual folder and should not accept email drops.
82-148: This code pattern is intentional and correct. The Action Required folder is a virtual, AI-curated folder that should not accept drag-and-drop operations from users. The key placement strategy—assigning_keytoMaterialwhenisActionRequiredis true and toDragTargetwhen false—prevents drag functionality on action-required folders, which is the desired behavior.The
isDraggingMailboxflag provides the intended visual feedback across all mailbox items, including preventing highlights on action-required folders during drag operations (see the explicit check on line 199:if (widget.isDraggingMailbox && widget.mailboxNode.item.isActionRequired)). UsingcandidateEmailsfor visual feedback is not necessary here because action-required folders do not accept email drops by design.Likely an incorrect or invalid review comment.
lib/features/mailbox/presentation/extensions/list_mailbox_node_extension.dart (5)
8-14: LGTM!The delegation to
insertAfterByPriorityis clean and well-documented. The method provides a convenient, specific API for the common case of inserting after Inbox.
16-26: LGTM!The priority-based insertion strategy is well-designed. The documentation clearly explains the cascading fallback behavior (Starred → Inbox → beginning).
57-59: LGTM!ID-based duplicate detection is the correct approach and prevents unintended duplicate insertions.
61-63: LGTM!The null-safe, case-insensitive comparison helper is correctly implemented.
47-55: Name-based fallback assumes JMAP standard mailbox names.The
_isInboxand_isStarredpredicates fall back to hardcoded English string matching ('inbox', 'starred') when role is null. This fallback is intentional and tested for the edge case where the role is missing. However, it assumes mailbox names follow the JMAP standard (English). If mailboxes from non-standard servers or APIs can have localized names without role values, this fallback will not match them. Clarify whether this limitation is acceptable or if non-English mailbox name matching is needed.model/lib/mailbox/presentation_mailbox.dart (1)
22-22: LGTM! Action Required folder constants follow established patterns.The implementation correctly mirrors the existing
favoriteFolderpattern, withactionRequiredRole,actionRequiredFolder, androleActionRequiredfollowing the same structure as the favorites implementation. The role name'needs-action'aligns with backend documentation.Also applies to: 30-34, 47-47
lib/features/thread/presentation/extensions/handle_pull_to_refresh_list_email_extension.dart (1)
53-53: LGTM! Correctly generalizes cache behavior for virtual folders.The change from
isFavoritetoisVirtualFolderappropriately extends the cache-skipping logic to all virtual folders (now including both Favorites and Action Required), since these are computed/filtered views rather than physical mailboxes.core/lib/presentation/extensions/color_extension.dart (1)
294-294: LGTM! Color constant addition for AI action tag UI.Straightforward addition following the existing pattern of color constants.
lib/features/thread/domain/model/email_filter.dart (1)
15-25: LGTM! Standard copyWith implementation.The
copyWithmethod follows the standard Dart pattern for immutable data classes, correctly handling all three fields with null-coalescing operators.model/lib/email/presentation_email.dart (1)
141-142: LGTM! Consistent keyword getter implementation.The
hasNeedActiongetter follows the same pattern as other keyword-based properties (hasRead,hasStarred,isSubscribed) and correctly handles null safety.server_settings/lib/server_settings/tmail_server_settings_extension.dart (1)
8-9: LGTM! Server setting getter follows established patterns.The
isAINeedsActionEnabledgetter correctly mirrors the pattern of existing settings getters (isDisplaySenderPriority,isAlwaysReadReceipts), providing a safefalsedefault when the setting is null.lib/features/mailbox/presentation/mixin/mailbox_widget_mixin.dart (1)
24-24: LGTM! Generalizes action list logic for virtual folders.Changing from
isFavoritetoisVirtualFoldercorrectly extends the limited-action behavior to all virtual folders (now including Action Required), which is appropriate since virtual folders should not support operations like creating subfolders or moving content.model/lib/extensions/keyword_identifier_extension.dart (1)
8-8: LGTM! Keyword identifier follows JMAP standards.The
needsActionMailconstant correctly uses the$needs-actionformat, consistent with JMAP keyword conventions (e.g.,$seen,$flagged) and confirmed by backend documentation. The dollar sign prefix is appropriate here, distinguishing it from the mailbox role name which doesn't use the prefix.Based on learnings from past comments confirming the 'needs-action' naming convention.
lib/features/composer/presentation/extensions/ai_scribe/handle_ai_scribe_in_composer_extension.dart (1)
11-24: LGTM! Centralized endpoint availability check.The refactoring correctly centralizes the AI Scribe endpoint availability check via
mailboxDashBoardController.isAIScribeEndpointAvailableand adds a mobile platform exclusion. The logic now ensures AI Scribe is available only when the config is enabled, the endpoint is available, and the platform is not mobile.lib/features/mailbox/presentation/action/mailbox_ui_action.dart (1)
38-42: LGTM! New action classes for action-required folder automation.The two new action classes
AutoCreateActionRequiredFolderMailboxActionandAutoRemoveActionRequiredFolderMailboxActionfollow the existing pattern and support the new AI action-required folder workflow.lib/features/manage_account/presentation/preferences/preferences_view.dart (1)
67-84: LGTM! AI capability-based filtering for preferences.The logic correctly filters available preference options based on AI capabilities:
- Non-local options exclude
aiNeedsAction- Local options conditionally include
aiScribeonly when AI capability is availableaiNeedsActionis appended when the setting option exists and AI capability is supportedThe implementation aligns with the PR's goal to add AI needs-action preferences.
test/features/search/verify_before_time_in_search_email_filter_test.dart (1)
860-860: LGTM! Centralized filter construction in tests.The test now uses
threadController.getEmailFilterForLoadMailbox()to obtain the email filter, aligning with the centralized filter construction pattern introduced in this PR. This change ensures consistent behavior across tests and production code.lib/features/mailbox/presentation/extensions/presentation_mailbox_extension.dart (3)
14-43: LGTM! Display name handling for action-required folder.The
getDisplayNamemethod correctly handles the newactionRequiredRoleby returning the appropriate localized display name.
76-133: LGTM! Icon mapping for action-required folder.The
getMailboxIconmethod correctly mapsactionRequiredRoletoicMailboxActionRequired.
140-145: ThefilterKeywordgetter is not used for filtering in this codebase. Filtering for both favorites and action-required folders is handled directly inMailboxFilterBuilder.buildFilterCondition(), which explicitly checksisFavoriteandisActionRequiredto build appropriate filter conditions. Action-required filtering already works correctly, using the "needs-action" keyword through_buildActionRequiredMailboxFilter(). ExtendingfilterKeywordwould be unnecessary since nothing invokes it.Likely an incorrect or invalid review comment.
test/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller_test.dart (1)
487-487: LGTM! Centralized filter construction in tests.The test correctly uses
threadController.getEmailFilterForLoadMailbox()for centralized filter construction, aligning with the pattern introduced in this PR.lib/features/thread/presentation/widgets/empty_emails_widget.dart (3)
17-17: LGTM! New field for action-required folder.The addition of
isActionRequiredFolderfield correctly extends the empty emails widget to support the new virtual folder.
83-90: LGTM! Virtual folder visibility logic.The logic correctly combines both
isFavoriteFolderandisActionRequiredFolderas virtual folders to determine when to show the email sub-message.
92-108: LGTM! Empty state message for action-required folder.The message handling correctly adds a case for the action-required folder, returning the appropriate localized message.
lib/features/search/mailbox/presentation/search_mailbox_controller.dart (1)
216-224: TheautoCreateVirtualFoldermethod is properly defined inBaseMailboxControllerat line 678, andSearchMailboxControllercorrectly extendsBaseMailboxController. The method calls at lines 216 and 222 are valid and will properly inherit the base implementation.lib/features/thread/presentation/widgets/web_tablet_body_email_item_widget.dart (1)
11-11: LGTM: AI action tag integration is clean.The
shouldShowAIActionparameter is properly added with a sensible default, and the conditional rendering ofAiActionTagWidgetfollows existing patterns in the widget.Also applies to: 26-26, 48-48, 171-174
lib/features/mailbox_dashboard/presentation/extensions/ai_scribe/setup_ai_needs_action_setting_extension.dart (1)
7-21: LGTM: Clean extension for AI needs-action setup.The extension method correctly handles the server setting, defaulting to
falsewhen options are null, and dispatches the appropriate UI actions to create or remove the Action Required virtual folder.server_settings/lib/server_settings/tmail_server_settings.dart (1)
43-44: LGTM: Server setting field properly integrated.The
aiNeedsActionEnabledfield is correctly added to the model with proper JSON serialization, constructor parameter,copyWithsupport, and Equatable props inclusion.Also applies to: 50-50, 62-62, 68-68, 77-77
lib/features/manage_account/presentation/preferences/preferences_controller.dart (1)
53-59: LGTM: Preferences controller properly extended.The new
isAIScribeCapabilityAvailablegetter andaiNeedsActionpreference handling follow existing patterns consistently.Also applies to: 190-194
lib/features/thread/presentation/extensions/handle_email_filter_extension.dart (1)
8-26: LGTM: Clean filter extension.The extension provides a well-structured API for building email filters, properly delegating to
MailboxFilterBuilderwith the necessary controller state.lib/features/base/widget/labels/ai_action_tag_widget.dart (1)
7-23: LGTM: Simple and focused widget.The
AiActionTagWidgetis a clean wrapper aroundTagWidgetwith appropriate localization and platform-specific tooltip behavior.lib/features/search/email/presentation/search_email_view.dart (1)
706-752: LGTM: Email item builder properly wired.The new
_buildEmailItemmethod cleanly encapsulates item rendering logic and correctly passesisAINeedsActionEnabledfrom the dashboard controller toEmailTileBuilder, enabling AI action tag display.lib/features/thread/presentation/widgets/email_tile_web_builder.dart (1)
12-12: LGTM: AI action tag properly integrated across responsive layouts.The
isAINeedsActionEnabledparameter is correctly threaded through all responsive layouts (mobile, tablet, desktop), with the computed_shouldShowAIActiongetter ensuring the tag only displays when both the feature is enabled and the email has the need-action keyword.Also applies to: 30-30, 45-45, 175-178, 195-195, 379-380, 428-429
lib/features/thread/presentation/thread_controller.dart (7)
503-529: Virtual-folder gating in_getAllEmailSuccessis reasonableUsing
currentMailboxIdplusselectedMailbox?.isVirtualFolderto early-return avoids applying mailbox-scoped results on top of a currently selected virtual folder, while still accepting results whencurrentMailboxIdisnull(e.g., virtual-folder filter-based queries). This preserves correctness for both normal and virtual mailboxes and keeps the log context useful.
547-576: Symmetric gating for refresh-changes success keeps virtual-folder state cleanMirroring the
currentMailboxId/isVirtualFoldercheck in_refreshChangesAllEmailSuccessensures refresh-change batches don’t overwrite the UI when a virtual folder is selected. Combined with the new virtual-folder-specific refresh path, this keeps the mailbox-scoped and virtual-folder-scoped flows separated.
578-600: Disabling cache when loading virtual folders avoids stale or mailbox-scoped cache issues
getAllEmailActionnow builds filters viagetEmailFilterForLoadMailbox()and setsuseCache: selectedMailbox?.isVirtualFolder != true. That means:
- Normal mailboxes continue to leverage cache.
- Virtual folders (like Action Required) always hit the server, which is appropriate since they span multiple mailboxes and are keyed by keywords instead of a single mailbox id.
640-658: WebSocket branch for virtual folders plus queue cleanup is soundRouting WebSocket processing to
_refreshChangeListEmailsInVirtualFolder()whenselectedMailbox?.isVirtualFolder == trueprevents mixing mailbox-based incremental updates with the virtual-folder view. The finalremoveMessagesUpToCurrent(...)call still usescurrentEmailState, which gets updated in_getAllEmailSuccess, so queue compaction remains correct after virtual-folder refreshes.
750-782: Virtual-folder refresh path correctly forces a fresh server snapshot
_refreshChangeListEmailsInVirtualFolder()first refreshes cache/state, then issues a non-cachedGetEmailsInMailboxInteractor.executewith the same filter (getEmailFilterForLoadMailbox()) and limit. This:
- Keeps thread-detail and cache in sync via
_refreshChangeListEmailCache().- Ensures the virtual-folder list reflects the latest state without relying on mailbox-id-based caches (
useCache: false).
Behavior is coherent with the rest of the virtual-folder handling.
784-805: Load-more semantics respect virtual folders and cache configurationUsing
getFilterConditionForLoadMailbox(oldestEmail: oldestEmail)alongsideuseCache: selectedMailbox?.isVirtualFolder != truemakes:
- Non-virtual mailboxes reuse cache and bound by
oldestEmail.receivedAt.- Virtual folders page strictly from the server using the mailbox-filter builder, which includes the
beforecutoff for all relevant conditions.
807-820: Relaxed mailbox-id check for virtual folders in_validatePresentationEmailis appropriateAllowing emails through when
selectedMailbox?.isVirtualFolder == true(while still de-duplicating byid) is necessary because Action Required and other virtual folders are defined by keyword filters, not bymailboxIdscontaining a dedicated virtual mailbox id. This keeps the validation strict for normal folders and permissive-but-safe for virtual ones.lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart (2)
307-315: AI needs-action server setting is correctly funneled into a dedicated extensionIntroducing
isAINeedsActionSettingEnabledand delegating both success and failure handling tosetupAINeedsActionSetting(...)keeps the controller lean and centralizes AI-needs-action enablement logic (including any side-effects like virtual-folder creation/removal) in the extension. This matches the requested behavior of honoring the backend/user setting before doing any “needs action” processing.Also applies to: 515-518, 573-575
1380-1408: Drag-to-move from virtual folders is correctly generalizedSwitching the condition from
selectedMailbox.value?.isFavorite == truetoselectedMailbox.value?.isVirtualFolder == trueensures that:
- Dragging from virtual folders (Favorites, Action Required, etc.) or from search results builds the per-mailbox
mapListEmailSelectedByMailBoxId.- “Normal” mailbox views still use the cheaper
{selectedMailbox.id: listEmails.listEmailIds}map.
Destination handling remains unchanged (Favorites as a special drop target), which preserves the requirement that users cannot drag emails into the Action Required virtual folder (UI DragTarget is disabled there per earlier learnings).lib/features/mailbox/presentation/mailbox_controller.dart (2)
71-89: autoCreateVirtualFolder integration centralizes favorite + action-required setupImporting
handle_action_required_tab_extension.dartandhandle_ai_needs_action_extension.dart, then calling:autoCreateVirtualFolder( mailboxDashBoardController.isAINeedsActionEnabled, );in
onDone()(forGetAllMailboxFailure,GetAllMailboxSuccess, andCreateDefaultMailboxAllSuccess) and in_handleRefreshChangeMailboxSuccessmakes virtual-folder creation/removal a single concern. This keeps favorite-folder behavior and the new Action Required virtual folder in sync whenever mailbox trees are (re)built or refreshed.Also applies to: 261-292, 608-623
306-337: Mailbox UI actions for Action Required folder creation/removal are well-scopedHandling
AutoCreateActionRequiredFolderMailboxActionandAutoRemoveActionRequiredFolderMailboxActionby delegating toaddActionRequiredFolder()/removeActionRequiredFolder()and clearing the UI action is clean. The extra guard:if (selectedMailbox?.isActionRequired == true) { _switchBackToMailboxDefault(); }on removal prevents the user from being left on a now-nonexistent virtual folder, gracefully sending them back to Inbox.
lib/features/thread/presentation/filters/mailbox_filter_builder.dart (1)
15-134: MailboxFilterBuilder cleanly unifies mailbox-specific filter logic (favorites, action-required, default)The builder encapsulates all mailbox +
FilterMessageOptioncombinations:
- For normal mailboxes,
buildEmailFilterForLoadMailbox()returns anEmailFilterwithmailboxId,filterOption, and a defaultEmailFilterCondition(all/unread/attachments/starred) with consistentbeforehandling.- For Favorites,
_buildFavoriteMailboxFilteralways enforcesemailFlagged, layering unread/attachments as needed.- For Action Required,
_buildActionRequiredMailboxFilterenforcesneeds-action+not $seen, with a starred variant expressed as anANDof “flagged” and “unread & needs-action”.beforenow applies uniformly to all conditions.- For virtual folders in general (
selectedMailbox?.isVirtualFolder == true),buildEmailFilterForLoadMailbox()omitsmailboxIdand relies purely on the composed filter, which aligns with the cross-mailbox nature of virtual folders.This centralization should make future tweaks to favorite/action-required semantics much easier.
lib/features/thread/presentation/thread_view.dart (4)
23-23: LGTM!The import follows the established pattern for extension imports in this file.
498-527: LGTM!The refactored method correctly wraps reactive state reads within
Obxand properly passes the newisAINeedsActionEnabledparameter toEmailTileBuilder.
530-639: LGTM!The method structure is well-designed: background widgets depend on the non-reactive
presentationEmailparameter and are correctly computed outsideObx, while reactive controller state is properly read inside theObxwrapper. TheisAINeedsActionEnabledparameter is correctly propagated toEmailTileBuilder.
779-798: LGTM!The generalization from
isFavoritetoisVirtualFoldercorrectly handles both the Favorites and Action Required virtual folders. The condition logic now properly avoids hiding the empty state for virtual folders that don't have real mailbox IDs. Based on learnings, Action Required is a virtual folder similar to Favorites.lib/l10n/intl_messages.arb (2)
4865-4882: LGTM!The AI Scribe localization keys are properly formatted with correct metadata structure and appropriate English text.
5151-5185: LGTM!The Action Required and AI needs-action localization keys are properly structured. The casing difference between "Action Required" (for tags) and "Action required" (for mailbox display) appears intentional for different UI contexts, consistent with other display name patterns in the file. Based on learnings, translations for other locales are managed automatically through the external workflow.
…le, web, tablet view
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
test/fixtures/widget_fixtures.dart (2)
24-38: Consider adding documentation for proper cleanup.The method correctly implements responsive testing setup, but since it modifies the global
debugDefaultTargetPlatformOverride, consider adding doc comments to remind users to callresetResponsivein their test'stearDownto avoid side effects on other tests.💡 Example documentation
+ /// Sets up responsive testing with the specified [logicalSize] and optional [platform]. + /// + /// Note: This method modifies global state via [debugDefaultTargetPlatformOverride]. + /// Always call [resetResponsive] in your test's tearDown to clean up: + /// + /// ```dart + /// tearDown(() { + /// WidgetFixtures.resetResponsive(tester); + /// }); + /// ``` static Future<void> setupResponsive(
45-60: Consider documenting widget wrapping expectations.The implementation is correct, but since this method doesn't automatically wrap the widget with
makeTestableWidget, consider adding a doc comment clarifying that callers should wrap their widget if they need localization or GetX context.💡 Example documentation
+ /// Pumps a widget with responsive setup for the specified [logicalSize] and optional [platform]. + /// + /// Note: This method does not wrap the widget with [makeTestableWidget]. + /// If your widget requires localization or GetX context, wrap it first: + /// + /// ```dart + /// await WidgetFixtures.pumpResponsiveWidget( + /// tester, + /// WidgetFixtures.makeTestableWidget(child: YourWidget()), + /// logicalSize: Size(800, 600), + /// ); + /// ``` static Future<void> pumpResponsiveWidget(test/features/thread/presentation/widgets/email_tile_builder_ai_action_test.dart (1)
17-21: Consider adding tearDownAll for GetX cleanup.While the setUpAll registers ResponsiveUtils and ImagePaths globally, there's no corresponding tearDownAll to clean up these dependencies. This could affect test isolation if other test suites run in the same process.
🔎 Proposed addition
setUpAll(() { Get.put(ResponsiveUtils()); Get.put(ImagePaths()); }); + + tearDownAll(() { + Get.delete<ResponsiveUtils>(); + Get.delete<ImagePaths>(); + });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
test/features/thread/presentation/widgets/email_tile_builder_ai_action_test.darttest/fixtures/widget_fixtures.dart
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4210
File: model/lib/mailbox/presentation_mailbox.dart:22-34
Timestamp: 2025-12-23T05:27:10.887Z
Learning: In the tmail-flutter codebase, the Action Required folder (actionRequiredFolder) is a virtual folder that is AI-curated and contains emails with the keyword "needs-action". Unlike the Favorites folder, users should NOT be able to drag emails into the Action Required folder. The UI correctly prevents this by not wrapping the Action Required folder item in a DragTarget (see mailbox_item_widget.dart).
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.
📚 Learning: 2025-12-23T05:27:47.287Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4210
File: lib/l10n/intl_messages.arb:5151-5182
Timestamp: 2025-12-23T05:27:47.287Z
Learning: In the tmail-flutter repository, localization (l10n) updates for locale files (intl_fr.arb, intl_vi.arb, intl_ru.arb, etc.) are managed automatically through a third-party website/platform. When new English keys are added to intl_messages.arb, translations to other locales are handled by this automated workflow rather than being manually added in pull requests.
Applied to files:
test/fixtures/widget_fixtures.dart
📚 Learning: 2025-12-18T09:19:32.437Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4203
File: scribe/pubspec.yaml:37-37
Timestamp: 2025-12-18T09:19:32.437Z
Learning: In the tmail-flutter repository, json_annotation version 4.8.0 is the standardized version across modules and should not be upgraded without considering the standardization practice.
Applied to files:
test/fixtures/widget_fixtures.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:
test/fixtures/widget_fixtures.darttest/features/thread/presentation/widgets/email_tile_builder_ai_action_test.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:
test/fixtures/widget_fixtures.darttest/features/thread/presentation/widgets/email_tile_builder_ai_action_test.dart
📚 Learning: 2025-12-23T05:27:10.887Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4210
File: model/lib/mailbox/presentation_mailbox.dart:22-34
Timestamp: 2025-12-23T05:27:10.887Z
Learning: In the tmail-flutter codebase, the Action Required folder (actionRequiredFolder) is a virtual folder that is AI-curated and contains emails with the keyword "needs-action". Unlike the Favorites folder, users should NOT be able to drag emails into the Action Required folder. The UI correctly prevents this by not wrapping the Action Required folder item in a DragTarget (see mailbox_item_widget.dart).
Applied to files:
test/features/thread/presentation/widgets/email_tile_builder_ai_action_test.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/thread/presentation/widgets/email_tile_builder_ai_action_test.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: analyze-test (rule_filter)
- GitHub Check: analyze-test (fcm)
- GitHub Check: analyze-test (email_recovery)
- GitHub Check: analyze-test (default)
- GitHub Check: analyze-test (model)
- GitHub Check: analyze-test (forward)
- GitHub Check: analyze-test (server_settings)
- GitHub Check: analyze-test (core)
- GitHub Check: analyze-test (contact)
- GitHub Check: Build web version and deploy
🔇 Additional comments (5)
test/fixtures/widget_fixtures.dart (2)
1-1: LGTM! Necessary imports for responsive testing utilities.The
flutter/foundation.dartimport providesdebugDefaultTargetPlatformOverrideandflutter_test/flutter_test.dartprovidesWidgetTester, both required for the new responsive testing helpers.Also applies to: 4-4
40-43: LGTM! Proper cleanup implementation.The method correctly resets both the platform override and the tester view, ensuring no side effects leak between tests.
test/features/thread/presentation/widgets/email_tile_builder_ai_action_test.dart (3)
23-36: LGTM!The test email fixtures are minimal and focused on the specific feature being tested (needs-action keyword). This is appropriate for unit testing the AI action tag display.
37-142: LGTM!The test suite comprehensively covers the AI action tag display across form factors (mobile, tablet, desktop), validates both positive and negative cases, and verifies localization. The consistent use of
WidgetFixtures.resetResponsive(tester)ensures proper cleanup between tests.
144-162: LGTM!The test correctly verifies that the AI action tag is hidden when the feature is disabled. Using plain
pumpWidgetinstead ofpumpResponsiveWidgetis appropriate here since responsive behavior is not relevant to the feature toggle test.
…per cleanup, widget wrapping expectations
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/features/thread/presentation/widgets/email_tile_builder_ai_action_test.dart (1)
144-162: Consider using pumpResponsiveWidget for consistency.This test uses
tester.pumpWidgetdirectly instead ofWidgetFixtures.pumpResponsiveWidget, unlike all other tests in this file. While functionally correct, using the same responsive testing pattern would maintain consistency.🔎 Proposed refactor for consistency
testWidgets( 'does not show AI Action tag', (tester) async { - await tester.pumpWidget( + await WidgetFixtures.pumpResponsiveWidget( + tester, WidgetFixtures.makeTestableWidget( child: EmailTileBuilder( presentationEmail: emailWithNeedsAction, selectAllMode: SelectMode.INACTIVE, isShowingEmailContent: false, isAINeedsActionEnabled: false, ), ), + logicalSize: const Size(375, 812), ); expect(find.byType(AiActionTagWidget), findsNothing); + + WidgetFixtures.resetResponsive(tester); }, );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
test/features/thread/presentation/widgets/email_tile_builder_ai_action_test.darttest/fixtures/widget_fixtures.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- test/fixtures/widget_fixtures.dart
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4210
File: model/lib/mailbox/presentation_mailbox.dart:22-34
Timestamp: 2025-12-23T05:27:10.887Z
Learning: In the tmail-flutter codebase, the Action Required folder (actionRequiredFolder) is a virtual folder that is AI-curated and contains emails with the keyword "needs-action". Unlike the Favorites folder, users should NOT be able to drag emails into the Action Required folder. The UI correctly prevents this by not wrapping the Action Required folder item in a DragTarget (see mailbox_item_widget.dart).
📚 Learning: 2025-12-23T05:27:10.887Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4210
File: model/lib/mailbox/presentation_mailbox.dart:22-34
Timestamp: 2025-12-23T05:27:10.887Z
Learning: In the tmail-flutter codebase, the Action Required folder (actionRequiredFolder) is a virtual folder that is AI-curated and contains emails with the keyword "needs-action". Unlike the Favorites folder, users should NOT be able to drag emails into the Action Required folder. The UI correctly prevents this by not wrapping the Action Required folder item in a DragTarget (see mailbox_item_widget.dart).
Applied to files:
test/features/thread/presentation/widgets/email_tile_builder_ai_action_test.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/thread/presentation/widgets/email_tile_builder_ai_action_test.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:
test/features/thread/presentation/widgets/email_tile_builder_ai_action_test.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:
test/features/thread/presentation/widgets/email_tile_builder_ai_action_test.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: analyze-test (email_recovery)
- GitHub Check: Build web version and deploy
- GitHub Check: analyze-test (forward)
- GitHub Check: analyze-test (rule_filter)
- GitHub Check: analyze-test (default)
- GitHub Check: analyze-test (fcm)
- GitHub Check: analyze-test (model)
- GitHub Check: analyze-test (contact)
- GitHub Check: analyze-test (server_settings)
- GitHub Check: analyze-test (core)
🔇 Additional comments (5)
test/features/thread/presentation/widgets/email_tile_builder_ai_action_test.dart (5)
1-15: LGTM!The imports are well-organized and necessary for the test setup, covering core utilities, test framework, GetX dependencies, JMAP models, and UI components.
17-21: LGTM!Proper GetX dependency initialization for ResponsiveUtils and ImagePaths, with corresponding cleanup in tearDownAll. This follows best practices for GetX test cleanup.
24-35: LGTM!Test fixtures appropriately cover both scenarios: email with needs-action keyword and email without. The minimal email properties keep tests focused.
37-142: Excellent test coverage across form factors and scenarios!The test suite thoroughly covers mobile, tablet, and desktop views, verifies both presence and absence of the AI Action tag, and includes localization validation. The consistent use of
WidgetFixtures.resetResponsive(tester)ensures proper cleanup between tests.
165-168: LGTM!Proper GetX cleanup in tearDownAll, ensuring ResponsiveUtils and ImagePaths are deleted after all tests complete. This prevents test pollution and follows best practices for GetX test cleanup.
| } | ||
| } | ||
|
|
||
| EmailFilterCondition buildDefaultMailboxFilter({UTCDate? before}) { |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/features/thread/presentation/filters/mailbox_filter_builder.dart (1)
78-111: Action Required filter correctly implements requirements.The filter properly excludes seen emails and includes the needs-action keyword as specified in the PR objectives. The use of
LogicFilterOperator.ANDfor the starred case is necessary since a singleEmailFilterConditioncannot include multiplehasKeywordconstraints.Minor observation: The
beforeparameter appears in both conditions within the AND operator (lines 86 and 91). While redundant, this is harmless and may even improve clarity.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/features/thread/presentation/filters/mailbox_filter_builder.dart
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4210
File: model/lib/mailbox/presentation_mailbox.dart:22-34
Timestamp: 2025-12-23T05:27:10.887Z
Learning: In the tmail-flutter codebase, the Action Required folder (actionRequiredFolder) is a virtual folder that is AI-curated and contains emails with the keyword "needs-action". Unlike the Favorites folder, users should NOT be able to drag emails into the Action Required folder. The UI correctly prevents this by not wrapping the Action Required folder item in a DragTarget (see mailbox_item_widget.dart).
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.
📚 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:
lib/features/thread/presentation/filters/mailbox_filter_builder.dart
📚 Learning: 2025-12-23T05:27:10.887Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4210
File: model/lib/mailbox/presentation_mailbox.dart:22-34
Timestamp: 2025-12-23T05:27:10.887Z
Learning: In the tmail-flutter codebase, the Action Required folder (actionRequiredFolder) is a virtual folder that is AI-curated and contains emails with the keyword "needs-action". Unlike the Favorites folder, users should NOT be able to drag emails into the Action Required folder. The UI correctly prevents this by not wrapping the Action Required folder item in a DragTarget (see mailbox_item_widget.dart).
Applied to files:
lib/features/thread/presentation/filters/mailbox_filter_builder.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/thread/presentation/filters/mailbox_filter_builder.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/thread/presentation/filters/mailbox_filter_builder.dart
🔇 Additional comments (5)
lib/features/thread/presentation/filters/mailbox_filter_builder.dart (5)
1-24: Clean class structure with proper immutability.The const constructor and null-safe design are well-implemented. The private getter
_mailboxIdprovides a clean accessor for the mailbox ID.
26-38: Correct virtual folder handling.The logic correctly omits
mailboxIdandfilterOptionfor virtual folders while including them for regular mailboxes. This aligns with the Action Required folder being a virtual, cross-mailbox view.
40-52: LGTM: Clear filter routing logic.The method correctly routes to specialized filters for virtual folders and falls back to default behavior for regular mailboxes. The
beforeparameter extraction supports pagination effectively.
54-76: Favorite mailbox filter correctly implemented.The filter properly treats favorites as a virtual folder by omitting
inMailbox, and correctly combines the flagged keyword with additional constraints for unread and attachments views.
113-139: Default mailbox filter correctly includesinMailboxin all cases.The past review comment about missing
inMailboxhas been addressed. All filter cases (all, unread, attachments, starred) now correctly includeinMailbox: _mailboxIdto constrain queries to the selected mailbox.
Issue
#4141
Resolved
demo-expection-action.mov
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.