aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-28 20:15:15 +0200
committerDanilo M. <danix@danix.xyz>2026-08-28 20:15:15 +0200
commitae2ae2df75ed780a88423b77af3b31fbe2b26389 (patch)
tree4550fbf6856ff03eb80487475514a979798c6641
parentdb38ca136c45f34f5fa2d342ee180f068073be37 (diff)
downloadqtmaildir-ae2ae2df75ed780a88423b77af3b31fbe2b26389.tar.gz
qtmaildir-ae2ae2df75ed780a88423b77af3b31fbe2b26389.zip
fix: list a conversation's first message under its row
setThreadMessages() dropped nodes.first(), which was correct while a thread row MEANT its first message: listing that message under itself would have shown it twice. Item 177 made the row stand for the conversation and render a dashboard instead, so the drop left the first message with no row anywhere: the user reported the list starting at the second message with the first unreachable. A conversation row now keeps every message, including the first; a thread of one keeps the old rule, since there it IS its message and must not be listed beneath itself. The choice reads what actually ARRIVED rather than summary.totalCount, which counts duplicates and can lie about whether a thread really is a conversation. Reply-scoped tests pointed at child 0, which was the first reply and is now the first message; they read child 1 instead, and two asserted a child count that grew by one. Two new model tests pin both halves, and both directions are mutation-checked.
-rw-r--r--src/threadlistmodel.cpp43
-rw-r--r--tests/test_mainwindow.cpp52
-rw-r--r--tests/test_threadlistmodel.cpp188
3 files changed, 215 insertions, 68 deletions
diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp
index 0559694..8bb9849 100644
--- a/src/threadlistmodel.cpp
+++ b/src/threadlistmodel.cpp
@@ -942,22 +942,37 @@ void ThreadListModel::setThreadMessages(const QString &threadId,
endRemoveRows();
}
- // Every message EXCEPT the first, which is the root card itself.
+ // A CONVERSATION row keeps every message, including the first.
//
- // Selecting on depth > 0 instead was wrong, and wrong in a way that
- // only showed on real mail: notmuch_thread_get_toplevel_messages
- // returns every message at depth 0 when a thread carries no usable
- // In-Reply-To, so a flat thread contributed no children at all. The
- // card advertised "3 replies" and expanded onto nothing. Measured in
- // the user's database: of 396 inbox threads, three are flat, one of
- // them nine messages long, and every two-message thread of this kind
- // was affected, which is why the fault looked like "the expander only
- // works with more than one reply".
+ // This dropped `nodes.first()` until item 177, and correctly: a thread
+ // row then MEANT its first message, so listing that message under
+ // itself would have shown it twice. Item 177 reversed the premise. A
+ // conversation row stands for the conversation and renders a dashboard
+ // rather than any one message, so dropping the first message left it
+ // with no row anywhere: the user reported the list starting at the
+ // second message with the first unreachable.
//
- // Position also happens to be the right rule rather than a workaround.
- // The root card IS the thread's first message, so the row under it is
- // the second message whatever depth notmuch assigns it.
- QVector<MessageNode> children = nodes.mid(1);
+ // A thread of one is still a message row, and there the old rule holds
+ // exactly as it did. Giving it a child would list the message beneath
+ // itself, which is the duplication the drop existed to prevent.
+ //
+ // Selecting on depth > 0 instead is wrong whichever premise holds, and
+ // wrong in a way that only showed on real mail:
+ // notmuch_thread_get_toplevel_messages returns every message at depth 0
+ // when a thread carries no usable In-Reply-To, so a flat thread
+ // contributed no children at all. The card advertised "3 replies" and
+ // expanded onto nothing. Measured in the user's database: of 396 inbox
+ // threads, three are flat, one of them nine messages long, and every
+ // two-message thread of this kind was affected, which is why the fault
+ // looked like "the expander only works with more than one reply".
+ // Position remains the rule; only where it starts has changed.
+ // Decided on what actually ARRIVED, never on summary.totalCount. That
+ // count includes duplicates, so a "thread of 2" can load one message;
+ // trusting it would give that row a single child, and isConversationRow()
+ // reads children once loaded, so the row would claim to be a
+ // conversation it cannot open. The two rules have to agree, and the
+ // messages are the truth.
+ QVector<MessageNode> children = nodes.size() > 1 ? nodes : nodes.mid(1);
// Kept so the root card can render its own message. It is the card the
// user clicks to read the thread's opening message.
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index cba4c96..aecefb7 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -1062,7 +1062,10 @@ void TestMainWindow::childRowsAreIndentedUnderTheirThread()
// visualRect reported an indent the text did not have; here it reports
// none while the text is indented.
const QModelIndex rootCell = model->index(0, 0, QModelIndex());
- const QModelIndex child = model->index(0, 0, root);
+ // Child 1, not child 0: since item 177 a conversation lists its FIRST
+ // message as a child too, so child 0 is m0 (depth 0) and the reply whose
+ // nesting is being measured is the one after it.
+ const QModelIndex child = model->index(1, 0, root);
QVERIFY(child.isValid());
// Guards before the claim: a probe that cannot see both rows can report
@@ -1635,7 +1638,10 @@ void TestMainWindow::anActionOnAMessageRowTagsThatMessageNotTheThread()
view->expand(threadRow);
QApplication::processEvents();
- const QModelIndex messageRow = model->index(0, 0, threadRow);
+ // Child 1, not child 0: since item 177 a conversation lists its first
+ // message as a child too, so child 0 is m0 and the reply this test acts on
+ // is the one after the card's own message.
+ const QModelIndex messageRow = model->index(1, 0, threadRow);
QVERIFY(model->isMessageRow(messageRow));
view->selectionModel()->select(
@@ -3227,7 +3233,10 @@ void TestMainWindow::theStaleNoticeCarriesTheMessageBeingRead()
model->setThreadMessages(QStringLiteral("T1"), { root, reply });
const QModelIndex threadIndex = model->index(0, 0, QModelIndex());
- const QModelIndex replyIndex = model->index(0, 0, threadIndex);
+ // Child 1, not child 0: since item 177 a conversation lists its first
+ // message as a child too, so the reply being read is the one after the
+ // card's own message.
+ const QModelIndex replyIndex = model->index(1, 0, threadIndex);
QVERIFY(replyIndex.isValid());
view->setCurrentIndex(replyIndex);
@@ -4008,9 +4017,12 @@ void TestMainWindow::recoveryFromAnExpandedThreadRestoresTheReply()
const QModelIndex threadIndex = model->index(0, 0, QModelIndex());
view->expand(threadIndex);
- QCOMPARE(model->rowCount(threadIndex), 3);
+ // All four messages are children since item 177: a conversation lists its
+ // first message under itself like every other.
+ QCOMPARE(model->rowCount(threadIndex), 4);
- const QModelIndex fourth = model->index(2, 0, threadIndex);
+ // The fourth message, now at child index 3 behind the first three.
+ const QModelIndex fourth = model->index(3, 0, threadIndex);
QVERIFY(fourth.isValid());
QCOMPARE(model->messageAt(fourth).messageId,
QStringLiteral("m3@example.org"));
@@ -4045,7 +4057,7 @@ void TestMainWindow::recoveryFromAnExpandedThreadRestoresTheReply()
QVERIFY2(view->isExpanded(back),
"the thread collapsed again once its replies arrived");
- QCOMPARE(model->rowCount(back), 3);
+ QCOMPARE(model->rowCount(back), 4);
const QModelIndex current = view->currentIndex();
QVERIFY2(current.isValid(), "recovery left nothing selected");
@@ -4917,13 +4929,20 @@ static QModelIndex expandSecondThreadAndSelectItsReply(
const QModelIndex threadRow = model->index(1, 0, QModelIndex());
view->expand(threadRow);
- const QModelIndex replyRow = model->index(0, 0, threadRow);
+ // Child 1, not child 0. Since item 177 a conversation lists its FIRST
+ // message as a child too, so child 0 is m0 and the reply this helper
+ // promises is the one after it. Every caller is about a reply
+ // specifically, and handing them the root message instead makes each one
+ // assert about the wrong message while still looking correct.
+ const QModelIndex replyRow = model->index(1, 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)
+ // A child row number that is also a plausible top-level one, which is the
+ // trap: threadAt() cannot tell the difference. Child 1 sits under the
+ // thread at top-level row 1, so threadAt() reading it still lands on the
+ // wrong object, which is what these tests exist to catch.
+ if (replyRow.row() != 1)
return {};
view->selectionModel()->select(
@@ -5831,8 +5850,10 @@ void TestMainWindow::taggingAnUnrelatedReplyLeavesTheStripAlone()
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);
+ // The FIRST reply is the one on display. Child 1, not child 0: since item
+ // 177 a conversation lists its first message as a child too, so child 0 is
+ // m0 (the card's own message) and the reply the strip describes is m1.
+ const QModelIndex displayed = model->index(1, 0, threadRow);
view->selectionModel()->select(
displayed,
QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows);
@@ -12268,9 +12289,12 @@ void TestMainWindow::deleteOnAReplyMovesThatReplyOnly()
QVERIFY2(conversation.isValid(), "no multi-message thread in the list");
view->expand(conversation);
- QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(conversation) == 1, 15000);
+ // Both messages are children since item 177: the conversation lists its
+ // first message under itself too.
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(conversation) == 2, 15000);
- const QModelIndex replyIndex = model->index(0, 0, conversation);
+ // The reply is child 1, behind the thread's first message.
+ const QModelIndex replyIndex = model->index(1, 0, conversation);
QVERIFY(model->isMessageRow(replyIndex));
QCOMPARE(model->messageAt(replyIndex).messageId,
QStringLiteral("reply@example.org"));
diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp
index dbff6b7..fba2d2d 100644
--- a/tests/test_threadlistmodel.cpp
+++ b/tests/test_threadlistmodel.cpp
@@ -106,6 +106,8 @@ private slots:
void aRowNamingNoMessageIsSkippedNotEscalated();
void aLoneMessageRowResolvesToItsMessage();
void aReplyRowResolvesToItsMessage();
+ void aConversationListsItsFirstMessageAsAChild();
+ void aMessageRowDoesNotListItselfBeneathItself();
void aMixedSelectionCarriesBothScopes();
};
@@ -142,19 +144,20 @@ void TestThreadListModel::repliesBecomeChildRowsUnderTheirThread()
model.appendBatch({ makeThread(QStringLiteral("t1"),
QStringLiteral("A subject")) });
- // Depth 0 is the thread's FIRST message and belongs on the root row, not in
- // the children: the user's model is "N replies", so a thread of three shows
- // one root and two children.
+ // EVERY message sits under a conversation row, the first included. This
+ // read "one root and two children" until item 177, when the root stopped
+ // meaning the first message and started meaning the conversation; the
+ // first message then had no row anywhere and the user could not reach it.
model.setThreadMessages(QStringLiteral("t1"),
{ makeNode(QStringLiteral("m0@example.org"), 0),
makeNode(QStringLiteral("m1@example.org"), 1),
makeNode(QStringLiteral("m2@example.org"), 2) });
const QModelIndex root = model.index(0, 0, QModelIndex());
- QCOMPARE(model.rowCount(root), 2);
+ QCOMPARE(model.rowCount(root), 3);
const QModelIndex child =
- model.index(0, 0, root);
+ model.index(1, 0, root);
QVERIFY(child.isValid());
QCOMPARE(model.parent(child), model.index(0, 0, QModelIndex()));
@@ -196,7 +199,7 @@ void TestThreadListModel::messageRowsShowTheirOwnSenderAndSubject()
QStringLiteral("Re: A subject")) });
const QModelIndex root = model.index(0, 0, QModelIndex());
- const QModelIndex reply = model.index(0, 0, root);
+ const QModelIndex reply = model.index(1, 0, root);
QCOMPARE(model.data(reply, ThreadListModel::SendersRole).toString(),
QStringLiteral("Bob <bob@example.org>"));
@@ -222,12 +225,15 @@ void TestThreadListModel::reloadingAThreadReplacesItsRepliesRatherThanRepeatingT
makeNode(QStringLiteral("m1@example.org"), 1)
};
+ // Two messages, two rows, both times. The number is what item 177 changed;
+ // the property under test is that a reload REPLACES rather than appends, so
+ // what matters is that the second call does not double it.
model.setThreadMessages(QStringLiteral("t1"), nodes);
const QModelIndex root = model.index(0, 0, QModelIndex());
- QCOMPARE(model.rowCount(root), 1);
+ QCOMPARE(model.rowCount(root), 2);
model.setThreadMessages(QStringLiteral("t1"), nodes);
- QCOMPARE(model.rowCount(root), 1);
+ QCOMPARE(model.rowCount(root), 2);
QAbstractItemModelTester tester(
&model, QAbstractItemModelTester::FailureReportingMode::Warning);
@@ -474,7 +480,7 @@ void TestThreadListModel::aMessageRowCarriesItsOwnSenderAndAddress()
model.setThreadMessages(QStringLiteral("t1"), { root, reply });
const QModelIndex replyIndex =
- model.index(0, 0, model.index(0, 0, QModelIndex()));
+ model.index(1, 0, model.index(0, 0, QModelIndex()));
QVERIFY(model.isMessageRow(replyIndex));
QCOMPARE(replyIndex.data(ThreadListModel::SenderAddressRole).toString(),
@@ -882,7 +888,7 @@ void TestThreadListModel::invalidIndexesReturnNothing()
QVERIFY(!model.index(-1, 0).isValid());
// A child index must yield nothing: this is a table, not a tree.
- const QModelIndex child = model.index(0, 0, model.index(0, 0));
+ const QModelIndex child = model.index(1, 0, model.index(0, 0));
QVERIFY(!child.isValid());
QVERIFY(!model.data(child, Qt::DisplayRole).isValid());
}
@@ -900,18 +906,25 @@ void TestThreadListModel::threadAtOutOfRangeIsSafe()
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.
+ // parent, so a child's row() indexes its siblings and threadAt() reads it
+ // as a position in the top-level list. threadFor() resolves through the
+ // parent instead, which is what every caller holding an index needs.
+ //
+ // THREE threads, and the message taken from the LAST one. The child row
+ // and the thread's own row have to differ, or threadAt() returns the right
+ // answer by coincidence and the test proves nothing. With the conversation
+ // at top-level row 2 and its message at child row 0, threadAt(0) names the
+ // first thread while the message belongs to the third.
ThreadListModel model;
model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("one")),
- makeThread(QStringLiteral("t2"), QStringLiteral("two")) });
- model.setThreadMessages(QStringLiteral("t2"),
+ makeThread(QStringLiteral("t2"), QStringLiteral("two")),
+ makeThread(QStringLiteral("t3"), QStringLiteral("three")) });
+ model.setThreadMessages(QStringLiteral("t3"),
{ 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 second = model.index(2, 0, QModelIndex());
+ QCOMPARE(model.threadFor(second).threadId, QStringLiteral("t3"));
const QModelIndex reply = model.index(0, 0, second);
QVERIFY(reply.isValid());
@@ -919,11 +932,16 @@ void TestThreadListModel::threadForResolvesAReplyThroughItsParent()
"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"));
+ QCOMPARE(model.threadFor(reply).threadId, QStringLiteral("t3"));
// 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.
+ // threadAt() reads a top-level row number and this index carries a child's.
QCOMPARE(model.threadAt(reply.row()).threadId, QStringLiteral("t1"));
+ QVERIFY2(model.threadAt(reply.row()).threadId
+ != model.threadFor(reply).threadId,
+ "threadAt() and threadFor() agree for this index, so the test no "
+ "longer demonstrates why the row-taking overload is unsafe");
// An invalid index gives an empty summary, which every caller treats as
// "nothing to do" rather than acting on row 0.
@@ -946,7 +964,7 @@ void TestThreadListModel::applyMessageTagChangeRepaintsThatReplyAlone()
model.setThreadMessages(QStringLiteral("t1"), { root, reply });
const QModelIndex threadIndex = model.index(0, 0, QModelIndex());
- const QModelIndex replyIndex = model.index(0, 0, threadIndex);
+ const QModelIndex replyIndex = model.index(1, 0, threadIndex);
QVERIFY(model.isMessageRow(replyIndex));
const QStringList threadTagsBefore =
@@ -971,7 +989,7 @@ void TestThreadListModel::applyMessageTagChangeRepaintsThatReplyAlone()
// 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,
+ QCOMPARE(model.messageAt(model.index(1, 0, threadIndex)).messageId,
QStringLiteral("m1@example.org"));
// An unknown message is a no-op rather than a wrong row repainted.
@@ -995,7 +1013,7 @@ void TestThreadListModel::aDeletedReplyIsPaintedAsDoomed()
model.setThreadMessages(QStringLiteral("t1"), { root, reply });
const QModelIndex threadIndex = model.index(0, 0, QModelIndex());
- const QModelIndex replyIndex = model.index(0, 0, threadIndex);
+ const QModelIndex replyIndex = model.index(1, 0, threadIndex);
const QVariant plainBackground =
model.data(replyIndex, Qt::BackgroundRole);
@@ -1033,7 +1051,7 @@ void TestThreadListModel::aDeletedReplyIsStruckThrough()
makeNode(QStringLiteral("m1@example.org"), 1) });
const QModelIndex replyIndex =
- model.index(0, 0, model.index(0, 0, QModelIndex()));
+ model.index(1, 0, model.index(0, 0, QModelIndex()));
QVERIFY(!model.data(replyIndex, Qt::FontRole).value<QFont>().strikeOut());
@@ -1069,7 +1087,7 @@ void TestThreadListModel::markingAReplyReadChangesItsForeground()
model.setThreadMessages(QStringLiteral("t1"), { root, reply });
const QModelIndex replyIndex =
- model.index(0, 0, model.index(0, 0, QModelIndex()));
+ model.index(1, 0, model.index(0, 0, QModelIndex()));
const QVariant unreadForeground =
model.data(replyIndex, Qt::ForegroundRole);
@@ -1109,7 +1127,7 @@ void TestThreadListModel::anUnreadReplyIsBoldAndStillSmallerThanItsThread()
model.setThreadMessages(QStringLiteral("t1"), { root, reply });
const QModelIndex threadIndex = model.index(0, 0, QModelIndex());
- const QModelIndex replyIndex = model.index(0, 0, threadIndex);
+ const QModelIndex replyIndex = model.index(1, 0, threadIndex);
const QFont unreadFont =
model.data(replyIndex, Qt::FontRole).value<QFont>();
@@ -1152,7 +1170,7 @@ void TestThreadListModel::aThreadTagChangeReachesItsLoadedReplies()
model.setThreadMessages(QStringLiteral("t1"), { root, reply });
const QModelIndex threadIndex = model.index(0, 0, QModelIndex());
- const QModelIndex replyIndex = model.index(0, 0, threadIndex);
+ const QModelIndex replyIndex = model.index(1, 0, threadIndex);
QVERIFY(model.messageAt(replyIndex).isUnread());
QSignalSpy spy(&model, &QAbstractItemModel::dataChanged);
@@ -1491,17 +1509,23 @@ void TestThreadListModel::aFlatThreadStillListsItsReplies()
const QModelIndex root = model.index(0, 0);
- // Two children, not zero: the FIRST message is the root card itself, and
- // the rest are its replies however flat the thread is.
- QCOMPARE(model.rowCount(root), 2);
+ // Three children, not zero: a flat thread's messages all arrive at depth 0,
+ // and selecting on depth would list none of them. Position is the rule, and
+ // since item 177 it starts at the first message rather than after it.
+ QCOMPARE(model.rowCount(root), 3);
QCOMPARE(model.index(0, 0, root).data(ThreadListModel::MessageIdRole)
.toString(),
+ QStringLiteral("m0@example.org"));
+ QCOMPARE(model.index(1, 0, root).data(ThreadListModel::MessageIdRole)
+ .toString(),
QStringLiteral("m1@example.org"));
- // And the count the card advertises must agree with the rows beneath it,
- // or the expander opens onto nothing.
+ // The pill counts REPLIES while the rows are MESSAGES, so since item 177
+ // the rows are one more than the pill: the conversation lists its first
+ // message too. They must still move together, or the expander opens onto a
+ // number the card never promised.
QCOMPARE(root.data(ThreadListModel::ReplyCountRole).toInt(),
- model.rowCount(root));
+ model.rowCount(root) - 1);
}
void TestThreadListModel::theRootCardKnowsItsOwnMessage()
@@ -1529,10 +1553,15 @@ void TestThreadListModel::theRootCardKnowsItsOwnMessage()
QCOMPARE(root.data(ThreadListModel::MessageIdRole).toString(),
QStringLiteral("m0@example.org"));
- // And it is the FIRST message, not just any of them: the reply must still
- // report its own.
+ // The card still knows its own message, which the dashboard's heading and
+ // the thread-scoped actions read. Child 0 is now that same first message
+ // rather than the first reply: since item 177 the conversation lists every
+ // message, so the row under the card is m0 and m1 follows it.
QCOMPARE(model.index(0, 0, root).data(ThreadListModel::MessageIdRole)
.toString(),
+ QStringLiteral("m0@example.org"));
+ QCOMPARE(model.index(1, 0, root).data(ThreadListModel::MessageIdRole)
+ .toString(),
QStringLiteral("m1@example.org"));
}
@@ -1565,7 +1594,7 @@ void TestThreadListModel::replyShowsOnlyItsOwnTags()
const QModelIndex threadIndex = model.index(0, 0);
QVERIFY(model.hasChildren(threadIndex));
- const QModelIndex replyIndex = model.index(0, 0, threadIndex);
+ const QModelIndex replyIndex = model.index(1, 0, threadIndex);
QVERIFY(replyIndex.isValid());
const QStringList own =
@@ -1601,7 +1630,7 @@ void TestThreadListModel::replySharingEveryThreadTagShowsNone()
root.depth = 0;
model.setThreadMessages(QStringLiteral("T1"), { root, reply });
- const QModelIndex replyIndex = model.index(0, 0, model.index(0, 0));
+ const QModelIndex replyIndex = model.index(1, 0, model.index(0, 0));
QVERIFY(replyIndex.isValid());
QVERIFY(replyIndex.data(ThreadListModel::MessageOwnTagsRole)
.toStringList()
@@ -1677,7 +1706,7 @@ void TestThreadListModel::reconcileKeepsSurvivingRowsAndTheirExpansion()
MessageNode root = makeNode(QStringLiteral("m1"), 0);
MessageNode reply = makeNode(QStringLiteral("m2"), 1);
model.setThreadMessages(QStringLiteral("t1"), { root, reply });
- QCOMPARE(model.rowCount(model.index(0, 0)), 1);
+ QCOMPARE(model.rowCount(model.index(0, 0)), 2);
const QPersistentModelIndex survivor(model.index(0, 0));
QVERIFY(survivor.isValid());
@@ -1693,8 +1722,10 @@ void TestThreadListModel::reconcileKeepsSurvivingRowsAndTheirExpansion()
"the open thread would be lost exactly as a reset loses them");
QCOMPARE(model.threadAt(survivor.row()).threadId, QStringLiteral("t1"));
- // Its loaded replies survive too, or the thread collapses under the reader.
- QCOMPARE(model.rowCount(model.index(survivor.row(), 0)), 1);
+ // Its loaded messages survive too, or the thread collapses under the
+ // reader. Two of them since item 177: a conversation lists its first
+ // message as well as its replies.
+ QCOMPARE(model.rowCount(model.index(survivor.row(), 0)), 2);
}
void TestThreadListModel::reconcileUpdatesTagsOnASurvivingThread()
@@ -2099,6 +2130,81 @@ void TestThreadListModel::aLoadedThreadTrustsItsChildrenOverItsCount()
"conversation after loading no replies at all");
}
+void TestThreadListModel::aConversationListsItsFirstMessageAsAChild()
+{
+ // The user's report: "I can't reach the first message in a thread. All
+ // messages were supposed to sit below the new thread row. They start
+ // from #2."
+ //
+ // setThreadMessages() dropped nodes.first() until item 177, and that was
+ // correct while a thread row MEANT its first message: listing the message
+ // under itself would have shown it twice. Item 177 made the row stand for
+ // the conversation and render a dashboard instead of any one message, so
+ // the drop left the first message with no row anywhere in the interface.
+ // Unreachable, not merely unlabelled: expanding was the only route to it.
+ ThreadListModel model;
+ ThreadSummary conversation =
+ makeThread(QStringLiteral("t1"), QStringLiteral("A subject"));
+ conversation.totalCount = 3;
+ conversation.firstMessageId = QStringLiteral("m0@example.org");
+ model.appendBatch({ conversation });
+
+ model.setThreadMessages(
+ QStringLiteral("t1"),
+ { makeNode(QStringLiteral("m0@example.org"), 0),
+ makeNode(QStringLiteral("m1@example.org"), 1),
+ makeNode(QStringLiteral("m2@example.org"), 2) });
+
+ const QModelIndex root = model.index(0, 0, QModelIndex());
+ QVERIFY2(model.isConversationRow(root),
+ "the fixture is not a conversation, so this test says nothing "
+ "about the case the defect lives in");
+
+ // Three messages, three rows. Counting is not enough on its own: a row
+ // could be added anywhere, so the first child is named explicitly.
+ QCOMPARE(model.rowCount(root), 3);
+ QCOMPARE(model.data(model.index(0, 0, root),
+ ThreadListModel::MessageIdRole).toString(),
+ QStringLiteral("m0@example.org"));
+ QCOMPARE(model.data(model.index(2, 0, root),
+ ThreadListModel::MessageIdRole).toString(),
+ QStringLiteral("m2@example.org"));
+
+ // And it is reachable as a MESSAGE row, which is what makes it selectable
+ // and actionable rather than merely drawn.
+ const QModelIndex first = model.index(0, 0, root);
+ QVERIFY(model.isMessageRow(first));
+ QCOMPARE(model.messageAt(first).messageId, QStringLiteral("m0@example.org"));
+}
+
+void TestThreadListModel::aMessageRowDoesNotListItselfBeneathItself()
+{
+ // The other half, and the reason the drop existed. A thread of one is
+ // still a MESSAGE row under item 177: its card renders that message, so a
+ // child carrying the same message would show it twice, once as the card
+ // and once beneath it.
+ //
+ // Decided on the messages that ARRIVED rather than on summary.totalCount,
+ // which counts duplicates: a "thread of 2" that loads one message must
+ // take this branch, or it gains a child and then claims through
+ // isConversationRow() to be a conversation it cannot open.
+ ThreadListModel model;
+ ThreadSummary lone =
+ makeThread(QStringLiteral("t1"), QStringLiteral("Alone"));
+ lone.totalCount = 2; // notmuch counted a duplicate; one message arrives.
+ lone.firstMessageId = QStringLiteral("m0@example.org");
+ model.appendBatch({ lone });
+
+ model.setThreadMessages(QStringLiteral("t1"),
+ { makeNode(QStringLiteral("m0@example.org"), 0) });
+
+ const QModelIndex root = model.index(0, 0, QModelIndex());
+ QCOMPARE(model.rowCount(root), 0);
+ QVERIFY2(!model.isConversationRow(root),
+ "a thread that loaded one message claims to be a conversation, so "
+ "it offers an expander onto nothing");
+}
+
void TestThreadListModel::aMessageRowIsNeverAConversationRow()
{
ThreadListModel model;
@@ -2118,7 +2224,7 @@ void TestThreadListModel::aMessageRowIsNeverAConversationRow()
model.setThreadMessages(QStringLiteral("t1"), { root, reply });
const QModelIndex thread = model.index(0, 0, QModelIndex());
- const QModelIndex replyRow = model.index(0, 0, thread);
+ const QModelIndex replyRow = model.index(1, 0, thread);
QVERIFY(model.isMessageRow(replyRow));
QVERIFY2(!model.isConversationRow(replyRow),
"a reply row answered yes, so an action on it would scope to the "
@@ -2220,8 +2326,10 @@ void TestThreadListModel::aReplyRowResolvesToItsMessage()
model.setThreadMessages(QStringLiteral("t2"), { root, reply });
const QModelIndex thread = model.index(1, 0, QModelIndex());
+ // Child 1, not child 0: since item 177 a conversation lists its first
+ // message too, so child 0 is m1 and the REPLY this test is about is m2.
const ActionScope scope =
- model.scopeForSelection({ model.index(0, 0, thread) });
+ model.scopeForSelection({ model.index(1, 0, thread) });
QCOMPARE(scope.messageIds, QStringList{ QStringLiteral("m2") });
QVERIFY(scope.threadIds.isEmpty());