diff options
| -rw-r--r-- | src/mainwindow.cpp | 114 | ||||
| -rw-r--r-- | src/mainwindow.h | 32 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 262 | ||||
| -rw-r--r-- | tests/test_notmuchworker.cpp | 105 | ||||
| -rw-r--r-- | translations/qtmaildir_it_IT.ts | 31 |
5 files changed, 482 insertions, 62 deletions
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 6c66953..37411e6 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -1784,9 +1784,8 @@ void MainWindow::registerActions() purgeSelected(); }); addAction(QStringLiteral("spam"), tr("Mark &spam"), - tr("Add spam and remove inbox"), [this]() { - tagSelected({ QStringLiteral("spam") }, { QStringLiteral("inbox") }, - tr("Mark spam")); + tr("Move the selected messages to the spam folder"), [this]() { + spamSelected(); }); // Item 57. The LABEL is "Important"; the action name and the tag are both // still `flag`/`flagged`, deliberately. The name is what a user writes in @@ -3972,7 +3971,8 @@ void MainWindow::refreshScopedActionLabels() tr("Move every message of the selected threads out of the " "trash")); relabel(QStringLiteral("spam"), tr("Mark thread as &spam"), - tr("Add spam and remove inbox on the selected threads")); + tr("Move every message of the selected threads to the spam " + "folder")); relabel(QStringLiteral("flag"), tr("&Important thread"), tr("Mark every message of the selected threads as important")); } else { @@ -3983,7 +3983,7 @@ void MainWindow::refreshScopedActionLabels() relabel(QStringLiteral("restore"), tr("&Restore from trash"), tr("Move the selected messages out of the trash")); relabel(QStringLiteral("spam"), tr("Mark &spam"), - tr("Add spam and remove inbox")); + tr("Move the selected messages to the spam folder")); relabel(QStringLiteral("flag"), tr("&Important"), tr("Add or remove the important tag")); } @@ -6413,6 +6413,105 @@ void MainWindow::trashThreads(const QStringList &threadIds) Q_ARG(QString, QStringLiteral("delete_thread"))); } +void MainWindow::spamSelected() +{ + const QModelIndexList rows = + m_threadView->selectionModel()->selectedRows(); + if (rows.isEmpty()) + return; + + // Delete's sibling, resolved the same per-row way (item 177): a + // conversation row spams its conversation, a thread of one spams its + // message. Both halves are run, because a selection really can hold one of + // each; they travel different routes for the reason trashSelected() + // records. + const ActionScope scope = m_model->scopeForSelection(rows); + + if (!scope.threadIds.isEmpty()) + spamThreads(scope.threadIds); + + if (scope.messageIds.isEmpty()) + return; + + QHash<QString, QString> pathById; + for (const QString &messageId : scope.messageIds) + pathById.insert(messageId, m_model->messageById(messageId).filePath); + + spamMessages(scope.messageIds, pathById, scope.messageIds.size()); +} + +void MainWindow::spamMessages(const QStringList &messageIds, + const QHash<QString, QString> &pathById, + int messageCount, + const QStringList &wholeThreadIds) +{ + if (messageIds.isEmpty()) + return; + + // Grouped by destination, exactly as trashMessages() is: moveMessages() + // takes one folder per call, and a selection can span accounts with + // different spam folders. + QHash<QString, QStringList> bySpam; + QStringList unconfigured; + for (const QString &messageId : messageIds) { + const Account account = + accountForMessagePath(pathById.value(messageId)); + if (account.spam.isEmpty()) { + unconfigured.append(messageId); + continue; + } + bySpam[account.maildir + QLatin1Char('/') + account.spam] + .append(messageId); + } + + // The second line of defence, as for trash: the config loader warns, but a + // user who never fixed it still needs the gesture to say it did nothing + // rather than move the file somewhere invented. + if (!unconfigured.isEmpty()) { + m_statusLabel->setText( + tr("%n message(s) could not be marked as spam: no spam folder is " + "configured for their account.", "", int(unconfigured.size()))); + } + + if (bySpam.isEmpty()) + return; + + for (auto it = bySpam.cbegin(); it != bySpam.cend(); ++it) { + // `unread` and `inbox` go with the message, exactly as Delete strips + // them: marking spam is a decision about the message, and without the + // `inbox` removal a message spammed FROM the inbox keeps the tag the + // Inbox filter matches on and stays in that view. The origin is the + // placeholder, resolved per message once the move is confirmed. + sendMove(it.value(), it.key(), + { QStringLiteral("spam"), kOriginTagPlaceholder() }, + { QStringLiteral("unread"), QStringLiteral("inbox") }, + tr("Mark spam"), false, wholeThreadIds); + } + + announceAction( + tr("%1: %n message(s)", "", messageCount).arg(tr("Mark spam"))); +} + +void MainWindow::spamThreads(const QStringList &threadIds) +{ + if (threadIds.isEmpty()) + return; + + // Asked of the WORKER rather than resolved here, and repainted HERE + // synchronously before it: the same shape as trashThreads(), for the same + // two reasons. An unexpanded thread's reply ids and paths exist only in + // the database; and the card must not wait for that round trip. + for (const QString &threadId : threadIds) + m_model->applyTagChange(threadId, { QStringLiteral("spam") }, + { QStringLiteral("inbox") }); + + m_pendingThreadScope = threadIds; + QMetaObject::invokeMethod(m_worker, "resolveThreadMessages", + Qt::QueuedConnection, + Q_ARG(QStringList, threadIds), + Q_ARG(QString, QStringLiteral("spam_thread"))); +} + void MainWindow::onThreadMessagesResolved(const QStringList &messageIds, const QStringList &paths, const QStringList &tags, @@ -6467,6 +6566,11 @@ void MainWindow::onThreadMessagesResolved(const QStringList &messageIds, return; } + if (requestTag == QStringLiteral("spam_thread")) { + spamMessages(messageIds, pathById, messageIds.size(), threadScope); + return; + } + if (requestTag == QStringLiteral("restore_messages")) { restoreResolvedMessages(messageIds, paths, tags); return; diff --git a/src/mainwindow.h b/src/mainwindow.h index 97d90e5..14bb06f 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -226,6 +226,15 @@ public: refreshTrashActions(); } + /// The trash predicate, exposed because `spam` is a DIFFERENT folder and + /// the predicate must not answer for it. A test seam rather than a + /// behavioural one: asserting on Delete's visibility would prove the same + /// thing only through the label refresh. + bool everySelectedRowIsInATrashFolderForTesting() const + { + return everySelectedRowIsInATrashFolder(); + } + /// Runs a purge without the confirmation, which a test cannot drive: a /// modal blocks the thread it is shown on (item 84). What this exists to /// cover is what happens AFTER the user confirms. @@ -1233,6 +1242,29 @@ private: /// back where it came from. void untrashThreads(const QStringList &threadIds); + /// Moves each selected row's message to its account's spam folder, tagging + /// it `spam` and recording where it came from. Delete's sibling. + void spamSelected(); + + /// The half of spamSelected() that does the work, given the messages and + /// their paths. + /// + /// Shaped exactly like trashMessages(): paths are passed in rather than + /// looked up, because the thread-scoped caller has messages the MODEL has + /// never seen. + void spamMessages(const QStringList &messageIds, + const QHash<QString, QString> &pathById, + int messageCount, + const QStringList &wholeThreadIds = {}); + + /// Moves every message of the named THREADS to their accounts' spam + /// folder. + /// + /// Asynchronous like trashThreads(), and for the same reason: the ids and + /// paths of an unexpanded thread's messages live only in the database, so + /// this asks the worker and finishes in onThreadMessagesResolved(). + void spamThreads(const QStringList &threadIds); + /// Runs the thread-scoped delete once the worker has resolved the /// threads to messages. void onThreadMessagesResolved(const QStringList &messageIds, diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index a92b3d1..5969da9 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -148,7 +148,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 +187,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 @@ -394,7 +400,7 @@ private slots: void importantOnAReplyReadsItsOwnStateNotItsThreads(); void editTagsOnAReplyCountsItsOwnThreadNotTheFirstInTheList(); void markCurrentThreadReadResolvesTheThreadThroughTheIndex(); - void deletingAReplyRepaintsThatReplyRow(); + void taggingAReplyRepaintsThatReplyRow(); void deleteIsHiddenOnMailAlreadyInTheTrash(); void aPartlyTrashedConversationIsNotJudgedOnOneMessage(); void restoreIsHiddenOnMailThatWasNeverDeleted(); @@ -509,6 +515,12 @@ 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(); + // 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 +5239,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 +5260,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 +5269,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 +5300,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 +5902,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 +5933,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 +5948,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 +6033,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 +6053,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 +6077,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 +6088,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 +6101,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 +6121,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 +6138,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 +6419,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 +6469,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 +6479,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"); } @@ -11948,6 +11960,158 @@ 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::deleteRecordsWhereTheMessageCameFrom() { // A Maildir filename does not record where a message came from, and once diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp index 2877894..b2810d1 100644 --- a/tests/test_notmuchworker.cpp +++ b/tests/test_notmuchworker.cpp @@ -100,6 +100,8 @@ private slots: void requestFoldersOnUnreadableConfigEmitsError(); void moveMessagesRelocatesTheFile(); + void moveMessagesToSpamRelocatesTheFileAndTagsIt(); + void aSecondMoveToSpamKeepsOnlyTheNewestOriginTag(); void moveMessagesReindexesAtTheNewPath(); void moveMessagesKeepsTheMessagesTags(); void moveMessagesReportsOnlyWhatMoved(); @@ -1796,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/translations/qtmaildir_it_IT.ts b/translations/qtmaildir_it_IT.ts index c0e336d..cb9b34e 100644 --- a/translations/qtmaildir_it_IT.ts +++ b/translations/qtmaildir_it_IT.ts @@ -293,6 +293,10 @@ Il messaggio È stato inviato. Non inviarlo di nuovo.</translation> <translation>L'account '%1' non ha un cestino configurato; aggiungere una chiave 'trash' alla sua sezione. L'eliminazione non funzionerà per questo account finché non verrà fatto.</translation> </message> <message> + <source>Account '%1' has no spam folder configured; add a 'spam' key to its section. Mark spam will not work for this account until it does.</source> + <translation>L'account '%1' non ha una cartella spam configurata; aggiungere una chiave 'spam' alla sua sezione. Segna come spam non funzionerà per questo account finché non verrà fatto.</translation> + </message> + <message> <source>[compose] quote_position '%1' is not recognised; expected above or below. Using below.</source> <translation>[compose] quote_position '%1' non è riconosciuto; atteso above o below. Uso below.</translation> </message> @@ -384,6 +388,10 @@ Il messaggio È stato inviato. Non inviarlo di nuovo.</translation> <source>Trash</source> <translation>Cestino</translation> </message> + <message> + <source>Spam</source> + <translation>Spam</translation> + </message> </context> <context> <name>HtmlBuilder</name> @@ -671,10 +679,6 @@ Il messaggio È stato inviato. Non inviarlo di nuovo.</translation> <translation>Segna come &spam</translation> </message> <message> - <source>Add spam and remove inbox</source> - <translation>Aggiunge spam e rimuove inbox</translation> - </message> - <message> <source>Mark spam</source> <translation>Segna come spam</translation> </message> @@ -746,6 +750,13 @@ Il messaggio È stato inviato. Non inviarlo di nuovo.</translation> <source>Syncing %1...</source> <translation>Sincronizzazione di %1...</translation> </message> + <message numerus="yes"> + <source>%n message(s) could not be marked as spam: no spam folder is configured for their account.</source> + <translation> + <numerusform>%n messaggio non è stato segnato come spam: nessuna cartella spam è configurata per il suo account.</numerusform> + <numerusform>%n messaggi non sono stati segnati come spam: nessuna cartella spam è configurata per il loro account.</numerusform> + </translation> + </message> <message> <source>Undelete thread</source> <translation>Ripristina conversazione</translation> @@ -861,6 +872,10 @@ Il messaggio È stato inviato. Non inviarlo di nuovo.</translation> <translation>Aggiunge il tag unread a ogni messaggio delle conversazioni selezionate</translation> </message> <message> + <source>Move every message of the selected threads to the spam folder</source> + <translation>Sposta nella cartella spam ogni messaggio delle conversazioni selezionate</translation> + </message> + <message> <source>Mark every message of the selected threads as important</source> <translation>Segna come importante ogni messaggio delle conversazioni selezionate</translation> </message> @@ -877,6 +892,10 @@ Il messaggio È stato inviato. Non inviarlo di nuovo.</translation> <translation>Elimina definitivamente i messaggi selezionati</translation> </message> <message> + <source>Move the selected messages to the spam folder</source> + <translation>Sposta nella cartella spam i messaggi selezionati</translation> + </message> + <message> <source>Edit the rules that tag mail as it arrives</source> <translation>Modifica le regole che etichettano la posta in arrivo</translation> </message> @@ -1309,10 +1328,6 @@ Il messaggio È stato inviato. Non inviarlo di nuovo.</translation> <translation>Sposta fuori dal cestino ogni messaggio delle conversazioni selezionate</translation> </message> <message> - <source>Add spam and remove inbox on the selected threads</source> - <translation>Aggiunge spam e rimuove inbox sulle conversazioni selezionate</translation> - </message> - <message> <source>&Important thread</source> <translation>Conversazione &importante</translation> </message> |