From 1c034f6358f17c5c1d0eeaa04c42c33fac125d93 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Wed, 26 Aug 2026 19:19:55 +0200 Subject: feat: resolve pending-change ids to subjects Item 119, second half of the data: the step that turns the snapshot's ids into something worth showing. resolvePendingSubjects() takes the rows' ids in order, each flagged as a thread id or a message id, and answers positionally: one subject per input, plus the thread's message total for a thread id and -1 for a message id. Positional rather than set-based, and that is load-bearing. The caller has already decided what its rows are and in what order, and one id can legitimately appear on several rows: a message with two outstanding actions is two rows carrying one id. A combined query returns a set, which loses both the order and the duplicate, so the walk is one lookup per row instead. The cost is bounded by what the user did by hand since the last sync, which is not a query-sized number. A missing id answers with an EMPTY subject rather than being dropped. The dialog still shows that row, because the count the user clicked has to equal the list they are shown, and dropping a row breaks that agreement in exactly the case where the user is most likely to notice. An index that cannot be opened answers the same way, one empty subject per row, so the list still shows the changes with only the subjects missing. The thread count is taken at snapshot time and says so: a held thread edit applies when the sync ends, and a reply arriving in between makes the real number larger. The row describes what the user is looking at, not what the write will touch. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01P88Q3MCSCSQxKDy7pmXh9F --- src/notmuchworker.cpp | 82 ++++++++++++++++++++++++++++++++++++++++++++ src/notmuchworker.h | 22 ++++++++++++ tests/test_notmuchworker.cpp | 67 ++++++++++++++++++++++++++++++++++++ 3 files changed, 171 insertions(+) diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp index e78d8c8..1f28973 100644 --- a/src/notmuchworker.cpp +++ b/src/notmuchworker.cpp @@ -1232,6 +1232,88 @@ void NotmuchWorker::resolveThreadMessages(const QStringList &threadIds, resolveQuery(terms.join(QStringLiteral(" or ")), requestTag); } +void NotmuchWorker::resolvePendingSubjects(const QStringList &ids, + const QList &areThreads) +{ + if (ids.isEmpty() || ids.size() != areThreads.size()) + return; + + QStringList subjects; + QList counts; + subjects.reserve(ids.size()); + counts.reserve(ids.size()); + + if (!openReadOnly()) { + // Answer anyway, one empty subject per row. The dialog must be able to + // show the user their pending changes even when the index cannot be + // opened: the ids and the actions are known without it, and only the + // subjects are missing. + for (int i = 0; i < ids.size(); ++i) { + subjects.append(QString()); + counts.append(-1); + } + emit pendingSubjectsResolved(subjects, counts); + return; + } + + // One lookup per id rather than one combined query, deliberately. The + // answer is POSITIONAL, and a combined query returns a set: it would lose + // both the order and the duplicates, and a message with two outstanding + // actions is exactly two rows carrying one id. + // + // The cost is bounded by what the user can have pending, which is what + // they did by hand since the last sync. This is not a query-sized walk. + for (int i = 0; i < ids.size(); ++i) { + QString subject; + int count = -1; + + if (areThreads.at(i)) { + NmQuery query(notmuch_query_create( + m_db, + QStringLiteral("thread:%1").arg(ids.at(i)).toUtf8().constData())); + notmuch_threads_t *raw = nullptr; + if (query + && notmuch_query_search_threads(query.get(), &raw) + == NOTMUCH_STATUS_SUCCESS) { + NmThreads threads(raw); + if (notmuch_threads_valid(threads.get())) { + NmThread thread(notmuch_threads_get(threads.get())); + if (thread) { + subject = QString::fromUtf8( + notmuch_thread_get_subject(thread.get())); + // At snapshot time, which is what the row reports. A + // held thread edit applies when the sync ends, and a + // reply landing in between makes the real number + // larger; the number describes what the user is + // looking at, not what the write will touch. + count = notmuch_thread_get_total_messages(thread.get()); + } + } + } + } else { + notmuch_message_t *raw = nullptr; + // find_message reports SUCCESS with a null message for an id that + // is not there, so both have to be checked. A missing id is not an + // error here: it is the stale row the dialog exists to show. + if (notmuch_database_find_message( + m_db, ids.at(i).toUtf8().constData(), &raw) + == NOTMUCH_STATUS_SUCCESS + && raw) { + NmMessage message(raw); + const char *header = + notmuch_message_get_header(message.get(), "Subject"); + if (header) + subject = QString::fromUtf8(header); + } + } + + subjects.append(subject); + counts.append(count); + } + + emit pendingSubjectsResolved(subjects, counts); +} + void NotmuchWorker::resolveQuery(const QString &query, const QString &requestTag) { diff --git a/src/notmuchworker.h b/src/notmuchworker.h index 77b14ec..8171f3c 100644 --- a/src/notmuchworker.h +++ b/src/notmuchworker.h @@ -213,6 +213,22 @@ public slots: /// whatever the current view happens to be showing. void resolveQueryMessages(const QString &query, const QString &requestTag); + /// Subjects for the list behind the unsynced-changes count (item 119). + /// + /// Takes the snapshot's ids in order, each flagged as a thread id or a + /// message id, and answers POSITIONALLY: one subject per input, plus a + /// message count for a thread id and -1 for a message id. Positional + /// because the caller has already decided what its rows are and in what + /// order; a set-based answer would make it match them back up by id, and + /// one id can legitimately appear on several rows. + /// + /// An id the index no longer holds yields an EMPTY subject rather than + /// being dropped. The dialog still shows that row: the count the user + /// clicked has to equal the list they are shown, and silently dropping a + /// row would break that for the one case where it matters most. + void resolvePendingSubjects(const QStringList &ids, + const QList &areThreads); + private: /// The shared walk behind resolveMessages() and resolveThreadMessages(): /// runs `query` and emits threadMessagesResolved() with each match's id, @@ -340,6 +356,12 @@ signals: const QStringList &paths, const QStringList &tags, const QString &requestTag); + /// One subject per requested id, in the SAME ORDER, and one count beside + /// it: the thread's message total, or -1 for a message id. An empty + /// subject means the index no longer holds that id. + void pendingSubjectsResolved(const QStringList &subjects, + const QList &messageCounts); + void allTagsReady(const QStringList &tags, quint64 generation); /// One entry per requested query, in the order they were asked for. A query diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp index 9a0896d..bcde45c 100644 --- a/tests/test_notmuchworker.cpp +++ b/tests/test_notmuchworker.cpp @@ -58,6 +58,8 @@ private slots: void applyTagsToThreadsSpansMultipleThreads(); void applyTagsToThreadsWithNoThreadsDoesNothing(); + void pendingSubjectsAnswerPositionally(); + void aMissingPendingIdYieldsAnEmptySubject(); void requestAllTagsReturnsSortedTags(); void requestAllTagsOnUnreadableConfigEmitsError(); @@ -962,6 +964,71 @@ void TestNotmuchWorker::applyTagsToThreadsWithNoThreadsDoesNothing() QVERIFY(errors.isEmpty()); } +void TestNotmuchWorker::pendingSubjectsAnswerPositionally() +{ + // Item 119. The dialog has already decided what its rows are and in what + // order, so the answer is positional: one subject per input id, in the + // same order, whatever those ids are. + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy spy(&worker, &NotmuchWorker::pendingSubjectsResolved); + + // The SAME message id twice, which is what a message with two outstanding + // actions produces. A combined query would return it once and the rows + // would no longer line up. + const QStringList ids { QStringLiteral("b1@example.org"), + QStringLiteral("b1@example.org") }; + worker.resolvePendingSubjects(ids, { false, false }); + + QCOMPARE(spy.size(), 1); + const QStringList subjects = spy.first().at(0).toStringList(); + const QList counts = spy.first().at(1).value>(); + QCOMPARE(subjects.size(), 2); + QCOMPARE(counts.size(), 2); + + // Both rows carry the subject, and neither claims a message count: a + // message id is not a thread. + QVERIFY2(!subjects.at(0).isEmpty(), "the subject did not resolve"); + QCOMPARE(subjects.at(0), subjects.at(1)); + QCOMPARE(counts.at(0), -1); + QCOMPARE(counts.at(1), -1); +} + +void TestNotmuchWorker::aMissingPendingIdYieldsAnEmptySubject() +{ + // A stale row: the id is no longer in the index. It must come back EMPTY + // rather than being dropped, because the dialog still has to show it. The + // count the user clicked has to equal the list they are shown, and a + // dropped row breaks that in the one case where it matters most. + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy spy(&worker, &NotmuchWorker::pendingSubjectsResolved); + + worker.resolvePendingSubjects( + { QStringLiteral("b1@example.org"), + QStringLiteral("gone@example.org") }, + { false, false }); + + QCOMPARE(spy.size(), 1); + const QStringList subjects = spy.first().at(0).toStringList(); + QCOMPARE(subjects.size(), 2); + QVERIFY(!subjects.at(0).isEmpty()); + QVERIFY2(subjects.at(1).isEmpty(), + "a missing id must answer empty, not drop its row"); + + // A thread id resolves to its subject AND its message count, which is what + // a thread-scoped row reports. + QSignalSpy threadSpy(&worker, &NotmuchWorker::pendingSubjectsResolved); + const QVector threads = + runQuery(QStringLiteral("subject:Preventivo")); + QCOMPARE(threads.size(), 1); + worker.resolvePendingSubjects({ threads.first().threadId }, { true }); + + QCOMPARE(threadSpy.size(), 1); + QCOMPARE(threadSpy.first().at(0).toStringList().size(), 1); + QVERIFY(!threadSpy.first().at(0).toStringList().at(0).isEmpty()); + QCOMPARE(threadSpy.first().at(1).value>().at(0), + threads.first().totalCount); +} + void TestNotmuchWorker::requestAllTagsReturnsSortedTags() { NotmuchWorker worker(m_fixture.configPath()); -- cgit v1.2.3