diff options
Diffstat (limited to 'tests')
| -rw-r--r-- | tests/test_carddelegate.cpp | 128 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 1001 | ||||
| -rw-r--r-- | tests/test_notmuchworker.cpp | 50 | ||||
| -rw-r--r-- | tests/test_threadlistmodel.cpp | 613 |
4 files changed, 1778 insertions, 14 deletions
diff --git a/tests/test_carddelegate.cpp b/tests/test_carddelegate.cpp index d5e55b5..a14671d 100644 --- a/tests/test_carddelegate.cpp +++ b/tests/test_carddelegate.cpp @@ -18,6 +18,9 @@ #include "carddelegate.h" +#include "cardlayout.h" +#include "tagchip.h" +#include "tagcolors.h" #include "threadlistmodel.h" #include <QTest> @@ -30,6 +33,9 @@ private slots: void theAccentLiftsAMutedAccountColour(); void theAccentKeepsEachAccountTellableApart(); void anAccountWithNoColourFallsBackToTheNeutralLine(); + void aSiblingChipIsMutedButStaysLegibleAndRecognisable(); + void aSiblingChipFontIsSmallerThanItsOwnTier(); + void aSiblingChipsPaddingShrinksWithItsFont(); }; namespace { @@ -131,5 +137,127 @@ void TestCardDelegate::anAccountWithNoColourFallsBackToTheNeutralLine() ThreadListModel::threadLineColour()); } +void TestCardDelegate::aSiblingChipIsMutedButStaysLegibleAndRecognisable() +{ + // Item 111: a card shows its own tags at full size and the rest of the + // conversation's smaller and muted. "Muted" has two hard requirements that + // a look at the screen will not catch, so they are asserted here. + for (const QColor &colour : sampleAccounts()) { + const QColor muted = CardDelegate::mutedChipColour(colour); + + // Actually muted, or the tier is not distinguishable at all. + QVERIFY2(saturationOf(muted) < saturationOf(colour), + qPrintable(QStringLiteral("%1 was not drained at all") + .arg(colour.name()))); + + // Same HUE. A sibling's `signed` has to stay recognisably the same + // colour as a full-size `signed` elsewhere in the list, or the muting + // reads as a different tag rather than a quieter one. + float h1 = 0, h2 = 0, s = 0, l = 0, a = 0; + colour.getHslF(&h1, &s, &l, &a); + muted.getHslF(&h2, &s, &l, &a); + QVERIFY2(qAbs(h1 - h2) < 0.001f, + qPrintable(QStringLiteral("%1 changed hue when muted") + .arg(colour.name()))); + + // Same LIGHTNESS, which is what keeps the text legible: TagColors + // picks the text colour from the fill, and a fill that drifted toward + // black or white could flip that choice or land mid-grey where neither + // works. Blending toward the background would do exactly that, which + // is the mistake accentLineColour() records. + QCOMPARE(lightnessOf(muted), lightnessOf(colour)); + QCOMPARE(TagColors::textColourOn(muted), + TagColors::textColourOn(colour)); + } + + // An invalid colour stays invalid rather than becoming a real one. + QVERIFY(!CardDelegate::mutedChipColour(QColor()).isValid()); +} + +void TestCardDelegate::aSiblingChipFontIsSmallerThanItsOwnTier() +{ + // Size is what says whose tag a chip is, so the two tiers must differ, and + // by enough to SEE. The first version subtracted a point from smallFont(), + // and the user reported the tiers as indistinguishable: on their 14pt + // desktop that gave 13 and 12, a 7% step. + // + // The step is now a fraction of the card font, so it does not shrink as + // the desktop's font grows. Asserted as a ratio rather than as a size, to + // keep this about the DISTINCTION rather than about the constant. + QFont card; + card.setPointSizeF(14.0); // The user's own desktop size. + const qreal own = CardLayout::smallFont(card).pointSizeF(); + const qreal sibling = CardLayout::siblingFont(card).pointSizeF(); + + QVERIFY(sibling < own); + QVERIFY2(sibling < own * 0.85, + qPrintable(QStringLiteral("sibling %1pt against own %2pt is under " + "a 15%% step, which reads as the same " + "size") + .arg(sibling) + .arg(own))); + + // Proportional, not a fixed subtraction: the step must survive a larger + // desktop font rather than becoming proportionally smaller. + QFont big; + big.setPointSizeF(28.0); + QVERIFY(CardLayout::siblingFont(big).pointSizeF() + < CardLayout::smallFont(big).pointSizeF() * 0.85); + + // The pixel branch too: qt6ct sets fonts in PIXELS, and pointSizeF() is -1 + // for those, so a point-only implementation silently returns the original + // size and both tiers render identically. CLAUDE.md records this trap. + QFont pixels; + pixels.setPixelSize(14); + QVERIFY(pixels.pointSizeF() < 0); + QVERIFY2(CardLayout::siblingFont(pixels).pixelSize() + < CardLayout::smallFont(pixels).pixelSize(), + "a pixel-sized desktop font gives both tiers the same size, so " + "the distinction disappears entirely"); + + // Floored rather than shrinking without limit. + QFont tiny; + tiny.setPointSizeF(6.0); + QVERIFY(CardLayout::siblingFont(tiny).pointSizeF() >= 6.0); +} + +void TestCardDelegate::aSiblingChipsPaddingShrinksWithItsFont() +{ + // Half of "smaller" is the padding, and leaving it fixed is why the first + // version still looked the same size. kPaddingX is 9 a side: on a sibling + // chip that is 18px of padding around roughly 30px of text, so the chip + // stayed wide while its letters shrank, which reads as "same chip, smaller + // text" rather than as a smaller chip. + QFont card; + card.setPointSizeF(14.0); + const QFontMetrics ownMetrics(CardLayout::smallFont(card)); + const QFontMetrics siblingMetrics(CardLayout::siblingFont(card)); + + const QString tag = QStringLiteral("signed"); + // Through CardDelegate::chipSize(), which is what the paint loop calls. + // Calling TagChip::sizeFor() directly here proved what THAT function does + // and nothing about whether the delegate asks it for a scaled padding: a + // mutation dropping the scale at the call site survived that version of + // this test. + const QSize own = CardDelegate::chipSize(ownMetrics, tag, true); + const QSize scaled = CardDelegate::chipSize(siblingMetrics, tag, false); + const QSize unscaled = TagChip::sizeFor(siblingMetrics, tag); + + // The font alone is not enough: scaling the padding as well takes off + // measurably more width. + QVERIFY2(scaled.width() < unscaled.width(), + "the padding did not scale, so the chip keeps full-size margins " + "around smaller letters"); + QVERIFY(scaled.width() < own.width()); + QVERIFY(scaled.height() < own.height()); + + // Floored rather than collapsing to nothing: the corner radius is half the + // height, so a chip with no horizontal padding has its text on the curve. + const QSize tiny = TagChip::sizeFor(siblingMetrics, tag, 0.0); + QVERIFY2(tiny.width() > siblingMetrics.horizontalAdvance(tag), + "a zero scale left no horizontal padding at all, so the text sits " + "on the chip's rounded end"); +} + QTEST_MAIN(TestCardDelegate) #include "test_carddelegate.moc" diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index f4357c6..07cc56b 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -57,6 +57,7 @@ #include <QComboBox> #include <QScrollBar> #include "tagchip.h" +#include "tagstrip.h" #include "threadlistmodel.h" #include "threadlistview.h" #include "notmuchfixture.h" @@ -273,6 +274,23 @@ private slots: void escapeBlanksTheMessagePane(); void deleteTogglesOnAnAlreadyDeletedThread(); void deleteOnAMixedSelectionDeletesRatherThanSplittingIt(); + void deleteOnAReplyReadsItsOwnThreadNotTheFirstInTheList(); + void toggleUnreadOnAReplyReadsItsOwnThreadNotTheFirstInTheList(); + void editTagsOnAReplyCountsItsOwnThreadNotTheFirstInTheList(); + void markCurrentThreadReadResolvesTheThreadThroughTheIndex(); + void deletingAReplyRepaintsThatReplyRow(); + void toggleUnreadOnAReplyReadsTheReplysOwnState(); + void toggleUnreadOnAReplyRepaintsItInBothDirections(); + void taggingTheOpenReplyUpdatesTheMessagePaneStrip(); + void taggingAnUnrelatedReplyLeavesTheStripAlone(); + void aHeldMessageEditIsSentWhenTheSyncEnds(); + void anActionOnAThreadRowActsOnTheMessageItDisplays(); + void theThreadSubmenuIsReachableFromBothMenus(); + void autoMarkReadTouchesOnlyTheMessageOnDisplay(); + void autoMarkReadArmsForAReplyToo(); + void taggingTheOpenRootMessageKeepsTheStripPopulated(); + void aLoadedMessageCorrectsTheStripFromTheThreadsUnion(); + void aLoadedRootMessageGivesTheCardItsOwnTags(); void aTransientStatusMessageExpires(); void theSelectionCountIsStateAndDoesNotExpire(); void anEditUndoneNettsBackToZero(); @@ -790,6 +808,13 @@ static ThreadSummary makeThread(const QString &id, const QStringList &tags) thread.subject = QStringLiteral("Subject ") + id; thread.authors = QStringLiteral("Someone <someone@example.org>"); thread.tags = tags; + + // The message the row displays, which the real worker fills in from the + // query. Required since item 108: an ordinary tag action resolves a thread + // row to THIS id, so a summary without one names no message and every + // action on it does nothing. A fixture missing it fails with "the action + // did not happen", which reads as a defect in the action. + thread.firstMessageId = id + QStringLiteral("-first@example.org"); return thread; } @@ -1372,8 +1397,11 @@ void TestMainWindow::anActionOnAThreadRowSaysItHitTheWholeThread() selectThreadRow(view, 0); QApplication::processEvents(); - auto *archive = window.findChild<QAction *>(QStringLiteral("archive")); - QVERIFY2(archive, "no archive action to trigger"); + // The THREAD action since item 108. The plain `archive` now acts on the + // one message a card displays, and would rightly not claim to have taken + // the whole thread; this suffix belongs to the action that really does. + auto *archive = window.findChild<QAction *>(QStringLiteral("archive_thread")); + QVERIFY2(archive, "no archive_thread action to trigger"); archive->trigger(); // Read BEFORE processEvents, deliberately. This binary has no worker @@ -4327,7 +4355,13 @@ void TestMainWindow::aHeldEditIsSentBeforeTheSyncEndRefreshReadsTheDatabase() // The write went out. Nothing else in this handler sends one, so its // presence is what proves the flush ran, and the refresh below is what it // has to have run BEFORE. - QVERIFY2(!window.pendingThreadIdsForTesting().isEmpty(), + // + // Either scope counts. The gesture is Toggle unread on a thread row, which + // is message-scoped since item 108, so the ids land in the message list; + // the ORDER this test exists for is the same either way, and pinning the + // scope here would make it fail for a reason it does not care about. + QVERIFY2(!window.pendingThreadIdsForTesting().isEmpty() + || !window.pendingMessageIdsForTesting().isEmpty(), "the held edit was dropped rather than sent"); const quint64 after = window.currentGenerationForTesting(); @@ -4593,7 +4627,10 @@ void TestMainWindow::deleteTogglesOnAnAlreadyDeletedThread() QVERIFY(model); auto *view = window.findChild<QTreeView *>(); QVERIFY(view); - auto *action = window.findChild<QAction *>(QStringLiteral("delete")); + // The THREAD action, since this is about a THREAD's state. Item 108 made + // the plain `delete` act on the one message a card displays, and a thread + // summary carrying `deleted` says nothing about that message's own tags. + auto *action = window.findChild<QAction *>(QStringLiteral("delete_thread")); QVERIFY(action); model->appendBatch({ makeThread(QStringLiteral("t1"), @@ -4621,7 +4658,7 @@ void TestMainWindow::deleteOnAMixedSelectionDeletesRatherThanSplittingIt() QVERIFY(model); auto *view = window.findChild<QTreeView *>(); QVERIFY(view); - auto *action = window.findChild<QAction *>(QStringLiteral("delete")); + auto *action = window.findChild<QAction *>(QStringLiteral("delete_thread")); QVERIFY(action); model->appendBatch({ makeThread(QStringLiteral("t1"), @@ -4637,6 +4674,889 @@ void TestMainWindow::deleteOnAMixedSelectionDeletesRatherThanSplittingIt() "a mixed selection split instead of deleting the whole selection"); } +/// Builds a window whose SECOND thread is expanded and carries one reply, with +/// the two threads deliberately in opposite states. +/// +/// Item 88's shape in one place. A tree numbers rows per parent, so the first +/// reply of any thread has row() == 0 and threadAt(0) answers about the FIRST +/// THREAD IN THE LIST. Every test below selects that reply and asserts on +/// behaviour that can only be right if the thread was resolved through the +/// index: with the row number, each one reads t1's state while acting on t2. +/// +/// The opposite states are what makes the tests able to fail. Two threads in +/// the same state give the same answer either way, which is how the reverted +/// item 87 fix passed while corrupting mail. +/// +/// \p replyTags defaults to the thread's own tags, which is the usual case. +/// Pass it explicitly to make a reply DISAGREE with its thread, which is what +/// separates "reads the right thread" from "reads the right message": a reply +/// can be unread inside a thread that is not, and vice versa. +static QModelIndex expandSecondThreadAndSelectItsReply( + QTreeView *view, ThreadListModel *model, const QStringList &firstTags, + const QStringList &secondTags, + const std::optional<QStringList> &replyTags = std::nullopt) +{ + ThreadSummary first = makeThread(QStringLiteral("t1"), firstTags); + ThreadSummary second = makeThread(QStringLiteral("t2"), secondTags); + second.totalCount = 2; + model->appendBatch({ first, second }); + + MessageNode root; + root.messageId = QStringLiteral("m0@example.org"); + root.threadId = QStringLiteral("t2"); + root.tags = secondTags; + root.depth = 0; + MessageNode reply; + reply.messageId = QStringLiteral("m1@example.org"); + reply.threadId = QStringLiteral("t2"); + reply.tags = replyTags.value_or(secondTags); + reply.depth = 1; + model->setThreadMessages(QStringLiteral("t2"), { root, reply }); + + const QModelIndex threadRow = model->index(1, 0, QModelIndex()); + view->expand(threadRow); + + const QModelIndex replyRow = model->index(0, 0, threadRow); + if (!replyRow.isValid() || !model->isMessageRow(replyRow)) + return {}; + + // Row 0 under its parent, which is the trap: the number is a plausible + // top-level row and threadAt() cannot tell the difference. + if (replyRow.row() != 0) + return {}; + + view->selectionModel()->select( + replyRow, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); + view->setCurrentIndex(replyRow); + QApplication::processEvents(); + return replyRow; +} + +void TestMainWindow::deleteOnAReplyReadsItsOwnThreadNotTheFirstInTheList() +{ + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *action = window.findChild<QAction *>(QStringLiteral("delete")); + QVERIFY(action); + + // t1 deleted, t2 not. Reading t1's state for a reply of t2 makes the + // toggle choose UNDELETE for a thread that was never deleted. + const QModelIndex reply = expandSecondThreadAndSelectItsReply( + view, model, { QStringLiteral("deleted") }, {}); + QVERIFY2(reply.isValid(), + "the fixture did not produce a reply row at row 0, so this test " + "would assert nothing about item 88's trap"); + + action->trigger(); + + // Delete, because the message's own thread is not deleted. The write goes + // through scopeFor() and lands on the message either way; what is under + // test is the DIRECTION, which is chosen from the state that was read. + QVERIFY2(window.pendingMessageIdsForTesting().contains( + QStringLiteral("m1@example.org")), + "Delete on a reply did not act on that reply"); + QCOMPARE(window.undoDepthForTesting(), 1); + QVERIFY2(window.undoTextForTesting().contains(QStringLiteral("Delete")), + qPrintable(QStringLiteral( + "Delete on a reply of an undeleted thread chose " + "the wrong direction: %1. It read the FIRST " + "thread's state, which is deleted.") + .arg(window.undoTextForTesting()))); +} + +void TestMainWindow::toggleUnreadOnAReplyReadsItsOwnThreadNotTheFirstInTheList() +{ + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *action = window.findChild<QAction *>(QStringLiteral("toggle_unread")); + QVERIFY(action); + + // t1 unread, t2 read. Reading t1's state marks an already-read message + // read again, which is a no-op write the user sees as a dead key. + const QModelIndex reply = expandSecondThreadAndSelectItsReply( + view, model, { QStringLiteral("unread") }, {}); + QVERIFY2(reply.isValid(), "the fixture did not produce a reply row at row 0"); + + action->trigger(); + + QCOMPARE(window.undoDepthForTesting(), 1); + QVERIFY2(window.undoTextForTesting().contains(QStringLiteral("Mark unread")), + qPrintable(QStringLiteral( + "Toggle unread on a reply of a READ thread chose " + "the wrong direction: %1. It read the FIRST " + "thread's state, which is unread.") + .arg(window.undoTextForTesting()))); +} + +void TestMainWindow::editTagsOnAReplyCountsItsOwnThreadNotTheFirstInTheList() +{ + // The tag dialog is modal, so what is tested is the count it is BUILT + // from. Those counts drive its tri-state checkboxes, so a wrong count + // offers to remove a tag the message does not carry and shows the ones it + // does as unset. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + + const QModelIndex reply = expandSecondThreadAndSelectItsReply( + view, model, { QStringLiteral("t1only") }, { QStringLiteral("t2only") }); + QVERIFY2(reply.isValid(), "the fixture did not produce a reply row at row 0"); + + const QHash<QString, int> counts = window.selectionTagCountsForTesting(); + + QVERIFY2(counts.contains(QStringLiteral("t2only")), + "the tag dialog would not offer the tag the selected reply " + "actually carries"); + QVERIFY2(!counts.contains(QStringLiteral("t1only")), + "the tag dialog counted the FIRST thread's tags for a reply of " + "the second, so it would offer to remove a tag that is not there"); +} + +void TestMainWindow::markCurrentThreadReadResolvesTheThreadThroughTheIndex() +{ + // Item 87 is blocked on this and will scope the write to one message. Today + // an unrelated guard hides the defect: onThreadSelected clears + // m_currentThreadId for a message row, so markCurrentThreadRead returns + // before it can read the wrong thread. That guard is not the protection + // this needs, and item 87 does not remove it, so the assertion here is on + // the resolution itself rather than on a write that cannot currently + // happen. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + + const QModelIndex reply = expandSecondThreadAndSelectItsReply( + view, model, { QStringLiteral("unread") }, { QStringLiteral("unread") }); + QVERIFY2(reply.isValid(), "the fixture did not produce a reply row at row 0"); + + // The question the timer's handler asks, in isolation: which thread is the + // current row part of. With the row number this answers "t1" for a reply + // of t2. + QCOMPARE(window.threadForCurrentRowForTesting().threadId, + QStringLiteral("t2")); +} + +void TestMainWindow::deletingAReplyRepaintsThatReplyRow() +{ + // The user's report, at the gesture level: "I'm hitting delete on a reply + // to a thread, I see the edits counter increasing but I have no feedback + // if that message is being deleted." The model-level test proves + // applyMessageTagChange works; this proves the action reaches it, which is + // the half that was actually missing. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *action = window.findChild<QAction *>(QStringLiteral("delete")); + QVERIFY(action); + + const QModelIndex reply = + expandSecondThreadAndSelectItsReply(view, model, {}, {}); + QVERIFY2(reply.isValid(), "the fixture did not produce a reply row at row 0"); + + // Nothing to see before the gesture, so the assertion after it means + // something. + QVERIFY(!model->messageAt(reply).isDeleted()); + const QVariant before = model->data(reply, Qt::BackgroundRole); + + QSignalSpy spy(model, &QAbstractItemModel::dataChanged); + action->trigger(); + + QVERIFY2(model->messageAt(reply).isDeleted(), + "Delete on a reply left the reply's own row unchanged, so the " + "pending count moved and the user saw nothing"); + QVERIFY2(spy.count() >= 1, "no repaint was requested for the reply's row"); + QVERIFY2(model->data(reply, Qt::BackgroundRole) != before, + "the deleted reply paints exactly as it did before"); + + // The THREAD row must not follow: it stands for the whole conversation, + // and one deleted reply does not doom it. + const QModelIndex threadRow = reply.parent(); + QVERIFY2(!model->threadFor(threadRow).isDeleted(), + "deleting one reply marked its whole thread deleted"); +} + +void TestMainWindow::toggleUnreadOnAReplyReadsTheReplysOwnState() +{ + // The user's report: "read/unread still doesn't trigger a repaint of the + // reply". The write was already message-scoped and the model already + // repaints a message row, so neither was the fault. The DIRECTION was: + // the action read threadFor(current).isUnread(), the THREAD's state, even + // when the selected row is a reply. + // + // The consequence is a dead key rather than a wrong write. On a read + // thread the answer is always "add unread", so pressing it on an + // already-unread reply re-adds a tag it has, which is a no-op the model + // correctly declines to repaint. Item 88 fixed WHICH thread this reads; + // this is about reading a message at all. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *action = window.findChild<QAction *>(QStringLiteral("toggle_unread")); + QVERIFY(action); + + // A thread that is READ carrying a reply that is UNREAD. That disagreement + // is the whole test: with the thread's state the answer is "mark unread", + // with the message's it is "mark read". + const QModelIndex reply = expandSecondThreadAndSelectItsReply( + view, model, {}, {}, QStringList{ QStringLiteral("unread") }); + QVERIFY2(reply.isValid(), "the fixture did not produce a reply row at row 0"); + QVERIFY(model->messageAt(reply).isUnread()); + QVERIFY2(!model->threadFor(reply).isUnread(), + "the fixture's thread is unread too, so this test cannot tell the " + "two sources apart"); + + action->trigger(); + + QVERIFY2(!model->messageAt(reply).isUnread(), + "Toggle unread on an unread reply did not mark it read: the " + "direction came from the THREAD, which is already read, so it " + "re-added a tag the reply already had and nothing changed"); + QVERIFY2(window.undoTextForTesting().contains(QStringLiteral("Mark read")), + qPrintable(QStringLiteral("wrong direction: %1") + .arg(window.undoTextForTesting()))); +} + +void TestMainWindow::toggleUnreadOnAReplyRepaintsItInBothDirections() +{ + // Visible BOTH ways. The user reached the repaint only by deleting and + // undoing, which is a different write forcing the row to redraw; the + // unread change itself has to do it on its own, in each direction. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *action = window.findChild<QAction *>(QStringLiteral("toggle_unread")); + QVERIFY(action); + + const QModelIndex reply = expandSecondThreadAndSelectItsReply( + view, model, {}, {}, QStringList{ QStringLiteral("unread") }); + QVERIFY(reply.isValid()); + + const QVariant unreadForeground = model->data(reply, Qt::ForegroundRole); + const QVariant unreadFont = model->data(reply, Qt::FontRole); + + QSignalSpy spy(model, &QAbstractItemModel::dataChanged); + action->trigger(); + + QVERIFY2(spy.count() >= 1, "marking a reply read requested no repaint"); + QVERIFY2(model->data(reply, Qt::ForegroundRole) != unreadForeground, + "a reply marked read paints exactly as it did while unread"); + QVERIFY2(model->data(reply, Qt::FontRole) != unreadFont, + "a reply marked read keeps the unread font"); + + // And back. A toggle that is only visible one way is half a toggle. + spy.clear(); + action->trigger(); + QVERIFY(model->messageAt(reply).isUnread()); + QVERIFY2(spy.count() >= 1, "marking a reply unread again requested no repaint"); + QCOMPARE(model->data(reply, Qt::ForegroundRole), unreadForeground); + QCOMPARE(model->data(reply, Qt::FontRole), unreadFont); +} + +void TestMainWindow::taggingTheOpenReplyUpdatesTheMessagePaneStrip() +{ + // The user's report: "the right pane chips are not [repainted], for it to + // sync I have to change message and go back to the edited one". + // + // sendThreadTagChange refreshes the strip when the edited thread is the + // open one. sendMessageTagChange had no equivalent, so a message-scoped + // write updated the list row and left the pane's chips describing the + // message as it was before the edit. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *strip = window.findChild<TagStrip *>(); + QVERIFY2(strip, "no tag strip in the message pane"); + + // visible + hidden: TagStrip collapses what does not fit into a "+N" chip, + // and an unshown window has no width, so visibleTags() alone measures the + // layout rather than the data. + const auto stripTags = [strip]() { + return strip->visibleTags() + strip->hiddenTags(); + }; + auto *action = window.findChild<QAction *>(QStringLiteral("delete")); + QVERIFY(action); + + // A tag the strip will actually draw. Account tags are filtered out by the + // strip, and `unread` and `inbox` are hidden on the card but not here, so + // the fixture uses a plain functional tag to keep the assertion honest. + const QModelIndex reply = expandSecondThreadAndSelectItsReply( + view, model, {}, {}, QStringList{ QStringLiteral("todo") }); + QVERIFY2(reply.isValid(), "the fixture did not produce a reply row at row 0"); + + // Selecting the reply is what puts it in the pane, and the strip has to be + // showing the reply's own tags before the edit or this asserts nothing. + QVERIFY2(stripTags().contains(QStringLiteral("todo")), + "the strip does not show the selected reply's tags, so this test " + "cannot tell a missing refresh from a strip that never had them"); + QVERIFY(!stripTags().contains(QStringLiteral("deleted"))); + + action->trigger(); + + QVERIFY2(stripTags().contains(QStringLiteral("deleted")), + "the message pane's chips still describe the reply as it was " + "before the edit; the user has to select away and back to see it"); +} + +void TestMainWindow::taggingAnUnrelatedReplyLeavesTheStripAlone() +{ + // The guard, not the refresh. The strip describes the message ON DISPLAY, + // so a write to a different message must not repaint it with that + // message's tags. The thread path has the same guard, keyed on + // m_currentThreadId. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *strip = window.findChild<TagStrip *>(); + QVERIFY(strip); + + // visible + hidden: TagStrip collapses what does not fit into a "+N" chip, + // and an unshown window has no width, so visibleTags() alone measures the + // layout rather than the data. + const auto stripTags = [strip]() { + return strip->visibleTags() + strip->hiddenTags(); + }; + + ThreadSummary thread = makeThread(QStringLiteral("t1"), {}); + thread.totalCount = 3; + model->appendBatch({ thread }); + + MessageNode root; + root.messageId = QStringLiteral("m0@example.org"); + root.threadId = QStringLiteral("t1"); + root.depth = 0; + MessageNode first; + first.messageId = QStringLiteral("m1@example.org"); + first.threadId = QStringLiteral("t1"); + first.tags = QStringList{ QStringLiteral("todo") }; + first.depth = 1; + MessageNode second; + second.messageId = QStringLiteral("m2@example.org"); + second.threadId = QStringLiteral("t1"); + second.tags = QStringList{ QStringLiteral("later") }; + second.depth = 1; + model->setThreadMessages(QStringLiteral("t1"), { root, first, second }); + + const QModelIndex threadRow = model->index(0, 0, QModelIndex()); + view->expand(threadRow); + + // The FIRST reply is the one on display. + const QModelIndex displayed = model->index(0, 0, threadRow); + view->selectionModel()->select( + displayed, + QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); + view->setCurrentIndex(displayed); + QApplication::processEvents(); + QVERIFY(stripTags().contains(QStringLiteral("todo"))); + + // A write to the OTHER reply, reaching the send path directly: driving it + // through the action would move the selection and change what is on + // display, which is the thing being held still. + window.sendMessageTagChangeForTesting({ QStringLiteral("m2@example.org") }, + { QStringLiteral("deleted") }, {}, + QStringLiteral("Delete")); + + QVERIFY2(!stripTags().contains(QStringLiteral("deleted")), + "the strip took on the tags of a message that is not the one in " + "the pane"); + QVERIFY2(stripTags().contains(QStringLiteral("todo")), + "the strip stopped describing the message on display"); +} + +void TestMainWindow::aHeldMessageEditIsSentWhenTheSyncEnds() +{ + // Found by reading while fixing the strip refresh, not reported. + // + // flushHeldEdits() looped over edit.threadIds and called + // sendThreadTagChange() only. A message-scoped edit held during a sync + // carries no thread ids at all, so the loop did nothing, the send + // early-returned on an empty list, and the edit was DROPPED: applied + // optimistically to the row, counted as unsynced, and never written. The + // user would have seen the change, been told it was pending, and lost it. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *action = window.findChild<QAction *>(QStringLiteral("delete")); + QVERIFY(action); + + const QModelIndex reply = + expandSecondThreadAndSelectItsReply(view, model, {}, {}); + QVERIFY(reply.isValid()); + + QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", + Q_ARG(SyncMonitor::State, + SyncMonitor::State::Running)); + action->trigger(); + QVERIFY2(window.hasEditAwaitingSend(), + "a message edit made during a sync was not held"); + + QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", + Q_ARG(SyncMonitor::State, + SyncMonitor::State::Idle)); + + QVERIFY2(!window.hasEditAwaitingSend(), + "the sync ending did not send the held message edit, so it was " + "silently dropped: shown on the row, counted as pending, never " + "written"); + + // Sent for the MESSAGE, not escalated to its thread. Losing the scope on + // the way out of the hold would delete every message in the thread. + QVERIFY2(window.pendingMessageIdsForTesting().contains( + QStringLiteral("m1@example.org")), + "the held edit was not sent with its message scope"); + QVERIFY2(window.pendingThreadIdsForTesting().isEmpty(), + "a held MESSAGE edit was sent as a thread edit, which would tag " + "every message in the thread"); + + // And the row still shows it: the flush takes the optimistic update back + // before re-sending, so a bug there leaves the row wrong in the other + // direction. + QVERIFY2(model->messageAt(reply).isDeleted(), + "sending the held edit lost the tag from the reply's row"); +} + +void TestMainWindow::anActionOnAThreadRowActsOnTheMessageItDisplays() +{ + // Item 108, the whole point of it. A root card renders ONE message since + // item 66, so acting on it acts on that message; the conversation is + // reached through the explicit thread actions. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + + ThreadSummary t = makeThread(QStringLiteral("t1"), {}); + t.totalCount = 7; + model->appendBatch({ t }); + selectThreadRow(view, 0); + + auto *deleteAction = window.findChild<QAction *>(QStringLiteral("delete")); + QVERIFY(deleteAction); + deleteAction->trigger(); + + QCOMPARE(window.pendingMessageIdsForTesting(), + QStringList{ QStringLiteral("t1-first@example.org") }); + QVERIFY2(window.pendingThreadIdsForTesting().isEmpty(), + "the ordinary Delete still acted on the whole thread, so it " + "touched six messages the card does not display"); + + // The thread action is how the conversation is reached, and it must still + // work from the same selection. + auto *deleteThread = + window.findChild<QAction *>(QStringLiteral("delete_thread")); + QVERIFY(deleteThread); + deleteThread->trigger(); + + QCOMPARE(window.pendingThreadIdsForTesting(), + QStringList{ QStringLiteral("t1") }); + + // Two commands, one per gesture, each recording the scope it used: a thread + // action that pushed the message command would undo a fraction of what it + // did. + QCOMPARE(window.undoDepthForTesting(), 2); +} + +void TestMainWindow::theThreadSubmenuIsReachableFromBothMenus() +{ + // The user asked for "a submenu when right clicking and the same submenu + // under Message in the top menu". Both, not one: the context menu is where + // the gesture starts and the menu bar is where a shortcut is discovered. + // + // A QMenu belongs to ONE menu tree, so these are two instances holding the + // same actions. Adding a single instance to both silently gives it to + // whichever added it last, which is the failure this pins. + const Config config; + MainWindow window(config); + + auto *context = + window.findChild<QMenu *>(QStringLiteral("threadContextMenu")); + QVERIFY(context); + + const QStringList expected = { + QStringLiteral("archive_thread"), + QStringLiteral("delete_thread"), + QStringLiteral("spam_thread"), + QStringLiteral("toggle_unread_thread"), + QStringLiteral("flag_thread"), + }; + + // Every submenu instance in the window, wherever it was added. + const QList<QMenu *> submenus = + window.findChildren<QMenu *>(QStringLiteral("threadActionsMenu")); + QVERIFY2(submenus.size() >= 2, + qPrintable(QStringLiteral("expected the thread submenu in both " + "the context menu and the menu bar, " + "found %1 instance(s)") + .arg(submenus.size()))); + + for (QMenu *menu : submenus) { + QStringList names; + for (QAction *action : menu->actions()) { + if (!action->isSeparator()) + names.append(action->objectName()); + } + QCOMPARE(names, expected); + } + + // One of them is the context menu's own, reached as a submenu rather than + // as a loose action. + bool inContextMenu = false; + for (QAction *action : context->actions()) { + if (action->menu() + && action->menu()->objectName() + == QStringLiteral("threadActionsMenu")) { + inContextMenu = true; + break; + } + } + QVERIFY2(inContextMenu, + "right-clicking a thread offers no whole-thread submenu"); +} + +void TestMainWindow::autoMarkReadTouchesOnlyTheMessageOnDisplay() +{ + // Item 87, reported 2026-08-14: "with the first message in a thread + // selected (not expanded), the 2s delay that marks it read applies to the + // whole thread, so all answers are marked read as well." + // + // Not cosmetic. maildir.synchronize_flags is on, so removing `unread` + // rewrites Maildir filenames and the next sync carries it to the server: + // mail the user never opened stops being unread everywhere, and nothing + // here can put it back except reading it again by hand. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString path = dir.filePath(QStringLiteral("qtmaildir.conf")); + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.write("[general]\nmark_read_delay_ms = 0\n"); + file.close(); + + Config config; + config.load(path); + QCOMPARE(config.markReadDelayMs(), 0); + + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); + QVERIFY(timer); + + ThreadSummary t = makeThread(QStringLiteral("t1"), + { QStringLiteral("unread") }); + t.totalCount = 7; + model->appendBatch({ t }); + + selectThreadRow(view, 0); + QApplication::processEvents(); + QVERIFY2(timer->isActive() || !window.pendingMessageIdsForTesting().isEmpty(), + "selecting an unread thread armed no mark-read at all"); + + // Fire it. A zero-interval timer still goes through the event loop. + QTRY_VERIFY_WITH_TIMEOUT(!timer->isActive(), 2000); + QApplication::processEvents(); + + // ONE message, the one the card renders, and named rather than merely + // counted: a thread of seven whose first message is the target is exactly + // the case where a count of one could still be the wrong one. + QCOMPARE(window.pendingMessageIdsForTesting(), + QStringList{ QStringLiteral("t1-first@example.org") }); + QVERIFY2(window.pendingThreadIdsForTesting().isEmpty(), + "the automatic mark-read still wrote to the whole thread, so six " + "messages the user never displayed were marked read and the next " + "sync carries that to the server"); + + // Still not on the undo stack. The user never took this action, so + // hijacking Ctrl+Z to reverse it would undo something they did not do. + QCOMPARE(window.undoDepthForTesting(), 0); +} + +void TestMainWindow::autoMarkReadArmsForAReplyToo() +{ + // Selecting a reply displays that message, so the same rule applies to it. + // Before item 87 the timer was deliberately not armed for a message row, + // because the write it would have made was thread-scoped and would have + // marked the whole conversation read. With the write scoped to one message + // that objection is gone, and leaving it unarmed would mean the message + // the user is reading is the one kind that never gets marked read. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString path = dir.filePath(QStringLiteral("qtmaildir.conf")); + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.write("[general]\nmark_read_delay_ms = 0\n"); + file.close(); + + Config config; + config.load(path); + + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer")); + QVERIFY(timer); + + // The reply is unread; its thread is not, so a thread-keyed timer would + // have declined to arm at all. + // + // No "is it still unread" guard before the wait: the delay is 0 and the + // helper pumps the event loop, so the write has already happened by the + // time selection returns. The assertions below are on the write itself, + // which is what this test is about, and the fixture above is what + // establishes the reply started unread. + const QModelIndex reply = expandSecondThreadAndSelectItsReply( + view, model, {}, {}, QStringList{ QStringLiteral("unread") }); + QVERIFY(reply.isValid()); + + QTRY_VERIFY_WITH_TIMEOUT(!timer->isActive(), 2000); + QApplication::processEvents(); + + QCOMPARE(window.pendingMessageIdsForTesting(), + QStringList{ QStringLiteral("m1@example.org") }); + QVERIFY2(window.pendingThreadIdsForTesting().isEmpty(), + "reading one reply marked its whole thread read"); + + // And the row shows it, which is the half item 105 built. + QVERIFY2(!model->messageAt(reply).isUnread(), + "the reply was marked read without its row following"); +} + +void TestMainWindow::taggingTheOpenRootMessageKeepsTheStripPopulated() +{ + // The user, 2026-08-16: "right pane loses the chip row when repainting, it + // simply disappears". + // + // The strip refresh added for item 105 reads the message's tags through + // messageById(), which searches only the loaded CHILDREN. A root card's + // message is never among them, so the lookup returned a default-constructed + // node and the refresh set the strip to that node's empty tag list, wiping + // a strip that had been correct a moment earlier. Worse than not + // refreshing: it actively destroyed what was there. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *strip = window.findChild<TagStrip *>(); + QVERIFY(strip); + + ThreadSummary t = makeThread(QStringLiteral("t1"), + { QStringLiteral("todo") }); + t.totalCount = 1; + model->appendBatch({ t }); + + selectThreadRow(view, 0); + QApplication::processEvents(); + + // visible + hidden, not visible alone. TagStrip is a single row that + // collapses whatever does not fit into a trailing "+N" chip, and this + // window is never shown, so it has no width to lay out with and puts + // almost everything in the hidden half. Asserting on visibleTags() alone + // measures the layout, not the data, and fails for a reason this test does + // not care about. + const auto stripTags = [strip]() { + return strip->visibleTags() + strip->hiddenTags(); + }; + + // The guard: the strip has to be showing something before the edit, or + // this cannot tell "wiped" from "never populated". + QVERIFY2(stripTags().contains(QStringLiteral("todo")), + "the strip never showed the selected thread's tags"); + + auto *action = window.findChild<QAction *>(QStringLiteral("delete")); + QVERIFY(action); + action->trigger(); + + QVERIFY2(!stripTags().isEmpty(), + "the chip row was emptied: the refresh looked the message up " + "among the loaded replies, where a root card's message never is, " + "and set the strip to the resulting empty tag list"); + QVERIFY2(stripTags().contains(QStringLiteral("todo")), + "the strip lost the tag the message still carries"); + QVERIFY2(stripTags().contains(QStringLiteral("deleted")), + "the strip did not pick up the tag just written"); +} + +void TestMainWindow::aLoadedMessageCorrectsTheStripFromTheThreadsUnion() +{ + // Reported by hand, 2026-08-16, against a real four-message thread whose + // root carried `unread` and whose THIRD message carried `signed`: + // "the right pane chips update and both signed and unread disappear ... + // changing message and going back makes them reappear". + // + // Neither half was the write's doing. Selecting a thread row sets the strip + // from ThreadSummary::tags, which is notmuch's UNION over the thread, so + // the pane claimed the root message was `signed` when a sibling was. The + // mark-read write then replaced it with the root's real tags, correctly + // dropping both, and reselecting put the union back. The pane was lying + // BEFORE the write, not after it. + // + // The load is the authority: MessageRef carries the message's own tags. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + auto *strip = window.findChild<TagStrip *>(); + QVERIFY(strip); + + const auto stripTags = [strip]() { + return strip->visibleTags() + strip->hiddenTags(); + }; + + // The union carries `signed`; the root message does not. + ThreadSummary t = makeThread(QStringLiteral("t1"), + { QStringLiteral("inbox"), + QStringLiteral("signed"), + QStringLiteral("unread") }); + t.totalCount = 4; + model->appendBatch({ t }); + + selectThreadRow(view, 0); + QApplication::processEvents(); + + // Before the load the strip can only show the union, which is what the + // model holds. That is the state the user saw and reported. + QVERIFY(stripTags().contains(QStringLiteral("signed"))); + + // The worker answers with the message's OWN tags. + MessageRef ref; + ref.messageId = QStringLiteral("t1-first@example.org"); + ref.tags = QStringList{ QStringLiteral("inbox"), QStringLiteral("unread") }; + QMetaObject::invokeMethod( + &window, "onMessageLoaded", Qt::DirectConnection, + Q_ARG(QVector<MessageRef>, QVector<MessageRef>{ ref }), + Q_ARG(quint64, window.currentGenerationForTesting())); + QApplication::processEvents(); + + QVERIFY2(!stripTags().contains(QStringLiteral("signed")), + "the pane still claims the root message is signed, which is a " + "sibling's tag: it is showing the thread's union rather than the " + "message on display"); + QVERIFY2(stripTags().contains(QStringLiteral("unread")), + "the pane lost a tag the message really carries"); +} + +void TestMainWindow::aLoadedRootMessageGivesTheCardItsOwnTags() +{ + // The same correction, reaching the MODEL, which is what fixes the two + // repaint reports: "if I mark the root message read the left pane entry + // doesn't repaint (stays bold)" and the same for delete. + // + // The card could not repaint because the model had no per-message tags for + // a root at all, so a message-scoped write updated the thread summary only + // when the thread was a single message. A load gives the root the same + // per-message node a reply has had all along, and from then on the card + // draws the message it displays. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + + ThreadSummary t = makeThread(QStringLiteral("t1"), + { QStringLiteral("inbox"), + QStringLiteral("signed"), + QStringLiteral("unread") }); + t.totalCount = 4; + model->appendBatch({ t }); + + selectThreadRow(view, 0); + QApplication::processEvents(); + + MessageRef ref; + ref.messageId = QStringLiteral("t1-first@example.org"); + ref.tags = QStringList{ QStringLiteral("inbox"), QStringLiteral("unread") }; + QMetaObject::invokeMethod( + &window, "onMessageLoaded", Qt::DirectConnection, + Q_ARG(QVector<MessageRef>, QVector<MessageRef>{ ref }), + Q_ARG(quint64, window.currentGenerationForTesting())); + QApplication::processEvents(); + + // The model now knows what the root message itself carries. + const MessageNode root = + model->messageById(QStringLiteral("t1-first@example.org")); + QCOMPARE(root.messageId, QStringLiteral("t1-first@example.org")); + QVERIFY2(!root.tags.contains(QStringLiteral("signed")), + "the root's node still carries a sibling's tag"); + + const QModelIndex threadIndex = model->index(0, 0, QModelIndex()); + QSignalSpy spy(model, &QAbstractItemModel::dataChanged); + + auto *toggle = window.findChild<QAction *>(QStringLiteral("toggle_unread")); + QVERIFY(toggle); + toggle->trigger(); + + QVERIFY2(spy.count() >= 1, "marking the root message read repainted nothing"); + QVERIFY2(!model->messageById(QStringLiteral("t1-first@example.org")) + .isUnread(), + "the root message is still unread after being marked read"); + + // The card now draws that message, so it stops looking unread. Asserted on + // the FONT, which is what the user means by "stays bold". + QVERIFY2(!model->data(threadIndex, Qt::FontRole).value<QFont>().bold(), + "the card still reads as unread, so the row stays bold and the " + "user sees nothing"); + QVERIFY2(model->threadAt(0).isUnread(), + "the thread summary was rewritten, claiming a four-message thread " + "is read when three of its messages still are not"); +} + void TestMainWindow::aTransientStatusMessageExpires() { // "Sync complete" describes an event, not a state, and reads as though it @@ -4796,7 +5716,10 @@ void TestMainWindow::anEditDuringABackgroundSyncIsNotSentYet() QVERIFY(model); auto *view = window.findChild<QTreeView *>(); QVERIFY(view); - auto *action = window.findChild<QAction *>(QStringLiteral("flag")); + // The THREAD action: this test asserts on the thread ROW, which a + // message-scoped write deliberately leaves alone since item 108. What + // is under test is the HOLD, which is identical either way. + auto *action = window.findChild<QAction *>(QStringLiteral("flag_thread")); QVERIFY2(action, "no flag action registered"); model->appendBatch({ makeThread(QStringLiteral("t1"), {}) }); @@ -4827,7 +5750,10 @@ void TestMainWindow::aHeldEditIsSentWhenTheBackgroundSyncEnds() QVERIFY(model); auto *view = window.findChild<QTreeView *>(); QVERIFY(view); - auto *action = window.findChild<QAction *>(QStringLiteral("flag")); + // The THREAD action: this test asserts on the thread ROW, which a + // message-scoped write deliberately leaves alone since item 108. What + // is under test is the HOLD, which is identical either way. + auto *action = window.findChild<QAction *>(QStringLiteral("flag_thread")); QVERIFY(action); model->appendBatch({ makeThread(QStringLiteral("t1"), {}) }); @@ -5360,15 +6286,19 @@ void TestMainWindow::theImportantActionStillWritesTheFlaggedTag() QVERIFY(action); action->trigger(); - // sendThreadTagChange() applies the change to the model optimistically, so - // the tag the action really wrote is observable here without a worker. - QVERIFY2(model->threadAt(0).isFlagged(), + // Asserted on the CHANGE that was sent rather than on the thread's row. + // Since item 108 this action is message-scoped, so it writes to the + // message the card displays and the thread summary is deliberately left + // alone. The tag name is what this test is about, and the change carries + // it whichever scope the action uses. + const TagChange sent = window.pendingChangeForTesting(); + QVERIFY2(sent.added.contains(QStringLiteral("flagged")), "the renamed action no longer writes the `flagged` tag"); - QVERIFY2(model->threadAt(0).tags.contains(QStringLiteral("flagged")), - "the tag written was not `flagged`"); - QVERIFY2(!model->threadAt(0).tags.contains(QStringLiteral("important")), + QVERIFY2(!sent.added.contains(QStringLiteral("important")), "the rename reached the mail store: an `important` tag was " "written, which no other tool reading this Maildir knows"); + QVERIFY2(sent.removed.isEmpty(), + "marking important removed a tag, which it must not"); } void TestMainWindow::theToolbarUsesTheConfiguredIconSize() @@ -5756,20 +6686,59 @@ void TestMainWindow::noTwoActionsShareAnIcon() // Compared by cacheKey() rather than by the theme NAME, which this window // does not keep. Two distinct names that resolve to the same art on a given // theme are just as ambiguous on screen, and that is what the user sees. + // Narrowed by item 108 to the actions that can reach the TOOLBAR, which is + // where the rule comes from: an icon-only toolbar makes the icon the whole + // control. The five whole-thread actions live only in the "Whole thread" + // submenu, whose entries always carry text, and each deliberately shares + // the icon of its message-scoped twin: same operation, wider scope, with + // the words saying which. Giving them five invented shapes would be less + // clear than the pairing. + // + // Named as an exception list rather than by asking the toolbar what it + // holds, so that PUTTING one of these on the toolbar fails this test + // rather than silently passing it. + static const QStringList menuOnlyThreadActions = { + QStringLiteral("archive_thread"), + QStringLiteral("delete_thread"), + QStringLiteral("spam_thread"), + QStringLiteral("toggle_unread_thread"), + QStringLiteral("flag_thread"), + }; + const Config config; MainWindow window(config); + // The exception must not become a hiding place: every one of them still + // has to carry an icon, which everyActionCarriesAnIcon asserts, and none + // may sit on the toolbar. + auto *toolBar = window.findChild<QToolBar *>(); + QVERIFY(toolBar); + for (const QString &name : menuOnlyThreadActions) { + auto *action = window.findChild<QAction *>(name); + QVERIFY2(action, qPrintable(QStringLiteral("no action named %1").arg(name))); + QVERIFY2(!toolBar->actions().contains(action), + qPrintable(QStringLiteral("%1 is on the toolbar, where a " + "shared icon is ambiguous, so it " + "cannot be exempt from this rule") + .arg(name))); + } + QHash<qint64, QString> owners; QStringList collisions; int withIcons = 0; + int compared = 0; for (const QString &name : KeyMap::knownActions()) { auto *action = window.findChild<QAction *>(name); QVERIFY2(action, qPrintable(QStringLiteral("no action named %1").arg(name))); + if (!action->icon().isNull()) + ++withIcons; + if (menuOnlyThreadActions.contains(name)) + continue; if (action->icon().isNull()) continue; + ++compared; - ++withIcons; const qint64 key = action->icon().cacheKey(); const auto existing = owners.constFind(key); if (existing != owners.constEnd()) { @@ -5789,6 +6758,10 @@ void TestMainWindow::noTwoActionsShareAnIcon() .arg(withIcons) .arg(KeyMap::knownActions().size()))); + // And the exception list did not swallow the comparison itself. + QCOMPARE(compared, KeyMap::knownActions().size() + - menuOnlyThreadActions.size()); + QVERIFY2(collisions.isEmpty(), qPrintable(QStringLiteral("actions sharing one icon: %1") .arg(collisions.join(QStringLiteral("; "))))); diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp index de96dbf..899fd11 100644 --- a/tests/test_notmuchworker.cpp +++ b/tests/test_notmuchworker.cpp @@ -52,6 +52,7 @@ private slots: void applyTagsIgnoresUnknownMessageIds(); void applyTagsWithNoIdsDoesNothing(); void queryStillWorksAfterWrite(); + void aThreadCarriesItsCardMessagesOwnTags(); void applyTagsToThreadsTagsEveryMessage(); void applyTagsToThreadsSpansMultipleThreads(); @@ -274,6 +275,55 @@ void TestNotmuchWorker::aQueryCarriesEachThreadsFirstMessageId() QVERIFY2(sawTheThread, "the two-message thread was not in the results"); } +void TestNotmuchWorker::aThreadCarriesItsCardMessagesOwnTags() +{ + // Item 111. A card draws its own message's tags at full size and the rest + // of the conversation's smaller, so it needs BOTH: `tags` is notmuch's + // union over the thread and `firstMessageTags` is the one message the card + // stands for. + // + // Derived from the message LOAD at first, which meant an unopened row had + // no split and drew everything as its own, correcting itself only when the + // user selected it. The user reported exactly that. The query knows, and + // the walk that finds firstMessageId is already holding the message, so + // this is the same index read rather than a second pass. + + // A tag on the REPLY only, which is the case that separates the two: a1 is + // the card's message, a2 its reply. + NotmuchWorker writer(m_fixture.configPath()); + writer.applyTags(TagChange{ { QStringLiteral("a2@example.org") }, + { QStringLiteral("signed") }, + {}, + QStringLiteral("Sign the reply") }); + + const QVector<ThreadSummary> threads = runQuery(QStringLiteral("*")); + bool sawTheThread = false; + for (const ThreadSummary &t : threads) { + if (t.subject != QStringLiteral("Release notes")) + continue; + sawTheThread = true; + + QCOMPARE(t.firstMessageId, QStringLiteral("a1@example.org")); + + // The union carries the reply's tag, as notmuch reports it. + QVERIFY2(t.tags.contains(QStringLiteral("signed")), + "the thread's own tags stopped being the union, which the " + "sibling tier is derived from"); + + // The card's message does not, and this is the whole point: without it + // the card claims a tag belonging to a message it does not display. + QVERIFY2(!t.firstMessageTags.isEmpty(), + "the query carried no per-message tags, so an unopened row " + "has no split and draws every chip at full size"); + QVERIFY2(!t.firstMessageTags.contains(QStringLiteral("signed")), + "the card's message was given its reply's tag"); + + // And it does carry its own. + QVERIFY(t.firstMessageTags.contains(QStringLiteral("inbox"))); + } + QVERIFY2(sawTheThread, "the two-message thread was not in the results"); +} + void TestNotmuchWorker::aSentQueryCarriesTheMatchedMessageNotTheThreadsFirst() { // THE case the Sent branch exists for, and the one hardest to get right. diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp index e338661..811b3e3 100644 --- a/tests/test_threadlistmodel.cpp +++ b/tests/test_threadlistmodel.cpp @@ -68,6 +68,20 @@ private slots: void threadIdIsReachableFromAnIndex(); void invalidIndexesReturnNothing(); void threadAtOutOfRangeIsSafe(); + void threadForResolvesAReplyThroughItsParent(); + void applyMessageTagChangeRepaintsThatReplyAlone(); + void aDeletedReplyIsPaintedAsDoomed(); + void aDeletedReplyIsStruckThrough(); + void markingAReplyReadChangesItsForeground(); + void anUnreadReplyIsBoldAndStillSmallerThanItsThread(); + void aThreadTagChangeReachesItsLoadedReplies(); + void messageScopeResolvesAThreadRowToTheMessageItDisplays(); + void messageScopeSkipsAThreadRowItCannotNameAMessageFor(); + void aMessageTagChangeReachesTheRootCardsOwnMessage(); + void aMessageTagChangeOnOneOfManyLeavesTheThreadSummaryAlone(); + void aCardListsItsOwnTagsBeforeItsSiblings(); + void theSplitIsKnownBeforeTheRowIsEverOpened(); + void reconcileRefreshesASurvivorsOwnMessageTags(); void updatesTagsForMessage(); void tagChangeIsIdempotent(); void tagChangeSignalsExactlyTheChangedRow(); @@ -877,6 +891,605 @@ void TestThreadListModel::threadAtOutOfRangeIsSafe() QCOMPARE(model.threadAt(0).threadId, QStringLiteral("t1")); } +void TestThreadListModel::threadForResolvesAReplyThroughItsParent() +{ + // Item 88. threadAt() takes a top-level row and a tree numbers rows per + // parent, so the first reply of ANY thread has row() == 0 and threadAt(0) + // answers "t1" for a reply of t2. threadFor() resolves through the parent + // instead, which is what every caller holding an index needs. + ThreadListModel model; + model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("one")), + makeThread(QStringLiteral("t2"), QStringLiteral("two")) }); + model.setThreadMessages(QStringLiteral("t2"), + { makeNode(QStringLiteral("m0@example.org"), 0), + makeNode(QStringLiteral("m1@example.org"), 1) }); + + const QModelIndex second = model.index(1, 0, QModelIndex()); + QCOMPARE(model.threadFor(second).threadId, QStringLiteral("t2")); + + const QModelIndex reply = model.index(0, 0, second); + QVERIFY(reply.isValid()); + QVERIFY2(model.isMessageRow(reply), + "the fixture did not produce a message row"); + QCOMPARE(reply.row(), 0); // The trap: a plausible top-level row number. + + QCOMPARE(model.threadFor(reply).threadId, QStringLiteral("t2")); + + // And the row-taking overload still does the wrong thing for that index, + // which is why it is documented as unsafe rather than merely deprecated. + QCOMPARE(model.threadAt(reply.row()).threadId, QStringLiteral("t1")); + + // An invalid index gives an empty summary, which every caller treats as + // "nothing to do" rather than acting on row 0. + QVERIFY(model.threadFor(QModelIndex()).threadId.isEmpty()); +} + +void TestThreadListModel::applyMessageTagChangeRepaintsThatReplyAlone() +{ + // The user's report: hitting Delete or Ctrl+U on a reply moved the pending + // count and changed nothing on screen. sendMessageTagChange made no + // optimistic update at all, on the correct reasoning that repainting the + // THREAD row would claim every message in it had changed. The row that + // should have repainted is the reply's own. + ThreadListModel model; + model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("one")) }); + + MessageNode root = makeNode(QStringLiteral("m0@example.org"), 0); + MessageNode reply = makeNode(QStringLiteral("m1@example.org"), 1); + reply.tags = QStringList{ QStringLiteral("unread") }; + model.setThreadMessages(QStringLiteral("t1"), { root, reply }); + + const QModelIndex threadIndex = model.index(0, 0, QModelIndex()); + const QModelIndex replyIndex = model.index(0, 0, threadIndex); + QVERIFY(model.isMessageRow(replyIndex)); + + const QStringList threadTagsBefore = + model.data(threadIndex, ThreadListModel::TagsRole).toStringList(); + + QSignalSpy spy(&model, &QAbstractItemModel::dataChanged); + + model.applyMessageTagChange(QStringLiteral("m1@example.org"), + { QStringLiteral("deleted") }, + { QStringLiteral("unread") }); + + // The node carries the change, which is what every reply-row role reads. + QCOMPARE(model.messageAt(replyIndex).isDeleted(), true); + QCOMPARE(model.messageAt(replyIndex).isUnread(), false); + + // And the view was told, or the change is invisible until something else + // happens to repaint the row. + QCOMPARE(spy.count(), 1); + QCOMPARE(spy.at(0).at(0).toModelIndex(), replyIndex); + + // The THREAD is untouched. Claiming the whole thread changed is the lie + // the missing update was avoiding, and it must stay avoided. + QCOMPARE(model.data(threadIndex, ThreadListModel::TagsRole).toStringList(), + threadTagsBefore); + QCOMPARE(model.messageAt(model.index(0, 0, threadIndex)).messageId, + QStringLiteral("m1@example.org")); + + // An unknown message is a no-op rather than a wrong row repainted. + spy.clear(); + model.applyMessageTagChange(QStringLiteral("nobody@example.org"), + { QStringLiteral("deleted") }, {}); + QCOMPARE(spy.count(), 0); +} + +void TestThreadListModel::aDeletedReplyIsPaintedAsDoomed() +{ + // Updating the node is not enough on its own: a reply row had no doomed + // branch at all, so a deleted reply repainted identically to an undeleted + // one and the user still saw nothing. The thread row has carried this cue + // since item 13. + ThreadListModel model; + model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("one")) }); + + MessageNode root = makeNode(QStringLiteral("m0@example.org"), 0); + MessageNode reply = makeNode(QStringLiteral("m1@example.org"), 1); + model.setThreadMessages(QStringLiteral("t1"), { root, reply }); + + const QModelIndex threadIndex = model.index(0, 0, QModelIndex()); + const QModelIndex replyIndex = model.index(0, 0, threadIndex); + + const QVariant plainBackground = + model.data(replyIndex, Qt::BackgroundRole); + + model.applyMessageTagChange(QStringLiteral("m1@example.org"), + { QStringLiteral("deleted") }, {}); + + const QVariant doomedBackground = + model.data(replyIndex, Qt::BackgroundRole); + QVERIFY2(doomedBackground != plainBackground, + "a deleted reply paints exactly like an undeleted one, so the " + "user has no way to see that Delete did anything"); + QCOMPARE(doomedBackground.value<QBrush>().color(), + ThreadListModel::deletedColour()); + + // Spam is the other half of isDoomed() and gets its own colour, so the two + // are told apart by hue rather than by shade. + model.applyMessageTagChange(QStringLiteral("m1@example.org"), + { QStringLiteral("spam") }, + { QStringLiteral("deleted") }); + QCOMPARE(model.data(replyIndex, Qt::BackgroundRole).value<QBrush>().color(), + ThreadListModel::spamColour()); +} + +void TestThreadListModel::aDeletedReplyIsStruckThrough() +{ + // The fill is not the only cue, deliberately: a strike-out survives a + // screenshot, a colourblind reader and a theme that overrides the + // background. The thread row has carried both since item 13; a reply had + // neither. + ThreadListModel model; + model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("one")) }); + model.setThreadMessages(QStringLiteral("t1"), + { makeNode(QStringLiteral("m0@example.org"), 0), + makeNode(QStringLiteral("m1@example.org"), 1) }); + + const QModelIndex replyIndex = + model.index(0, 0, model.index(0, 0, QModelIndex())); + + QVERIFY(!model.data(replyIndex, Qt::FontRole).value<QFont>().strikeOut()); + + model.applyMessageTagChange(QStringLiteral("m1@example.org"), + { QStringLiteral("deleted") }, {}); + + QVERIFY2(model.data(replyIndex, Qt::FontRole).value<QFont>().strikeOut(), + "a deleted reply is not struck through, so the only cue it has " + "is a background colour"); + + // The reply's smaller font is not lost to the strike-out branch: a reply + // reads as subordinate whatever its tags say. + const QFont threadFont = + model.data(model.index(0, 0, QModelIndex()), Qt::FontRole).value<QFont>(); + const QFont replyFont = + model.data(replyIndex, Qt::FontRole).value<QFont>(); + if (threadFont.pointSize() > 0 && replyFont.pointSize() > 0) + QVERIFY(replyFont.pointSize() < threadFont.pointSize()); +} + +void TestThreadListModel::markingAReplyReadChangesItsForeground() +{ + // The user's second report: marking a reply read or unread moved the + // counter with no visible change. A reply is deliberately NEVER bold, so + // unlike a thread row its only cue is the foreground dimming. That cue has + // to at least exist and change, which is what this pins. + ThreadListModel model; + model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("one")) }); + + MessageNode root = makeNode(QStringLiteral("m0@example.org"), 0); + MessageNode reply = makeNode(QStringLiteral("m1@example.org"), 1); + reply.tags = QStringList{ QStringLiteral("unread") }; + model.setThreadMessages(QStringLiteral("t1"), { root, reply }); + + const QModelIndex replyIndex = + model.index(0, 0, model.index(0, 0, QModelIndex())); + + const QVariant unreadForeground = + model.data(replyIndex, Qt::ForegroundRole); + + model.applyMessageTagChange(QStringLiteral("m1@example.org"), {}, + { QStringLiteral("unread") }); + + const QVariant readForeground = model.data(replyIndex, Qt::ForegroundRole); + QVERIFY2(readForeground != unreadForeground, + "marking a reply read changed nothing about how its row paints"); + QCOMPARE(readForeground.value<QBrush>().color(), + ThreadListModel::readColour()); + + // And back, so the toggle is visible in both directions rather than only + // on the way to read. + model.applyMessageTagChange(QStringLiteral("m1@example.org"), + { QStringLiteral("unread") }, {}); + QCOMPARE(model.data(replyIndex, Qt::ForegroundRole), unreadForeground); +} + +void TestThreadListModel::anUnreadReplyIsBoldAndStillSmallerThanItsThread() +{ + // Requested by the user on 2026-08-16: "I prefer the bold on replies + // combined with the dimming." Replies were deliberately never bold before + // that, so this pins the decision rather than describing the code. + // + // Both halves matter. Bold is the second cue, next to the dimming; the + // smaller size is what still separates a reply from the thread heading + // above it, and dropping it would make an unread reply indistinguishable + // from a thread row. + ThreadListModel model; + model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("one")) }); + + MessageNode root = makeNode(QStringLiteral("m0@example.org"), 0); + MessageNode reply = makeNode(QStringLiteral("m1@example.org"), 1); + reply.tags = QStringList{ QStringLiteral("unread") }; + model.setThreadMessages(QStringLiteral("t1"), { root, reply }); + + const QModelIndex threadIndex = model.index(0, 0, QModelIndex()); + const QModelIndex replyIndex = model.index(0, 0, threadIndex); + + const QFont unreadFont = + model.data(replyIndex, Qt::FontRole).value<QFont>(); + QVERIFY2(unreadFont.bold(), "an unread reply is not bold"); + + const QFont threadFont = + model.data(threadIndex, Qt::FontRole).value<QFont>(); + if (threadFont.pointSize() > 0 && unreadFont.pointSize() > 0) { + QVERIFY2(unreadFont.pointSize() < threadFont.pointSize(), + "a bold reply is the same size as its thread row, so the two " + "kinds of row no longer read apart"); + } + + // Bold is the unread cue specifically, not decoration on every reply. + model.applyMessageTagChange(QStringLiteral("m1@example.org"), {}, + { QStringLiteral("unread") }); + QVERIFY2(!model.data(replyIndex, Qt::FontRole).value<QFont>().bold(), + "a read reply is still bold, so bold says nothing"); +} + +void TestThreadListModel::aThreadTagChangeReachesItsLoadedReplies() +{ + // The user's report: "if I hit read/unread on the main thread message [...] + // only the main message is repainted [...] the replies don't get + // repainted." + // + // A thread-scoped write reaches every message in the thread IN THE + // DATABASE. applyTagChange only ever updated the thread's summary, so an + // expanded thread kept showing replies with their old tags: bold, undimmed + // and unstruck, describing a state the database no longer held. The rows + // corrected themselves on the next query, which is what made this look + // like a repaint problem rather than a stale-model one. + ThreadListModel model; + model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("one")) }); + + MessageNode root = makeNode(QStringLiteral("m0@example.org"), 0); + root.tags = QStringList{ QStringLiteral("unread") }; + MessageNode reply = makeNode(QStringLiteral("m1@example.org"), 1); + reply.tags = QStringList{ QStringLiteral("unread") }; + model.setThreadMessages(QStringLiteral("t1"), { root, reply }); + + const QModelIndex threadIndex = model.index(0, 0, QModelIndex()); + const QModelIndex replyIndex = model.index(0, 0, threadIndex); + QVERIFY(model.messageAt(replyIndex).isUnread()); + + QSignalSpy spy(&model, &QAbstractItemModel::dataChanged); + + model.applyTagChange(QStringLiteral("t1"), {}, + { QStringLiteral("unread") }); + + QVERIFY2(!model.messageAt(replyIndex).isUnread(), + "a thread marked read left its loaded replies carrying unread, so " + "the rows describe a state the database does not hold"); + + // The reply's row was told to repaint, not merely mutated behind the view. + bool replyRepainted = false; + for (const QList<QVariant> &call : spy) { + const QModelIndex from = call.at(0).toModelIndex(); + const QModelIndex to = call.at(1).toModelIndex(); + if (from.parent() == threadIndex && replyIndex.row() >= from.row() + && replyIndex.row() <= to.row()) { + replyRepainted = true; + break; + } + } + QVERIFY2(replyRepainted, + "no dataChanged covered the reply rows, so the view has no reason " + "to redraw them"); + + // Both directions, since a toggle is only fixed if it is visible each way. + model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("unread") }, {}); + QVERIFY(model.messageAt(replyIndex).isUnread()); +} + +void TestThreadListModel::messageScopeResolvesAThreadRowToTheMessageItDisplays() +{ + // Item 108. A thread root RENDERS one message since item 66, so acting on + // it acts on that message. The thread's other messages are reached through + // the explicit thread actions, which still resolve through scopeFor(). + ThreadListModel model; + ThreadSummary t = makeThread(QStringLiteral("t1"), + QStringLiteral("A subject")); + t.totalCount = 7; + t.firstMessageId = QStringLiteral("m0@example.org"); + model.appendBatch({ t }); + + const QModelIndex root = model.index(0, 0, QModelIndex()); + + // Unexpanded, which is the case that matters: the id comes from the query, + // so this needs no children loaded. + QCOMPARE(model.rowCount(root), 0); + + const ActionScope scope = model.messageScopeFor({ root }); + QCOMPARE(scope.messageIds, QStringList{ QStringLiteral("m0@example.org") }); + QVERIFY2(scope.threadIds.isEmpty(), + "a thread row still resolved to its whole thread, so every action " + "on a root card would touch messages it does not display"); + QCOMPARE(scope.messageCount, 1); + QVERIFY2(!scope.wholeThread, + "the status bar would claim '(whole thread)' for a one-message " + "action"); + + // The old resolver is unchanged and is what the thread actions use. + const ActionScope threadScope = model.scopeFor({ root }); + QCOMPARE(threadScope.threadIds, QStringList{ QStringLiteral("t1") }); + QCOMPARE(threadScope.messageCount, 7); + QVERIFY(threadScope.wholeThread); + + // A reply row is unchanged in both: it always stood for one message. + model.setThreadMessages(QStringLiteral("t1"), + { makeNode(QStringLiteral("m0@example.org"), 0), + makeNode(QStringLiteral("m1@example.org"), 1) }); + const QModelIndex reply = model.index(0, 0, root); + QCOMPARE(model.messageScopeFor({ reply }).messageIds, + QStringList{ QStringLiteral("m1@example.org") }); + + // A root and one of its own replies is two DISTINCT messages, not one + // deduplicated to the thread. + const ActionScope both = model.messageScopeFor({ root, reply }); + QCOMPARE(both.messageIds, + (QStringList{ QStringLiteral("m0@example.org"), + QStringLiteral("m1@example.org") })); + QCOMPARE(both.messageCount, 2); +} + +void TestThreadListModel::messageScopeSkipsAThreadRowItCannotNameAMessageFor() +{ + // firstMessageId is populated by the worker from the query. A summary that + // arrived without one names no message, and the tempting fallback is to + // act on the whole thread instead. That is exactly the silent escalation + // item 108 exists to remove: the user would ask to act on one message and + // hit the conversation. + ThreadListModel model; + ThreadSummary t = makeThread(QStringLiteral("t1"), + QStringLiteral("A subject")); + t.totalCount = 4; + t.firstMessageId.clear(); + model.appendBatch({ t }); + + const QModelIndex root = model.index(0, 0, QModelIndex()); + const ActionScope scope = model.messageScopeFor({ root }); + + QVERIFY2(scope.isEmpty(), + "a thread row with no message id was escalated to its whole " + "thread rather than skipped"); + QCOMPARE(scope.messageCount, 0); +} + +void TestThreadListModel::aMessageTagChangeReachesTheRootCardsOwnMessage() +{ + // The user, 2026-08-16: "delete single message on the root message of a + // thread doesn't trigger the repaint, delete whole thread does". + // + // applyMessageTagChange only searched `children`, and the root message is + // never there: setThreadMessages drops depth 0 because the root row stands + // for it. So a write to the message a root card displays found nothing, + // updated nothing and repainted nothing, while the same write on a reply + // worked. Item 108 made this the ORDINARY case, so the every-day gesture + // was the broken one. + ThreadListModel model; + ThreadSummary t = makeThread(QStringLiteral("t1"), QStringLiteral("one")); + t.totalCount = 1; // A single-message thread. + t.firstMessageId = QStringLiteral("m0@example.org"); + t.tags = QStringList{ QStringLiteral("inbox") }; + model.appendBatch({ t }); + + const QModelIndex threadIndex = model.index(0, 0, QModelIndex()); + QSignalSpy spy(&model, &QAbstractItemModel::dataChanged); + + model.applyMessageTagChange(QStringLiteral("m0@example.org"), + { QStringLiteral("deleted") }, {}); + + // The card has to repaint, which is the whole report. + QCOMPARE(spy.count(), 1); + QCOMPARE(spy.at(0).at(0).toModelIndex(), threadIndex); + + // And it has to LOOK deleted. For a one-message thread the thread's tags + // ARE that message's tags: notmuch_thread_get_tags is a union over the + // thread, and a union over one message is that message. + QVERIFY2(model.threadAt(0).isDeleted(), + "the root card does not show the state of the message it " + "displays, so Delete on it looks like it did nothing"); + + // Works before the thread has ever been expanded, which is the case the + // user hits: nothing loads a root's node until then. + QCOMPARE(model.rowCount(threadIndex), 0); +} + +void TestThreadListModel::aMessageTagChangeOnOneOfManyLeavesTheThreadSummaryAlone() +{ + // The other half, and the reason the fix is not "write it to the summary". + // A thread's tags are a UNION over its messages, so deleting one message of + // seven does not make the conversation deleted, and painting the card + // crimson would claim it did. + ThreadListModel model; + ThreadSummary t = makeThread(QStringLiteral("t1"), QStringLiteral("one")); + t.totalCount = 7; + t.firstMessageId = QStringLiteral("m0@example.org"); + t.tags = QStringList{ QStringLiteral("inbox"), QStringLiteral("unread") }; + model.appendBatch({ t }); + + const QModelIndex threadIndex = model.index(0, 0, QModelIndex()); + QSignalSpy spy(&model, &QAbstractItemModel::dataChanged); + + model.applyMessageTagChange(QStringLiteral("m0@example.org"), + { QStringLiteral("deleted") }, + { QStringLiteral("unread") }); + + // Still repaints: MessageIdRole and anything else keyed on the root's own + // node has changed, and the row is what the user is looking at. + QCOMPARE(spy.count(), 1); + + QVERIFY2(!model.threadAt(0).isDeleted(), + "deleting one message of a seven-message thread painted the whole " + "conversation as deleted"); + QVERIFY2(model.threadAt(0).isUnread(), + "marking one message of a seven-message thread read claimed the " + "whole conversation was read, though six messages still are not"); +} + +void TestThreadListModel::aCardListsItsOwnTagsBeforeItsSiblings() +{ + // The user, 2026-08-16, looking at a real four-message thread: the card + // showed `mailing-list/SBo` and `signed`, and `signed` vanished the moment + // the row was selected, because it belongs to a SIBLING and item 110 made + // the card stop claiming it. + // + // Their answer, which is better than either extreme: show both, and let + // size say whose is whose. Own tags first at full size, the thread's other + // tags after, smaller and muted. Nothing disappears; a chip only shrinks + // once the split becomes known. + ThreadListModel model; + ThreadSummary t = makeThread(QStringLiteral("t1"), QStringLiteral("one")); + t.totalCount = 4; + t.firstMessageId = QStringLiteral("m0@example.org"); + // The UNION, as notmuch reports it: `signed` is a sibling's. + t.tags = QStringList{ QStringLiteral("inbox"), + QStringLiteral("mailing-list/SBo"), + QStringLiteral("signed"), + QStringLiteral("unread") }; + model.appendBatch({ t }); + + const QModelIndex threadIndex = model.index(0, 0, QModelIndex()); + + // Before the row is opened there is no per-message answer, so every chip + // is in the own tier. This is what stops anything from appearing to vanish + // later: the split narrows the tier, it does not remove a chip. + const QStringList before = + model.data(threadIndex, ThreadListModel::PillTagsRole).toStringList(); + QVERIFY(before.contains(QStringLiteral("mailing-list/SBo"))); + QVERIFY(before.contains(QStringLiteral("signed"))); + QCOMPARE(model.data(threadIndex, ThreadListModel::PillOwnCountRole).toInt(), + before.size()); + + // The message loads, carrying what it really has. + model.setRootMessageTags(QStringLiteral("m0@example.org"), + { QStringLiteral("inbox"), + QStringLiteral("mailing-list/SBo"), + QStringLiteral("unread") }); + + const QStringList after = + model.data(threadIndex, ThreadListModel::PillTagsRole).toStringList(); + + // Same chips, still all present. The user explicitly did not want the + // sibling's tag dropped. + QVERIFY2(after.contains(QStringLiteral("signed")), + "the sibling's tag was dropped from the card rather than being " + "shown smaller, which is what looked like a bug"); + QVERIFY2(after.contains(QStringLiteral("mailing-list/SBo")), + "the card lost a tag the message really carries"); + + // Own first, siblings after, and the count is where the delegate switches + // fonts. + const int own = + model.data(threadIndex, ThreadListModel::PillOwnCountRole).toInt(); + QVERIFY2(own > 0 && own < after.size(), + "the split did not happen: every chip is in one tier"); + QCOMPARE(after.mid(0, own), + QStringList{ QStringLiteral("mailing-list/SBo") }); + QCOMPARE(after.mid(own), QStringList{ QStringLiteral("signed") }); + + // Colours stay aligned with the tags, since the delegate walks them in + // step and a shift would colour a chip with its neighbour's colour. + QCOMPARE(model.data(threadIndex, ThreadListModel::PillColoursRole) + .toList() + .size(), + after.size()); +} + +void TestThreadListModel::theSplitIsKnownBeforeTheRowIsEverOpened() +{ + // The user, 2026-08-16: "not selecting the thread shows the chips at 'main' + // size, not smaller, not dimmed. After selecting the thread the unioned + // chips repaint to the correct size/color." + // + // The first version derived the split from the message LOAD, so an unopened + // row had no per-message answer and put every chip in the own tier. That is + // honest and useless: the list is mostly unopened rows, so the feature was + // invisible exactly where it was meant to be read, and selecting a row + // still changed the card. + // + // The query knows. The worker already walks to the card's message to get + // its id, so it reads that message's tags in the same pass and the split + // arrives with the row. + ThreadListModel model; + ThreadSummary t = makeThread(QStringLiteral("t1"), QStringLiteral("one")); + t.totalCount = 4; + t.firstMessageId = QStringLiteral("m0@example.org"); + t.tags = QStringList{ QStringLiteral("inbox"), + QStringLiteral("mailing-list/SBo"), + QStringLiteral("signed"), + QStringLiteral("unread") }; + // What the worker now supplies: the CARD's message, not the thread. + t.firstMessageTags = QStringList{ QStringLiteral("inbox"), + QStringLiteral("mailing-list/SBo"), + QStringLiteral("unread") }; + model.appendBatch({ t }); + + const QModelIndex threadIndex = model.index(0, 0, QModelIndex()); + + // Never opened, never expanded. + QCOMPARE(model.rowCount(threadIndex), 0); + + const QStringList pills = + model.data(threadIndex, ThreadListModel::PillTagsRole).toStringList(); + const int own = + model.data(threadIndex, ThreadListModel::PillOwnCountRole).toInt(); + + QVERIFY2(own < pills.size(), + "an unopened row still puts every chip in the own tier, so the " + "card renders them all at full size and only corrects itself " + "when the row is selected"); + QCOMPARE(pills.mid(0, own), QStringList{ QStringLiteral("mailing-list/SBo") }); + QCOMPARE(pills.mid(own), QStringList{ QStringLiteral("signed") }); + + // And a message-scoped write still lands, without a load having happened. + model.applyMessageTagChange(QStringLiteral("m0@example.org"), + { QStringLiteral("deleted") }, {}); + QVERIFY(model.messageById(QStringLiteral("m0@example.org")).isDeleted()); + QVERIFY2(!model.threadAt(0).isDeleted(), + "the thread summary was rewritten for a one-message edit on a " + "four-message thread"); +} + +void TestThreadListModel::reconcileRefreshesASurvivorsOwnMessageTags() +{ + // reconcile() keeps a surviving row's NODE, deliberately: its children and + // its loaded flag are the expansion state the method exists to preserve. + // That means the per-message tags have to be refreshed explicitly, and the + // change detector has to notice when only they moved. + // + // The case: a sync where the root message alone changed, which is exactly + // what an external `notmuch tag` or another client does. The thread's union + // can be identical while the card's own message is not. + ThreadListModel model; + ThreadSummary before = makeThread(QStringLiteral("t1"), + QStringLiteral("one")); + before.totalCount = 2; + before.firstMessageId = QStringLiteral("m0@example.org"); + before.tags = QStringList{ QStringLiteral("inbox"), + QStringLiteral("unread") }; + before.firstMessageTags = QStringList{ QStringLiteral("inbox"), + QStringLiteral("unread") }; + model.appendBatch({ before }); + + QVERIFY(model.messageById(QStringLiteral("m0@example.org")).isUnread()); + + // The root was read elsewhere. The THREAD is still unread, because its + // reply is, so the union does not move at all. + ThreadSummary after = before; + after.firstMessageTags = QStringList{ QStringLiteral("inbox") }; + + QSignalSpy spy(&model, &QAbstractItemModel::dataChanged); + model.reconcile({ after }); + + QVERIFY2(!model.messageById(QStringLiteral("m0@example.org")).isUnread(), + "a sync that changed only the card's own message left the row " + "showing the old per-message tags"); + QVERIFY2(spy.count() >= 1, + "the change was applied without telling the view, so the card " + "keeps its old pixels until something else repaints it"); + + // The expansion state is still what reconcile() exists to preserve. + QCOMPARE(model.threadAt(0).threadId, QStringLiteral("t1")); +} + void TestThreadListModel::updatesTagsForMessage() { ThreadListModel model; |