diff options
| -rw-r--r-- | src/mainwindow.cpp | 15 | ||||
| -rw-r--r-- | src/threadlistmodel.cpp | 81 | ||||
| -rw-r--r-- | src/threadlistmodel.h | 17 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 58 |
4 files changed, 167 insertions, 4 deletions
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 17a5a65..ae5e8fc 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -3482,10 +3482,17 @@ void MainWindow::sendMessageTagChange(const QStringList &messageIds, if (messageIds.isEmpty()) return; - // No optimistic model update. applyTagChange is keyed by THREAD and would - // repaint the whole row as though every message in it had changed, which - // for a one-message edit is a lie the user would see and then watch - // silently correct itself on the next query. + // Optimistic, but scoped to the message. applyTagChange() is keyed by + // THREAD and would repaint the whole row as though every message in it had + // changed, which for a one-message edit is a lie; applyMessageTagChange() + // updates that message and lets the thread's own tags follow only when the + // answer is unambiguous. + // + // Not optional for auto mark-read: without it the write goes out, the + // status bar counts an unsynced edit, and the card stays bold with + // `unread` on it until the next query. The user reported exactly that. + for (const QString &messageId : messageIds) + m_model->applyMessageTagChange(messageId, add, remove); // The accounts this touches, resolved through the containing threads: the // account is a property of the thread, and the sync needs the channel diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp index 3e079ed..04a5493 100644 --- a/src/threadlistmodel.cpp +++ b/src/threadlistmodel.cpp @@ -888,3 +888,84 @@ void ThreadListModel::applyTagChange(const QString &threadId, return; } } + +void ThreadListModel::applyMessageTagChange(const QString &messageId, + const QStringList &added, + const QStringList &removed) +{ + const auto retag = [&added, &removed](QStringList *tags) { + for (const QString &tag : removed) + tags->removeAll(tag); + for (const QString &tag : added) { + if (!tags->contains(tag)) + tags->append(tag); + } + }; + + for (int row = 0; row < m_threads.size(); ++row) { + ThreadNode &node = m_threads[row]; + bool touched = false; + + // `first` is a copy of the opening message rather than an alias into + // children, so both have to be updated when they name the same one. + if (node.first.messageId == messageId) { + retag(&node.first.tags); + touched = true; + } + + for (int child = 0; child < node.children.size(); ++child) { + if (node.children.at(child).messageId != messageId) + continue; + retag(&node.children[child].tags); + touched = true; + const QModelIndex childIndex = index(child, 0, index(row, 0)); + emit dataChanged(childIndex, childIndex); + } + + // The thread has not been expanded and does not open with this + // message, so nothing here holds it. The summary may still need to + // follow, which the totalCount check below decides. + if (!touched && node.summary.firstMessageId != messageId + && node.summary.totalCount > 1) { + continue; + } + + // The thread's own tags follow only when the answer is unambiguous. + // + // A thread carries `unread` while ANY of its messages does, so a + // one-message change can only clear it from the thread when there is + // nothing else left to carry it. With one message in the thread that + // is certain. With more, the honest answer needs every message's tags, + // which are only loaded once the thread has been expanded; until then + // the summary is left alone rather than guessed at, and the next query + // corrects it. + const bool wholeThread = + node.summary.totalCount <= 1 + || (!node.children.isEmpty() + && node.children.size() >= node.summary.totalCount); + if (!wholeThread) { + if (touched) + emit dataChanged(index(row, 0), index(row, 0)); + continue; + } + + for (const QString &tag : removed) { + bool stillHeld = false; + for (const MessageNode &child : node.children) { + if (child.messageId != messageId && child.tags.contains(tag)) { + stillHeld = true; + break; + } + } + if (!stillHeld) + node.summary.tags.removeAll(tag); + } + for (const QString &tag : added) { + if (!node.summary.tags.contains(tag)) + node.summary.tags.append(tag); + } + + emit dataChanged(index(row, 0), index(row, 0)); + return; + } +} diff --git a/src/threadlistmodel.h b/src/threadlistmodel.h index 2b8d2b0..f7abaf8 100644 --- a/src/threadlistmodel.h +++ b/src/threadlistmodel.h @@ -273,6 +273,23 @@ public: void applyTagChange(const QString &threadId, const QStringList &added, const QStringList &removed); + /// The same, scoped to ONE message. + /// + /// Updates that message's own tags wherever it is held: as a child row if + /// the thread is expanded, and as `first` when it is the thread's opening + /// message. The thread's summary tags follow only when the change is + /// unambiguous for the whole thread, which for `unread` means no other + /// message still carries it, since a thread reads as unread while any of + /// its messages does. + /// + /// Exists because auto mark-read touches one message and the card still + /// has to stop looking unread. applyTagChange() above cannot serve that: + /// it rewrites the thread's tags directly, which for a multi-message + /// thread would claim every reply had been read. + void applyMessageTagChange(const QString &messageId, + const QStringList &added, + const QStringList &removed); + private: /// One thread root and the message rows expanded under it. /// diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index ea42223..5347493 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -186,6 +186,7 @@ private slots: void anUnexpandedRootRendersOneMessageNotTheConversation(); void aFirstClickIntoAnUnfocusedListStillRenders(); void autoMarkReadTouchesOnlyTheMessageOnScreen(); + void autoMarkReadClearsUnreadOnTheCardImmediately(); void autoSyncIsNotArmedWhenDisabledOrWithNothingPending(); void autoSyncSkipsWhileABackgroundSyncIsRunning(); void aSuccessfulSyncRefreshesRatherThanRerunningTheQuery(); @@ -6555,4 +6556,61 @@ void TestMainWindow::autoMarkReadTouchesOnlyTheMessageOnScreen() QStringList{ QStringLiteral("root@example.org") }); } +void TestMainWindow::autoMarkReadClearsUnreadOnTheCardImmediately() +{ + // Reported by the user against the message-scoped mark-read: the write + // went out, the status bar counted an unsynced edit, and the card stayed + // bold with `unread` still on it. sendMessageTagChange deliberately makes + // no optimistic model update, because applyTagChange is keyed by THREAD + // and repainting a whole row for a one-message edit would be a lie. + // + // For an explicit tag edit that trade is fine. For auto mark-read it is + // not: the visible change IS the feature, and the 2s delay exists to give + // the user that feedback. + // + // One message in the thread, so the thread's own unread state and the + // message's are the same fact and the card must stop reading as unread. + WorkerBackedWindow backed; + backed.setGeneralKey(QStringLiteral("mark_read_delay_ms"), + QStringLiteral("0")); + QVERIFY(backed.fixture().addMessage( + QStringLiteral("inbox"), QStringLiteral("only@example.org"), + QStringLiteral("A single message"), + QStringLiteral("sender@example.org"), + QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"), + QStringLiteral("The only message."), /*unread=*/true)); + QVERIFY2(backed.build(), qPrintable(backed.error())); + + MainWindow window(backed.config()); + + QLineEdit *queryEdit = + window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY2(queryEdit, "no query bar"); + auto *view = window.findChild<ThreadListView *>(); + QVERIFY2(view, "no thread list view"); + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY2(model, "no thread list model"); + + queryEdit->setText(QStringLiteral("tag:inbox")); + queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); + + const QModelIndex root = model->index(0, 0, QModelIndex()); + QVERIFY(root.isValid()); + + // Unread to begin with, or the assertion below proves nothing. + QVERIFY2(model->data(root, ThreadListModel::TagsRole) + .toStringList() + .contains(QStringLiteral("unread")), + "the thread was not unread to begin with"); + + view->setCurrentIndex(root); + + // The card must stop reading as unread without waiting for a new query. + QTRY_VERIFY_WITH_TIMEOUT(!model->data(root, ThreadListModel::TagsRole) + .toStringList() + .contains(QStringLiteral("unread")), + 15000); +} + #include "test_mainwindow.moc" |