summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-09-14 14:11:18 +0200
committerDanilo M. <danix@danix.xyz>2026-09-14 14:11:18 +0200
commitc1c196ebeef9a17defe8a3070e8f10d548ffdf40 (patch)
treeb02cf24e29b1a0aef13c4dfd9fd0b2f0cc1bd136
parent40328fb336ebcec716a39d1f78cdee95b89bc75b (diff)
downloadqtmaildir-c1c196ebeef9a17defe8a3070e8f10d548ffdf40.tar.gz
qtmaildir-c1c196ebeef9a17defe8a3070e8f10d548ffdf40.zip
fix: keep the message bar populated on a spam reply
The spam branch of populateMessageBar() was keyed on the path predicate alone, but its only action is hidden on a reply, so a reply inside an expanded spam conversation lost Reply, Forward and Star. Skip the branch when the reply guard is set, so the ordinary branch populates instead. Extend notSpamIsOfferedInTheSpamView with the message-bar assertions the QAction-only check missed, waiting for the reply row to load first, and mark Task 9's step checkboxes done in the plan.
-rw-r--r--docs/superpowers/plans/2026-09-13-spam-view.md16
-rw-r--r--src/mainwindow.cpp7
-rw-r--r--tests/test_mainwindow.cpp50
3 files changed, 64 insertions, 9 deletions
diff --git a/docs/superpowers/plans/2026-09-13-spam-view.md b/docs/superpowers/plans/2026-09-13-spam-view.md
index ebd8a6a..6c33edc 100644
--- a/docs/superpowers/plans/2026-09-13-spam-view.md
+++ b/docs/superpowers/plans/2026-09-13-spam-view.md
@@ -475,18 +475,18 @@ lupdate-qt6 src/ -ts translations/qtmaildir_it_IT.ts -no-obsolete -locations non
**Behaviour (approved 2026-09-14).** Shown when the selection is in a spam folder, hidden on reply rows and in the trash, like Delete/Restore. Moves each message back to the folder its `moved-from:` tag names, strips `spam` + that origin, adds `inbox` when the destination is the account's inbox. A message with NO origin (provider-caught) falls back to the account's inbox and is reported in the status bar, exactly as `restoreResolvedMessages()` already does for trash. A conversation row acts on the whole conversation, a message row on that message. Undoable, no confirmation, no default shortcut.
-- [ ] **Step 1: Register.** Add `QStringLiteral("not_spam")` to `KeyMap::knownActions()`, no `defaultBindings()` entry. Icon table: `{ QStringLiteral("not_spam"), { QStringLiteral("mail-mark-notjunk"), QString() } }` (`mail-mark-notjunk` ships in Breeze and Adwaita and is unused in the table, so no icon exception is needed).
-- [ ] **Step 2: Action + menus.** `addAction(QStringLiteral("not_spam"), tr("Not s&pam"), ...)` with tip `tr("Move the selected messages out of the spam folder")`; choose a mnemonic free in the Message menu (`&p` is `Re&ply`, `&s` is `Mark &spam`; `noMenuHasTwoEntriesSharingAMnemonic()` must stay green). Add the action to `messageMenu` and `m_threadContextMenu`.
-- [ ] **Step 3: Predicate + gating.** Add `bool MainWindow::everySelectedRowIsInASpamFolder() const` mirroring `everySelectedRowIsInATrashFolder()` but comparing against `account.spam`. In `refreshTrashActions()` compute `const bool inSpam = everySelectedRowIsInASpamFolder();` and set `not_spam` visible with `haveSelection && inSpam && !m_replySelectionHidesDelete`, so it is hidden on a reply row and everywhere outside the spam folder.
-- [ ] **Step 4: Message bar.** In `populateMessageBar()`, add a branch keyed on `everySelectedRowIsInASpamFolder() && !selection empty`, between the trash branch and the draft branch, whose list is `{ not_spam }`.
-- [ ] **Step 5: The three methods.** `notSpamSelected()` mirrors `restoreSelectedFromTrash()` but resolves through the worker with request tag `"not_spam_messages"`; `notSpamThreads()` mirrors `untrashThreads()` with `"not_spam_thread"`. Handle both in `onThreadMessagesResolved()` beside `"restore_messages"` and `"undelete_thread"`, clearing `spam` instead of `deleted`. **Prefer parameterising the existing `restoreResolvedMessages()` and the `undelete_thread` branch with the cleared tag and the undo description over copying them**, so the two scopes cannot drift; if you copy instead, say why.
-- [ ] **Step 6: Tests** in `tests/test_mainwindow.cpp`, `WorkerBackedWindow`, `QTRY_VERIFY_WITH_TIMEOUT`, assertions against the database:
+- [x] **Step 1: Register.** Add `QStringLiteral("not_spam")` to `KeyMap::knownActions()`, no `defaultBindings()` entry. Icon table: `{ QStringLiteral("not_spam"), { QStringLiteral("mail-mark-notjunk"), QString() } }` (`mail-mark-notjunk` ships in Breeze and Adwaita and is unused in the table, so no icon exception is needed).
+- [x] **Step 2: Action + menus.** `addAction(QStringLiteral("not_spam"), tr("Not s&pam"), ...)` with tip `tr("Move the selected messages out of the spam folder")`; choose a mnemonic free in the Message menu (`&p` is `Re&ply`, `&s` is `Mark &spam`; `noMenuHasTwoEntriesSharingAMnemonic()` must stay green). Add the action to `messageMenu` and `m_threadContextMenu`.
+- [x] **Step 3: Predicate + gating.** Add `bool MainWindow::everySelectedRowIsInASpamFolder() const` mirroring `everySelectedRowIsInATrashFolder()` but comparing against `account.spam`. In `refreshTrashActions()` compute `const bool inSpam = everySelectedRowIsInASpamFolder();` and set `not_spam` visible with `haveSelection && inSpam && !m_replySelectionHidesDelete`, so it is hidden on a reply row and everywhere outside the spam folder.
+- [x] **Step 4: Message bar.** In `populateMessageBar()`, add a branch keyed on `everySelectedRowIsInASpamFolder() && !selection empty`, between the trash branch and the draft branch, whose list is `{ not_spam }`.
+- [x] **Step 5: The three methods.** `notSpamSelected()` mirrors `restoreSelectedFromTrash()` but resolves through the worker with request tag `"not_spam_messages"`; `notSpamThreads()` mirrors `untrashThreads()` with `"not_spam_thread"`. Handle both in `onThreadMessagesResolved()` beside `"restore_messages"` and `"undelete_thread"`, clearing `spam` instead of `deleted`. **Prefer parameterising the existing `restoreResolvedMessages()` and the `undelete_thread` branch with the cleared tag and the undo description over copying them**, so the two scopes cannot drift; if you copy instead, say why.
+- [x] **Step 6: Tests** in `tests/test_mainwindow.cpp`, `WorkerBackedWindow`, `QTRY_VERIFY_WITH_TIMEOUT`, assertions against the database:
- mark a message spam, then Not spam: the file returns to its exact original folder, `spam` and `moved-from:` are gone, and `inbox` is back when the origin is the inbox;
- a provider-caught message (in the spam folder, no `moved-from:`) goes to the account's inbox and the status reports that it had no origin;
- visibility: offered in the spam view, hidden on a reply row and in the trash view;
- undo restores it.
-- [ ] **Step 7: Docs + i18n.** Add a clause to the changelog's `[Unreleased]` Added entry, mention the action in `README.md`, run `lupdate-qt6`/`lrelease-qt6`, translate the new strings, and keep `ctest -R translations` green.
-- [ ] **Step 8: Run + commit.** `ctest --test-dir build -R 'keymap|mainwindow|translations'`, then `git commit -S -m "feat: a Not spam action"`.
+- [x] **Step 7: Docs + i18n.** Add a clause to the changelog's `[Unreleased]` Added entry, mention the action in `README.md`, run `lupdate-qt6`/`lrelease-qt6`, translate the new strings, and keep `ctest -R translations` green.
+- [x] **Step 8: Run + commit.** `ctest --test-dir build -R 'keymap|mainwindow|translations'`, then `git commit -S -m "feat: a Not spam action"`.
---
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 68cdde7..2443ead 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -2467,11 +2467,18 @@ void MainWindow::populateMessageBar()
// need no colour to tell them from each other, only from this one.
tinted = { m_actions.value(QStringLiteral("restore")) };
} else if (everySelectedRowIsInASpamFolder()
+ && !m_replySelectionHidesDelete
&& !m_threadView->selectionModel()->selectedRows().isEmpty()) {
// Item 201. Mail in a spam folder affords one thing the ordinary bar
// cannot offer: a way back out. Ordered after the trash branch and
// before the draft one, because a draft thrown into the spam folder is
// no more useful to edit than one thrown into the trash.
+ //
+ // NOT taken on a reply row. The only action here is not_spam and
+ // refreshTrashActions() hides it on a reply, so taking the branch left
+ // the bar EMPTY: a reply inside an expanded spam conversation lost
+ // Reply, Forward and Star. The trash branch needs no such guard because
+ // purge and empty_trash stay visible on a reply.
messageActions = { m_actions.value(QStringLiteral("not_spam")) };
} else if (currentMessageIsADraft()) {
messageActions = { m_actions.value(QStringLiteral("edit_draft")) };
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index 0dfd834..e87843d 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -17429,12 +17429,25 @@ void TestMainWindow::notSpamIsOfferedInTheSpamView()
// 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()));
@@ -17444,10 +17457,20 @@ void TestMainWindow::notSpamIsOfferedInTheSpamView()
auto *view = window.findChild<ThreadListView *>();
auto *queryEdit =
window.findChild<QLineEdit *>(QStringLiteral("queryEdit"));
- QVERIFY(model && view && 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);
@@ -17456,6 +17479,31 @@ void TestMainWindow::notSpamIsOfferedInTheSpamView()
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()