diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-29 09:49:29 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-29 09:49:29 +0200 |
| commit | f051dd657757bca3ac7e36454d7f2fc21f322f73 (patch) | |
| tree | 5804923ed3eb747ee2cd84284e1b36fe0817ab8e /docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | |
| parent | d128919ad80ecc37b1f58d2ba83a7487362c126f (diff) | |
| download | qtmaildir-f051dd657757bca3ac7e36454d7f2fc21f322f73.tar.gz qtmaildir-f051dd657757bca3ac7e36454d7f2fc21f322f73.zip | |
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).
Diffstat (limited to 'docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 66 |
1 files changed, 66 insertions, 0 deletions
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md index f062106..55c5ee7 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md @@ -9249,3 +9249,69 @@ mail was repaired by hand the same day. The regression test builds a thread whose messages DISAGREE about the tag, since two messages in the same state answer identically whichever way the code resolves them, which is the trap item 87 already records. + +## 178. Delete and Restore judge a conversation on one message + +**Done 2026-08-29**, unreleased, on `thread-row-identity`. Split out of item +168 by item 177 and flagged in that spec's "Open, deliberately". + +**Observed.** `MainWindow::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 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.** Delete and Archive are ABSENT on a +reply row (item 177, at the user's own decision) and Restore is thread-scoped, +so every path through this application is all-or-nothing. Two things outside it +produce one: another client trashing a single message, which is live rather +than hypothetical since the user runs Thunderbird (item 104), and a reply +arriving after the conversation was trashed, which needs no other client at +all. + +**Cause and fix.** The summary carries one path because the query walk stops at +one message, and it stops there deliberately: the Sent branch's own measurement +records that walk as free only because it breaks at the first match. Collecting +every path on every query would make 36,000 rows pay for a question about the +one the user clicked. + +`ThreadDigest` already walks every message of the selected conversation, for +the 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: +`ThreadDigest::messagePaths`, relative to the mail root for the reason +`firstMessagePath` records, filled in `loadThreadDigest()` and consumed by +`onThreadDigestLoaded()`. The predicate tests every path for a conversation row +and one path for a message row. + +**One limit, stated 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 being +given a second, differently wrong rule of its own. Making it exhaustive costs a +per-query walk over every message, which is what this fix exists to avoid. + +**Tests.** Two, both mutation-checked. + +- `aDigestCarriesEveryMessagePath` puts the two messages in DIFFERENT folders, + since two in one folder answer identically whichever way the code resolves + them (item 87's rule). It also asserts the paths are relative, because an + absolute one matches no account and silently resolves every row to none. +- `aPartlyTrashedConversationIsNotJudgedOnOneMessage` asserts both directions: + Delete survives and Restore hides on a partly trashed conversation, and the + wholly trashed case still answers as it always did, which is what says the + fix narrowed nothing. Reverting the predicate fails it with the reported + symptom. + +The second test sets `totalCount` explicitly and asserts `isConversationRow()` +before proceeding. A summary left at the default is a MESSAGE row, so a test +meaning to exercise a conversation would quietly exercise the other branch and +pass for the wrong reason, which is the fixture trap the top-level document +records. |
