aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-09-14 14:02:21 +0200
committerDanilo M. <danix@danix.xyz>2026-09-14 14:02:21 +0200
commit40328fb336ebcec716a39d1f78cdee95b89bc75b (patch)
tree5564c0b731a8f0af4cf56791ffac04062c6f03dd
parent9fdac493c9a6e35f36dc45ff0e13583ddf88605e (diff)
downloadqtmaildir-40328fb336ebcec716a39d1f78cdee95b89bc75b.tar.gz
qtmaildir-40328fb336ebcec716a39d1f78cdee95b89bc75b.zip
fix: the Not spam review findings
Close the two Important test gaps and fold in the minor notes. notSpamIsAbsentOnAReplyRow passed for the wrong reason: the reply node had no filePath and the selected child was the first message, so the predicate answered false on the empty path and hid the action with or without the reply guard. Give the reply a real spam path and select the actual reply child; mutation-checked that removing the guard now fails the test. Add notSpamThreadMovesEveryMessageHome, the thread-scoped coverage notSpamThreads()/m_pendingThreadScope/wholeThreadIds had none of, and assert the folded thread-scoped trash Restore re-adds the inbox tag. Rename the label Not junk -> Not spam (no free mnemonic in the Message menu) to match the rest of the UI, with the Italian translation updated, and add a changelog line for the thread-scoped Restore inbox-tag fix.
-rw-r--r--CHANGELOG.md8
-rw-r--r--README.md4
-rw-r--r--src/mainwindow.cpp16
-rw-r--r--tests/test_mainwindow.cpp124
-rw-r--r--translations/qtmaildir_it_IT.ts12
5 files changed, 145 insertions, 19 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index fd8d858..0765935 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -47,7 +47,7 @@ point at which they are stable.
- **A spam button on the message pane's bar**, beside Star and Archive. It is
drawn with a bug rather than the theme's junk glyph, and falls back to
`mail-mark-junk` on a theme that ships no bug.
-- **Not junk**, the way back out of the spam folder. Selecting a message in the
+- **Not spam**, the way back out of the spam folder. Selecting a message in the
Spam view offers it on both the Message menu and the message pane's bar, and
it returns each message to the folder its `moved-from:` origin names, or to
the account's inbox when a provider put it there and no origin was recorded,
@@ -124,6 +124,12 @@ point at which they are stable.
- **A skipped sync no longer looks like a successful one.** When a run exits
because another sync already holds the lock, it carried nothing, and the
pending count stays where it was.
+- **A thread-scoped Restore from the trash re-adds the `inbox` tag when it
+ returns mail to an inbox.** The message-scoped restore already did this; the
+ thread-scoped one moved the file back to the inbox folder but left the tag
+ off, so the conversation sat in the folder invisible to the Inbox view until
+ the next hook run. Both routes now share one implementation and both restore
+ the tag.
### Upgrading
diff --git a/README.md b/README.md
index 62bac67..67fb16e 100644
--- a/README.md
+++ b/README.md
@@ -596,7 +596,7 @@ From: account re-seeds it only until you pick one yourself.
Archive, mark-important and toggle-unread write fixed tags. Delete and spam
move the file instead: each adds a fixed tag (`deleted` or `spam`) and records
the folder the file left in a `moved-from:<folder>` origin. Restore (in the
-trash) and Not junk (in the spam folder) are their inverses: each moves the
+trash) and Not spam (in the spam folder) are their inverses: each moves the
message back to the folder its origin names, and mail caught by a provider with
no origin at all goes to the account's inbox, which the status line reports. For
anything else, **Ctrl+T** opens a dialog over the selected threads: type tags to
@@ -783,7 +783,7 @@ Defaults, all rebindable through `[keys]`:
Every action in this table carries a default binding, and every one appears in
a menu. Seven actions carry none, because a chord for them would be arbitrary:
-Check for stranded spam, Empty trash, Delete permanently, Empty spam, Not junk,
+Check for stranded spam, Empty trash, Delete permanently, Empty spam, Not spam,
Edit draft and Save message. They are reachable from the menus, and the shortcut
reference prints them as unbound.
**Help > Keyboard shortcuts** lists the current bindings, generated from the
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 383e257..68cdde7 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -1813,11 +1813,13 @@ void MainWindow::registerActions()
// back to the folder its `moved-from:` tag names, falling back to the
// account's inbox for mail a provider caught, which carries no origin.
//
- // The mnemonic is on "&junk" rather than "spam": Alt+P is already Re&ply
- // and Alt+S is a frozen collision, and no letter of "Not spam" is free in
- // this menu. `junk` is the theme's own name for the folder and what its
- // icon (`mail-mark-notjunk`) draws, so the wording stays honest.
- addAction(QStringLiteral("not_spam"), tr("Not &junk"),
+ // No mnemonic, deliberately: every letter of "Not spam" is already used in
+ // this menu (N, o, t, s, p, a, m), Alt+P and Alt+S are taken by Re&ply and
+ // the frozen Mark &spam collision, and no free letter spells "spam". The
+ // mnemonic test skips an entry that carries none, and the terminology
+ // matches the rest of the UI ("spam folder", the `spam` key) rather than
+ // inventing "junk" for a menu that says spam everywhere else.
+ addAction(QStringLiteral("not_spam"), tr("Not spam"),
tr("Move the selected messages out of the spam folder"), [this]() {
notSpamSelected();
});
@@ -6773,7 +6775,7 @@ void MainWindow::onThreadMessagesResolved(const QStringList &messageIds,
if (requestTag == QStringLiteral("not_spam_thread")) {
restoreResolvedMessages(messageIds, paths, tags,
- QStringLiteral("spam"), tr("Not junk"),
+ QStringLiteral("spam"), tr("Not spam"),
true, false, threadScope);
return;
}
@@ -6844,7 +6846,7 @@ void MainWindow::notSpamMessages(const QStringList &messageIds,
const QStringList &tags)
{
restoreResolvedMessages(messageIds, paths, tags, QStringLiteral("spam"),
- tr("Not junk"), true, false);
+ tr("Not spam"), true, false);
}
void MainWindow::notSpamThreads(const QStringList &threadIds)
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index 66f11c0..0dfd834 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -547,6 +547,7 @@ private slots:
void notSpamIsOfferedInTheSpamView();
void notSpamIsAbsentOnAReplyRow();
void notSpamIsHiddenOutsideTheSpamFolder();
+ void notSpamThreadMovesEveryMessageHome();
void undoOfNotSpamReturnsTheFileToTheSpamFolder();
// ComposeWindow, item 123. These need a window but no worker: the composer
@@ -12818,6 +12819,16 @@ void TestMainWindow::deleteThreadMovesEveryMessageAndRepaintsTheRootCard()
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()
@@ -17468,15 +17479,24 @@ void TestMainWindow::notSpamIsAbsentOnAReplyRow()
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);
- const QModelIndex replyRow = model->index(0, 0, 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);
@@ -17609,4 +17629,106 @@ void TestMainWindow::undoOfNotSpamReturnsTheFileToTheSpamFolder()
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/translations/qtmaildir_it_IT.ts b/translations/qtmaildir_it_IT.ts
index dfd2ff9..f4d7688 100644
--- a/translations/qtmaildir_it_IT.ts
+++ b/translations/qtmaildir_it_IT.ts
@@ -550,6 +550,10 @@ Il messaggio È stato inviato. Non inviarlo di nuovo.</translation>
<translation>Aggiunge o rimuove l&apos;etichetta deleted</translation>
</message>
<message>
+ <source>Not spam</source>
+ <translation>Non spam</translation>
+ </message>
+ <message>
<source>Remove the unread tag from every message of the selected threads</source>
<translation>Rimuove il tag unread da ogni messaggio delle conversazioni selezionate</translation>
</message>
@@ -770,10 +774,6 @@ Il messaggio È stato inviato. Non inviarlo di nuovo.</translation>
<translation>Svuota spam</translation>
</message>
<message>
- <source>Not junk</source>
- <translation>Non spam</translation>
- </message>
- <message>
<source>Undelete thread</source>
<translation>Ripristina conversazione</translation>
</message>
@@ -928,10 +928,6 @@ Il messaggio È stato inviato. Non inviarlo di nuovo.</translation>
<translation>Sposta nella cartella spam i messaggi selezionati</translation>
</message>
<message>
- <source>Not &amp;junk</source>
- <translation>&amp;Non spam</translation>
- </message>
- <message>
<source>Move the selected messages out of the spam folder</source>
<translation>Sposta fuori dalla cartella spam i messaggi selezionati</translation>
</message>