From f051dd657757bca3ac7e36454d7f2fc21f322f73 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Sat, 29 Aug 2026 09:49:29 +0200 Subject: 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). --- src/threaddigest.h | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) (limited to 'src/threaddigest.h') diff --git a/src/threaddigest.h b/src/threaddigest.h index e78a2ac..8c68e16 100644 --- a/src/threaddigest.h +++ b/src/threaddigest.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include "types.h" @@ -41,6 +42,30 @@ struct ThreadDigest int totalCount = 0; + /// Every message's file, RELATIVE to the database path, in the walk's own + /// order. + /// + /// Item 178. Delete and Restore ask whether a row is in the trash, and a + /// CONVERSATION is in the trash only when all of its messages are. + /// ThreadSummary carries one path, which was the right answer while a + /// thread row meant its first message and is not one now, so a partly + /// trashed conversation answered on whichever message the query returned + /// first: Delete hidden on a thread with mail outside the trash, Restore + /// offered on one that mostly is not. + /// + /// Relative and not absolute, for the same reason + /// ThreadSummary::firstMessagePath is: the UI knows an account only by its + /// `maildir`, itself a database-relative prefix, so an absolute path + /// matches no account and silently resolves every row to none. + /// + /// Free, like everything else here: the digest already walks every message + /// for the sender counts, and a filename is served from the INDEX rather + /// than the message file. A message with several files contributes only + /// its first, which is what notmuch_message_get_filename returns; the + /// question is which FOLDER a message lives in, and a caller testing + /// "every path is under trash" is answered correctly by any one of them. + QStringList messagePaths; + /// Always kBuckets entries. A fixed count is what keeps the sparkline's /// geometry testable; a thread spanning five days and one spanning two /// years cannot share a bucket size, so the span is what varies and the -- cgit v1.2.3