From ae2ae2df75ed780a88423b77af3b31fbe2b26389 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 28 Aug 2026 20:15:15 +0200 Subject: 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. --- tests/test_mainwindow.cpp | 52 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 14 deletions(-) (limited to 'tests/test_mainwindow.cpp') 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")); -- cgit v1.2.3