fix: judge a conversation's trash state on all of its messages

Item 178. everySelectedRowIsInATrashFolder() read
ThreadSummary::firstMessagePath for any row that was not a message row.
That was correct while a thread row MEANT that message (item 108) and
stopped being correct when item 177 made it mean the conversation. A
conversation is in the trash only when ALL of its messages are, so a
partly trashed thread answered on whichever message the query returned
first: Delete could be hidden on a conversation that still had mail
outside the trash, and Restore offered on one that mostly did not.

Not data-affecting. Both actions are no-ops in the wrong direction:
Delete on already-trashed mail takes moveMessages()' already-there
branch, and Restore on mail that was never trashed finds nothing to
move.

qtmaildir cannot produce such a thread itself, since Delete is absent on
a reply row and Restore is thread-scoped. Two things outside it can:
another client trashing a single message, and a reply arriving after the
conversation was trashed.

ThreadDigest already walks every message of the selected conversation
for its sender counts, and a filename is served from the index like
everything else in it, so the paths ride along on a request the
selection already makes rather than costing a walk on every query.
ThreadDigest::messagePaths is relative to the mail root, for the reason
firstMessagePath records: an absolute path matches no account and
silently resolves every row to none. MainWindow keeps them beside the
dashboard's thread id and clears them when the dashboard is left, so a
late digest cannot answer about another row.

One limit, stated in the code rather than hidden. The digest is
requested only for a single selected conversation row, so that is the
only case with a real answer; any other selection falls back to the
summary's one path. That fallback IS the pre-177 answer and is wrong in
exactly the same partial case, which is the point: a multi-row selection
is left no worse than it was, rather than given a second, differently
wrong rule of its own. Making it exhaustive costs a per-query walk over
every message, which is what this avoids.

Two tests, both mutation-checked. The worker test puts its two messages
in different folders, since two in one folder answer identically
whichever way the code resolves them. The window test asserts both
directions, so a fix that simply hid Delete everywhere would fail it,
and sets totalCount explicitly: a summary left at the default is a
message row, and the test would otherwise exercise the other branch and
pass for the wrong reason.

Suite: 42 of 43, with undoMovesTheMessageBack failing as it does on
master (item 136).

1 files changed, 55 insertions, 20 deletions

diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 321c608..eb8f4c6 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -3667,29 +3667,53 @@ bool MainWindow::everySelectedRowIsInATrashFolder() const
         return false;
 
     for (const QModelIndex &index : rows) {
-        // The row's own file: a reply row's message, a thread row's displayed
-        // message. Same rule as everySelectedRowHasTag(), and for the same
-        // reason: a thread row acts on the message its card shows.
-        const QString path =
-            m_model->isMessageRow(index)
-                ? m_model->messageAt(index).filePath
-                : m_model->threadFor(index).firstMessagePath;
-        if (path.isEmpty())
-            return false;
+        // A MESSAGE row is its one file. A CONVERSATION is in the trash 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.
+        //
+        // That was the pre-177 rule and it is now wrong in a way that is
+        // invisible: a partly trashed thread answered on whichever message the
+        // query returned first, hiding Delete on a conversation with mail
+        // outside the trash and offering Restore on one that mostly is not.
+        QStringList paths;
+        if (m_model->isMessageRow(index)) {
+            paths = { m_model->messageAt(index).filePath };
+        } else {
+            const ThreadSummary thread = m_model->threadFor(index);
+            // Known only for the single conversation row the dashboard is
+            // showing, since that is the one the digest was requested for.
+            // Anything else falls back to the summary's one path, which is the
+            // pre-177 answer: wrong in the same partial case, so a multi-row
+            // selection is left no worse than it was rather than being given a
+            // second, differently wrong rule of its own.
+            paths = (!m_conversationPaths.isEmpty()
+                     && m_conversationPathsThreadId == thread.threadId)
+                        ? m_conversationPaths
+                        : QStringList{ thread.firstMessagePath };
+        }
 
-        const Account account = accountForMessagePath(path);
-        if (account.maildir.isEmpty() || account.trash.isEmpty())
+        if (paths.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('/');
-        // accountForMessagePath() accepts both shapes, so this must too: a
-        // thread row's path is database-relative and a reply row's absolute.
-        if (!path.contains(prefix))
-            return false;
+        for (const QString &path : std::as_const(paths)) {
+            if (path.isEmpty())
+                return false;
+
+            const Account account = accountForMessagePath(path);
+            if (account.maildir.isEmpty() || account.trash.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('/');
+            // accountForMessagePath() accepts both shapes, so this must too: a
+            // thread row's path is database-relative and a reply row's
+            // absolute.
+            if (!path.contains(prefix))
+                return false;
+        }
     }
     return true;
 }
@@ -4149,6 +4173,10 @@ void MainWindow::onThreadSelected(const QModelIndex &current,
     }
 
     m_dashboardThreadId.clear();
+    // Stale the moment the dashboard is not showing this thread: leaving them
+    // would answer a later conversation's question with an earlier one's paths.
+    m_conversationPathsThreadId.clear();
+    m_conversationPaths.clear();
 
     // The message the card displays, not the thread. The summary's `unread` is
     // a union over the conversation, so this can arm for a thread whose first
@@ -4210,6 +4238,13 @@ void MainWindow::onThreadDigestLoaded(const ThreadDigest &digest,
     if (digest.threadId != m_dashboardThreadId)
         return;
 
+    // Item 178: what Delete and Restore need to judge the CONVERSATION rather
+    // than its first message. Kept beside the thread id so a late digest for
+    // another row cannot answer about this one.
+    m_conversationPathsThreadId = digest.threadId;
+    m_conversationPaths = digest.messagePaths;
+    refreshTrashActions();
+
     m_messageView->showDashboard(digest);
 }