diff options
Diffstat (limited to 'tests')
| -rw-r--r-- | tests/test_config.cpp | 119 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 1270 | ||||
| -rw-r--r-- | tests/test_notmuchworker.cpp | 180 | ||||
| -rw-r--r-- | tests/test_tagdialog.cpp | 4 |
4 files changed, 1468 insertions, 105 deletions
diff --git a/tests/test_config.cpp b/tests/test_config.cpp index e69a073..70e3705 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -120,11 +120,17 @@ private slots: void anAccountCarriesItsTrashFolder(); void aBracketedTrashFolderIsQuoted(); void anAccountWithoutATrashFolderWarns(); + void anAccountCarriesItsSpamFolder(); + void aBracketedSpamFolderIsQuoted(); + void anAccountWithoutASpamFolderWarns(); + void allSpamQueryJoinsAndSkips(); void theDraftsFilterComposesPerAccount(); void theDraftsFilterMatchesNothingWithoutAFolder(); void theDraftsFilterIsFlatLikeSent(); void theTrashFilterComposesPerAccount(); void theTrashFilterMatchesNothingWithoutAFolder(); + void theSpamFilterComposesPerAccount(); + void theSpamFilterMatchesNothingWithoutAFolder(); void anAccountWithoutASendCommandIsReceiveOnly(); void composeSettingsDefaultWhenTheSectionIsAbsent(); void aZeroSendDelayIsHonouredRatherThanTreatedAsUnset(); @@ -451,7 +457,8 @@ void TestConfig::absentSyncCommandIsNoticeNotProblem() const QString path = writeIni(dir, QStringLiteral( "[account.work]\n" "maildir=work-mail\n" - "trash=Trash\n")); + "trash=Trash\n" + "spam=Spam\n")); Config config; config.load(path); @@ -472,7 +479,8 @@ void TestConfig::brokenSyncCommandIsAProblem() "\n" "[account.work]\n" "maildir=work-mail\n" - "trash=Trash\n")); + "trash=Trash\n" + "spam=Spam\n")); Config config; config.load(path); @@ -508,7 +516,8 @@ void TestConfig::validConfigHasNoProblems() "[account.work]\n" "maildir=work-mail\n" "address=user@example.org\n" - "trash=Trash\n")); + "trash=Trash\n" + "spam=Spam\n")); Config config; config.load(path); @@ -929,7 +938,8 @@ void TestConfig::sentQueryIsEmptyWithoutTheKey() config.load(writeIni(dir, QStringLiteral( "[account.provider-c]\n" "maildir = provider-c\n" - "trash = Trash\n"))); + "trash = Trash\n" + "spam = Spam\n"))); QCOMPARE(config.accounts().size(), 1); QVERIFY(config.accounts().at(0).sentQuery().isEmpty()); @@ -1028,6 +1038,52 @@ void TestConfig::anAccountWithoutATrashFolderWarns() QVERIFY(joined.contains(QStringLiteral("trash"))); } +void TestConfig::anAccountCarriesItsSpamFolder() +{ + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.work]\nmaildir=work\nspam=Spam\n"))); + const Account account = config.account(QStringLiteral("work")); + QCOMPARE(account.spam, QStringLiteral("Spam")); + QCOMPARE(account.spamQuery(), QStringLiteral("path:\"work/Spam/**\"")); +} + +void TestConfig::aBracketedSpamFolderIsQuoted() +{ + Account account; + account.maildir = QStringLiteral("provider-a"); + account.spam = QStringLiteral("[Provider]/Spam"); + QCOMPARE(account.spamQuery(), + QStringLiteral("path:\"provider-a/[Provider]/Spam/**\"")); +} + +void TestConfig::anAccountWithoutASpamFolderWarns() +{ + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.work]\nmaildir=work\n"))); + QVERIFY(config.account(QStringLiteral("work")).isValid()); + const QString joined = config.warnings().join(QLatin1Char('\n')); + QVERIFY(joined.contains(QStringLiteral("work"))); + QVERIFY(joined.contains(QStringLiteral("spam"))); +} + +void TestConfig::allSpamQueryJoinsAndSkips() +{ + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.work]\nmaildir=work\nspam=Spam\n" + "\n[account.personal]\nmaildir=personal\nspam=[Provider]/Spam\n" + "\n[account.none]\nmaildir=none\n"))); + const QString all = config.allSpamQuery(); + QVERIFY(all.contains(QStringLiteral("path:\"work/Spam/**\""))); + QVERIFY(all.contains(QStringLiteral("path:\"personal/[Provider]/Spam/**\""))); + QVERIFY(!all.contains(QStringLiteral("none"))); +} + void TestConfig::theDraftsFilterComposesPerAccount() { // Item 138. Follows `sent` and `trash`, which match a FOLDER: `draft` is a @@ -1144,6 +1200,34 @@ void TestConfig::theTrashFilterMatchesNothingWithoutAFolder() QCOMPARE(config.resolvedQuery(trash, QString()), Config::matchNothingQuery()); } +void TestConfig::theSpamFilterComposesPerAccount() +{ + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.work]\nmaildir=work\nspam=Spam\n" + "\n[account.personal]\nmaildir=personal\nspam=[Provider]/Spam\n"))); + const SavedQuery spam = Config::builtinFilter(QStringLiteral("spam")); + QVERIFY(spam.isGenerated()); + QVERIFY2(!spam.flat, "spam must be threaded, like trash"); + const QString all = config.resolvedQuery(spam, QString()); + QVERIFY(all.contains(QStringLiteral("path:\"work/Spam/**\""))); + QVERIFY(all.contains(QStringLiteral("path:\"personal/[Provider]/Spam/**\""))); + const QString scoped = config.resolvedQuery(spam, QStringLiteral("work")); + QCOMPARE(scoped, QStringLiteral("path:\"work/Spam/**\"")); + QVERIFY(!scoped.contains(QStringLiteral("personal"))); +} + +void TestConfig::theSpamFilterMatchesNothingWithoutAFolder() +{ + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[account.work]\nmaildir=work\n"))); + const SavedQuery spam = Config::builtinFilter(QStringLiteral("spam")); + QCOMPARE(config.resolvedQuery(spam, QString()), Config::matchNothingQuery()); +} + void TestConfig::sentQueryComposesWithScopedQuery() { // A Sent view under one account must not show another account's sent mail. @@ -1265,10 +1349,12 @@ void TestConfig::theStartupAccountIsReadAndValidated() "[account.work]\n" "maildir=work\n" "trash=Trash\n" + "spam=Spam\n" "\n" "[account.personal]\n" "maildir=personal\n" - "trash=Trash\n"))); + "trash=Trash\n" + "spam=Spam\n"))); QCOMPARE(config.startupAccount(), QStringLiteral("work")); QVERIFY(config.problems().isEmpty()); @@ -1294,7 +1380,8 @@ void TestConfig::theStartupAccountIsReadAndValidated() "\n" "[account.work]\n" "maildir=work\n" - "trash=Trash\n"))); + "trash=Trash\n" + "spam=Spam\n"))); QVERIFY2(wrong.startupAccount().isEmpty(), "an unknown startup account was passed through rather than " "falling back to All accounts"); @@ -1320,7 +1407,8 @@ void TestConfig::theStartupAccountTakesTheKeyNotTheSyncChannel() "[account.provider-work.mailbox]\n" "maildir=provider-work.mailbox\n" "channel=provider-workmailbox\n" - "trash=Trash\n"))); + "trash=Trash\n" + "spam=Spam\n"))); QCOMPARE(config.accounts().size(), 1); QCOMPARE(config.accounts().constFirst().key, @@ -1343,7 +1431,8 @@ void TestConfig::theStartupAccountTakesTheKeyNotTheSyncChannel() "[account.provider-work.mailbox]\n" "maildir=provider-work.mailbox\n" "channel=provider-workmailbox\n" - "trash=Trash\n"))); + "trash=Trash\n" + "spam=Spam\n"))); QVERIFY2(byChannel.startupAccount().isEmpty(), "the sync channel was accepted as an account key"); @@ -1428,7 +1517,8 @@ void TestConfig::theStartupQuerySurvivesATranslatedFilterName() "[account.work]\n" "maildir=work\n" "sent=Sent\n" - "trash=Trash\n"))); + "trash=Trash\n" + "spam=Spam\n"))); QVERIFY2(!config.savedQueries().isEmpty(), "queries.json did not load, so the warning path is unreachable"); @@ -1454,7 +1544,8 @@ void TestConfig::theStartupQuerySurvivesATranslatedFilterName() "[account.work]\n" "maildir=work\n" "sent=Sent\n" - "trash=Trash\n"))); + "trash=Trash\n" + "spam=Spam\n"))); QVERIFY(!byLabel.savedQueries().isEmpty()); QCOMPARE(byLabel.startupSavedQuery().generated, QStringLiteral("inbox")); QVERIFY(byLabel.problems().isEmpty()); @@ -1567,7 +1658,7 @@ void TestConfig::everyBuiltinFilterIsAKnownGenerator() Config config; const QList<SavedQuery> filters = config.builtinFilters(); - QCOMPARE(filters.size(), 6); + QCOMPARE(filters.size(), 7); QStringList names; for (const SavedQuery &filter : filters) { @@ -1590,7 +1681,8 @@ void TestConfig::everyBuiltinFilterIsAKnownGenerator() QStringLiteral("Important"), QStringLiteral("Sent"), QStringLiteral("Drafts"), - QStringLiteral("Trash") })); + QStringLiteral("Trash"), + QStringLiteral("Spam") })); } void TestConfig::aFilterAcrossAllAccountsIsTheUnscopedQuery() @@ -1684,7 +1776,8 @@ void TestConfig::draftsQueryIsEmptyWithoutTheKey() config.load(writeIni(dir, QStringLiteral( "[account.provider-c]\n" "maildir = provider-c\n" - "trash = Trash\n"))); + "trash = Trash\n" + "spam = Spam\n"))); QCOMPARE(config.accounts().size(), 1); QVERIFY(config.accounts().at(0).draftsQuery().isEmpty()); diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index b7f7846..e87843d 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. @@ -148,7 +153,8 @@ public: bool build(const QString &accountKey = QString(), const QString &accountMaildir = QString(), - const QString &accountTrash = QString()) + const QString &accountTrash = QString(), + const QString &accountSpam = QString()) { if (!m_fixture.isValid()) { m_error = QStringLiteral("fixture directory invalid"); @@ -186,6 +192,11 @@ public: << "maildir=" << accountMaildir << "\n"; if (!accountTrash.isEmpty()) out << "trash=" << accountTrash << "\n"; + // Mark spam moves a file into this folder, exactly as Delete + // does into the trash, so a spam test needs it declared for + // the same reason a delete test needs `trash`. + if (!accountSpam.isEmpty()) + out << "spam=" << accountSpam << "\n"; // The fixture's folders are lowercase, unlike the Maildir // convention Account::inboxFolder() defaults to. Stated rather // than assumed, which is the whole point of the key: naming a @@ -210,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(); @@ -394,7 +407,7 @@ private slots: void importantOnAReplyReadsItsOwnStateNotItsThreads(); void editTagsOnAReplyCountsItsOwnThreadNotTheFirstInTheList(); void markCurrentThreadReadResolvesTheThreadThroughTheIndex(); - void deletingAReplyRepaintsThatReplyRow(); + void taggingAReplyRepaintsThatReplyRow(); void deleteIsHiddenOnMailAlreadyInTheTrash(); void aPartlyTrashedConversationIsNotJudgedOnOneMessage(); void restoreIsHiddenOnMailThatWasNeverDeleted(); @@ -459,6 +472,7 @@ private slots: void placeholderCountsDropAnUncountableQuery(); void flatModeDoesNotSurviveTheNextQuery(); void noTwoActionsShareAnIcon(); + void theSpamActionCarriesTheBugIconWithAFallback(); void theMessagePaneCarriesItsOwnActionBar(); void theMainToolbarKeepsOnlyListWideActions(); void theMessageBarSitsAboveTheBodyAndBelowTheHeader(); @@ -509,6 +523,33 @@ private slots: void theRefreshAfterARestoreLeavesUndoIntact(); void deletingOutsideTheTrashViewLeavesTheRowInPlace(); + // Mark spam is Delete's sibling: it moves the file into the account's spam + // folder rather than only tagging it. + void spamMovesTheMessageToTheSpamFolder(); + void undoOfMarkingSpamReturnsTheFileAndDropsBothTags(); + void aMessageInTheSpamFolderIsNotInTheTrash(); + void emptySpamMovesEachAccountsMailToItsOwnTrash(); + void emptySpamRewritesTheOriginToTheSpamFolder(); + void emptySpamRefusesAnUnconfiguredFolder(); + void theSpamCleanupQueryExcludesTheSpamFolder(); + void theSpamCleanupQueryWithoutASpamFolderIsJustTheTag(); + + // Item 187's final review. Mark spam is Delete's sibling, so it is hidden + // exactly where Delete is, and the model keeps one origin per message. + void spamIsAbsentOnAReplyRow(); + void spamIsHiddenOnMailAlreadyInTheTrash(); + void restoringAfterTwoMovesReturnsToTheLatestOrigin(); + + // Item 201: a message in the spam folder could not be un-spammed. Not spam + // is Mark spam's inverse, and Restore is its template. + void notSpamReturnsAMessageToItsOriginFolder(); + void notSpamFallsBackToInboxWithoutAnOriginTag(); + void notSpamIsOfferedInTheSpamView(); + void notSpamIsAbsentOnAReplyRow(); + void notSpamIsHiddenOutsideTheSpamFolder(); + void notSpamThreadMovesEveryMessageHome(); + void undoOfNotSpamReturnsTheFileToTheSpamFolder(); + // ComposeWindow, item 123. These need a window but no worker: the composer // never touches NotmuchWorker, it reads its context from the value struct // MainWindow hands it, so a Config written to a temporary INI is the whole @@ -5227,13 +5268,15 @@ void TestMainWindow::markCurrentThreadReadResolvesTheThreadThroughTheIndex() QStringLiteral("t2")); } -void TestMainWindow::deletingAReplyRepaintsThatReplyRow() +void TestMainWindow::taggingAReplyRepaintsThatReplyRow() { - // `spam`, not `delete`. Since item 103 Delete MOVES the file, so it needs - // an account with a configured trash folder and a worker to do the move; - // this bare window has neither, and Delete correctly refuses. What is - // under test here is unchanged by that: `spam` is the other message-scoped - // tag-only action, and it paints the same doomed state. + // `flag`, not `delete` and not `spam`. Since item 103 Delete MOVES the + // file, and `spam` now does too, neither needs only a tag write; this bare + // window has no worker and no configured folders, and both correctly + // refuse. `flag` is the remaining message-scoped tag-only action, and what + // is under test is unchanged by that: the action reaches + // applyMessageTagChange on the reply's OWN row. + // // The user's report, at the gesture level: "I'm hitting delete on a reply // to a thread, I see the edits counter increasing but I have no feedback // if that message is being deleted." The model-level test proves @@ -5246,7 +5289,7 @@ void TestMainWindow::deletingAReplyRepaintsThatReplyRow() QVERIFY(model); auto *view = window.findChild<QTreeView *>(); QVERIFY(view); - auto *action = window.findChild<QAction *>(QStringLiteral("spam")); + auto *action = window.findChild<QAction *>(QStringLiteral("flag")); QVERIFY(action); const QModelIndex reply = @@ -5255,24 +5298,21 @@ void TestMainWindow::deletingAReplyRepaintsThatReplyRow() // Nothing to see before the gesture, so the assertion after it means // something. - QVERIFY(!model->messageAt(reply).isSpam()); - const QVariant before = model->data(reply, Qt::BackgroundRole); + QVERIFY(!model->messageAt(reply).isFlagged()); QSignalSpy spy(model, &QAbstractItemModel::dataChanged); action->trigger(); - QVERIFY2(model->messageAt(reply).isSpam(), - "Delete on a reply left the reply's own row unchanged, so the " + QVERIFY2(model->messageAt(reply).isFlagged(), + "the action on a reply left the reply's own row unchanged, so the " "pending count moved and the user saw nothing"); QVERIFY2(spy.count() >= 1, "no repaint was requested for the reply's row"); - QVERIFY2(model->data(reply, Qt::BackgroundRole) != before, - "the deleted reply paints exactly as it did before"); // The THREAD row must not follow: it stands for the whole conversation, - // and one deleted reply does not doom it. + // and one changed reply does not change the conversation. const QModelIndex threadRow = reply.parent(); - QVERIFY2(!model->threadFor(threadRow).isSpam(), - "deleting one reply marked its whole thread deleted"); + QVERIFY2(!model->threadFor(threadRow).isFlagged(), + "changing one reply marked its whole thread flagged"); } /// A window whose one account owns `acct/`, with its trash at `acct/trash`. @@ -5289,6 +5329,7 @@ static Config configWithTrash(QTemporaryDir &dir) out << "[account.acct]\n" << "maildir = acct\n" << "trash = trash\n" + << "spam = spam\n" << "inbox = inbox\n"; } Config config; @@ -5890,11 +5931,11 @@ void TestMainWindow::toggleUnreadOnAReplyRepaintsItInBothDirections() void TestMainWindow::taggingTheOpenReplyUpdatesTheMessagePaneStrip() { - // `spam`, not `delete`. Since item 103 Delete MOVES the file, so it needs - // an account with a configured trash folder and a worker to do the move; - // this bare window has neither, and Delete correctly refuses. What is - // under test here is unchanged by that: `spam` is the other message-scoped - // tag-only action, and it paints the same doomed state. + // `flag`, not `delete` and not `spam`. Since item 103 Delete MOVES the + // file, and `spam` now does too, neither is a tag-only action; this bare + // window has no worker and no configured folders, and both correctly + // refuse. `flag` is the remaining message-scoped tag-only action, and it + // exercises the same sendMessageTagChange() strip refresh. // The user's report: "the right pane chips are not [repainted], for it to // sync I have to change message and go back to the edited one". // @@ -5921,7 +5962,7 @@ void TestMainWindow::taggingTheOpenReplyUpdatesTheMessagePaneStrip() const auto stripTags = [strip]() { return strip->visibleTags() + strip->hiddenTags(); }; - auto *action = window.findChild<QAction *>(QStringLiteral("spam")); + auto *action = window.findChild<QAction *>(QStringLiteral("flag")); QVERIFY(action); // A tag the strip will actually draw. Account tags are filtered out by the @@ -5936,11 +5977,11 @@ void TestMainWindow::taggingTheOpenReplyUpdatesTheMessagePaneStrip() QVERIFY2(stripTags().contains(QStringLiteral("todo")), "the strip does not show the selected reply's tags, so this test " "cannot tell a missing refresh from a strip that never had them"); - QVERIFY(!stripTags().contains(QStringLiteral("spam"))); + QVERIFY(!stripTags().contains(QStringLiteral("flagged"))); action->trigger(); - QVERIFY2(stripTags().contains(QStringLiteral("spam")), + QVERIFY2(stripTags().contains(QStringLiteral("flagged")), "the message pane's chips still describe the reply as it was " "before the edit; the user has to select away and back to see it"); } @@ -6021,11 +6062,11 @@ void TestMainWindow::taggingAnUnrelatedReplyLeavesTheStripAlone() void TestMainWindow::aHeldMessageEditIsSentWhenTheSyncEnds() { - // `spam`, not `delete`. Since item 103 Delete MOVES the file, so it needs - // an account with a configured trash folder and a worker to do the move; - // this bare window has neither, and Delete correctly refuses. What is - // under test here is unchanged by that: `spam` is the other message-scoped - // tag-only action, and it paints the same doomed state. + // `flag`, not `delete` and not `spam`. Since item 103 Delete MOVES the + // file, and `spam` now does too, neither is a tag-only action; this bare + // window has no worker and no configured folders, and both correctly + // refuse. `flag` is the remaining message-scoped tag-only action, and the + // held-edit path it exercises is the same for every tag write. // Found by reading while fixing the strip refresh, not reported. // // flushHeldEdits() looped over edit.threadIds and called @@ -6041,7 +6082,7 @@ void TestMainWindow::aHeldMessageEditIsSentWhenTheSyncEnds() QVERIFY(model); auto *view = window.findChild<QTreeView *>(); QVERIFY(view); - auto *action = window.findChild<QAction *>(QStringLiteral("spam")); + auto *action = window.findChild<QAction *>(QStringLiteral("flag")); QVERIFY(action); const QModelIndex reply = @@ -6065,7 +6106,7 @@ void TestMainWindow::aHeldMessageEditIsSentWhenTheSyncEnds() "written"); // Sent for the MESSAGE, not escalated to its thread. Losing the scope on - // the way out of the hold would delete every message in the thread. + // the way out of the hold would tag every message in the thread. QVERIFY2(window.pendingMessageIdsForTesting().contains( QStringLiteral("m1@example.org")), "the held edit was not sent with its message scope"); @@ -6076,7 +6117,7 @@ void TestMainWindow::aHeldMessageEditIsSentWhenTheSyncEnds() // And the row still shows it: the flush takes the optimistic update back // before re-sending, so a bug there leaves the row wrong in the other // direction. - QVERIFY2(model->messageAt(reply).isSpam(), + QVERIFY2(model->messageAt(reply).isFlagged(), "sending the held edit lost the tag from the reply's row"); } @@ -6089,10 +6130,10 @@ void TestMainWindow::anActionOnAConversationRowTakesTheConversation() // with replies is now the conversation, and a row without them is still // its message. // - // `spam`, not `delete`. Since item 103 Delete MOVES the file, so it needs - // an account with a configured trash folder and a worker to do the move; - // this bare window has neither. `spam` is the other tag-only action and - // resolves its scope through the same tagSelected(). + // `flag`, not `delete` and not `spam`. Since item 103 Delete MOVES the + // file, and `spam` now does too, neither is tag-only any more; this bare + // window has no worker and no configured folders. `flag` is the remaining + // tag-only action and resolves its scope through the same tagSelected(). const Config config; MainWindow window(config); @@ -6109,12 +6150,12 @@ void TestMainWindow::anActionOnAConversationRowTakesTheConversation() one.totalCount = 1; model->appendBatch({ many, one }); - auto *spam = window.findChild<QAction *>(QStringLiteral("spam")); - QVERIFY(spam); + auto *flag = window.findChild<QAction *>(QStringLiteral("flag")); + QVERIFY(flag); selectThreadRow(view, 0); QApplication::processEvents(); - spam->trigger(); + flag->trigger(); QCOMPARE(window.pendingThreadIdsForTesting(), QStringList{ QStringLiteral("t1") }); @@ -6126,7 +6167,7 @@ void TestMainWindow::anActionOnAConversationRowTakesTheConversation() // than a blanket escalation: a thread of one is still its message. selectThreadRow(view, 1); QApplication::processEvents(); - spam->trigger(); + flag->trigger(); QCOMPARE(window.pendingMessageIdsForTesting(), QStringList{ QStringLiteral("t2-first@example.org") }); @@ -6407,11 +6448,11 @@ void TestMainWindow::autoMarkReadArmsForAReplyToo() void TestMainWindow::taggingTheOpenRootMessageKeepsTheStripPopulated() { - // `spam`, not `delete`. Since item 103 Delete MOVES the file, so it needs - // an account with a configured trash folder and a worker to do the move; - // this bare window has neither, and Delete correctly refuses. What is - // under test here is unchanged by that: `spam` is the other message-scoped - // tag-only action, and it paints the same doomed state. + // `flag`, not `delete` and not `spam`. Since item 103 Delete MOVES the + // file, and `spam` now does too, neither is a tag-only action; this bare + // window has no worker and no configured folders, and both correctly + // refuse. `flag` is the remaining message-scoped tag-only action and + // exercises the same strip refresh. // The user, 2026-08-16: "right pane loses the chip row when repainting, it // simply disappears". // @@ -6457,7 +6498,7 @@ void TestMainWindow::taggingTheOpenRootMessageKeepsTheStripPopulated() QVERIFY2(stripTags().contains(QStringLiteral("todo")), "the strip never showed the selected thread's tags"); - auto *action = window.findChild<QAction *>(QStringLiteral("spam")); + auto *action = window.findChild<QAction *>(QStringLiteral("flag")); QVERIFY(action); action->trigger(); @@ -6467,7 +6508,7 @@ void TestMainWindow::taggingTheOpenRootMessageKeepsTheStripPopulated() "and set the strip to the resulting empty tag list"); QVERIFY2(stripTags().contains(QStringLiteral("todo")), "the strip lost the tag the message still carries"); - QVERIFY2(stripTags().contains(QStringLiteral("spam")), + QVERIFY2(stripTags().contains(QStringLiteral("flagged")), "the strip did not pick up the tag just written"); } @@ -8471,7 +8512,8 @@ void TestMainWindow::theMessagePaneCarriesItsOwnActionBar() // this after seeing the first version, and the split is now by what the // action needs rather than by what it is about. const QStringList expected = { QStringLiteral("reply"), - QStringLiteral("forward") }; + QStringLiteral("forward"), + QStringLiteral("spam") }; for (const QString &name : expected) { auto *action = window.findChild<QAction *>(name); QVERIFY2(action, qPrintable(QStringLiteral("no action %1").arg(name))); @@ -8676,6 +8718,18 @@ 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"), + // Find stranded spam shares `cleanup_stranded`'s `system-search`, + // Task 7. Same property again: a Message-menu-only entry that always + // carries its text and never reaches the main toolbar, allowed for the + // same reason and caught here if it is ever put on the toolbar. + QStringLiteral("cleanup_stranded_spam"), }; const Config config; @@ -8744,6 +8798,25 @@ void TestMainWindow::noTwoActionsShareAnIcon() .arg(collisions.join(QStringLiteral("; "))))); } +void TestMainWindow::theSpamActionCarriesTheBugIconWithAFallback() +{ + // Task 5. `bug` is the glyph the user asked for, and it is not a + // freedesktop name every theme carries, so the table pairs it with + // `mail-mark-junk` as a fallback: a theme without the bug still draws a + // junk icon rather than degrading the action to text alone. + const Config config; + MainWindow window(config); + + auto *spam = window.findChild<QAction *>(QStringLiteral("spam")); + QVERIFY2(spam, "no action named spam"); + QVERIFY2(!spam->icon().isNull(), "the spam action carries no icon"); + + const QPair<QString, QString> names = + MainWindow::iconNamesForTesting(QStringLiteral("spam")); + QCOMPARE(names.first, QStringLiteral("bug")); + QCOMPARE(names.second, QStringLiteral("mail-mark-junk")); +} + // Constructing a MainWindow needs a QApplication and a platform plugin. The // test has no display under ctest, so it runs offscreen unless the caller // asked for something else. @@ -11008,12 +11081,13 @@ void TestMainWindow::everyBuiltinFilterButtonCarriesAnIconAndItsText() }); // A trash key too, or the Trash filter finds nothing and is skipped from // the row entirely (item 103), leaving no trashButton for this loop to - // find. Drafts behaves the same way since item 138. + // find. Drafts and Spam behave the same way since item 138. { QSettings s(path, QSettings::IniFormat); s.beginGroup(QStringLiteral("account.work")); s.setValue(QStringLiteral("trash"), QStringLiteral("Trash")); s.setValue(QStringLiteral("drafts"), QStringLiteral("Drafts")); + s.setValue(QStringLiteral("spam"), QStringLiteral("Spam")); s.endGroup(); } Config config; @@ -11947,6 +12021,306 @@ void TestMainWindow::deleteMovesTheMessageToTrash() QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); } +void TestMainWindow::spamMovesTheMessageToTheSpamFolder() +{ + // Mark spam is Delete's sibling: it MOVES the file into the account's spam + // folder, tagging it `spam` and recording `moved-from:inbox`. Before this + // item it only added a tag, so spam mail sat in the inbox for good. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("spam1@example.org"), + QStringLiteral("Spam me"), 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<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(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 inbox = root + QStringLiteral("/acct/inbox/new"); + const QString stem = QStringLiteral("spam1.example.org"); + QVERIFY(folderHasMessageFile(inbox, stem)); + + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("spam"))->trigger(); + + // The filesystem half. cur/, never new/: a file in new/ is re-announced as + // fresh mail by every reader of the Maildir. + const QString spam = root + QStringLiteral("/acct/Spam/cur"); + QTRY_VERIFY_WITH_TIMEOUT(folderHasMessageFile(spam, stem), 15000); + QVERIFY2(!folderHasMessageFile(inbox, stem), + "the file is in the spam folder and still in the inbox"); + QVERIFY2(!folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), + stem), + "the file is in the spam folder and still in the inbox"); + + // The tags land only once the worker confirms the move, so they are waited + // for separately. `unread` goes with it, exactly as Delete strips it: a + // decision about the message must not leave the unread count including it. + const QString cfg = backed.fixture().configPath(); + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, QStringLiteral("id:spam1@example.org and tag:spam " + "and tag:\"moved-from:inbox\" and " + "not tag:unread")) == 1, + 15000); +} + +void TestMainWindow::undoOfMarkingSpamReturnsTheFileAndDropsBothTags() +{ + // Undo is this project's stand-in for a confirmation dialog, so a move + // into the spam folder that cannot be undone is one with no safety net. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("spamundo@example.org"), + QStringLiteral("Undo my spam"), 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<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(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 stem = QStringLiteral("spamundo.example.org"); + const QString spam = root + QStringLiteral("/acct/Spam/cur"); + + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("spam"))->trigger(); + QTRY_VERIFY_WITH_TIMEOUT(folderHasMessageFile(spam, stem), 15000); + + // The command reaches the stack only once the worker CONFIRMS the move, and + // the file appearing is that move's rename, which lands a moment earlier. + // Undo before the push is a no-op, and the assertion below would then blame + // the move-back for a race in the test. + QTRY_VERIFY_WITH_TIMEOUT(window.undoDepthForTesting() >= 1, 15000); + + window.findChild<QAction *>(QStringLiteral("undo"))->trigger(); + + // Back in its ORIGIN folder, not guessed. A move-back to a hardcoded inbox + // would pass a laxer assertion than this one. + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), stem) + || folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"), + stem), + 15000); + QVERIFY2(!folderHasMessageFile(spam, stem), + "undo returned the file and left a copy in the spam folder"); + + // Both tags gone, asked of the database: a `moved-from:` left behind makes + // Restore offer to move a message that is already home. Waited separately, + // since the undo's tag writes land after its rename. + const QString cfg = backed.fixture().configPath(); + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, QStringLiteral("id:spamundo@example.org and " + "(tag:spam or " + "tag:\"moved-from:inbox\")")) == 0, + 15000); + // The guard the assertion above needs: a message that vanished would + // satisfy it too. + QCOMPARE(notmuchCount(cfg, QStringLiteral("id:spamundo@example.org")), 1); +} + +void TestMainWindow::aMessageInTheSpamFolderIsNotInTheTrash() +{ + // The trash predicate must answer for the TRASH folder only. `spam` is a + // different folder, so mail in it is offered Delete like any other mail + // and never Restore. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const Config config = configWithTrash(dir); + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + + model->appendBatch({ + threadAtPath(QStringLiteral("t1"), + QStringLiteral("acct/trash/cur/1:2,S")), + threadAtPath(QStringLiteral("t2"), + QStringLiteral("acct/spam/cur/2:2,S")), + }); + + view->setCurrentIndex(model->index(0, 0, {})); + QVERIFY2(window.everySelectedRowIsInATrashFolderForTesting(), + "the predicate does not recognise mail in the trash, so it " + "cannot tell the spam case apart"); + + view->setCurrentIndex(model->index(1, 0, {})); + QVERIFY2(!window.everySelectedRowIsInATrashFolderForTesting(), + "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<QAction *>(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<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(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<QAction *>(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); + + auto *emptySpam = window.findChild<QAction *>(QStringLiteral("empty_spam")); + QVERIFY2(emptySpam, "empty_spam does not exist"); + emptySpam->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<QLabel *>(QStringLiteral("statusMessage")); + QVERIFY(status); + auto *action = window.findChild<QAction *>(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 @@ -11980,7 +12354,7 @@ void TestMainWindow::deleteRecordsWhereTheMessageCameFrom() // would report the tag whether or not the write ever landed. // Re-queried by id and asserted on the TAG LIST the database returns. // - // Not with `tag:"deleted-from:inbox"` in the query: notmuch's parser does + // Not with `tag:"moved-from:inbox"` in the query: notmuch's parser does // not match a quoted tag containing a colon that way, so such a query // returns nothing against a perfectly tagged message and reads as the // feature being broken. Asking for the message and inspecting its tags @@ -11997,7 +12371,7 @@ void TestMainWindow::deleteRecordsWhereTheMessageCameFrom() QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); const QStringList tags = model->threadAt(0).tags; tagged = tags.contains(QStringLiteral("deleted")) - && tags.contains(QStringLiteral("deleted-from:inbox")); + && tags.contains(QStringLiteral("moved-from:inbox")); if (!tagged) QTest::qWait(200); } @@ -12016,8 +12390,8 @@ void TestMainWindow::deletingTwiceLeavesNoOriginTagBehind() // onMessagesMoved() resolved the origin placeholder from the folder the // WORKER reported, which is where the message came FROM. On a delete that // is the inbox and correct. On a restore it is the TRASH, so the restore - // asked to remove `deleted-from:Trash`, a tag that had never been written, - // while the real `deleted-from:inbox` was never named and stayed on the + // asked to remove `moved-from:Trash`, a tag that had never been written, + // while the real `moved-from:inbox` was never named and stayed on the // message. It came home still claiming to have been deleted from // somewhere, which makes Restore offer to move a message already at home. // @@ -12065,7 +12439,7 @@ void TestMainWindow::deletingTwiceLeavesNoOriginTagBehind() QTRY_VERIFY_WITH_TIMEOUT( notmuchCount(backed.fixture().configPath(), QStringLiteral("id:twice@example.org and " - "tag:\"deleted-from:inbox\"")) == 1, + "tag:\"moved-from:inbox\"")) == 1, 15000); // Second press on the same message, which restores it. @@ -12095,11 +12469,11 @@ void TestMainWindow::deletingTwiceLeavesNoOriginTagBehind() == 0, 15000); - // BOTH tags gone, asked of the database. `deleted-from:` left behind is + // BOTH tags gone, asked of the database. `moved-from:` left behind is // the defect this covers, and it survived a green suite before. // The origin tag specifically, asserted on its OWN query. // - // A combined `tag:deleted or tag:"deleted-from:inbox"` query is NOT + // A combined `tag:deleted or tag:"moved-from:inbox"` query is NOT // equivalent and passed against the bug: `deleted` is removed correctly // and promptly, so the disjunction went to zero on that term alone while // the origin tag was still on the message. Split, so the assertion can @@ -12119,18 +12493,18 @@ void TestMainWindow::deletingTwiceLeavesNoOriginTagBehind() QCOMPARE(notmuchCount(cfg, QStringLiteral("id:twice@example.org")), 1); // The origin tag is gone. This is the defect: it used to survive the - // restore, because the placeholder resolved to `deleted-from:Trash`, the + // restore, because the placeholder resolved to `moved-from:Trash`, the // folder the message was coming FROM, and stripped a tag that had never // been written. QCOMPARE(notmuchCount(cfg, QStringLiteral("id:twice@example.org and " - "tag:\"deleted-from:inbox\"")), + "tag:\"moved-from:inbox\"")), 0); // And no tag naming the trash was invented in its place. QCOMPARE(notmuchCount(cfg, QStringLiteral("id:twice@example.org and " - "tag:\"deleted-from:Trash\"")), + "tag:\"moved-from:Trash\"")), 0); // `deleted` itself, so a fix that dropped this one instead cannot hide. @@ -12150,7 +12524,7 @@ void TestMainWindow::undoOfADeleteRemovesTheOriginTagToo() // placeholder for the tags it wrote to the database, but handed the undo // command the raw list. Undo then asked to remove a tag by the // placeholder's literal name, which no message carries, so the removal - // was a silent no-op and `deleted-from:inbox` survived. The message came + // was a silent no-op and `moved-from:inbox` survived. The message came // home still claiming to have been deleted from somewhere, which makes // Restore offer to move a message that is already at home. // @@ -12192,7 +12566,7 @@ void TestMainWindow::undoOfADeleteRemovesTheOriginTagToo() // about it being REMOVED rather than never having existed. QTRY_VERIFY_WITH_TIMEOUT( notmuchCount(cfg, QStringLiteral("id:undotag@example.org and " - "tag:\"deleted-from:inbox\"")) == 1, + "tag:\"moved-from:inbox\"")) == 1, 15000); window.findChild<QAction *>(QStringLiteral("undo"))->trigger(); @@ -12216,11 +12590,11 @@ void TestMainWindow::undoOfADeleteRemovesTheOriginTagToo() QCOMPARE(notmuchCount(cfg, QStringLiteral("id:undotag@example.org")), 1); QCOMPARE(notmuchCount(cfg, QStringLiteral("id:undotag@example.org and " - "tag:\"deleted-from:inbox\"")), + "tag:\"moved-from:inbox\"")), 0); QCOMPARE(notmuchCount(cfg, QStringLiteral("id:undotag@example.org and " - "tag:\"deleted-from:Trash\"")), + "tag:\"moved-from:Trash\"")), 0); } @@ -12236,8 +12610,8 @@ void TestMainWindow::deletingALoneMessageRemovesItFromTheInboxAndUndoReturnsIt() // from the mismatch: the toggle asked a thread ROW about its thread's // tags, which notmuch gives as a UNION, so deleting the root left the // union carrying no `deleted` and a second press ran Delete AGAIN, - // trash-to-trash, producing `deleted-from:inbox` and - // `deleted-from:Trash` at once with no way back. Item 177 dissolves the + // trash-to-trash, producing `moved-from:inbox` and + // `moved-from:Trash` at once with no way back. Item 177 dissolves the // mismatch rather than patching it: the row and the write now agree about // what they are for. The trash-to-trash assertions stay, because they are // what proves a delete cannot run twice on one message. @@ -12285,7 +12659,7 @@ void TestMainWindow::deletingALoneMessageRemovesItFromTheInboxAndUndoReturnsIt() QTRY_VERIFY_WITH_TIMEOUT(folderHasMessageFile(trash, stem), 15000); QTRY_VERIFY_WITH_TIMEOUT( notmuchCount(cfg, QStringLiteral("id:tlone@example.org and " - "tag:\"deleted-from:inbox\"")) == 1, + "tag:\"moved-from:inbox\"")) == 1, 15000); // There is no second press to make any more, and that is the point. @@ -12329,12 +12703,12 @@ void TestMainWindow::deletingALoneMessageRemovesItFromTheInboxAndUndoReturnsIt() // query bar passes against any state of the database. QCOMPARE(notmuchCount(cfg, QStringLiteral("id:tlone@example.org")), 1); QCOMPARE(notmuchCount(cfg, QStringLiteral("id:tlone@example.org and " - "tag:\"deleted-from:inbox\"")), + "tag:\"moved-from:inbox\"")), 0); // The tag a re-delete would invent. Its presence is the signature of a // trash-to-trash move rather than a variation on the origin-tag defects. QCOMPARE(notmuchCount(cfg, QStringLiteral("id:tlone@example.org and " - "tag:\"deleted-from:Trash\"")), + "tag:\"moved-from:Trash\"")), 0); QVERIFY2(!folderHasMessageFile(trash, stem), "the message was left in the trash"); @@ -12348,7 +12722,7 @@ void TestMainWindow::deleteThreadMovesEveryMessageAndRepaintsTheRootCard() // when Delete became a move, so a whole conversation stayed in the inbox // wearing a `deleted` chip, which is the half-deleted state item 103 // existed to remove. It moves every message now, each carrying its own - // `deleted-from:` origin so a thread spanning folders reassembles. + // `moved-from:` origin so a thread spanning folders reassembles. // // And the ROOT card did not repaint until it was clicked, while its // replies did. A thread-scoped move updated each message's node; @@ -12417,7 +12791,7 @@ void TestMainWindow::deleteThreadMovesEveryMessageAndRepaintsTheRootCard() // Each with its own origin, which is what makes the move reversible. QCOMPARE(notmuchCount(cfg, thread + QStringLiteral(" and " - "tag:\"deleted-from:inbox\"")), + "tag:\"moved-from:inbox\"")), 3); // The ROOT CARD's own state, which is what the user watches. Read from the @@ -12440,19 +12814,29 @@ void TestMainWindow::deleteThreadMovesEveryMessageAndRepaintsTheRootCard() QCOMPARE(notmuchCount(cfg, thread), 3); QCOMPARE(notmuchCount(cfg, thread + QStringLiteral(" and " - "tag:\"deleted-from:inbox\"")), + "tag:\"moved-from:inbox\"")), 0); QVERIFY(!folderHasMessageFile(trash, QStringLiteral("dt0.example.org"))); QVERIFY(!folderHasMessageFile(trash, QStringLiteral("dt1.example.org"))); QVERIFY(!folderHasMessageFile(trash, QStringLiteral("dt2.example.org"))); + + // And the `inbox` tag came back with each message, not only the file. The + // message-scoped restore always did this; the thread-scoped one did not, + // because it passed an empty add list. Item 201 folded the two routes into + // one implementation, and this is the property that was silently missing: + // the conversation sat in the inbox FOLDER with no `inbox` tag, so the + // Inbox view could not see it until the next hook run. + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, thread + QStringLiteral(" and tag:inbox")) == 3, + 15000); } void TestMainWindow::aFolderNameWithASpaceSurvivesTheRoundTrip() { // A notmuch tag MAY contain a space, and a Maildir folder name may too. // The worker reported each message's tags as one space-joined string, so - // `deleted-from:Inbox/SlackBuilds users` was split back into - // "deleted-from:Inbox/SlackBuilds" and "users", and Restore moved the + // `moved-from:Inbox/SlackBuilds users` was split back into + // "moved-from:Inbox/SlackBuilds" and "users", and Restore moved the // messages to the truncated folder, CREATING it. On the user's real // Maildir that put four messages into a directory mbsync does not sync, // beside the real folder of 808, and they read as missing. @@ -12504,7 +12888,7 @@ void TestMainWindow::aFolderNameWithASpaceSurvivesTheRoundTrip() // The origin tag carries the WHOLE folder name, space included. QCOMPARE(notmuchCount(cfg, thread - + QStringLiteral(" and tag:\"deleted-from:" + + QStringLiteral(" and tag:\"moved-from:" "Inbox/SlackBuilds users\"")), 2); @@ -12520,13 +12904,13 @@ void TestMainWindow::aFolderNameWithASpaceSurvivesTheRoundTrip() // could see, could not type, and could not remove. QCOMPARE(notmuchCount(cfg, thread - + QStringLiteral(" and tag:\"deleted-from:" + + QStringLiteral(" and tag:\"moved-from:" "Inbox/SlackBuilds users\"")), 0); // Nor a truncated one, which is what a space-split would have written. QCOMPARE(notmuchCount(cfg, thread - + QStringLiteral(" and tag:\"deleted-from:" + + QStringLiteral(" and tag:\"moved-from:" "Inbox/SlackBuilds\"")), 0); @@ -12753,7 +13137,7 @@ void TestMainWindow::restoreReturnsAMessageToItsOriginFolder() // removes the race rather than papering over it with a longer timeout. QTRY_VERIFY_WITH_TIMEOUT( notmuchCount(cfg, QStringLiteral("id:ro1@example.org and " - "tag:\"deleted-from:inbox\"")) == 0, + "tag:\"moved-from:inbox\"")) == 0, 15000); QTRY_VERIFY_WITH_TIMEOUT( notmuchCount(cfg, @@ -12783,7 +13167,7 @@ void TestMainWindow::restoreReturnsAMessageToItsOriginFolder() void TestMainWindow::restoreFallsBackToInboxWithoutAnOriginTag() { // A message trashed by ANOTHER client: it sits in the trash folder and - // carries no `deleted-from:` tag, because nothing here put it there. The + // carries no `moved-from:` tag, because nothing here put it there. The // real Maildir has such messages, which is why the trash view is path // based rather than tag based. // @@ -12813,7 +13197,7 @@ void TestMainWindow::restoreFallsBackToInboxWithoutAnOriginTag() // The guard this test needs: no origin tag, so the fallback is what is // under test rather than an ordinary restore. QCOMPARE(notmuchCount(cfg, QStringLiteral("id:foreign@example.org and " - "tag:\"deleted-from:inbox\"")), + "tag:\"moved-from:inbox\"")), 0); queryEdit->setText(QStringLiteral("path:\"acct/Trash/**\"")); @@ -12881,10 +13265,10 @@ void TestMainWindow::undoMovesTheMessageBack() QVERIFY2(!folderHasMessageFile(trash, stem), "undo restored the file and left a copy in the trash"); - // Both tags gone, asked of the database. `deleted-from:` left behind would + // Both tags gone, asked of the database. `moved-from:` left behind would // make Restore offer to move a message that is already home. queryEdit->setText(QStringLiteral( - "id:del3@example.org and (tag:deleted or tag:\"deleted-from:inbox\")")); + "id:del3@example.org and (tag:deleted or tag:\"moved-from:inbox\")")); queryEdit->returnPressed(); QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 0, 15000); // The guard the assertion above needs: a query that matches nothing @@ -13100,7 +13484,7 @@ void TestMainWindow::twoDeletesToOneTrashBothGetTheirTags() // Deletes in one account before the first confirmation arrived both named // `acct/Trash`: the second insert overwrote the first and the second // confirmation took an empty entry. That file reached the trash carrying - // neither `deleted` nor `deleted-from:`, which makes it unrestorable by + // neither `deleted` nor `moved-from:`, which makes it unrestorable by // Restore and invisible to a `tag:deleted` query. WorkerBackedWindow backed; QVERIFY(backed.fixture().addMessage( @@ -13155,7 +13539,7 @@ void TestMainWindow::twoDeletesToOneTrashBothGetTheirTags() // the defect was a write that never happened, and the model would have // shown the optimistic state either way. queryEdit->setText(QStringLiteral( - "tag:deleted and tag:\"deleted-from:inbox\" and " + "tag:deleted and tag:\"moved-from:inbox\" and " "(id:two1@example.org or id:two2@example.org)")); queryEdit->returnPressed(); QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 2, 15000); @@ -13327,6 +13711,54 @@ void TestMainWindow::theCleanupQueryExcludesMailAlreadyInTrash() QCOMPARE(notmuchCount(cfg, queryEdit->text()), 0); } +void TestMainWindow::theSpamCleanupQueryExcludesTheSpamFolder() +{ + // Task 7, mirroring theCleanupQueryExcludesMailAlreadyInTrash(). Properly + // spammed mail carries the tag AND sits in the spam folder, so without the + // exclusion this reports every message ever moved to spam. + WorkerBackedWindow backed; + QVERIFY2(backed.build(QStringLiteral("work"), QStringLiteral("work"), + QString(), QStringLiteral("Spam")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + auto *cleanup = + window.findChild<QAction *>(QStringLiteral("cleanup_stranded_spam")); + QVERIFY(queryEdit); + QVERIFY2(cleanup, "there is no cleanup_stranded_spam action"); + + cleanup->trigger(); + + // The composed query, asserted whole: the exclusion has to wrap the + // account's own spam path, and the tag has to be there. + QCOMPARE(queryEdit->text(), + QStringLiteral("tag:spam and not (path:\"work/Spam/**\")")); +} + +void TestMainWindow::theSpamCleanupQueryWithoutASpamFolderIsJustTheTag() +{ + // The other branch of the same composition. An empty exclusion must never + // be written as `not ()`: notmuch parses that happily and matches nothing, + // so an account with no spam folder would report a clean database. + WorkerBackedWindow backed; + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + auto *cleanup = + window.findChild<QAction *>(QStringLiteral("cleanup_stranded_spam")); + QVERIFY(queryEdit); + QVERIFY2(cleanup, "there is no cleanup_stranded_spam action"); + + cleanup->trigger(); + + QCOMPARE(queryEdit->text(), QStringLiteral("tag:spam")); +} + void TestMainWindow::aMoveThatRelocatesNothingWritesNoTag() { // The spec's ordering bullet, at the UI level: a failed rename must leave @@ -13384,7 +13816,7 @@ void TestMainWindow::aMoveThatRelocatesNothingWritesNoTag() QCOMPARE(notmuchCount(cfg, QStringLiteral("id:nomove@example.org and " "tag:deleted")), 0); QCOMPARE(notmuchCount(cfg, QStringLiteral("id:nomove@example.org and " - "tag:\"deleted-from:inbox\"")), 0); + "tag:\"moved-from:inbox\"")), 0); // And the file never left. QVERIFY(folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"), @@ -16681,4 +17113,670 @@ void TestMainWindow::undoingAMarkReadRestoresOnlyWhatWasUnread() 0); } +void TestMainWindow::spamIsAbsentOnAReplyRow() +{ + // Item 187's final review. Mark spam is Delete's sibling, so it follows the + // same rule (item 177): a single reply cannot be moved out of its + // conversation, and moving only that one message is not a gesture this + // application offers. Absent on a reply, back on the conversation row. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(model && view); + + ThreadSummary first = makeThread(QStringLiteral("t1"), {}); + first.totalCount = 1; + ThreadSummary many = makeThread(QStringLiteral("t2"), {}); + many.totalCount = 2; + model->appendBatch({ first, many }); + + MessageNode root; + root.messageId = QStringLiteral("m1"); + root.threadId = QStringLiteral("t2"); + root.depth = 0; + MessageNode reply; + reply.messageId = QStringLiteral("m2"); + reply.threadId = QStringLiteral("t2"); + reply.depth = 1; + model->setThreadMessages(QStringLiteral("t2"), { root, reply }); + + const QModelIndex thread = model->index(1, 0, QModelIndex()); + view->expand(thread); + const QModelIndex replyRow = model->index(0, 0, thread); + view->selectionModel()->select( + replyRow, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); + view->setCurrentIndex(replyRow); + QApplication::processEvents(); + + auto *spam = window.findChild<QAction *>(QStringLiteral("spam")); + QVERIFY(spam); + QVERIFY2(!spam->isVisible() || !spam->isEnabled(), + "Mark spam is offered on a reply: one reply cannot be moved out of " + "its conversation, the same rule Delete and Archive follow"); + + // And the mirror: on the conversation row it is back, so the hide is about + // what the row IS and not a stuck flag. + selectThreadRow(view, 1); + QApplication::processEvents(); + QVERIFY2(spam->isVisible() && spam->isEnabled(), + "Mark spam stayed hidden on a conversation row"); +} + +void TestMainWindow::spamIsHiddenOnMailAlreadyInTheTrash() +{ + // The trash view does not afford Mark spam, exactly as it does not afford + // Delete: the message is already thrown away, and a move from the trash + // into the spam folder is not a gesture the view offers. Asked of the PATH, + // never the `deleted` tag, for the reason Delete is. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const Config config = configWithTrash(dir); + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *spam = window.findChild<QAction *>(QStringLiteral("spam")); + QVERIFY(spam); + + model->appendBatch({ + threadAtPath(QStringLiteral("t1"), + QStringLiteral("acct/inbox/cur/1:2,S")), + threadAtPath(QStringLiteral("t2"), + QStringLiteral("acct/trash/cur/2:2,S")), + }); + + view->setCurrentIndex(model->index(0, 0, {})); + QVERIFY2(spam->isVisible(), + "Mark spam is hidden on mail that is NOT in the trash, so this " + "test cannot tell the two cases apart"); + + view->setCurrentIndex(model->index(1, 0, {})); + QVERIFY2(!spam->isVisible(), + "Mark spam is still offered on mail already in the trash"); + + // A folder whose name STARTS with the trash folder's is a different folder, + // so the prefix must be compared with its separator here too. + model->appendBatch({ threadAtPath(QStringLiteral("t3"), + QStringLiteral("acct/trash-old/cur/3:2,S")) }); + view->setCurrentIndex(model->index(2, 0, {})); + QVERIFY2(spam->isVisible(), + "Mark spam vanished on mail in acct/trash-old, which is not the " + "trash"); +} + +void TestMainWindow::restoringAfterTwoMovesReturnsToTheLatestOrigin() +{ + // Item 187's final review. The worker keeps exactly ONE `moved-from:` per + // message, overwriting the old one (NotmuchWorker::applyTags). The + // optimistic MODEL update did not: on a second move it added the new origin + // beside the old, and restoreSelected() reads the model and takes the FIRST + // `moved-from:` with a break(). So a message that travelled + // inbox -> Spam -> Trash restored to the INBOX rather than to the spam + // folder it actually came from, silently and with no way back. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("twoorig@example.org"), + QStringLiteral("Two origins"), 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<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + const QString root = backed.fixture().maildirPath(); + const QString cfg = backed.fixture().configPath(); + const QString stem = QStringLiteral("twoorig.example.org"); + + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + + // inbox -> Spam, origin moved-from:inbox. + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("spam"))->trigger(); + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/Spam/cur"), stem), + 15000); + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, QStringLiteral("id:twoorig@example.org and " + "tag:\"moved-from:inbox\"")) == 1, + 15000); + + // Spam -> Trash, which overwrites the origin with moved-from:Spam. An + // `id:` query keeps the row in the model across the move, so the second + // Delete press can select it. + queryEdit->setText(QStringLiteral("id:twoorig@example.org")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("delete"))->trigger(); + + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/Trash/cur"), stem), + 15000); + + // The DATABASE holds exactly one origin, and it names the spam folder. + // This is the worker's overwrite rule, already correct. + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, QStringLiteral("id:twoorig@example.org and " + "tag:\"moved-from:Spam\"")) == 1, + 15000); + QCOMPARE(notmuchCount(cfg, QStringLiteral("id:twoorig@example.org and " + "tag:\"moved-from:inbox\"")), + 0); + + // A second Delete press restores it. The MODEL restoreSelected() reads must + // also hold one origin; without the model-side overwrite it still held + // moved-from:inbox and sent the message home to the inbox. + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("delete"))->trigger(); + + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/Spam/cur"), stem), + 15000); + QVERIFY2(!folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), + stem) + && !folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"), + stem), + "the restore went to the inbox instead of the spam folder the " + "message actually came from: the model held two origins"); +} + +void TestMainWindow::notSpamReturnsAMessageToItsOriginFolder() +{ + // Item 201. A message marked spam had no way back: the Spam view offered + // Mark spam and Delete, and Restore is trash-only. Not spam is Mark spam's + // inverse and Restore's twin, and it must return the file to the EXACT + // folder the origin tag names rather than to a guessed inbox. + // + // The origin here is a folder that is NOT the inbox, deliberately: a + // move-back that hardcoded the inbox would pass a laxer assertion, and it + // must also NOT add the `inbox` tag, which would make the message claim to + // belong to a view it was never returned to. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/Archive"), QStringLiteral("ns1@example.org"), + QStringLiteral("Not spam me"), 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<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + const QString root = backed.fixture().maildirPath(); + const QString cfg = backed.fixture().configPath(); + const QString stem = QStringLiteral("ns1.example.org"); + const QString spam = root + QStringLiteral("/acct/Spam/cur"); + + // Mark it spam first, from an `id:` query so the row survives the move. + queryEdit->setText(QStringLiteral("id:ns1@example.org")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("spam"))->trigger(); + QTRY_VERIFY_WITH_TIMEOUT(folderHasMessageFile(spam, stem), 15000); + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, QStringLiteral("id:ns1@example.org and tag:spam " + "and tag:\"moved-from:Archive\"")) == 1, + 15000); + + // Not spam, from the Spam view: the action is only offered there. + queryEdit->setText(QStringLiteral("path:\"acct/Spam/**\"")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("not_spam"))->trigger(); + + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/Archive/cur"), stem) + || folderHasMessageFile(root + QStringLiteral("/acct/Archive/new"), + stem), + 15000); + QVERIFY2(!folderHasMessageFile(spam, stem), + "Not spam left a copy in the spam folder"); + + // The tags this test is about: `spam` and the origin are gone, and `inbox` + // did NOT come back, because the destination is not an inbox. + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, QStringLiteral("id:ns1@example.org and " + "(tag:spam or " + "tag:\"moved-from:Archive\")")) == 0, + 15000); + QCOMPARE(notmuchCount(cfg, QStringLiteral("id:ns1@example.org and " + "tag:inbox")), + 0); + // The guard the assertion above needs: a message that vanished satisfies it. + QCOMPARE(notmuchCount(cfg, QStringLiteral("id:ns1@example.org")), 1); +} + +void TestMainWindow::notSpamFallsBackToInboxWithoutAnOriginTag() +{ + // A provider-caught message: it sits in the spam folder and carries no + // `moved-from:` tag, because nothing here put it there. The inbox is the + // documented fallback, and it is REPORTED: a guess the user is not told + // about is worse than the guess. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/Spam"), QStringLiteral("nsfb@example.org"), + QStringLiteral("Caught upstream"), 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<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + const QString root = backed.fixture().maildirPath(); + const QString cfg = backed.fixture().configPath(); + const QString stem = QStringLiteral("nsfb.example.org"); + + // The guard: no origin, so the fallback is what is under test. + QCOMPARE(notmuchCount(cfg, QStringLiteral("id:nsfb@example.org and " + "tag:\"moved-from:inbox\"")), + 0); + + queryEdit->setText(QStringLiteral("path:\"acct/Spam/**\"")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("not_spam"))->trigger(); + + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), stem) + || folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"), + stem), + 15000); + QVERIFY2(!folderHasMessageFile(root + QStringLiteral("/acct/Spam/cur"), + stem), + "the provider-caught message was copied rather than moved"); + + // And the user is told the destination was a fallback. The status is the + // only place that says so. + auto *status = + window.findChild<QLabel *>(QStringLiteral("statusMessage")); + QVERIFY(status); + QTRY_VERIFY_WITH_TIMEOUT( + status->text().contains(QStringLiteral("no record")), 15000); +} + +void TestMainWindow::notSpamIsOfferedInTheSpamView() +{ + // The action is offered where it means something and only there: a + // selection in a spam folder. Asked of the PATH, never the `spam` tag: a + // provider-caught message carries no tag of ours and must still be + // un-spammable. + // + // Asserted on the message BAR as well as on the QAction. The two are set by + // different code (populateMessageBar() vs refreshTrashActions()), so + // deleting the bar's spam branch leaves a QAction-only test green. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/Spam"), QStringLiteral("nsv@example.org"), + QStringLiteral("Offered here"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Body text."))); + // A reply in the same conversation, so the bar's reply behaviour inside a + // spam folder can be checked too: the only action the spam branch offers is + // hidden on a reply, so taking that branch would leave the bar EMPTY. + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/Spam"), QStringLiteral("nsv2@example.org"), + QStringLiteral("Re: Offered here"), QStringLiteral("other@example.org"), + QStringLiteral("Fri, 14 Aug 2026 11:00:00 +0200"), + QStringLiteral("Reply text."), true, + QStringLiteral("nsv@example.org"))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash"), QStringLiteral("Spam")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + auto *bar = window.findChild<QToolBar *>(QStringLiteral("message_toolbar")); + QVERIFY(model && view && queryEdit && bar); + auto *notSpam = window.findChild<QAction *>(QStringLiteral("not_spam")); + QVERIFY(notSpam); + + const auto barHolds = [&](const QString &name) { + const auto actions = bar->actions(); + return std::any_of(actions.cbegin(), actions.cend(), + [&](const QAction *action) { + return action && action->objectName() == name + && action->isVisible(); + }); + }; + + queryEdit->setText(QStringLiteral("path:\"acct/Spam/**\"")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + QApplication::processEvents(); + + QVERIFY2(notSpam->isVisible() && notSpam->isEnabled(), + "Not spam is not offered in the spam view, where it is the point"); + QVERIFY2(barHolds(QStringLiteral("not_spam")), + "the message bar does not carry Not spam in the spam view"); + + // The reply inside the spam conversation. Not spam is absent there, as on + // any reply, but the bar must fall back to the ordinary actions rather than + // go empty. + const QModelIndex thread = model->index(0, 0, QModelIndex()); + view->expand(thread); + // Both messages are children since item 177; wait for the worker round trip + // or the reply row is not there and the selection below is a no-op. + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(thread) == 2, 15000); + const QModelIndex replyRow = model->index(1, 0, thread); + QVERIFY(model->isMessageRow(replyRow)); + view->selectionModel()->select( + replyRow, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); + view->setCurrentIndex(replyRow); + QApplication::processEvents(); + + QVERIFY2(!barHolds(QStringLiteral("not_spam")), + "Not spam is on the message bar for a reply inside the spam folder"); + QVERIFY2(barHolds(QStringLiteral("reply")), + "the message bar went empty on a reply inside the spam folder: it " + "took the spam branch, whose only action is hidden on a reply"); + QVERIFY2(barHolds(QStringLiteral("forward")), + "the message bar lost Forward on a reply inside the spam folder"); +} + +void TestMainWindow::notSpamIsAbsentOnAReplyRow() +{ + // Mark spam is Delete's sibling and Not spam is Restore's, so both follow + // item 177: a single reply cannot be moved out of its conversation. Absent + // on a reply, back on the conversation row. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const Config config = configWithTrash(dir); + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(model && view); + + model->appendBatch({ threadAtPath(QStringLiteral("t1"), + QStringLiteral("acct/spam/cur/1:2,S")) }); + + MessageNode root; + root.messageId = QStringLiteral("m1"); + root.threadId = QStringLiteral("t1"); + root.depth = 0; + root.filePath = QStringLiteral("acct/spam/cur/1:2,S"); + MessageNode reply; + reply.messageId = QStringLiteral("m2"); + reply.threadId = QStringLiteral("t1"); + reply.depth = 1; + // A REAL spam path, so the predicate answers true for this row and the + // ONLY thing that can hide Not spam is the reply guard. Without it the + // empty path made everySelectedRowIsInAFolder() return false and the test + // would pass against a missing guard. + reply.filePath = QStringLiteral("acct/spam/cur/2:2,S"); + model->setThreadMessages(QStringLiteral("t1"), { root, reply }); + + const QModelIndex thread = model->index(0, 0, QModelIndex()); + view->expand(thread); + // Child 1, not child 0: since item 177 a conversation lists its FIRST + // message as a child too, so child 0 is the root message and child 1 is + // the reply whose row is under test. + const QModelIndex replyRow = model->index(1, 0, thread); + view->selectionModel()->select( + replyRow, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); + view->setCurrentIndex(replyRow); + QApplication::processEvents(); + + auto *notSpam = window.findChild<QAction *>(QStringLiteral("not_spam")); + QVERIFY(notSpam); + QVERIFY2(!notSpam->isVisible(), + "Not spam is offered on a reply: one reply cannot be moved out of " + "its conversation, the rule Delete and Mark spam follow"); + + // The mirror: on the conversation row it is back, so the hide is about what + // the row IS and not a stuck flag. + selectThreadRow(view, 0); + QApplication::processEvents(); + QVERIFY2(notSpam->isVisible(), + "Not spam stayed hidden on a conversation in the spam folder"); +} + +void TestMainWindow::notSpamIsHiddenOutsideTheSpamFolder() +{ + // Everywhere else the message is not in a spam folder, so there is nothing + // to come out of. Trash included: mail already thrown away is not offered a + // second move. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const Config config = configWithTrash(dir); + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *notSpam = window.findChild<QAction *>(QStringLiteral("not_spam")); + QVERIFY(notSpam); + + model->appendBatch({ + threadAtPath(QStringLiteral("t1"), + QStringLiteral("acct/spam/cur/1:2,S")), + threadAtPath(QStringLiteral("t2"), + QStringLiteral("acct/inbox/cur/2:2,S")), + threadAtPath(QStringLiteral("t3"), + QStringLiteral("acct/trash/cur/3:2,S")), + }); + + view->setCurrentIndex(model->index(1, 0, {})); + QVERIFY2(!notSpam->isVisible(), + "Not spam is offered on mail in the inbox"); + + view->setCurrentIndex(model->index(2, 0, {})); + QVERIFY2(!notSpam->isVisible(), + "Not spam is offered on mail already in the trash"); + + // A folder whose name STARTS with the spam folder's is a different folder, + // so the prefix must be compared with its separator here too. + model->appendBatch({ threadAtPath(QStringLiteral("t4"), + QStringLiteral("acct/spam-old/cur/4:2,S")) }); + view->setCurrentIndex(model->index(3, 0, {})); + QVERIFY2(!notSpam->isVisible(), + "Not spam is offered on mail in acct/spam-old, which is not the " + "spam folder"); + + view->setCurrentIndex(model->index(0, 0, {})); + QVERIFY2(notSpam->isVisible(), + "Not spam is hidden on mail in the spam folder, so this test " + "cannot tell the cases apart"); +} + +void TestMainWindow::undoOfNotSpamReturnsTheFileToTheSpamFolder() +{ + // Undo is the safety net that replaces the confirmation dialog, so an + // un-spam that cannot be retracted is one with no net at all. The undo puts + // the file back in the spam folder AND restores the tags the move removed. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("nsundo@example.org"), + QStringLiteral("Undo my not-spam"), 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<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + const QString root = backed.fixture().maildirPath(); + const QString cfg = backed.fixture().configPath(); + const QString stem = QStringLiteral("nsundo.example.org"); + const QString spam = root + QStringLiteral("/acct/Spam/cur"); + + queryEdit->setText(QStringLiteral("id:nsundo@example.org")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("spam"))->trigger(); + QTRY_VERIFY_WITH_TIMEOUT(folderHasMessageFile(spam, stem), 15000); + QTRY_VERIFY_WITH_TIMEOUT(window.undoDepthForTesting() >= 1, 15000); + + // Not spam, and wait for its own command to reach the stack before undoing + // it: undo before the push is a no-op and would blame the wrong move. The + // query above cleared the stack, so this is the only entry on it. + queryEdit->setText(QStringLiteral("path:\"acct/Spam/**\"")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("not_spam"))->trigger(); + QTRY_VERIFY_WITH_TIMEOUT(window.undoDepthForTesting() >= 1, 15000); + + window.findChild<QAction *>(QStringLiteral("undo"))->trigger(); + + QTRY_VERIFY_WITH_TIMEOUT(folderHasMessageFile(spam, stem), 15000); + QVERIFY2(!folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), stem) + && !folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"), + stem), + "undo returned the file to the inbox instead of the spam folder"); + + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, QStringLiteral("id:nsundo@example.org and tag:spam " + "and tag:\"moved-from:inbox\"")) == 1, + 15000); + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, QStringLiteral("id:nsundo@example.org and " + "tag:inbox")) == 0, + 15000); + QCOMPARE(notmuchCount(cfg, QStringLiteral("id:nsundo@example.org")), 1); +} + +void TestMainWindow::notSpamThreadMovesEveryMessageHome() +{ + // The thread-scoped half of Not spam, mirroring + // deleteThreadMovesEveryMessageAndRepaintsTheRootCard: a conversation row + // moves its whole conversation, each message back to its OWN origin. The + // single-message test cannot see notSpamThreads(), the optimistic + // applyTagChange, m_pendingThreadScope or the wholeThreadIds repaint. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("nst0@example.org"), + QStringLiteral("NST root"), QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("Root body."))); + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("nst1@example.org"), + QStringLiteral("Re: NST root"), QStringLiteral("other@example.org"), + QStringLiteral("Fri, 14 Aug 2026 11:00:00 +0200"), + QStringLiteral("Reply one."), true, QStringLiteral("nst0@example.org"))); + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("nst2@example.org"), + QStringLiteral("Re: NST root"), QStringLiteral("third@example.org"), + QStringLiteral("Fri, 14 Aug 2026 12:00:00 +0200"), + QStringLiteral("Reply two."), true, QStringLiteral("nst0@example.org"))); + QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"), + QStringLiteral("Trash"), QStringLiteral("Spam")), + qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + auto *view = window.findChild<ThreadListView *>(); + auto *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(model && view && queryEdit); + + const QString root = backed.fixture().maildirPath(); + const QString cfg = backed.fixture().configPath(); + const QString spam = root + QStringLiteral("/acct/Spam/cur"); + const QString thread = QStringLiteral("thread:{id:nst0@example.org}"); + + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + QCOMPARE(notmuchCount(cfg, thread), 3); + + // A conversation row, so the move is thread-scoped. + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("spam"))->trigger(); + + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(spam, QStringLiteral("nst0.example.org")) + && folderHasMessageFile(spam, QStringLiteral("nst1.example.org")) + && folderHasMessageFile(spam, QStringLiteral("nst2.example.org")), + 15000); + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, thread + QStringLiteral(" and tag:spam")) == 3, + 15000); + QCOMPARE(notmuchCount(cfg, thread + + QStringLiteral(" and " + "tag:\"moved-from:inbox\"")), + 3); + + // Now Not spam on the same conversation row, from the Spam view. + queryEdit->setText(QStringLiteral("path:\"acct/Spam/**\"")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + view->setCurrentIndex(model->index(0, 0, QModelIndex())); + window.findChild<QAction *>(QStringLiteral("not_spam"))->trigger(); + + // Every message back in the inbox folder, none left in spam, each carrying + // the `inbox` tag its origin move stripped. + QTRY_VERIFY_WITH_TIMEOUT( + (folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), + QStringLiteral("nst0.example.org")) + || folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"), + QStringLiteral("nst0.example.org"))) + && (folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), + QStringLiteral("nst1.example.org")) + || folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"), + QStringLiteral("nst1.example.org"))) + && (folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), + QStringLiteral("nst2.example.org")) + || folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"), + QStringLiteral("nst2.example.org"))), + 15000); + QVERIFY(!folderHasMessageFile(spam, QStringLiteral("nst0.example.org"))); + QVERIFY(!folderHasMessageFile(spam, QStringLiteral("nst1.example.org"))); + QVERIFY(!folderHasMessageFile(spam, QStringLiteral("nst2.example.org"))); + + QCOMPARE(notmuchCount(cfg, thread), 3); + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, thread + QStringLiteral(" and tag:spam")) == 0, + 15000); + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, thread + + QStringLiteral(" and " + "tag:\"moved-from:inbox\"")) == 0, + 15000); + QTRY_VERIFY_WITH_TIMEOUT( + notmuchCount(cfg, thread + QStringLiteral(" and tag:inbox")) == 3, + 15000); +} + #include "test_mainwindow.moc" diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp index 970f51d..b2810d1 100644 --- a/tests/test_notmuchworker.cpp +++ b/tests/test_notmuchworker.cpp @@ -54,6 +54,7 @@ private slots: void applyTagsWithNoIdsDoesNothing(); void applyTagsReportsOnlyTheMessagesItChanged(); void applyTagsThatChangeNothingEmitNothing(); + void applyTagsKeepsOnlyTheNewestOriginTag(); void queryStillWorksAfterWrite(); void aThreadCarriesItsCardMessagesOwnTags(); @@ -99,6 +100,8 @@ private slots: void requestFoldersOnUnreadableConfigEmitsError(); void moveMessagesRelocatesTheFile(); + void moveMessagesToSpamRelocatesTheFileAndTagsIt(); + void aSecondMoveToSpamKeepsOnlyTheNewestOriginTag(); void moveMessagesReindexesAtTheNewPath(); void moveMessagesKeepsTheMessagesTags(); void moveMessagesReportsOnlyWhatMoved(); @@ -142,8 +145,10 @@ private: QString fileOf(const QString &messageId, const QString &configPath = QString()); - /// Tags of one message, read back through a fresh worker query. - QStringList tagsOf(const QString &messageId); + /// Tags of one message, read back through a fresh worker query. Defaults to + /// the shared fixture; a test with its own fixture passes its own path. + QStringList tagsOf(const QString &messageId, + const QString &configPath = QString()); QVector<MessageRef> messagesOfThread(const QString &threadId, const QString &matchQuery = QString(), bool matchedOnly = false); @@ -401,9 +406,11 @@ QVector<MessageRef> TestNotmuchWorker::messagesOfThread(const QString &threadId, return loaded.first().at(0).value<QVector<MessageRef>>(); } -QStringList TestNotmuchWorker::tagsOf(const QString &messageId) +QStringList TestNotmuchWorker::tagsOf(const QString &messageId, + const QString &configPath) { - NotmuchWorker worker(m_fixture.configPath()); + NotmuchWorker worker(configPath.isEmpty() ? m_fixture.configPath() + : configPath); QSignalSpy loaded(&worker, &NotmuchWorker::threadLoaded); worker.loadThread(QStringLiteral("{id:%1}").arg(messageId), QString(), 1); if (loaded.isEmpty()) @@ -1117,6 +1124,68 @@ void TestNotmuchWorker::applyTagsThatChangeNothingEmitNothing() QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); } +void TestNotmuchWorker::applyTagsKeepsOnlyTheNewestOriginTag() +{ + // A message that travelled inbox -> spam -> trash must hold exactly ONE + // `moved-from:` tag, the newest, or Restore's first-match scan picks an + // origin arbitrarily. Its own fixture, so the tags this test leaves behind + // cannot move the shared one's thread counts. + NotmuchFixture fixture; + QVERIFY(fixture.addMessage(QStringLiteral("inbox"), + QStringLiteral("origin1@example.org"), + QStringLiteral("Travelled"), + QStringLiteral("Erin <erin@example.org>"), + QStringLiteral("Sun, 7 Jun 2026 10:00:00 +0000"), + QStringLiteral("body"), false)); + QVERIFY(fixture.addMessage(QStringLiteral("inbox"), + QStringLiteral("origin2@example.org"), + QStringLiteral("Fresh"), + QStringLiteral("Erin <erin@example.org>"), + QStringLiteral("Sun, 7 Jun 2026 11:00:00 +0000"), + QStringLiteral("body"), false)); + QVERIFY(fixture.index()); + + NotmuchWorker worker(fixture.configPath()); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + + // An earlier move left an origin behind. + worker.applyTags(TagChange{ { QStringLiteral("origin1@example.org") }, + { QStringLiteral("moved-from:inbox") }, + {}, + QStringLiteral("Earlier move") }); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + QVERIFY(tagsOf(QStringLiteral("origin1@example.org"), fixture.configPath()) + .contains(QStringLiteral("moved-from:inbox"))); + + // The move under test: a new origin lands while the old one is still there. + worker.applyTags(TagChange{ + { QStringLiteral("origin1@example.org"), + QStringLiteral("origin2@example.org") }, + { QStringLiteral("moved-from:Spam"), QStringLiteral("spam") }, + { QStringLiteral("inbox"), QStringLiteral("unread") }, + QStringLiteral("Mark spam") }); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + + const QStringList travelled = + tagsOf(QStringLiteral("origin1@example.org"), fixture.configPath()); + QVERIFY(!travelled.contains(QStringLiteral("moved-from:inbox"))); + QVERIFY(travelled.contains(QStringLiteral("moved-from:Spam"))); + int origins = 0; + for (const QString &tag : travelled) { + if (tag.startsWith(QStringLiteral("moved-from:"))) + ++origins; + } + QCOMPARE(origins, 1); + + // The message that never carried an origin keeps exactly the new one, and + // the move still removed what a move removes. + const QStringList fresh = + tagsOf(QStringLiteral("origin2@example.org"), fixture.configPath()); + QVERIFY(fresh.contains(QStringLiteral("moved-from:Spam"))); + QVERIFY(fresh.contains(QStringLiteral("spam"))); + QVERIFY(!fresh.contains(QStringLiteral("inbox"))); +} + void TestNotmuchWorker::queryStillWorksAfterWrite() { // applyTags closes the read-only handle to take the write lock. The same @@ -1729,6 +1798,109 @@ void TestNotmuchWorker::moveMessagesRelocatesTheFile() QVERIFY(!QFile::exists(before)); } +void TestNotmuchWorker::moveMessagesToSpamRelocatesTheFileAndTagsIt() +{ + // Mark spam is Delete's sibling, and its own fixture rather than the + // shared one: this needs an UNREAD message so the `unread` removal the + // account's spam folder config implies is actually observable. The shared + // fixture's movable messages are all read, which would make that assertion + // pass against nothing. + NotmuchFixture fixture; + QVERIFY(fixture.isValid()); + const QString id = QStringLiteral("spam1@example.org"); + QVERIFY(fixture.addMessage(QStringLiteral("inbox"), id, + QStringLiteral("Suspect"), + QStringLiteral("Erin <erin@example.org>"), + QStringLiteral("Sun, 7 Jun 2026 10:00:00 +0000"), + QStringLiteral("body"), true)); + QVERIFY2(fixture.index(), qPrintable(fixture.error())); + + QVERIFY2(tagsOf(id, fixture.configPath()).contains(QStringLiteral("unread")), + "the fixture message is already read, so the unread removal below " + "would prove nothing"); + + NotmuchWorker worker(fixture.configPath()); + QSignalSpy moved(&worker, &NotmuchWorker::messagesMoved); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + + worker.moveMessages({ id }, QStringLiteral("spam")); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + + QCOMPARE(moved.size(), 1); + QCOMPARE(moved.first().at(0).toStringList(), QStringList{ id }); + QCOMPARE(moved.first().at(1).toString(), QStringLiteral("spam")); + + const QString expectedDir = + fixture.maildirPath() + QStringLiteral("/spam/cur"); + const QString after = fileOf(id, fixture.configPath()); + QVERIFY2(!after.isEmpty(), + "the message is not in the database after the move"); + QCOMPARE(QFileInfo(after).absolutePath(), expectedDir); + QVERIFY2(QFile::exists(after), qPrintable(after)); + + // The tag half travels with the move the way onMessagesMoved() composes + // it: `spam` and the origin in, `unread` and `inbox` out. + worker.applyTags(TagChange{ { id }, + { QStringLiteral("spam"), + QStringLiteral("moved-from:inbox") }, + { QStringLiteral("unread"), + QStringLiteral("inbox") }, + QStringLiteral("Mark spam") }); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + + const QStringList tags = tagsOf(id, fixture.configPath()); + QVERIFY(tags.contains(QStringLiteral("spam"))); + QVERIFY(tags.contains(QStringLiteral("moved-from:inbox"))); + QVERIFY(!tags.contains(QStringLiteral("unread"))); +} + +void TestNotmuchWorker::aSecondMoveToSpamKeepsOnlyTheNewestOriginTag() +{ + // The overwrite rule, reached through a second move: inbox -> spam -> a + // later move that writes `moved-from:Spam` must leave exactly ONE + // `moved-from:` tag, the newest. Two would make Restore's first-match scan + // pick an origin arbitrarily, and the message would go home by a coin toss. + NotmuchFixture fixture; + QVERIFY(fixture.isValid()); + const QString id = QStringLiteral("reorigin@example.org"); + QVERIFY(fixture.addMessage(QStringLiteral("inbox"), id, + QStringLiteral("Travelled"), + QStringLiteral("Erin <erin@example.org>"), + QStringLiteral("Sun, 7 Jun 2026 10:00:00 +0000"), + QStringLiteral("body"), true)); + QVERIFY2(fixture.index(), qPrintable(fixture.error())); + + NotmuchWorker worker(fixture.configPath()); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + + // An earlier move left an origin behind. + worker.applyTags(TagChange{ { id }, + { QStringLiteral("moved-from:inbox") }, + {}, + QStringLiteral("Earlier move") }); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + + // The move under test: a new origin lands while the old one is still there. + worker.applyTags(TagChange{ { id }, + { QStringLiteral("spam"), + QStringLiteral("moved-from:Spam") }, + { QStringLiteral("unread"), + QStringLiteral("inbox") }, + QStringLiteral("Mark spam") }); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + + const QStringList tags = tagsOf(id, fixture.configPath()); + QVERIFY(!tags.contains(QStringLiteral("moved-from:inbox"))); + QVERIFY(tags.contains(QStringLiteral("moved-from:Spam"))); + + int origins = 0; + for (const QString &tag : tags) { + if (tag.startsWith(QStringLiteral("moved-from:"))) + ++origins; + } + QCOMPARE(origins, 1); +} + void TestNotmuchWorker::purgeMessagesDoesNotClaimAnIdItCouldNotDelete() { // The report drives what the UI tells the user, and the one number they diff --git a/tests/test_tagdialog.cpp b/tests/test_tagdialog.cpp index 9b12109..93a7229 100644 --- a/tests/test_tagdialog.cpp +++ b/tests/test_tagdialog.cpp @@ -176,14 +176,14 @@ void TestTagDialog::aTagWithASpaceCanStillBeRemoved() // they can see and cannot get rid of. // // Reached by a real Maildir: a folder named "Inbox/SlackBuilds users" - // produced `deleted-from:Inbox/SlackBuilds users`, and the one dialog that + // produced `moved-from:Inbox/SlackBuilds users`, and the one dialog that // could have cleared it refused the only text that names it. // // Only the TYPED route was blocked. Unchecking appends to the removal list // after validation has run, so it worked throughout; that asymmetry is why // both routes are asserted here rather than just the one that failed. const QString spaced = - QStringLiteral("deleted-from:Inbox/SlackBuilds users"); + QStringLiteral("moved-from:Inbox/SlackBuilds users"); QHash<QString, int> current; current.insert(spaced, 1); |