diff options
| author | Danilo M. <danix@danix.xyz> | 2026-09-14 13:46:33 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-09-14 13:46:33 +0200 |
| commit | 9fdac493c9a6e35f36dc45ff0e13583ddf88605e (patch) | |
| tree | ff561fba18bd543306b3166c8eb3591cb1d220e3 | |
| parent | d714483b6027425923340d1bcfa0263b8e7ac0bc (diff) | |
| download | qtmaildir-9fdac493c9a6e35f36dc45ff0e13583ddf88605e.tar.gz qtmaildir-9fdac493c9a6e35f36dc45ff0e13583ddf88605e.zip | |
feat: a Not spam action
Backlog item 201. A message in the Spam view had no way back out: spam is
one-way and Restore is hidden outside the trash. not_spam moves each message
to the folder its moved-from: origin names, falling back to the account's
inbox (reported) for provider-caught mail with no origin.
The action is labelled "Not junk" on the free Alt+J: every letter of "Not
spam" is taken in the Message menu, and Alt+P (Re&ply) and Alt+S (Mark &spam,
frozen) are unavailable.
restoreResolvedMessages() and the undelete_thread branch are parameterised
with the cleared tag and undo description rather than copied, so Delete and
Not spam cannot drift.
| -rw-r--r-- | CHANGELOG.md | 7 | ||||
| -rw-r--r-- | README.md | 16 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 66 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-09-13-spam-view.md | 36 | ||||
| -rw-r--r-- | src/keymap.cpp | 5 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 280 | ||||
| -rw-r--r-- | src/mainwindow.h | 48 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 336 | ||||
| -rw-r--r-- | translations/qtmaildir_it_IT.ts | 12 |
9 files changed, 713 insertions, 93 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index e3bba3b..fd8d858 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,13 @@ 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 + 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, + which the status line reports. It is a move with an undo behind it, so it asks + nothing, and like Mark spam it is hidden on a reply row and everywhere outside + the spam folder. ### Changed @@ -595,9 +595,13 @@ 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. For anything else, -**Ctrl+T** opens a dialog over the selected threads: type tags to add or -remove, separated by commas, or clear a checkbox to drop a tag already present. +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 +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 +add or remove, separated by commas, or clear a checkbox to drop a tag already +present. Both fields complete against every tag in your database, which is a guard against typing `shoppping` beside `shopping`, not a restriction: a tag that does @@ -778,9 +782,9 @@ Defaults, all rebindable through `[keys]`: | `Ctrl+Q` | `quit` | Quit | Every action in this table carries a default binding, and every one appears in -a menu. Six actions carry none, because a chord for them would be arbitrary: -Check for stranded spam, Empty trash, Delete permanently, Empty spam, Edit -draft and Save message. They are reachable from the menus, and the shortcut +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, +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 actions themselves, so it shows your overrides rather than these defaults. diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md index 6792353..9d3979f 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md @@ -274,6 +274,7 @@ taking that too literally. | 199 | The window chrome uses the system icon theme, and the user wants a shipped set | presentation | M-L | open, 2026-09-13, from the notes ("we should ship our own icons, color themeable to be consistent in every theme a user may implement, since icons are a brand identity"). This deliberately REVERSES item 70, which drew the split as "panes are ours, chrome is the system's" and shipped `Marks` for the panes only; the note asks for the other half too, so it is a decision to revisit rather than a defect. Verified: the `themeIcons` table at `mainwindow.cpp:2211` and six `QIcon::fromTheme` sites in `composewindow.cpp` are every chrome icon, all resolved from the desktop theme. The mechanism already exists and is proven, `Marks::pixmap` compositing `fill="currentColor"` with `CompositionMode_SourceIn` so one asset serves a light and a dark palette, and `src/marks.h` records why it is compiled-in string literals rather than a `.qrc`. The size is the ARTWORK, not the code: item 70's six marks are shipped, this is roughly forty actions, each needing a drawing. Needs a decision from the user on scope before it can be sized honestly, and on whether the system theme stays as a fallback for an action with no shipped icon | | 200 | qtmaildir cannot be launched at a given account, thread or message | workflow | M | open, **specified 2026-09-13** in `specs/2026-09-13-cli-selectors-design.md`; read that rather than this row. The user settled three things: a second launch STEERS the running window over a `QLocalServer` rather than opening a second one, the selectors are `--account`/`--thread`/`--message` (`--query` dropped as the one with no caller), and a selector matching nothing opens the window normally and says so in the status bar. The design shrank on one side and grew on the other: `recoverStaleThread()` already runs `thread:<id>` with a deferred selection and is reused as a third caller, so the selectors are the small half, while the socket (connect-first ordering, stale-socket recovery, a degrade path when no socket is possible) is the real work and adds `Qt6::Network` to the component list. Original entry: open, 2026-09-13, from the notes ("the program should accept cli parameters like `--account` or `--thread`/`--message`, so that another app can launch qtmaildir opening that account's inbox or a certain message/thread"). Verified: `main.cpp:38-66` hand-rolls a `strcmp` loop over `argv` for `--version` and `--help` only, both answering before `QApplication` exists, which is deliberate and documented. Parsing is the small half and `QCommandLineParser` covers it; the item is bigger than it looks for two reasons. There is NO single-instance mechanism (no `QLocalServer` anywhere in `src/`), so a second launch opens a second window against the same notmuch database rather than steering the running one, and notmuch permits only one open handle per process. And the selector has to reach a query the startup path does not currently take, since `--thread` names a row that may not be in the configured startup view at all. Needs a decision from the user first: whether a second launch should focus the running window (which is the useful behaviour for "another app launches qtmaildir" and is the whole cost of the item) or simply start with a different query | | 197 | No way to say a message is not spam | workflow | S | open, 2026-09-10, split out of the 187 design at the user's decision rather than built into it. Restore already covers what qtmaildir moved: a message it marked carries `moved-from:` and goes back where it came from. The gap is mail the PROVIDER's filter caught, which was never in an inbox and carries no origin tag, so "not spam" has no recorded destination to return it to. Needs two answers before it can be planned: where such a message goes (the account's inbox is the obvious guess and is a guess), and whether anything should tell the provider its filter was wrong, which is network work this application does not do and would belong in a sidecar like item 194's. No seam is needed in the meantime: `sendMove()` already takes any destination and any tags | +| 201 | A message in the Spam view cannot be un-spammed, even one qtmaildir put there | defect | S | open, 2026-09-14, found while testing the `spam-view` branch. Corrects item 197's claim that Restore already covers what qtmaildir moved: the CAPABILITY does (`restoreSelectedFromTrash()` reads the origin from the database), but no SURFACE offers it for spam, so only Ctrl+Z immediately after the move reverses it. The user asked for it to be built on `spam-view` before that branch merges. See the section | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -1632,6 +1633,12 @@ destination and any tag lists, so a Not-spam action is a caller rather than a capability. Provisioning for it now would be a hook with one hypothetical caller, which is what YAGNI names. +**Corrected 2026-09-14:** the interface half is item 201. Restore's visibility +is coupled to `everySelectedRowIsInATrashFolder()`, which by design never +answers for spam, so "Restore already covers what qtmaildir moved" was a +capability claim that no surface offered. Read item 201 for the narrower, +decision-free half. + ## 198. The unsynced-changes list never says which account a message belongs to **Observed (user, from the notes):** "when clicking on the bottom right status @@ -1766,3 +1773,62 @@ in the notes. `--thread` value reaches a notmuch query, so it goes through `SearchTerm`'s quoting like every other query this application builds, rather than being concatenated at the call site. + +## 201. A message in the Spam view cannot be un-spammed, even one qtmaildir put there + +**Observed (user, 2026-09-14, testing the `spam-view` branch).** "If a message +is in spam, how do I unmark it spam?" The only built-in answer found was Ctrl+Z +immediately after the move, before any other action; an edit of the `spam` tag +alone leaves the file in the spam folder. The user asked for this to be built on +`spam-view` before that branch merges. + +**Cause.** Verified in `src/`. Three facts together: + +1. `spam` is one-way, not a toggle. `MainWindow::spamSelected()` always resolves + the selection and calls `spamMessages()`/`spamThreads()`; unlike `delete` + (`mainwindow.cpp:1745`) and `flag` (`mainwindow.cpp:1832`) it never asks + `everySelectedRowHasTag("spam")`, so pressing Mark spam again moves the file + toward the folder it is already in rather than reverting it. +2. Restore is hidden outside the trash. `refreshTrashActions()` + (`mainwindow.cpp:3936`) sets `restore`'s visibility to + `(!haveSelection || inTrash) && !m_replySelectionHidesDelete`, where `inTrash` + is `everySelectedRowIsInATrashFolder()`, which compares `account.trash` only + BY DESIGN so the predicate never answers for spam (the spec requires that, so + Delete is not hidden and Purge not offered there). Restore is coupled to the + same predicate, which is what hid it in the Spam view. +3. The write side already works. `restoreSelectedFromTrash()` + (`mainwindow.cpp:6907`) resolves the origin from the DATABASE and calls + `sendMove()`, which takes any destination and any tag lists. Only the entry + point and the gating are missing. + +So item 197's "Restore already covers what qtmaildir moved" is true of the +function and false of the interface. Its stated gap was the PROVIDER-caught +message with no origin; this item is the narrower half, mail qtmaildir itself +moved and can put back, and it needs no new decision. + +**Approach.** One of two, and the difference is what the user should decide: + +- **Widen Restore into the Spam view**, decoupling Restore's visibility from + `everySelectedRowIsInATrashFolder()` so a message carrying a `moved-from:` + origin can be restored from spam too. Smallest change; makes one action serve + both folders. +- **Add a distinct `not_spam` action** shown on a spam-folder selection, doing + the existing restore and also stripping `spam`. Clearer on the bar and in the + menu, and costs the five registration places the rule names + (`KeyMap::knownActions()`, `defaultBindings()` optional, the icon table, the + action, a menu). + +Either reuses `sendMove()` and resolves the origin from the database, never the +model (the message-scoped `restoreSelected()` reads the model, the path item 176 +and the spam-view final review warn against). + +**Constraints.** A move's origin is resolved by the worker, never the model +(`CLAUDE.md`). An undo covers what the write CHANGED, not what it asked for +(item 176). No confirmation: this is a move and it is undoable. Item 197's open +questions, where PROVIDER-caught mail with no origin goes (the inbox guess) and +whether to tell the provider its filter was wrong, stay out of scope here. + +**Verification.** A `WorkerBackedWindow` test using `QTRY_VERIFY_WITH_TIMEOUT`: +mark a message spam, then un-spam it through the new path, asserting the file +returns to the folder it came from, `spam` and `moved-from:` are gone, and the +row leaves the Spam view. Because it is a move, assert the undo as well. diff --git a/docs/superpowers/plans/2026-09-13-spam-view.md b/docs/superpowers/plans/2026-09-13-spam-view.md index f6bec47..ebd8a6a 100644 --- a/docs/superpowers/plans/2026-09-13-spam-view.md +++ b/docs/superpowers/plans/2026-09-13-spam-view.md @@ -460,9 +460,39 @@ lupdate-qt6 src/ -ts translations/qtmaildir_it_IT.ts -no-obsolete -locations non --- +### Task 9: Not spam + +**Files:** +- Modify: `src/keymap.cpp` (`knownActions()`) +- Modify: `src/mainwindow.h` (`notSpamSelected()`, `notSpamMessages()`, `notSpamThreads()`, `everySelectedRowIsInASpamFolder()`) +- Modify: `src/mainwindow.cpp` (action, icon table, both menus, message-bar spam branch, gating, the three methods, the `not_spam_*` resolution branches) +- Modify: `tests/test_mainwindow.cpp` +- Modify: `CHANGELOG.md`, `README.md`, `translations/qtmaildir_it_IT.ts` + +**Interfaces:** +- Consumes: `Account::spam`, `accountForMessagePath()`, `originTagFor()`, `sendMove()`, the `resolveMessages`/`resolveThreadMessages` worker calls, `kOriginTagPrefix`, `m_replySelectionHidesDelete`. +- Produces: `MainWindow::notSpamSelected()`, `notSpamMessages(const QStringList &, const QStringList &, const QStringList &)`, `notSpamThreads(const QStringList &)`, `bool MainWindow::everySelectedRowIsInASpamFolder() const`. + +**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: + - 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"`. + +--- + ## Self-review -- **Spec coverage:** config key + queries (Task 1), the Spam filter (Task 2), origin rename (Task 3), the move action (Task 4), message bar + icon (Task 5), Empty Spam (Task 6), stranded cleanup (Task 7), translations/docs (Task 8). The trash-predicate test is in Task 4 Step 6. Item 196 (abusectl auto-tagging) and item 197 (not-spam) are out of scope by the spec. -- **Type consistency:** `spamQuery`/`allSpamQuery`/`isShowingSpam`/`spamSelected`/`spamMessages`/`spamThreads`/`emptySpam`/`showStrandedSpamMail`/`kOriginTagPrefix` are used with one spelling throughout. -- **Decided:** Empty Spam removes `spam`, `unread` and `inbox` (user confirmed `unread`), and adds `deleted` + the origin placeholder. It carries no confirmation dialog and no default shortcut; it is a move and is undoable. +- **Spec coverage:** config key + queries (Task 1), the Spam filter (Task 2), origin rename (Task 3), the move action (Task 4), message bar + icon (Task 5), Empty Spam (Task 6), stranded cleanup (Task 7), translations/docs (Task 8), Not spam (Task 9, added 2026-09-14 from backlog item 201 after the branch's final review). The trash-predicate test is in Task 4 Step 6. Item 196 (abusectl auto-tagging) and item 197's provider-notification half are out of scope by the spec; item 197's destination question is settled as the inbox fallback in Task 9. +- **Type consistency:** `spamQuery`/`allSpamQuery`/`isShowingSpam`/`spamSelected`/`spamMessages`/`spamThreads`/`emptySpam`/`showStrandedSpamMail`/`notSpamSelected`/`notSpamMessages`/`notSpamThreads`/`everySelectedRowIsInASpamFolder`/`kOriginTagPrefix` are used with one spelling throughout. +- **Decided:** Empty Spam removes `spam`, `unread` and `inbox` (user confirmed `unread`), and adds `deleted` + the origin placeholder. It carries no confirmation dialog and no default shortcut; it is a move and is undoable. Not spam falls back to the account's inbox for provider-caught mail, at the user's decision on 2026-09-14. - **Known blast radius:** Task 3 renames a string literal in ~35 test sites; `grep -rn "deleted-from" src tests` must be empty afterwards. diff --git a/src/keymap.cpp b/src/keymap.cpp index d62ba3e..97399e2 100644 --- a/src/keymap.cpp +++ b/src/keymap.cpp @@ -54,6 +54,11 @@ QStringList KeyMap::knownActions() // is menu-reachable rather than broken. QStringLiteral("empty_spam"), QStringLiteral("spam"), + // Item 201. Mark spam's inverse: a message in the spam folder had no + // way back. No default binding, matching empty_spam: moving mail out of + // a folder is reached from a menu, and since item 132 an unbound action + // is menu-reachable rather than broken. + QStringLiteral("not_spam"), QStringLiteral("toggle_unread"), QStringLiteral("mark_all_read"), QStringLiteral("edit_tags"), diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 4b116ba..383e257 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -1809,6 +1809,18 @@ void MainWindow::registerActions() tr("Move the selected messages to the spam folder"), [this]() { spamSelected(); }); + // Item 201. Mark spam's inverse, and Restore's twin: it moves each message + // 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"), + tr("Move the selected messages out of the spam folder"), [this]() { + notSpamSelected(); + }); // 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 // the config's [keys] section, and `flagged` is a notmuch tag that neomutt, @@ -2111,6 +2123,11 @@ const QHash<QString, QPair<QString, QString>> kThemeIcons = { // `bug` first, per the user's choice, with the standard junk name behind // it so a theme without the bug still draws a junk icon. { QStringLiteral("spam"), { QStringLiteral("bug"), QStringLiteral("mail-mark-junk") } }, + // Item 201. Mark spam's inverse. `mail-mark-notjunk` is the theme's own + // name for taking a message back out of the junk folder, it ships in Breeze + // and Adwaita, and nothing else in this table uses it, so no + // noTwoActionsShareAnIcon() exception is needed. + { QStringLiteral("not_spam"), { QStringLiteral("mail-mark-notjunk"), QString() } }, // A STAR, which is what the user asked for and what every other mail // client draws for this. NOT mail-mark-important: Breeze draws that // as an exclamation mark, so on the icon-only message bar the action @@ -2218,6 +2235,9 @@ void MainWindow::buildMenus() // that it exists. messageMenu->addAction(m_actions.value(QStringLiteral("purge"))); messageMenu->addAction(m_actions.value(QStringLiteral("spam"))); + // Beside Mark spam, whose inverse it is, and hidden everywhere Mark spam + // cannot be undone from: outside the spam folder it means nothing. + messageMenu->addAction(m_actions.value(QStringLiteral("not_spam"))); messageMenu->addSeparator(); messageMenu->addAction(m_actions.value(QStringLiteral("toggle_unread"))); messageMenu->addAction(m_actions.value(QStringLiteral("mark_all_read"))); @@ -2296,6 +2316,7 @@ void MainWindow::buildMenus() m_threadContextMenu->addAction(m_actions.value(QStringLiteral("delete"))); m_threadContextMenu->addAction(m_actions.value(QStringLiteral("restore"))); m_threadContextMenu->addAction(m_actions.value(QStringLiteral("spam"))); + m_threadContextMenu->addAction(m_actions.value(QStringLiteral("not_spam"))); m_threadContextMenu->addSeparator(); m_threadContextMenu->addAction(m_actions.value(QStringLiteral("toggle_unread"))); m_threadContextMenu->addAction(m_actions.value(QStringLiteral("flag"))); @@ -2443,6 +2464,13 @@ void MainWindow::populateMessageBar() // Restore alone. The two purges are the same act at two scopes and // need no colour to tell them from each other, only from this one. tinted = { m_actions.value(QStringLiteral("restore")) }; + } else if (everySelectedRowIsInASpamFolder() + && !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. + messageActions = { m_actions.value(QStringLiteral("not_spam")) }; } else if (currentMessageIsADraft()) { messageActions = { m_actions.value(QStringLiteral("edit_draft")) }; } else { @@ -3853,7 +3881,7 @@ void MainWindow::showThreadContextMenu(const QPoint &pos) m_threadContextMenu->popup(m_threadView->viewport()->mapToGlobal(pos)); } -bool MainWindow::everySelectedRowIsInATrashFolder() const +bool MainWindow::everySelectedRowIsInAFolder(QString Account::*folder) const { const QModelIndexList rows = m_threadView->selectionModel()->selectedRows(); @@ -3861,7 +3889,7 @@ bool MainWindow::everySelectedRowIsInATrashFolder() const return false; for (const QModelIndex &index : rows) { - // A MESSAGE row is its one file. A CONVERSATION is in the trash only + // A MESSAGE row is its one file. A CONVERSATION is in the folder only // when ALL of its messages are (item 178), so it answers on every path // the digest reported rather than on the one the summary carries. // @@ -3894,14 +3922,15 @@ bool MainWindow::everySelectedRowIsInATrashFolder() const return false; const Account account = accountForMessagePath(path); - if (account.maildir.isEmpty() || account.trash.isEmpty()) + const QString named = account.*folder; + if (account.maildir.isEmpty() || named.isEmpty()) return false; // Compared as a path segment, never with startsWith(): `trash-old` // starts with `trash` and is a different folder. The same trap the // attachment-save check records. const QString prefix = account.maildir + QLatin1Char('/') - + account.trash + QLatin1Char('/'); + + named + QLatin1Char('/'); // accountForMessagePath() accepts both shapes, so this must too: a // thread row's path is database-relative and a reply row's // absolute. @@ -3912,9 +3941,20 @@ bool MainWindow::everySelectedRowIsInATrashFolder() const return true; } +bool MainWindow::everySelectedRowIsInATrashFolder() const +{ + return everySelectedRowIsInAFolder(&Account::trash); +} + +bool MainWindow::everySelectedRowIsInASpamFolder() const +{ + return everySelectedRowIsInAFolder(&Account::spam); +} + void MainWindow::refreshTrashActions() { const bool inTrash = everySelectedRowIsInATrashFolder(); + const bool inSpam = everySelectedRowIsInASpamFolder(); const bool haveSelection = !m_threadView->selectionModel()->selectedRows().isEmpty(); @@ -3940,6 +3980,16 @@ void MainWindow::refreshTrashActions() && !m_replySelectionHidesDelete); } + // Item 201. Not spam is Restore's twin, but scoped to the SPAM folder + // rather than the trash: shown when the selection is in a spam folder, and + // hidden on a reply row by the same flag Delete reads. Hidden rather than + // disabled, like Mark spam beside it: offering it where it does not apply + // teaches nothing. + if (auto *notSpam = m_actions.value(QStringLiteral("not_spam"))) { + notSpam->setVisible(haveSelection && inSpam + && !m_replySelectionHidesDelete); + } + // The mirror, which shipped beside it: Restore was added unconditionally // to both menus and so was offered on mail that was never deleted. if (auto *restore = m_actions.value(QStringLiteral("restore"))) { @@ -6710,77 +6760,36 @@ void MainWindow::onThreadMessagesResolved(const QStringList &messageIds, } if (requestTag == QStringLiteral("restore_messages")) { - restoreResolvedMessages(messageIds, paths, tags); + restoreResolvedMessages(messageIds, paths, tags, + QStringLiteral("deleted"), tr("Restore"), + true, false); return; } - if (requestTag != QStringLiteral("undelete_thread")) + if (requestTag == QStringLiteral("not_spam_messages")) { + notSpamMessages(messageIds, paths, tags); return; - - // Restore, resolved per message: each one goes back to the folder its own - // `moved-from:` tag names, so a thread whose messages were deleted from - // different folders reassembles correctly rather than collapsing into one. - const QString prefix = QString::fromLatin1(kOriginTagPrefix); - QHash<QString, QStringList> byOrigin; - QStringList unknown; - for (int i = 0; i < messageIds.size(); ++i) { - // Split on TAB, matching resolveThreadMessages(). A space is not a - // safe separator: a folder name containing one produces a tag - // containing one, and splitting there silently truncates the origin - // to its first word. - const QStringList messageTags = - tags.at(i).split(QLatin1Char('\t'), Qt::SkipEmptyParts); - QString origin; - for (const QString &tag : messageTags) { - if (tag.startsWith(prefix)) { - origin = tag.mid(prefix.length()); - break; - } - } - // A message with no `deleted` tag is not in the trash and has nothing - // to come back from. A thread-scoped restore reaches every message, - // including ones the user never deleted, and moving those would drag - // untouched mail out of whatever folder it legitimately sits in. - if (!messageTags.contains(QStringLiteral("deleted"))) - continue; - const Account account = - accountForMessagePath(paths.at(i)); - if (origin.isEmpty() || account.maildir.isEmpty()) { - unknown.append(messageIds.at(i)); - continue; - } - byOrigin[account.maildir + QLatin1Char('/') + origin] - .append(messageIds.at(i)); } - if (!unknown.isEmpty()) { - // No origin recorded: deleted by an older version or tagged by hand. - // The tag comes off so the row stops claiming to be deleted, but no - // file moves, since guessing a folder would put the message somewhere - // the user never had it. - auto *command = new MessageTagCommand(this, unknown, {}, - { QStringLiteral("deleted") }, - tr("Undelete thread")); - awaitTagConfirmation(command); - sendMessageTagChange(unknown, {}, { QStringLiteral("deleted") }, - tr("Undelete thread")); - m_undoStack.push(command); + if (requestTag == QStringLiteral("not_spam_thread")) { + restoreResolvedMessages(messageIds, paths, tags, + QStringLiteral("spam"), tr("Not junk"), + true, false, threadScope); + return; } - for (auto it = byOrigin.cbegin(); it != byOrigin.cend(); ++it) { - // The origin tag is named here, not left as the placeholder: on a - // restore the placeholder would resolve to the folder the message is - // coming FROM, which is the trash, and strip a tag never written. - const QString origin = originTagFor(it.key()); - QStringList remove{ QStringLiteral("deleted") }; - if (!origin.isEmpty()) - remove.append(origin); - sendMove(it.value(), it.key(), {}, remove, tr("Undelete thread"), - false, threadScope); - } + if (requestTag != QStringLiteral("undelete_thread")) + return; - showTransientStatus(tr("%1: %n message(s)", "", messageIds.size()) - .arg(tr("Undelete thread"))); + // Restore, resolved per message: each one goes back to the folder its own + // `moved-from:` tag names, so a thread whose messages were deleted from + // different folders reassembles correctly rather than collapsing into one. + // + // The same walk serves Not spam and both message-scoped callers; the flags + // say what a message with no origin gets, so the four paths cannot drift. + restoreResolvedMessages(messageIds, paths, tags, + QStringLiteral("deleted"), tr("Undelete thread"), + false, true, threadScope); } void MainWindow::untrashThreads(const QStringList &threadIds) @@ -6799,6 +6808,62 @@ void MainWindow::untrashThreads(const QStringList &threadIds) Q_ARG(QString, QStringLiteral("undelete_thread"))); } +void MainWindow::notSpamSelected() +{ + const QModelIndexList rows = + m_threadView->selectionModel()->selectedRows(); + if (rows.isEmpty()) + return; + + // Mark spam's inverse, resolved the same per-row way (item 177): a + // conversation row moves its whole conversation, a thread of one moves its + // message. Both halves run, because a selection really can hold one of + // each; they travel different routes for the reason spamSelected() + // records. + const ActionScope scope = m_model->scopeForSelection(rows); + + if (!scope.threadIds.isEmpty()) + notSpamThreads(scope.threadIds); + + if (scope.messageIds.isEmpty()) + return; + + // Resolved by the WORKER, not read from the model. A row whose move into + // the spam folder has not been re-queried still carries its pre-move tags, + // so reading the origin off the model would find none and send the message + // to the inbox instead of the folder it came from. The same reasoning + // restoreSelectedFromTrash() records. + QMetaObject::invokeMethod( + m_worker, "resolveMessages", Qt::QueuedConnection, + Q_ARG(QStringList, scope.messageIds), + Q_ARG(QString, QStringLiteral("not_spam_messages"))); +} + +void MainWindow::notSpamMessages(const QStringList &messageIds, + const QStringList &paths, + const QStringList &tags) +{ + restoreResolvedMessages(messageIds, paths, tags, QStringLiteral("spam"), + tr("Not junk"), true, false); +} + +void MainWindow::notSpamThreads(const QStringList &threadIds) +{ + if (threadIds.isEmpty()) + return; + + // Repainted synchronously, as the mark-spam direction is: the card is what + // the user watches, and it must not wait for the worker round trip. + for (const QString &threadId : threadIds) + m_model->applyTagChange(threadId, {}, { QStringLiteral("spam") }); + + m_pendingThreadScope = threadIds; + QMetaObject::invokeMethod( + m_worker, "resolveThreadMessages", Qt::QueuedConnection, + Q_ARG(QStringList, threadIds), + Q_ARG(QString, QStringLiteral("not_spam_thread"))); +} + QString MainWindow::inboxFolderFor(const Account &account) const { // Discovered from the account's OWN inbox query, never hardcoded. @@ -6829,7 +6894,12 @@ QString MainWindow::inboxFolderFor(const Account &account) const void MainWindow::restoreResolvedMessages(const QStringList &messageIds, const QStringList &paths, - const QStringList &tags) + const QStringList &tags, + const QString &clearedTag, + const QString &description, + bool moveUnoriginToInbox, + bool skipUnmarked, + const QStringList &wholeThreadIds) { if (messageIds.size() != paths.size() || messageIds.size() != tags.size()) return; @@ -6837,11 +6907,24 @@ void MainWindow::restoreResolvedMessages(const QStringList &messageIds, const QString prefix = QString::fromLatin1(kOriginTagPrefix); QHash<QString, QStringList> byOrigin; QHash<QString, QStringList> byInbox; + QStringList unmarked; QStringList stranded; for (int i = 0; i < messageIds.size(); ++i) { + // Split on TAB, matching resolveThreadMessages(). A space is not a + // safe separator: a folder name containing one produces a tag + // containing one, and splitting there silently truncates the origin + // to its first word. const QStringList messageTags = tags.at(i).split(QLatin1Char('\t'), Qt::SkipEmptyParts); + + // A thread-scoped restore reaches every message, including ones that + // never moved, so it skips the ones that do not carry the tag this + // move clears. The message-scoped callers selected mail that is + // demonstrably IN the folder, so they ask nothing. + if (skipUnmarked && !messageTags.contains(clearedTag)) + continue; + QString origin; for (const QString &tag : messageTags) { if (tag.startsWith(prefix)) { @@ -6851,30 +6934,53 @@ void MainWindow::restoreResolvedMessages(const QStringList &messageIds, } const Account account = accountForMessagePath(paths.at(i)); - if (account.maildir.isEmpty()) { - stranded.append(messageIds.at(i)); - continue; - } - if (origin.isEmpty()) { - // Trashed by another client, so there is no record of where it - // belongs. Inbox is the documented fallback, and it is reported: - // a guess the user is not told about is worse than the guess. - byInbox[account.maildir + QLatin1Char('/') - + account.inboxFolder()] - .append(messageIds.at(i)); - continue; + if (!moveUnoriginToInbox) { + // The thread-scoped restore out of the TRASH is also reached from + // Delete's toggle on stranded mail that sits outside the trash, so + // a message with no origin (or no account) only has its tag taken + // off: guessing a folder would put it somewhere the user never had + // it. + if (origin.isEmpty() || account.maildir.isEmpty()) { + unmarked.append(messageIds.at(i)); + continue; + } + } else { + if (account.maildir.isEmpty()) { + stranded.append(messageIds.at(i)); + continue; + } + if (origin.isEmpty()) { + // In the folder with no record of where it came from, usually + // because a provider put it there. Inbox is the documented + // fallback, and it is reported: a guess the user is not told + // about is worse than the guess. + byInbox[account.maildir + QLatin1Char('/') + + account.inboxFolder()] + .append(messageIds.at(i)); + continue; + } } byOrigin[account.maildir + QLatin1Char('/') + origin] .append(messageIds.at(i)); } + if (!unmarked.isEmpty()) { + // The tag comes off so the row stops claiming to be there, but no file + // moves. The undo is the same tag write, pushed like any other. + auto *command = new MessageTagCommand(this, unmarked, {}, + { clearedTag }, description); + awaitTagConfirmation(command); + sendMessageTagChange(unmarked, {}, { clearedTag }, description); + m_undoStack.push(command); + } + for (auto it = byOrigin.cbegin(); it != byOrigin.cend(); ++it) { // The origin tag is named here rather than left as the placeholder, // which onMessagesMoved() would resolve to the folder the message is - // coming FROM, namely the trash. + // coming FROM, namely the folder it is leaving. const QString origin = originTagFor(it.key()); - QStringList remove{ QStringLiteral("deleted") }; + QStringList remove{ clearedTag }; if (!origin.isEmpty()) remove.append(origin); @@ -6913,7 +7019,8 @@ void MainWindow::restoreResolvedMessages(const QStringList &messageIds, break; } - sendMove(it.value(), it.key(), add, remove, tr("Restore")); + sendMove(it.value(), it.key(), add, remove, description, false, + wholeThreadIds); } for (auto it = byInbox.cbegin(); it != byInbox.cend(); ++it) { @@ -6921,9 +7028,16 @@ void MainWindow::restoreResolvedMessages(const QStringList &messageIds, // message with no origin tag, and the folder it names is the // account's own inbox. So the tag always comes with it. sendMove(it.value(), it.key(), { QStringLiteral("inbox") }, - { QStringLiteral("deleted") }, tr("Restore")); + { clearedTag }, description, false, wholeThreadIds); } + // A thread-scoped move is announced as a count, matching the delete and + // mark-spam directions. The message-scoped callers leave the bar's own + // status alone. + if (!wholeThreadIds.isEmpty()) { + showTransientStatus(tr("%1: %n message(s)", "", messageIds.size()) + .arg(description)); + } if (!byInbox.isEmpty()) { m_statusLabel->setText( tr("%n message(s) had no record of where they came from and were " diff --git a/src/mainwindow.h b/src/mainwindow.h index 478de2d..c3395c9 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -1130,6 +1130,17 @@ private: /// Delete on exactly the mail a trash view is full of. bool everySelectedRowIsInATrashFolder() const; + /// Whether every selected row's file already sits in its account's spam + /// folder. Empty selection answers false. The spam twin of + /// everySelectedRowIsInATrashFolder(), and asked of the PATH for the same + /// reason: provider-caught mail in the spam folder carries no `spam` tag. + bool everySelectedRowIsInASpamFolder() const; + + /// The one walk both predicates above share: \p folder names the Account + /// member (`&Account::trash` or `&Account::spam`) whose folder is asked + /// about. + bool everySelectedRowIsInAFolder(QString Account::*folder) const; + /// Opens the tag dialog on the current selection and applies its result. /// /// The only route to an arbitrary tag: every other tag action writes a @@ -1276,6 +1287,22 @@ private: /// this asks the worker and finishes in onThreadMessagesResolved(). void spamThreads(const QStringList &threadIds); + /// Moves each selected row's message back OUT of the spam folder, to the + /// folder its `moved-from:` tag names. Mark spam's inverse and Restore's + /// twin, offered only on a selection that is in a spam folder. + void notSpamSelected(); + + /// The half of notSpamSelected() that does the work, given the tags and + /// paths the WORKER reported. Forwards to restoreResolvedMessages() with + /// `spam` as the cleared tag, so the two restore directions cannot drift. + void notSpamMessages(const QStringList &messageIds, + const QStringList &paths, + const QStringList &tags); + + /// Moves every message of the named THREADS out of their accounts' spam + /// folders. Asynchronous like spamThreads(), and for the same reason. + void notSpamThreads(const QStringList &threadIds); + /// Runs the thread-scoped delete once the worker has resolved the /// threads to messages. void onThreadMessagesResolved(const QStringList &messageIds, @@ -1360,9 +1387,28 @@ private: /// Moves each resolved message home, using the tags and paths the WORKER /// reported rather than anything the model holds. + /// + /// One walk serves all four restore routes: Delete out of the trash + /// (message and thread scoped) and Not spam out of the spam folder + /// (message and thread scoped). \p clearedTag is the folder's marker tag + /// (`deleted` or `spam`) stripped by the move; \p description names the + /// action in the status line and on the undo entry. + /// + /// \p moveUnoriginToInbox picks what a message with no `moved-from:` + /// origin gets: the inbox, reported (the message-scoped routes and Not + /// spam, where the mail is demonstrably in the folder and must come out), + /// or a tag-only removal with no file move (the thread-scoped TRASH route, + /// which Delete's toggle also reaches on stranded mail sitting outside the + /// trash). \p skipUnmarked drops messages that do not carry \p clearedTag, + /// which only the thread-scoped trash route needs. void restoreResolvedMessages(const QStringList &messageIds, const QStringList &paths, - const QStringList &tags); + const QStringList &tags, + const QString &clearedTag, + const QString &description, + bool moveUnoriginToInbox, + bool skipUnmarked, + const QStringList &wholeThreadIds = {}); /// The messages a resolveMessages() request was made for. QStringList m_pendingRestoreIds; diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index b9a8f0f..66f11c0 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -540,6 +540,15 @@ private slots: 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 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 @@ -17273,4 +17282,331 @@ void TestMainWindow::restoringAfterTwoMovesReturnsToTheLatestOrigin() "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. + 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."))); + 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); + auto *notSpam = window.findChild<QAction *>(QStringLiteral("not_spam")); + QVERIFY(notSpam); + + 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"); +} + +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; + MessageNode reply; + reply.messageId = QStringLiteral("m2"); + reply.threadId = QStringLiteral("t1"); + reply.depth = 1; + 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); + 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); +} + #include "test_mainwindow.moc" diff --git a/translations/qtmaildir_it_IT.ts b/translations/qtmaildir_it_IT.ts index 53a9dc3..dfd2ff9 100644 --- a/translations/qtmaildir_it_IT.ts +++ b/translations/qtmaildir_it_IT.ts @@ -770,6 +770,10 @@ 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> @@ -924,6 +928,14 @@ Il messaggio È stato inviato. Non inviarlo di nuovo.</translation> <translation>Sposta nella cartella spam i messaggi selezionati</translation> </message> <message> + <source>Not &junk</source> + <translation>&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> + <message> <source>Edit the rules that tag mail as it arrives</source> <translation>Modifica le regole che etichettano la posta in arrivo</translation> </message> |
