From 0bbb2d2c8d75c2b4392b17214ec3ec3768305ba1 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Sat, 8 Aug 2026 11:13:10 +0200 Subject: feat(ui): render a single message when its row is selected loadMessage queries by id: and returns one MessageRef, always matched, since the user asked for that message by clicking its row and a stub would answer the wrong question. An unknown id emits an empty vector rather than an error: a stale row after a reindex is an ordinary race, not a failure worth the status bar. The signal fires even when empty so the UI handler runs instead of waiting for a reply that never comes. The branch in onThreadSelected is placed BEFORE threadAt(), which is the whole trap. threadAt takes a top-level row number and a child's row number indexes its siblings, so handing a message row's number to it loads whichever thread happens to sit at that position. Mutation-checked: with the branch disabled the test reports thread 't1' for a reply belonging to 't2', a wrong answer plausible enough to survive review. m_currentMessageId and m_currentThreadId are mutually exclusive and each clears the other, so a queued reply can tell which kind of selection it belongs to. onMessageLoaded carries a third guard onThreadLoaded does not need: a reply landing after the selection moved to a thread row would render one message where the conversation belongs. No mark-read timer for a message row in this pass. Marking one message of a thread read is a per-message tag write and the pending-edit map is keyed by thread; item 28 is the record of what happens when that count goes wrong. --- tests/test_mainwindow.cpp | 72 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) (limited to 'tests/test_mainwindow.cpp') diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 86cfb62..79a3a98 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -96,6 +96,7 @@ private slots: void noTagStripIsPaintedUnderAMessageRow(); void replyRowsKeepTheirTextUnderTheThreadLine(); void clickingTheExpanderTogglesTheThread(); + void selectingAMessageRowTargetsThatMessageNotItsThread(); void markAllReadIsDisabledUntilTheQueryFinishes(); void markAllReadActsOnEveryRowAndUndoesInOneStep(); void markAllReadDoesNothingWhenNothingIsUnread(); @@ -663,6 +664,77 @@ void TestMainWindow::aThreadWithRepliesDrawsAVisibleExpander() QCOMPARE(control, 0); } +void TestMainWindow::selectingAMessageRowTargetsThatMessageNotItsThread() +{ + // test_mainwindow has no worker (backlog item 36), so this cannot assert on + // what the pane renders. What it CAN assert is the decision the UI makes: + // a message row must stop tracking a current thread, or a reply arriving + // for either kind of selection cannot tell which one it belongs to. + // + // The trap this covers is specific. threadAt() takes a TOP-LEVEL row + // number, and a child's row number indexes its siblings, so handing a + // message row's number to it loads whichever thread happens to sit at that + // position in the list. Row 0 under a thread is a plausible-looking wrong + // answer, which is why the fixture puts the reply under the SECOND thread. + const Config config; + MainWindow window(config); + + auto *model = window.findChild(); + QVERIFY(model); + auto *view = window.findChild(); + QVERIFY(view); + + ThreadSummary first = makeThread(QStringLiteral("t1"), {}); + ThreadSummary second = makeThread(QStringLiteral("t2"), {}); + second.totalCount = 2; + model->appendBatch({ first, second }); + + MessageNode root; + root.messageId = QStringLiteral("m0@example.org"); + root.threadId = QStringLiteral("t2"); + root.depth = 0; + MessageNode reply; + reply.messageId = QStringLiteral("m1@example.org"); + reply.threadId = QStringLiteral("t2"); + reply.depth = 1; + model->setThreadMessages(QStringLiteral("t2"), { root, reply }); + + window.resize(1400, 300); + window.show(); + QVERIFY(QTest::qWaitForWindowExposed(&window)); + + // Start on a thread row, so the transition to a message row is what is + // being observed rather than the initial state. + const QModelIndex threadRow = model->index(1, 0, QModelIndex()); + selectThreadRow(view, 1); + QApplication::processEvents(); + QCOMPARE(window.currentThreadId(), QStringLiteral("t2")); + + view->expand(threadRow); + QApplication::processEvents(); + + const QModelIndex messageRow = model->index(0, 0, threadRow); + QVERIFY(messageRow.isValid()); + QVERIFY2(model->isMessageRow(messageRow), + "the fixture did not produce a message row, so this test would " + "assert nothing about one"); + + view->selectionModel()->select( + messageRow, + QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); + view->setCurrentIndex(messageRow); + QApplication::processEvents(); + + // The thread is no longer what the pane is about. Left set, a late + // loadThread reply would repaint the whole conversation over the single + // message the user asked for. + QVERIFY2(window.currentThreadId().isEmpty(), + qPrintable(QStringLiteral("selecting a reply left the current " + "thread set to '%1': the pane is still " + "tracking the conversation") + .arg(window.currentThreadId()))); +} + void TestMainWindow::clickingTheExpanderTogglesTheThread() { // The glyph being VISIBLE and the glyph being CLICKABLE are separate -- cgit v1.2.3