fix: gate spam like delete, and keep one origin in the model

3 files changed, 67 insertions, 9 deletions

diff --git a/src/config.cpp b/src/config.cpp
index 390848a..184c756 100644
--- a/src/config.cpp
+++ b/src/config.cpp
@@ -68,8 +68,8 @@ const QStringList kQueryGenerators = { QStringLiteral("unread"),
                                        QStringLiteral("spam") };
 
 /// The tag a generator matches, for the three filters that are a plain tag
-/// query. Empty for "sent", "drafts" and "trash", which compose from each
-/// account's folder instead and are handled separately.
+/// query. Empty for "sent", "drafts", "trash" and "spam", which compose from
+/// each account's folder instead and are handled separately.
 QString generatorTag(const QString &generator)
 {
     if (generator == QStringLiteral("unread"))
@@ -85,8 +85,9 @@ QString generatorTag(const QString &generator)
 /// user's own message back into the conversation it answers, and "drafts" is
 /// worse: a thread row stands for its first matched message, which for a draft
 /// reply is the message being replied TO, so the draft itself is unreachable.
-/// "trash" stays threaded, since a deleted message still belongs to its
-/// conversation. Closed set, and the one place the three views are decided.
+/// "trash" and "spam" stay threaded, since a deleted or spammed message still
+/// belongs to its conversation. Closed set, and the one place the three views
+/// are decided.
 bool generatorIsFlat(const QString &generator)
 {
     return generator == QStringLiteral("sent")
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 476ff18..4b116ba 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -3931,6 +3931,15 @@ void MainWindow::refreshTrashActions()
                         && !m_replySelectionHidesDelete);
     }
 
+    // Mark spam follows Delete exactly: one reply cannot be moved out of its
+    // conversation, and the trash view does not afford it. Same two hides, ORed
+    // rather than fought over, so it reads the same flag instead of walking the
+    // selection a second time.
+    if (auto *spam = m_actions.value(QStringLiteral("spam"))) {
+        spam->setVisible((!haveSelection || !inTrash)
+                         && !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"))) {
@@ -6466,6 +6475,29 @@ QString MainWindow::originTagFor(const QString &dbRelativeFolder) const
     return QString(kOriginTagPrefix) + accountRelative;
 }
 
+QStringList MainWindow::originTagsToStrip(const QStringList &messageIds,
+                                          const QStringList &added) const
+{
+    const bool writingOrigin =
+        std::any_of(added.cbegin(), added.cend(), [](const QString &tag) {
+            return tag == kOriginTagPlaceholder()
+                   || tag.startsWith(QLatin1String(kOriginTagPrefix));
+        });
+    if (!writingOrigin)
+        return {};
+
+    QStringList stripped;
+    for (const QString &messageId : messageIds) {
+        for (const QString &tag : m_model->messageById(messageId).tags) {
+            if (tag.startsWith(QLatin1String(kOriginTagPrefix))
+                && !added.contains(tag) && !stripped.contains(tag)) {
+                stripped.append(tag);
+            }
+        }
+    }
+    return stripped;
+}
+
 void MainWindow::trashThreads(const QStringList &threadIds)
 {
     if (threadIds.isEmpty())
@@ -7331,6 +7363,11 @@ void MainWindow::sendMove(const QStringList &messageIds,
         if (tag != kOriginTagPlaceholder())
             displayRemove.append(tag);
     }
+    // The model's half of the worker's overwrite rule. This move is writing a
+    // new origin, so every `moved-from:` the model still holds is stale and
+    // goes with it; otherwise the confirmed update below appends the new tag
+    // beside the old one and restoreSelected() reads whichever comes first.
+    displayRemove += originTagsToStrip(messageIds, add);
     // A thread-scoped move already repainted its rows in
     // trashThreads() / untrashThreads(), synchronously, before
     // the worker was asked to resolve the threads at all. Repeating it here
@@ -7586,7 +7623,11 @@ void MainWindow::onMessagesMoved(const QMap<QString, QString> &originByMessageId
         };
 
         const QStringList resolvedAdd = resolve(pending.add);
-        const QStringList resolvedRemove = resolve(pending.remove);
+        QStringList resolvedRemove = resolve(pending.remove);
+        // The confirmed half of the same rule, and the one that matters when
+        // the optimistic update was skipped (a whole-thread move): the new
+        // origin replaces any other the model still holds.
+        resolvedRemove += originTagsToStrip(it.value(), resolvedAdd);
         sendMessageTagChange(it.value(), resolvedAdd, resolvedRemove,
                              pending.description);
 
diff --git a/src/mainwindow.h b/src/mainwindow.h
index a7fbf26..478de2d 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -1107,12 +1107,13 @@ private:
     SelectionKind selectionKind() const;
 
     /// Whether the selection holds a reply row, which is what hides Delete,
-    /// Restore and Archive (item 177).
+    /// Restore, Archive and Mark spam: all conversation-level acts, and a
+    /// single reply cannot be removed from its thread (item 177).
     ///
     /// Written by refreshScopedActionLabels() and read by
-    /// refreshTrashActions(), which runs after it and owns the same two
-    /// actions' visibility. A flag rather than a second walk over the
-    /// selection, so the two cannot answer differently.
+    /// refreshTrashActions(), which runs after it and owns the same actions'
+    /// visibility. A flag rather than a second walk over the selection, so the
+    /// two cannot answer differently.
     bool m_replySelectionHidesDelete = false;
 
     /// Hides Delete on mail already in the trash, and Restore on mail that
@@ -1393,6 +1394,21 @@ private:
     /// and a restore stripped a tag that had never been written.
     QString originTagFor(const QString &dbRelativeFolder) const;
 
+    /// The `moved-from:` tags these messages currently carry that a new origin
+    /// must replace, read from the MODEL.
+    ///
+    /// Mirrors the worker's overwrite rule (NotmuchWorker::applyTags): a
+    /// message carries exactly one origin, so writing a new one strips any
+    /// other. The worker does it against the database; this does it against
+    /// the optimistic model, where restoreSelected() reads the origin back and
+    /// a stale second tag would send the message to the wrong folder.
+    ///
+    /// Returns nothing unless `added` is writing a new origin, which is either
+    /// a resolved `moved-from:` tag or the unresolved placeholder the
+    /// optimistic update drops before painting.
+    QStringList originTagsToStrip(const QStringList &messageIds,
+                                  const QStringList &added) const;
+
     /// The account whose maildir contains `path`, or an invalid account when
     /// no configured maildir does.
     ///