summaryrefslogtreecommitdiffstats
path: root/tests/test_mainwindow.cpp
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-29 09:49:29 +0200
committerDanilo M. <danix@danix.xyz>2026-08-29 09:49:29 +0200
commitf051dd657757bca3ac7e36454d7f2fc21f322f73 (patch)
tree5804923ed3eb747ee2cd84284e1b36fe0817ab8e /tests/test_mainwindow.cpp
parentd128919ad80ecc37b1f58d2ba83a7487362c126f (diff)
downloadqtmaildir-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 'tests/test_mainwindow.cpp')
-rw-r--r--tests/test_mainwindow.cpp81
1 files changed, 81 insertions, 0 deletions
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