From 3d986788b9576451ef1215e0f423d587629b3f81 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 14 Sep 2026 12:15:35 +0200 Subject: feat: empty spam moves mail to the trash, per account --- src/keymap.cpp | 6 ++ src/mainwindow.cpp | 102 ++++++++++++++++++++++++++++- src/mainwindow.h | 16 +++++ tests/test_mainwindow.cpp | 163 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 285 insertions(+), 2 deletions(-) diff --git a/src/keymap.cpp b/src/keymap.cpp index b7cf8f5..098a85d 100644 --- a/src/keymap.cpp +++ b/src/keymap.cpp @@ -41,6 +41,12 @@ QStringList KeyMap::knownActions() // carries no default binding for exactly the same reason: the act is // identical and so is the hazard. QStringLiteral("purge"), + // Empty trash's other sibling, per ACCOUNT: it MOVES every message in + // the spam folder to that account's trash, so it is undoable and + // carries no confirmation. No default binding either: a bulk move + // deserves a deliberate gesture, and since item 132 an unbound action + // is menu-reachable rather than broken. + QStringLiteral("empty_spam"), QStringLiteral("spam"), QStringLiteral("toggle_unread"), QStringLiteral("mark_all_read"), diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 4a4a8d1..9198839 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -1783,6 +1783,16 @@ void MainWindow::registerActions() tr("Permanently delete the selected messages"), [this]() { purgeSelected(); }); + // Empty Trash's other sibling, but a MOVE rather than a purge: it moves + // every message in the spam folder to that account's trash, so it is + // undoable and asks nothing. The mnemonic is on "f&older" rather than the + // brief's `s&pam` because Alt+P is already Re&ply and Alt+S is Mark &spam, + // and no letter of "Empty spam" is free in this menu. + addAction(QStringLiteral("empty_spam"), tr("Empty spam f&older..."), + tr("Move every message in the spam folder to the trash"), + [this]() { + emptySpam(); + }); addAction(QStringLiteral("spam"), tr("Mark &spam"), tr("Move the selected messages to the spam folder"), [this]() { spamSelected(); @@ -2072,6 +2082,13 @@ const QHash> kThemeIcons = { // different amounts of mail must not look identical. `user-trash` is // the theme's own wastebasket, which reads as "this one, gone". { QStringLiteral("purge"), { QStringLiteral("user-trash"), QString() } }, + // Empty Spam SHARES purge's `user-trash`, deliberately: both move mail + // into the trash, and the standard wastebasket is the honest glyph. The + // no-duplicates rule is about the icon-only TOOLBAR, and this is a + // Message-menu-only entry that always carries its text, so it is named in + // noTwoActionsShareAnIcon()'s exception list. Putting it on the toolbar + // fails that test rather than passing silently. + { QStringLiteral("empty_spam"), { QStringLiteral("user-trash"), QString() } }, { QStringLiteral("undo"), { QStringLiteral("edit-undo"), QString() } }, // `bug` first, per the user's choice, with the standard junk name behind // it so a theme without the bug still draws a junk icon. @@ -2197,6 +2214,7 @@ void MainWindow::buildMenus() // the five filters would read as one of them. messageMenu->addAction(m_actions.value(QStringLiteral("cleanup_stranded"))); messageMenu->addAction(m_actions.value(QStringLiteral("empty_trash"))); + messageMenu->addAction(m_actions.value(QStringLiteral("empty_spam"))); messageMenu->addAction(m_actions.value(QStringLiteral("tag_rules"))); auto *viewMenu = menuBar()->addMenu(tr("&View")); @@ -3718,6 +3736,28 @@ bool MainWindow::isShowingTrash() const return false; } +bool MainWindow::isShowingSpam() const +{ + // Exact sibling of isShowingTrash(), and path-based for the same reason: + // the Spam filter matches a folder rather than a tag, so a message put + // there by another client is in the view and carries no tag of ours. Both + // scopes, because the view composes with the account dropdown. + const QString query = m_lastQuery.trimmed(); + if (query.isEmpty()) + return false; + + const QString all = m_config.allSpamQuery().trimmed(); + if (!all.isEmpty() && query == all) + return true; + + for (const Account &account : m_config.accounts()) { + const QString spam = account.spamQuery().trimmed(); + if (!spam.isEmpty() && query == spam) + return true; + } + return false; +} + void MainWindow::updateViewWideActions() { // Only meaningful on mail that is actually in a trash folder. An enabled @@ -6576,6 +6616,31 @@ void MainWindow::onThreadMessagesResolved(const QStringList &messageIds, return; } + if (requestTag == QStringLiteral("empty_spam")) { + // Grouped per account, because each group travels to that account's + // own trash: one destination composed once would put one account's + // junk in another's trash. The origin tag is the placeholder, resolved + // per message by onMessagesMoved() to the spam folder it is leaving; + // Task 3's overwrite rule strips the previous origin, so a message + // that travelled inbox -> spam -> trash keeps exactly one. + QHash byTrash; + for (int i = 0; i < messageIds.size(); ++i) { + const Account account = accountForMessagePath(paths.at(i)); + if (account.maildir.isEmpty() || account.trash.isEmpty()) + continue; + byTrash[account.maildir + QLatin1Char('/') + account.trash] + .append(messageIds.at(i)); + } + for (auto it = byTrash.cbegin(); it != byTrash.cend(); ++it) { + sendMove(it.value(), it.key(), + { QStringLiteral("deleted"), kOriginTagPlaceholder() }, + { QStringLiteral("spam"), QStringLiteral("unread"), + QStringLiteral("inbox") }, + tr("Empty spam")); + } + return; + } + if (requestTag == QStringLiteral("purge_selection")) { // No scope named: the prompt says "the selection", which is what the // user pointed at, rather than an account they did not. @@ -6899,6 +6964,37 @@ void MainWindow::emptyTrash() Q_ARG(QString, QStringLiteral("empty_trash"))); } +void MainWindow::emptySpam() +{ + // Scoped to the account selector, like emptyTrash(): All accounts empties + // every configured spam folder, a selected account only its own. Unlike + // emptyTrash() there is no confirmation, because this is a MOVE and every + // mutation that can be undone gets undo instead of a dialog. + const QString accountKey = m_accountBox->currentData().toString(); + const QString query = accountKey.isEmpty() + ? m_config.allSpamQuery() + : m_config.account(accountKey).spamQuery(); + + // An account with no spam folder configured produces an EMPTY query, and + // an empty notmuch query matches EVERYTHING. Refusing here rather than + // relying on the worker's guard is the whole safety of the action: the + // message names the cause. + if (query.isEmpty()) { + showTransientStatus(tr("No spam folder is configured")); + return; + } + + if (!m_worker) { + showTransientStatus(tr("Not connected to the mail index")); + return; + } + + QMetaObject::invokeMethod(m_worker, "resolveQueryMessages", + Qt::QueuedConnection, + Q_ARG(QString, query), + Q_ARG(QString, QStringLiteral("empty_spam"))); +} + void MainWindow::purgeSelected() { const QModelIndexList rows = @@ -7488,8 +7584,10 @@ void MainWindow::onMessagesMoved(const QMap &originByMessageId // // Gated on isShowingTrash() and not on the destination: a Delete is a move // too and reaches this same slot, and refreshing after every delete would - // make a row vanish from under the user in every other view. - if (isShowingTrash()) + // make a row vanish from under the user in every other view. The Spam view + // is the same case: Empty Spam is path-based, so moved rows stop matching + // and only a refresh can say so. + if (isShowingTrash() || isShowingSpam()) refreshCurrentQuery(); // The undo entries are pushed inside the loop above, one per origin diff --git a/src/mainwindow.h b/src/mainwindow.h index 1fc8822..ab65f1d 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -1321,6 +1321,17 @@ private: /// which holds whatever the current view happens to show. void emptyTrash(); + /// Moves every message in the spam folder to that account's trash. Unlike + /// emptyTrash() this is NOT a purge: it is a move, so it is undoable and + /// asks nothing. Scoped to the account selector like every other + /// account-aware surface, and grouped per account because the destination + /// differs: one account's spam moves to that account's own trash. + /// + /// Asynchronous for the same reason emptyTrash() is, but no confirm follows: + /// the answer arrives at onThreadMessagesResolved() tagged `empty_spam`, + /// which resolves the destinations from each message's own path. + void emptySpam(); + /// Empty trash's sibling, scoped to the SELECTION rather than to the /// account's whole trash. The act is identical, `purgeMessages()` in both /// cases, and so are its safeguards: it confirms, and it carries no @@ -1362,6 +1373,11 @@ private: /// in it, and such a message carries no tag of ours. bool isShowingTrash() const; + /// Whether the current query IS a spam view, for either scope. The exact + /// sibling of isShowingTrash(), for the same reason: Empty Spam drops rows + /// from a path-based view that no tag change can express. + bool isShowingSpam() const; + /// The `moved-from:` tag naming `dbRelativeFolder`, or empty when no /// account owns it. /// diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 4c0f32a..d581933 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -130,6 +130,11 @@ public: /// account without one sends and files nothing, which is a real /// configuration rather than an error. QString sent; + /// Where spam is filed. Written only when non-empty, like trash: an + /// account without one has no Spam filter and no Empty Spam, and a + /// test that needs the per-account grouping has to say where each + /// account's spam folder is. + QString spam; }; /// Writes several accounts, for the compose cases. @@ -216,6 +221,8 @@ public: out << "drafts=" << account.drafts << "\n"; if (!account.sent.isEmpty()) out << "sent=" << account.sent << "\n"; + if (!account.spam.isEmpty()) + out << "spam=" << account.spam << "\n"; } } file.close(); @@ -521,6 +528,9 @@ private slots: void spamMovesTheMessageToTheSpamFolder(); void undoOfMarkingSpamReturnsTheFileAndDropsBothTags(); void aMessageInTheSpamFolderIsNotInTheTrash(); + void emptySpamMovesEachAccountsMailToItsOwnTrash(); + void emptySpamRewritesTheOriginToTheSpamFolder(); + void emptySpamRefusesAnUnconfiguredFolder(); // ComposeWindow, item 123. These need a window but no worker: the composer // never touches NotmuchWorker, it reads its context from the value struct @@ -8689,6 +8699,13 @@ void TestMainWindow::noTwoActionsShareAnIcon() // rather than silently passing it. static const QStringList menuOnlySharedIconActions = { QStringLiteral("reply_no_quote"), + // Empty Spam shares `purge`'s `user-trash`, Task 6. It is a + // Message-menu-only entry that always carries its text and never + // reaches the main toolbar, so the icon is not the whole control, + // exactly as for reply_no_quote above. The assertion below still + // fails if it is ever put on the toolbar, so this is not a hiding + // place. + QStringLiteral("empty_spam"), }; const Config config; @@ -12132,6 +12149,152 @@ void TestMainWindow::aMessageInTheSpamFolderIsNotInTheTrash() "mail in the spam folder is judged to be in the trash"); } +void TestMainWindow::emptySpamMovesEachAccountsMailToItsOwnTrash() +{ + // Empty Spam is per-ACCOUNT: one gesture over the All accounts view moves + // each account's spam to that account's own trash. A single destination + // composed once would put one account's junk in the other's trash, where + // its files' paths and its Restore origin would both be wrong. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct1/Spam"), QStringLiteral("espam1@example.org"), + QStringLiteral("First spam"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct2/Junk"), QStringLiteral("espam2@example.org"), + QStringLiteral("Second spam"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 11:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY2(backed.buildWithAccounts( + { { QStringLiteral("acct1"), QStringLiteral("acct1"), + QStringLiteral("Trash"), {}, {}, {}, {}, + QStringLiteral("Spam") }, + { QStringLiteral("acct2"), QStringLiteral("acct2"), + QStringLiteral("Trash"), {}, {}, {}, {}, + QStringLiteral("Junk") } }), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *action = window.findChild(QStringLiteral("empty_spam")); + QVERIFY2(action, "empty_spam does not exist"); + action->trigger(); + + const QString root = backed.fixture().maildirPath(); + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct1/Trash/cur"), + QStringLiteral("espam1.example.org")), + 15000); + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct2/Trash/cur"), + QStringLiteral("espam2.example.org")), + 15000); + + QVERIFY2(!folderHasMessageFile(root + QStringLiteral("/acct2/Trash/cur"), + QStringLiteral("espam1.example.org")), + "the first account's spam landed in the second account's trash"); + QVERIFY2(!folderHasMessageFile(root + QStringLiteral("/acct1/Trash/cur"), + QStringLiteral("espam2.example.org")), + "the second account's spam landed in the first account's trash"); +} + +void TestMainWindow::emptySpamRewritesTheOriginToTheSpamFolder() +{ + // A message that travelled inbox -> spam -> trash carries exactly one + // origin, and it names the folder it left LAST: the spam folder. A stale + // `moved-from:inbox` left behind would make Restore send it back to the + // inbox instead of where Empty Spam took it from. The overwrite rule is + // Task 3's; this pins that Empty Spam asks for it. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("origin1@example.org"), + QStringLiteral("Travels"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash"), QStringLiteral("Spam")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild(); + auto *view = window.findChild(); + auto *queryEdit = + window.findChild(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + + const QString root = backed.fixture().maildirPath(); + const QString cfg = backed.fixture().configPath(); + const QString stem = QStringLiteral("origin1.example.org"); + + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild(QStringLiteral("spam"))->trigger(); + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/Spam/cur"), stem), + 15000); + // Wait for the FIRST move's origin tag before emptying, or the assertion + // below could observe a moment when neither tag has landed. + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, QStringLiteral("id:origin1@example.org and " + "tag:\"moved-from:inbox\"")) == 1, + 15000); + + window.findChild(QStringLiteral("empty_spam"))->trigger(); + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/Trash/cur"), stem), + 15000); + + // Exactly one origin remains, and it names the spam folder. + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, QStringLiteral("tag:\"moved-from:Spam\"")) == 1, + 15000); + QCOMPARE(notmuchCount(cfg, QStringLiteral("tag:\"moved-from:inbox\"")), 0); + QCOMPARE(notmuchCount(cfg, QStringLiteral("tag:\"moved-from:Spam\"")), 1); + // The guard the count above needs: a message that vanished would satisfy + // the two assertions too. + QCOMPARE(notmuchCount(cfg, QStringLiteral("id:origin1@example.org")), 1); +} + +void TestMainWindow::emptySpamRefusesAnUnconfiguredFolder() +{ + // An account with no spam folder produces an EMPTY query, and an empty + // notmuch query matches EVERYTHING. Without the guard Empty Spam would + // move the whole Maildir into the trash, so the refusal is the whole + // safety of the action. Asserting the message is still in the inbox is + // the observable consequence of refusing; the status names the cause. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("guard1@example.org"), + QStringLiteral("Untouched"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *status = window.findChild(QStringLiteral("statusMessage")); + QVERIFY(status); + auto *action = window.findChild(QStringLiteral("empty_spam")); + QVERIFY2(action, "empty_spam does not exist"); + + action->trigger(); + + QCOMPARE(status->text(), QStringLiteral("No spam folder is configured")); + + const QString root = backed.fixture().maildirPath(); + const QString stem = QStringLiteral("guard1.example.org"); + QVERIFY2(folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"), + stem), + "the guard ran an empty query and moved the inbox"); + QVERIFY2(!folderHasMessageFile(root + QStringLiteral("/acct/Trash/cur"), + stem), + "an unconfigured spam folder moved mail to the trash anyway"); +} + void TestMainWindow::deleteRecordsWhereTheMessageCameFrom() { // A Maildir filename does not record where a message came from, and once -- cgit v1.2.3