diff options
| -rw-r--r-- | CHANGELOG.md | 10 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 66 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 2 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 75 | ||||
| -rw-r--r-- | src/mainwindow.h | 29 | ||||
| -rw-r--r-- | src/notmuchworker.cpp | 14 | ||||
| -rw-r--r-- | src/threaddigest.h | 25 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 81 | ||||
| -rw-r--r-- | tests/test_notmuchworker.cpp | 72 |
9 files changed, 353 insertions, 21 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 4bdc536..34d9d2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -143,6 +143,16 @@ point at which they are stable. ### Fixed +- **Delete and Restore now judge a whole conversation.** They asked whether a + row was in the trash by looking at one of its messages, so a conversation + with some messages trashed and some not answered on whichever 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. Both + actions were no-ops in the wrong direction rather than destructive. A + conversation is now in the trash only when every one of its messages is. + qtmaildir cannot produce such a thread itself, since it deletes and restores + whole conversations; another mail client trashing a single message can, and + so can a reply arriving after a conversation was trashed. - **Undo no longer rewrites messages the action never touched.** Undoing a thread-scoped action inverted its tags while keeping the whole thread as its scope, so undoing "mark thread read" on a conversation of 44 messages that 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. 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 bf347c8..1f5a45d 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 @@ -249,7 +249,7 @@ taking that too literally. | 173 | The composer is a plain-text editor, not WYSIWYG | v2 | L | open, 2026-08-27, **asked for by the user** while hand-testing 171. This is a GUI mail client and should edit rich text the way one does: the forwarded original, and the user's own formatting, visible and editable in place. Supersedes the preview 171 shipped as a middle ground, and **subsumes item 133** (markdown syntax highlighting), which is the same want answered cheaply. See the entry: the draft format and the markdown-as-source-of-truth model both change | | 176 | Undoing a thread-scoped action applies its inverse to messages it never changed | defect | S | **done 2026-08-28**, unreleased, on `thread-row-identity`. `NotmuchWorker::applyTags()` reads each message's tags before writing and reports only the ids whose tags actually MOVED; a `TagCommand` base carries that effective set for both `ThreadTagCommand` and `MessageTagCommand`, which had the same defect on a multi-row selection. `tagsApplied` does NOT fire on an empty effective list, since an empty change would push an undo entry whose inverse adds a tag no message ever carried, the same bug one step later. `sendThreadTagChange` gained `onlyMessageIds` so it keeps its thread-scoped REPAINT while restricting the WRITE: the card that changed on screen and the messages that changed on disk are different sets on purpose. **The spec's own plan said item 177 would make a thread undo honest and shrink this to the multi-row case; that was wrong and is corrected in the spec**, an undo inverts an EFFECT, not a scope | | 177 | A thread row means both a message and a conversation, and neither consistently | design | L | **done 2026-08-28**, unreleased, on `thread-row-identity`, eleven commits. Spec: `specs/2026-08-28-thread-row-identity-design.md`. `ThreadListModel::isConversationRow()` is the single predicate and `scopeForSelection()` the single resolver, replacing the `scopeFor()`/`messageScopeFor()` pair that made the CALLER choose. A summary with `totalCount == 1` is unchanged. **Reverses items 108, 110 and 111**, and the user confirmed they are happy to lose the two-tier chips; the `*_thread` submenu and its five action names are deleted with an `### Upgrading` note. Item 112's hiding rule is reversed too: with the absolute entries gone, hiding the toggle on a mixed selection leaves no way to act, so it is a catch-all and the write direction moves with the label. Membership is the union, with two user decisions kept (never evict the current row; an asked-for write evicts at once, an automatic one defers) and one documented lag (a long thread's summary is not updated by a message write, so reading its last unread message waits for the next query). Dashboard from a `ThreadDigest` read by its own worker walk. Two traps found while building: a `QStackedWidget` takes the LARGEST minimum width of its pages and the hidden dashboard was raising the pane's minimum to 395px over MainWindow's 300px floor, caught by an existing resize test; and the pane now holds two `TagStrip`s, so both are named | -| 178 | Delete and Restore judge a conversation on one message | defect | XS | open, 2026-08-28, split out of item 168 by item 177 and flagged in that spec's "Open, deliberately". `MainWindow::everySelectedRowIsInATrashFolder()` reads `ThreadSummary::firstMessagePath` for any row that is not a message row, which was correct while a thread row MEANT that message and is not correct now that it means the conversation. A conversation is in the trash when ALL of its messages are, so a partly-trashed thread currently answers on whichever message the query returned first: Delete can be hidden on a conversation that still has mail outside the trash, and Restore offered on one that mostly does not. Not data-affecting, both actions are no-ops in the wrong direction, but it is an inconsistency the row-kind rule was supposed to remove. Needs the summary to carry the answer, or the paths of every message, which the digest walk already reads | +| 178 | Delete and Restore judge a conversation on one message | defect | XS | **done 2026-08-29**, unreleased, on `thread-row-identity`. `ThreadDigest` carries every message's path, collected by the walk it already makes, so the predicate tests the whole conversation. Known for the SINGLE selected conversation row the digest was requested for; any other selection falls back to the summary's one path, which is the pre-177 answer, deliberately left no worse rather than given a second differently-wrong rule. Section in the closed file | | 174 | An external `notmuch new` reaches the index without the pending count noticing | defect | S | open, 2026-08-28, from the notes. Item 54 cleared the count for a sync run by `mailsync.sh`, which is what `SyncMonitor` watches; a bare `notmuch new` (a hand run, or a cron entry that is not the script) takes notmuch's own write lock and touches `/tmp/mbsync.lock` not at all, so nothing observes it. The user's framing is the approach: we own `mailsync.sh` and the whole process | | 175 | The send countdown says Undo, and cannot be skipped | presentation | XS | open, 2026-08-28, from the notes. Two changes in one control: the button reads Abort, and a second button sends immediately rather than waiting the countdown out | | 179 | Undo is one level deep in practice, and there is no Redo | workflow | ? | open, 2026-08-29, from the notes. The `QUndoStack` is real and multi-level; what is missing is a `redo` action (absent from `knownActions()`, never called) and an answer to the stack being CLEARED on every new query (`mainwindow.cpp:3458`), which is what makes a deep stack behave like a shallow one. The clear has a correct reason and cannot simply be removed. Redo re-applies a write to real mail, so item 176's rule binds it too | 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 ¤t, } 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); } diff --git a/src/mainwindow.h b/src/mainwindow.h index 16bf8c4..d7579f4 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -202,6 +202,16 @@ public: /// command was pushed, which is what "this did nothing" has to assert. int undoDepthForTesting() const { return m_undoStack.count(); } + /// Item 178. Stands in for the digest round trip, which a bare window has + /// no worker to make. Sets what onThreadDigestLoaded() would have set. + void setConversationPathsForTesting(const QString &threadId, + const QStringList &paths) + { + m_conversationPathsThreadId = threadId; + m_conversationPaths = paths; + refreshTrashActions(); + } + /// Runs a purge without the confirmation, which a test cannot drive: a /// modal blocks the thread it is shown on (item 84). What this exists to /// cover is what happens AFTER the user confirms. @@ -1640,6 +1650,25 @@ private: /// which is also set for a thread of one message that renders normally. QString m_dashboardThreadId; + /// Every message path of the conversation the dashboard is showing, keyed + /// by its thread id so a late digest cannot answer about another row. + /// + /// 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; the + /// summary carries ONE path, which was the right answer while a thread row + /// meant its first message (item 108) and stopped being right when item + /// 177 made it mean the conversation. + /// + /// Filled from ThreadDigest, which the selection already requests and + /// which already walks every message, so this costs no query of its own. + /// It is therefore known only for a SINGLE selected conversation row, and + /// everySelectedRowIsInATrashFolder() falls back to the summary's one path + /// when it is absent: the fallback is the pre-177 answer, wrong in exactly + /// the same partial case, so a multi-row selection is no better than + /// before and no worse. + QString m_conversationPathsThreadId; + QStringList m_conversationPaths; + /// The message a dashboard entry asked for and the thread it is in, both /// empty when nothing is waiting. See selectMessageInCurrentThread(). QString m_dashboardSelectMessageId; diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp index 821c575..3a0b2c1 100644 --- a/src/notmuchworker.cpp +++ b/src/notmuchworker.cpp @@ -776,6 +776,11 @@ void NotmuchWorker::loadThreadDigest(const QString &threadId, QVector<MessageNode> unread; QVector<qint64> timestamps; + // mailRootOf(), never notmuch_database_get_path(): under a split index the + // latter returns the XAPIAN directory, and a path relative to that matches + // no account (item 124). + const QString dbRoot = QDir(mailRootOf(m_db)).absolutePath(); + notmuch_messages_t *messages = notmuch_thread_get_messages(thread.get()); for (; notmuch_messages_valid(messages); notmuch_messages_move_to_next(messages)) { @@ -786,6 +791,15 @@ void NotmuchWorker::loadThreadDigest(const QString &threadId, ++digest.totalCount; + // Item 178: which FOLDER each message lives in, so Delete and Restore + // can judge a conversation on all of it rather than on whichever + // message the query returned first. Read from the index by the walk + // that is already running. + if (const char *rawName = notmuch_message_get_filename(message)) { + digest.messagePaths.append( + QDir(dbRoot).relativeFilePath(QString::fromUtf8(rawName))); + } + const qint64 when = notmuch_message_get_date(message); timestamps.append(when); 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 <QMetaType> #include <QPair> #include <QString> +#include <QStringList> #include <QVector> #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 diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 2e7d220..e2254f1 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -391,6 +391,7 @@ private slots: void markCurrentThreadReadResolvesTheThreadThroughTheIndex(); void deletingAReplyRepaintsThatReplyRow(); void deleteIsHiddenOnMailAlreadyInTheTrash(); + void aPartlyTrashedConversationIsNotJudgedOnOneMessage(); void restoreIsHiddenOnMailThatWasNeverDeleted(); void deleteAlsoMarksTheMessageRead(); void emptyTrashAsksBeforeDestroyingAnything(); @@ -5243,6 +5244,86 @@ static ThreadSummary threadAtPath(const QString &id, const QString &filePath, return thread; } +/// Item 178. A CONVERSATION is in the trash only when ALL of its messages are. +/// +/// The predicate read ThreadSummary::firstMessagePath for any row that is not +/// a message row, which was right while a thread row MEANT that message (item +/// 108) and stopped being right when item 177 made it mean the conversation. +/// So a partly trashed thread answered on whichever message the query returned +/// first: Delete hidden on a conversation that still has mail outside the +/// trash, Restore offered on one that mostly does not. +/// +/// qtmaildir cannot itself produce such a thread, since Delete is hidden on a +/// reply row and Restore is thread-scoped. Two things outside it can: another +/// client trashing one message (the user runs Thunderbird, item 104), and a +/// reply arriving after the conversation was trashed. +/// +/// The two messages are in DIFFERENT folders deliberately. Two in the same +/// folder answer identically whichever way the code resolves them, which is +/// the trap AGENTS.md records for item 87. +void TestMainWindow::aPartlyTrashedConversationIsNotJudgedOnOneMessage() +{ + 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 *deleteAction = + window.findChild<QAction *>(QStringLiteral("delete")); + QVERIFY(deleteAction); + auto *restoreAction = + window.findChild<QAction *>(QStringLiteral("restore")); + QVERIFY(restoreAction); + + // totalCount is what item 177 reads to decide a row is a conversation. 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. + ThreadSummary partly = threadAtPath(QStringLiteral("t1"), + QStringLiteral("acct/trash/cur/1:2,S")); + partly.totalCount = 2; + model->appendBatch({ partly }); + + const QModelIndex row = model->index(0, 0, {}); + QVERIFY2(model->isConversationRow(row), + "the fixture is a message row, so this test cannot see item 178 " + "at all: set totalCount"); + + view->setCurrentIndex(row); + + // The paths the worker reports for this conversation: the displayed + // message is in the trash, the other is not. Delete must survive and + // Restore must not be offered, because the conversation is NOT wholly + // trashed however its first message looks. + window.setConversationPathsForTesting( + QStringLiteral("t1"), + { QStringLiteral("acct/trash/cur/1:2,S"), + QStringLiteral("acct/inbox/cur/2:2,S") }); + + QVERIFY2(deleteAction->isVisible(), + "Delete was hidden on a conversation with mail outside the " + "trash: it judged the thread on its first message"); + QVERIFY2(!restoreAction->isVisible(), + "Restore was offered on a conversation that is only partly " + "trashed"); + + // And the whole-conversation case still answers as it always did, which is + // what says the fix narrowed nothing. + window.setConversationPathsForTesting( + QStringLiteral("t1"), + { QStringLiteral("acct/trash/cur/1:2,S"), + QStringLiteral("acct/trash/cur/2:2,S") }); + + QVERIFY2(!deleteAction->isVisible(), + "Delete is still offered on a wholly trashed conversation"); + QVERIFY2(restoreAction->isVisible(), + "Restore vanished on a wholly trashed conversation"); +} + void TestMainWindow::deleteIsHiddenOnMailAlreadyInTheTrash() { // Item 168, from the user: "I noticed I can hit delete via context menu on diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp index 1766589..7148f0c 100644 --- a/tests/test_notmuchworker.cpp +++ b/tests/test_notmuchworker.cpp @@ -123,6 +123,7 @@ private slots: void aDigestCountsSendersAndUnread(); void aDigestCapsItsUnreadListButNotItsCount(); + void aDigestCarriesEveryMessagePath(); void aOneMessageThreadGivesASaneSpan(); private: @@ -2242,6 +2243,77 @@ void TestNotmuchWorker::aDigestCountsSendersAndUnread() QVERIFY(digest.firstTimestamp <= digest.lastTimestamp); } +/// Item 178. Delete and Restore judge a CONVERSATION, and the summary carries +/// one message's path, so a partly-trashed thread answered on whichever +/// message the query returned first. +/// +/// The paths are collected by the walk the digest already makes over every +/// message, so this costs no extra query and no extra file read: a filename +/// comes from the INDEX, like everything else in ThreadDigest. +/// +/// The fixture puts the two messages in DIFFERENT folders deliberately. Two +/// messages in one folder answer identically whichever way the code resolves +/// them, which is the trap AGENTS.md records for item 87's opposite states. +void TestNotmuchWorker::aDigestCarriesEveryMessagePath() +{ + NotmuchFixture fixture; + QVERIFY(fixture.addMessage(QStringLiteral("inbox"), + QStringLiteral("p0@example.org"), + QStringLiteral("Split thread"), + QStringLiteral("alice@example.org"), + QStringLiteral("Mon, 24 Aug 2026 10:00:00 +0200"), + QStringLiteral("Root."), false)); + QVERIFY(fixture.addMessage(QStringLiteral("Trash"), + QStringLiteral("p1@example.org"), + QStringLiteral("Re: Split thread"), + QStringLiteral("bob@example.org"), + QStringLiteral("Tue, 25 Aug 2026 10:00:00 +0200"), + QStringLiteral("Trashed reply."), false, + QStringLiteral("p0@example.org"))); + QVERIFY2(fixture.index(), qPrintable(fixture.error())); + + NotmuchWorker worker(fixture.configPath()); + QSignalSpy spy(&worker, &NotmuchWorker::threadDigestLoaded); + + const QString threadId = worker.threadIdForTesting( + QStringLiteral("id:p0@example.org")); + QVERIFY(!threadId.isEmpty()); + worker.loadThreadDigest(threadId, 1); + + QCOMPARE(spy.count(), 1); + const ThreadDigest digest = spy.at(0).at(0).value<ThreadDigest>(); + + QCOMPARE(digest.totalCount, 2); + QCOMPARE(digest.messagePaths.size(), 2); + + // RELATIVE to the mail root, for the reason ThreadSummary::firstMessagePath + // records: the UI knows an account only by its `maildir`, itself a + // database-relative prefix, so an absolute path here matches no account and + // silently resolves every row to none. + for (const QString &path : digest.messagePaths) { + QVERIFY2(!path.startsWith(QLatin1Char('/')), + qPrintable(QStringLiteral("absolute path: %1").arg(path))); + } + + // One in each folder, which is what makes a partly-trashed conversation + // answerable at all. + int inInbox = 0; + int inTrash = 0; + for (const QString &path : digest.messagePaths) { + // The mail root IS the fixture's maildir, so a relative path begins + // with the folder name and carries no leading separator. + if (path.startsWith(QStringLiteral("inbox/"))) + ++inInbox; + if (path.startsWith(QStringLiteral("Trash/"))) + ++inTrash; + } + QVERIFY2(inInbox + inTrash == 2, + qPrintable(QStringLiteral("unexpected paths: %1") + .arg(digest.messagePaths.join(QStringLiteral(", "))))); + QCOMPARE(inInbox, 1); + QCOMPARE(inTrash, 1); +} + void TestNotmuchWorker::aDigestCapsItsUnreadListButNotItsCount() { NotmuchFixture fixture; |