Skip to content

TF-4392 [Team Mailbox] Delete email in Trash #4416

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dab246 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

TF-4392 [Team Mailbox] Delete email in Trash #4416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5b238e1
TF-4392 [Team Mailbox] Delete emails within the team mailbox trash
dab246 Mar 26, 2026
fd13b5b
TF-4392 [Team Mailbox] Empty the team mailbox trash
dab246 Mar 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 2 additions & 8 deletions lib/features/base/mixin/mailbox_action_handler_mixin.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,7 @@ mixin MailboxActionHandlerMixin {
..onConfirmAction(AppLocalizations.of(context).delete, () {
popBack();
if (mailbox.countTotalEmails > 0) {
dashboardController.emptyTrashFolderAction(
trashFolderId: mailbox.id,
totalEmails: mailbox.countTotalEmails
);
dashboardController.emptyTrashFolderAction(trashMailbox: mailbox);
} else {
appToast.showToastWarningMessage(
context,
Expand All @@ -92,10 +89,7 @@ mixin MailboxActionHandlerMixin {
onConfirmAction: () {
popBack();
if (mailbox.countTotalEmails > 0) {
dashboardController.emptyTrashFolderAction(
trashFolderId: mailbox.id,
totalEmails: mailbox.countTotalEmails
);
dashboardController.emptyTrashFolderAction(trashMailbox: mailbox);
} else {
appToast.showToastWarningMessage(
context,
Expand Down
3 changes: 1 addition & 2 deletions lib/features/mailbox/presentation/mailbox_controller.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1600,8 +1600,7 @@ class MailboxController extends BaseMailboxController
log('MailboxController::emptyMailboxAction:presentationMailbox: ${presentationMailbox.name}');
if (presentationMailbox.isTrash) {
mailboxDashBoardController.emptyTrashFolderAction(
trashFolderId: presentationMailbox.id,
totalEmails: presentationMailbox.countTotalEmails
trashMailbox: presentationMailbox,
);
} else if (presentationMailbox.isSpam) {
mailboxDashBoardController.emptySpamFolderAction(
Expand Down
4 changes: 3 additions & 1 deletion lib/features/mailbox/presentation/mixin/mailbox_widget_mixin.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ mixin MailboxWidgetMixin {
if (mailbox.isSubscribedMailbox)
MailboxActions.disableMailbox
else
MailboxActions.enableMailbox
MailboxActions.enableMailbox,
if (mailbox.isTrash && mailbox.myRights?.mayRemoveItems == true)
MailboxActions.emptyTrash,
];
}

Expand Down
31 changes: 16 additions & 15 deletions lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1832,41 +1832,40 @@ class MailboxDashBoardController extends ReloadableController

void emptyTrashFolderAction({
Function? onCancelSelectionEmail,
MailboxId? trashFolderId,
int totalEmails = 0,
PresentationMailbox? trashMailbox,
}) {
onCancelSelectionEmail?.call();

final trashMailboxId = trashFolderId ?? mapDefaultMailboxIdByRole[PresentationMailbox.roleTrash];
final trashFolder = trashMailbox
?? (selectedMailbox.value?.isTrash == true ? selectedMailbox.value : null)
?? mapMailboxById[mapDefaultMailboxIdByRole[PresentationMailbox.roleTrash]];
final accountId = this.accountId.value;
final session = sessionCurrent;

if (accountId == null || sessionCurrent == null) {
if (accountId == null || session == null) {
consumeState(Stream.value(Left(EmptyTrashFolderFailure(NotFoundSessionException()))));
return;
}

if (trashMailboxId == null) {
if (trashFolder == null) {
consumeState(Stream.value(Left(EmptyTrashFolderFailure(NotFoundMailboxException()))));
return;
}

if (CapabilityIdentifier.jmapMailboxClear.isSupported(sessionCurrent!, accountId)) {
if (CapabilityIdentifier.jmapMailboxClear.isSupported(session, accountId) &&
trashFolder.isTrashTeamMailbox != true) {
clearMailbox(
sessionCurrent!,
session,
accountId,
trashMailboxId,
trashFolder.id,
PresentationMailbox.roleTrash,
);
} else {
final totalEmailsInTrash = totalEmails == 0
? mapMailboxById[trashMailboxId]?.countTotalEmails ?? 0
: totalEmails;

consumeState(_emptyTrashFolderInteractor.execute(
sessionCurrent!,
session,
accountId,
trashMailboxId,
totalEmailsInTrash,
trashFolder.id,
trashFolder.countTotalEmails,
progressStateController,
Comment on lines 1833 to 1869
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Enforce mayRemoveItems inside emptyTrashFolderAction.

Lines 1839-1869 resolve and execute against any trash mailbox, but never verify trashFolder.myRights?.mayRemoveItems == true. lib/features/mailbox/presentation/mixin/mailbox_widget_mixin.dart still adds MailboxActions.emptyTrash for default trash mailboxes without that guard, so some callers can reach this path and only fail after the server rejects the request. Add the permission check here as the final gate.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart`
around lines 1833 - 1869, Inside emptyTrashFolderAction, add a final permission
gate that checks trashFolder.myRights?.mayRemoveItems == true before attempting
clearMailbox or calling _emptyTrashFolderInteractor.execute; if the check fails,
short-circuit by calling consumeState with a Left(EmptyTrashFolderFailure(...))
containing an appropriate "not allowed" permission error and return. Place this
check after resolving trashFolder and before the existing
CapabilityIdentifier.jmapMailboxClear branch so both clearMailbox and
_emptyTrashFolderInteractor paths honor mayRemoveItems.

));
}
Expand Down Expand Up @@ -2397,6 +2396,7 @@ class MailboxDashBoardController extends ReloadableController
) {
return mailbox != null &&
mailbox.isTrash &&
mailbox.myRights?.mayRemoveItems == true &&
mailbox.countTotalEmails > 0 &&
!searchController.isSearchActive() &&
responsiveUtils.isWebDesktop(context);
Expand All @@ -2408,6 +2408,7 @@ class MailboxDashBoardController extends ReloadableController
) {
return mailbox != null &&
mailbox.isTrash &&
mailbox.myRights?.mayRemoveItems == true &&
mailbox.countTotalEmails > 0 &&
!searchController.isSearchActive() &&
!responsiveUtils.isWebDesktop(context);
Expand Down
13 changes: 12 additions & 1 deletion model/lib/extensions/presentation_mailbox_extension.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,18 @@ extension PresentationMailboxExtension on PresentationMailbox {

bool get isVirtualFolder => isFavorite || isActionRequired;

bool get isTrash => role == PresentationMailbox.roleTrash;
bool get isTrash {
if (isPersonal) {
return role == PresentationMailbox.roleTrash;
} else {
return isTrashTeamMailbox;
}
}

bool get isTrashTeamMailbox {
return isChildOfTeamMailboxes &&
name?.name.toLowerCase() == PresentationMailbox.trashRole.toLowerCase();
}
Comment on lines +43 to +54
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

isTrashTeamMailbox matches any descendant named trash.

Line 52 only checks hasParentId() via isChildOfTeamMailboxes, so folders like Team A/Projects/Trash will also return true. Line 43 then propagates that broader match to every isTrash consumer, which can expose empty-trash and permanent-delete behavior on ordinary folders. Tighten this to the actual first-level team trash mailbox.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@model/lib/extensions/presentation_mailbox_extension.dart` around lines 43 -
54, isTrashTeamMailbox currently returns true for any descendant named "trash"
because it only checks isChildOfTeamMailboxes (presence of any parent) and the
folder name; update isTrashTeamMailbox to only match the first-level team trash
mailbox by verifying the folder's immediate parent is a team mailbox root (not
any ancestor) and the folder's name equals PresentationMailbox.trashRole
(case-insensitive). Locate isTrashTeamMailbox and replace the broad parent check
(isChildOfTeamMailboxes/hasParentId) with a check that the direct parent is a
team mailbox root (e.g., compare parent type/id to the team mailbox identifier
or use an isImmediateChildOfTeamMailbox helper) so isTrash consumers only get
true for the top-level team Trash.


bool get isDrafts => role == PresentationMailbox.roleDrafts;

Expand Down
Loading