diff options
| -rw-r--r-- | docs/superpowers/plans/2026-09-13-spam-view.md | 16 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 7 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 50 |
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() |