Skip to content

Conversation

@rafaeltonholo
Copy link
Member

@rafaeltonholo rafaeltonholo commented Oct 3, 2025

Fixes #6276.

This PR introduces the following changes:

  • Introduce a warning within the Message Compose screen when no sent folder is detected
  • Add a confirmation dialog so users can choose to send the message without setting up a sent folder and warn them the message will be deleted locally

@rafaeltonholo rafaeltonholo requested a review from a team as a code owner October 3, 2025 13:19
@rafaeltonholo rafaeltonholo requested a review from asoucar October 3, 2025 13:19
@rafaeltonholo rafaeltonholo changed the title Fix/6276/add warning in app notification sent folder not found feat(notifications): trigger sent folder not found on message compose Oct 3, 2025
@rafaeltonholo rafaeltonholo changed the title feat(notifications): trigger sent folder not found on message compose feat(notifications): trigger sent folder not found in-app notification on message compose Oct 3, 2025
@rafaeltonholo
Copy link
Member Author

@asoucar: Why is this notification its own class? Creating a class like this makes it seem like there will be different types of SentFolderNotFound notifications, which is doesn't seem like a normal use case. Why is this not simply create a notification with the correct fields?
#9900 (comment)

Short answer:

We intentionally model recurring domain events as concrete types. A dedicated SentFolderNotFoundNotification encodes the invariant fields (severity, icon, action set, and style) so every call site behaves consistently. It also makes it easier to change these invariants when needed because they’re centralized in one place.

Long answer:

I will split the answer into a few topics.

Why is this notification its own class?

Because the model is intentionally type-driven: each domain event encodes its invariants (severity, icon, actions, and style) once, in a named type. That prevents call-site drift and gives us a single place for localization and UX rules. It also lets us reconstruct the same notification deterministically (e.g., from SentFolderNotFound + accountUuid) to dismiss or re-emit without carrying an ID around.

Creating a class like this makes it seem like there will be different types of SentFolderNotFound notifications, which is doesn't seem like a normal use case.

Not different types, just different instances. This event is account-scoped; two accounts with no Sent folder produce two independent notifications. The factory takes accountUuid for that reason.

Although the notification UI may look the same, they’re distinct because they’re tied to different accounts.

Why is this not simply create a notification with the correct fields?

Ad-hoc field composition pushes UX decisions to every caller and is easy to get wrong (severity/actions/styling drift, inconsistent strings). Encoding those invariants in a dedicated type keeps behavior consistent, testable, and localizable. That's the core reason we avoid a generic "fill the fields" builder.

Let me know if that address your concerns.

@rafaeltonholo rafaeltonholo added the merge block: soft freeze PR to main is blocked: risky code or feature flag enablement must wait until soft freeze lifts. label Oct 3, 2025
@rafaeltonholo rafaeltonholo marked this pull request as draft October 10, 2025 10:59
@rafaeltonholo rafaeltonholo force-pushed the fix/6276/add-warning-in-app-notification-sent-folder-not-found branch from a1cea26 to 4ab9edb Compare October 14, 2025 18:23
@rafaeltonholo rafaeltonholo marked this pull request as ready for review October 14, 2025 18:24
@wmontwe wmontwe removed the merge block: soft freeze PR to main is blocked: risky code or feature flag enablement must wait until soft freeze lifts. label Oct 17, 2025
@rafaeltonholo rafaeltonholo force-pushed the fix/6276/add-warning-in-app-notification-sent-folder-not-found branch 3 times, most recently from d1b475c to f92c2fa Compare October 21, 2025 18:36
@rafaeltonholo rafaeltonholo force-pushed the fix/6276/add-warning-in-app-notification-sent-folder-not-found branch from f92c2fa to 232f79a Compare October 21, 2025 18:56
@rafaeltonholo rafaeltonholo merged commit cb7a741 into thunderbird:main Oct 22, 2025
12 checks passed
@rafaeltonholo rafaeltonholo deleted the fix/6276/add-warning-in-app-notification-sent-folder-not-found branch October 22, 2025 19:09
@thunderbird-botmobile thunderbird-botmobile bot added this to the Thunderbird 15 milestone Oct 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Warn if Sent folder not found

3 participants