TF-4227 Display view with all mail with a tag #4228
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR introduces support for label-based mailboxes throughout the application. It adds a new Possibly related PRs
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 |
|
This PR has been deployed to https://linagora.github.io/tmail-flutter/4228. |
dab246
force-pushed
the
features/tf-4227-display-view-with-all-mail-with-a-tag
branch
from
761cd30 to
f932b0e
Compare
|
@coderabbitai please review this |
|
@chibenwa: I'll review the changes in this PR for you. ✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
lib/features/thread/domain/model/email_filter.dart (1)
15-25: copyWith cannot be used to clear fields to nullThe
copyWithimplementation is fine for updating fields, but note that because parameters are nullable and use??, there’s no way to explicitly setfilter,filterOption, ormailboxIdback tonullvia this method. If you anticipate needing to clear any of these, consider a sentinel‑basedcopyWithsignature (e.g., usingObject?params plus a private sentinel) so callers can distinguish “keep current value” from “set to null”.lib/features/thread/presentation/filters/mailbox_filter_builder.dart (1)
77-90: Consider addingbeforeto both conditions for clarity.In the starred case,
beforeis only set on the secondEmailFilterCondition. While functionally correct (JMAP ANDs all conditions on the same email), it may cause confusion during maintenance. Consider adding it to both for consistency with other cases.🔎 Optional: Add before to both conditions for clarity
case FilterMessageOption.starred: return LogicFilterOperator( Operator.AND, { EmailFilterCondition( hasKeyword: KeyWordIdentifier.emailFlagged.value, + before: oldestEmail?.receivedAt, ), EmailFilterCondition( hasKeyword: keyword, - before: oldestEmail?.receivedAt, ), }, );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
lib/features/mailbox/presentation/base_mailbox_view.dartlib/features/mailbox/presentation/extensions/presentation_mailbox_extension.dartlib/features/mailbox/presentation/model/presentation_label_mailbox.dartlib/features/mailbox/presentation/widgets/labels/label_list_item.dartlib/features/mailbox/presentation/widgets/labels/label_list_view.dartlib/features/thread/domain/model/email_filter.dartlib/features/thread/presentation/filters/mailbox_filter_builder.dartlib/features/thread/presentation/thread_controller.dartlib/features/thread/presentation/widgets/empty_emails_widget.dartlib/l10n/intl_messages.arblib/main/localizations/app_localizations.darttest/features/search/verify_before_time_in_search_email_filter_test.dart
🧰 Additional context used
🧠 Learnings (11)
📚 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:
lib/main/localizations/app_localizations.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/main/localizations/app_localizations.dartlib/features/thread/presentation/widgets/empty_emails_widget.dartlib/features/thread/domain/model/email_filter.dartlib/features/mailbox/presentation/widgets/labels/label_list_view.dartlib/features/mailbox/presentation/model/presentation_label_mailbox.dartlib/features/thread/presentation/filters/mailbox_filter_builder.dartlib/features/mailbox/presentation/base_mailbox_view.darttest/features/search/verify_before_time_in_search_email_filter_test.dartlib/features/mailbox/presentation/extensions/presentation_mailbox_extension.dartlib/features/mailbox/presentation/widgets/labels/label_list_item.dartlib/features/thread/presentation/thread_controller.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/main/localizations/app_localizations.dartlib/features/thread/presentation/widgets/empty_emails_widget.dartlib/features/thread/domain/model/email_filter.dartlib/features/mailbox/presentation/widgets/labels/label_list_view.dartlib/features/mailbox/presentation/model/presentation_label_mailbox.dartlib/features/thread/presentation/filters/mailbox_filter_builder.dartlib/features/mailbox/presentation/base_mailbox_view.darttest/features/search/verify_before_time_in_search_email_filter_test.dartlib/features/mailbox/presentation/extensions/presentation_mailbox_extension.dartlib/features/mailbox/presentation/widgets/labels/label_list_item.dartlib/features/thread/presentation/thread_controller.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:
lib/features/thread/presentation/widgets/empty_emails_widget.dartlib/features/thread/domain/model/email_filter.dartlib/features/mailbox/presentation/model/presentation_label_mailbox.dartlib/features/thread/presentation/filters/mailbox_filter_builder.dartlib/features/mailbox/presentation/base_mailbox_view.darttest/features/search/verify_before_time_in_search_email_filter_test.dartlib/features/mailbox/presentation/extensions/presentation_mailbox_extension.dartlib/features/mailbox/presentation/widgets/labels/label_list_item.dartlib/features/thread/presentation/thread_controller.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/widgets/empty_emails_widget.dartlib/features/mailbox/presentation/widgets/labels/label_list_view.dartlib/features/mailbox/presentation/model/presentation_label_mailbox.dartlib/features/mailbox/presentation/base_mailbox_view.dartlib/features/mailbox/presentation/extensions/presentation_mailbox_extension.dartlib/features/mailbox/presentation/widgets/labels/label_list_item.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/widgets/empty_emails_widget.dartlib/features/mailbox/presentation/base_mailbox_view.darttest/features/search/verify_before_time_in_search_email_filter_test.dartlib/features/mailbox/presentation/extensions/presentation_mailbox_extension.dartlib/features/thread/presentation/thread_controller.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/widgets/empty_emails_widget.dartlib/features/mailbox/presentation/model/presentation_label_mailbox.dartlib/features/mailbox/presentation/extensions/presentation_mailbox_extension.dartlib/features/mailbox/presentation/widgets/labels/label_list_item.dartlib/features/thread/presentation/thread_controller.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-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:
test/features/search/verify_before_time_in_search_email_filter_test.dartlib/features/thread/presentation/thread_controller.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/presentation/extensions/presentation_mailbox_extension.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). (1)
- GitHub Check: Build web version and deploy
🔇 Additional comments (14)
lib/l10n/intl_messages.arb (1)
5308-5314: New label empty‑state localization is wired correctlyKey
youDoNotHaveAnyEmailTaggedWithThisand its metadata follow the existing ARB pattern and match the new getter inAppLocalizations, so this addition looks good.lib/features/mailbox/presentation/model/presentation_label_mailbox.dart (1)
1-36: PresentationLabelMailbox implementation is clean and consistentThe subclass cleanly forwards all mailbox fields to
PresentationMailbox, adds thelabelfield, and correctly extendspropswith[label, ...super.props]for Equatable. Theinitialfactory is a nice convenience for creating label mailboxes from aLabel.lib/main/localizations/app_localizations.dart (1)
5627-5633: Getter for label empty‑state string matches ARB definition
youDoNotHaveAnyEmailTaggedWithThiscorrectly mirrors the ARB key and message text and follows the existing localization pattern.lib/features/mailbox/presentation/base_mailbox_view.dart (1)
7-8: Label selection wiring is sound; verify label.id non‑null invariantThe new logic in
buildLabelsListto:
- Derive
labelIdSelectedonly whenselectedMailbox is PresentationLabelMailbox, and- Pass
labelIdSelectedplus anonOpenLabelCallbackthat callsopenMailbox(context, PresentationLabelMailbox.initial(label))is a clean way to keep label selection state in sync between the mailbox view and the label list.
One thing to double‑check:
PresentationLabelMailbox.initialusesMailboxId(label.id!), and herelabelcomes fromlabelController.labels. If there’s any transient state where aLabelcan exist in that list with a nullid(e.g., just‑created but not yet persisted), this will throw at runtime. If such a state is possible, adding a guard (skip labels without an id) or an assertion around creation would make this safer.Also applies to: 19-20, 397-402, 415-419
lib/features/thread/presentation/thread_controller.dart (2)
601-626: Good refactoring to centralize filter construction.The delegation to
MailboxFilterBuilderproperly encapsulates the filter construction logic, making it easier to maintain and test. The three methods (getEmailFilterForLoadMailbox,getFilterConditionForLoadMailbox,getFilterCondition) now have clear, focused responsibilities.
594-594: Appropriate use ofisCacheablefor cache toggle.Using
isCacheablecorrectly extends the non-caching behavior to label mailboxes alongside favorites, as both are virtual/keyword-based mailboxes.lib/features/thread/presentation/widgets/empty_emails_widget.dart (1)
103-107: LGTM!The label mailbox condition is correctly placed in the priority chain, and the localized message provides appropriate feedback for empty label views.
lib/features/mailbox/presentation/widgets/labels/label_list_view.dart (1)
31-40: LGTM!The selection state wiring is clean: extracting the label variable improves readability, and the
isSelectedcomputation vialabel.id == labelIdSelectedis straightforward.test/features/search/verify_before_time_in_search_email_filter_test.dart (1)
863-863: Good practice: testing through the actual API.Using
threadController.getEmailFilterForLoadMailbox()instead of manually constructing the expectedEmailFilterensures the test remains synchronized with implementation changes and validates the real behavior.lib/features/mailbox/presentation/widgets/labels/label_list_item.dart (2)
10-26: Clean widget API extension.The callback typedef and selection state additions follow established patterns in the codebase. Making
onOpenLabelCallbackrequired ensures labels are always interactive.
80-81: Selection styling looks good.Using
AppColor.blue100for selected state andColors.transparentfor unselected is a clean approach that integrates with the existing design system.lib/features/mailbox/presentation/extensions/presentation_mailbox_extension.dart (1)
142-154: Well-designed extension properties.The three new getters provide clear, focused responsibilities:
isCacheablecorrectly identifies mailboxes suitable for cachingisLabelMailboxprovides a clean type check abstractionfilterKeywordencapsulates the keyword resolution logic for virtual mailboxeslib/features/thread/presentation/filters/mailbox_filter_builder.dart (2)
14-56: Good builder pattern implementation.The
MailboxFilterBuildercleanly centralizes filter construction logic with clear separation between keyword-based and mailbox-based filtering paths. The const constructor and immutable design are appropriate.
28-40: SetfilterOptionfor label mailboxes in thebuild()method.Label mailboxes currently omit
filterOptionwhen copying the base filter (line 33), while regular mailboxes include it (lines 36–39). This causesemailFilter.filterOptionto benullfor labels, breaking the_isApproveFilterOptionvalidation inthread_repository_impl.dartwhich expects a non-null value. AddfilterOption: filterOptionto thecopyWith()call for label mailboxes to match the regular mailbox behavior.⛔ Skipped due to 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.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.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).
…_email_with_tag_scenario
…en_open_tag_scenario
🧪 Should display view with all email with tag correctly when open label
✅ 1. waitUntilVisible widgets with type "TwakeWelcomeView".
✅ 2. tap widgets with text "Use company server".
✅ 3. waitUntilVisible widgets with type "LoginView".
✅ 4. tap widgets with type "TextField" descending from widgets with key [<'dns_lookup_input_form'>].
✅ 5. enterText widgets with type "TextField" descending from widgets with key [<'dns_lookup_input_form'>].
✅ 6. waitUntilVisible widgets with text "bob".
✅ 7. tap widgets with text "Next".
✅ 8. waitUntilVisible widgets with key [<'base_url_form'>] descending from widgets with type "LoginView".
✅ Should display view with all email with tag correctly when open label (integration_test/tests/labels/display_view_with_all_email_with_tag_test.dart) (23s)
Test summary:
📝 Total: 1
✅ Successful: 1
❌ Failed: 0
⏩ Skipped: 0
📊 Report: file:///Users/datvu/WorkingSpace/tmail-flutter/build/app/reports/androidTests/connected/index.html
⏱️ Duration: 100sScreen_recording_20260107_120802.webm
🧪 Should display empty view when open tag with no email
✅ 1. waitUntilVisible widgets with type "TwakeWelcomeView".
✅ 2. tap widgets with text "Use company server".
✅ 3. waitUntilVisible widgets with type "LoginView".
✅ 4. tap widgets with type "TextField" descending from widgets with key [<'dns_lookup_input_form'>].
✅ 5. enterText widgets with type "TextField" descending from widgets with key [<'dns_lookup_input_form'>].
✅ 6. waitUntilVisible widgets with text "bob".
✅ Should display empty view when open tag with no email (integration_test/tests/labels/display_empty_view_when_open_tag_test.dart) (18s)
Test summary:
📝 Total: 1
✅ Successful: 1
❌ Failed: 0
⏩ Skipped: 0
📊 Report: file:///Users/datvu/WorkingSpace/tmail-flutter/build/app/reports/androidTests/connected/index.html
⏱️ Duration: 94s
demo-empty.webm |
Issue
#4227
Resolved
demo.mov
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.