aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/mainwindow.cpp58
-rw-r--r--src/mainwindow.h42
-rw-r--r--src/types.h39
-rw-r--r--tests/test_mainwindow.cpp85
4 files changed, 215 insertions, 9 deletions
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 59edf53..0c6092e 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -18,6 +18,8 @@
#include "mainwindow.h"
+#include <algorithm>
+
#include "maildirname.h"
#include <QAction>
@@ -4440,9 +4442,9 @@ void MainWindow::onTagsApplied(const TagChange &change)
// message are two independent changes and must not cancel each other.
for (const QString &messageId : change.messageIds) {
for (const QString &tag : change.added)
- recordPendingEdit(messageId, tag, true);
+ recordPendingEdit(messageId, tag, true, change.description);
for (const QString &tag : change.removed)
- recordPendingEdit(messageId, tag, false);
+ recordPendingEdit(messageId, tag, false, change.description);
}
updatePendingIndicator();
@@ -4989,7 +4991,7 @@ void MainWindow::runAutoSync()
}
void MainWindow::recordPendingEdit(const QString &messageId, const QString &tag,
- bool added)
+ bool added, const QString &action)
{
const QString key = messageId + QLatin1Char('\n') + tag;
@@ -4998,12 +5000,12 @@ void MainWindow::recordPendingEdit(const QString &messageId, const QString &tag,
// long session of tagging and untagging.
const auto existing = m_pendingTagEdits.constFind(key);
if (existing != m_pendingTagEdits.constEnd()) {
- if (*existing != added)
+ if (existing->added != added)
m_pendingTagEdits.erase(m_pendingTagEdits.find(key));
return;
}
- m_pendingTagEdits.insert(key, added);
+ m_pendingTagEdits.insert(key, PendingEdit{ added, action });
}
QStringList MainWindow::pendingSyncChannels() const
@@ -5060,6 +5062,52 @@ int MainWindow::pendingEditCount() const
return m_pendingTagEdits.size() + held + heldMoves;
}
+QVector<PendingChange> MainWindow::pendingChangeSnapshot() const
+{
+ QVector<PendingChange> rows;
+
+ // The netted per-(message, tag) edits. The key is `messageId\ntag`, built
+ // by recordPendingEdit(), so the id is everything before the first
+ // newline: a TAG may contain almost anything, but a message id cannot
+ // contain a newline and neither separator can be confused for the other.
+ for (auto it = m_pendingTagEdits.cbegin(); it != m_pendingTagEdits.cend();
+ ++it) {
+ const QString id = it.key().section(QLatin1Char('\n'), 0, 0);
+ rows.append(PendingChange{ id, false, it->action, QString(), -1 });
+ }
+
+ // Held THREAD edits, which stay thread-scoped: a `*_thread` action is what
+ // made them, and reporting the messages instead would claim the user acted
+ // on each one. One row per thread the edit named, since a single edit can
+ // cover a multi-row selection.
+ for (const HeldEdit &edit : m_heldEdits) {
+ for (const QString &threadId : edit.threadIds) {
+ rows.append(PendingChange{ threadId, true, edit.change.description,
+ QString(), -1 });
+ }
+ }
+
+ // Held MOVES, which are message-scoped. A move is not a tag change and is
+ // queued separately for that reason, but it is the same kind of row here:
+ // one message, one action the user took.
+ for (const HeldMove &move : m_heldMoves) {
+ for (const QString &messageId : move.messageIds)
+ rows.append(PendingChange{ messageId, false, move.description,
+ QString(), -1 });
+ }
+
+ // Grouped by id so a message with several outstanding actions appears
+ // ONCE with its actions beneath it, which is the layout the user asked
+ // for. A stable sort, so the actions under one message keep the order
+ // they were made in rather than an arbitrary one; QHash has no order of
+ // its own, so without this the list reshuffles between openings.
+ std::stable_sort(rows.begin(), rows.end(),
+ [](const PendingChange &a, const PendingChange &b) {
+ return a.id < b.id;
+ });
+ return rows;
+}
+
void MainWindow::updatePendingIndicator()
{
const int pending = pendingEditCount();
diff --git a/src/mainwindow.h b/src/mainwindow.h
index 25a9a7f..532a5ea 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -105,6 +105,24 @@ public:
/// the worker, which test_mainwindow has no database to drive.
bool hasEditAwaitingSend() const { return !m_heldEdits.isEmpty(); }
+ /// Every outstanding change, as rows, for the list behind the count.
+ ///
+ /// A SNAPSHOT: taken once when the user opens the list and never refreshed
+ /// under them. Subjects are empty here, filled by the resolve step, so
+ /// this is testable with no worker and no database.
+ ///
+ /// Scope follows the ACTION. The three queues already encode it: a held
+ /// thread edit carries thread ids because a `*_thread` action made it,
+ /// while a netted tag edit and a held move both carry message ids. Nothing
+ /// is expanded, and nothing is escalated.
+ /// Net changes the index holds that a sync has not carried over.
+ ///
+ /// Public beside pendingChangeSnapshot(), which must agree with it: the
+ /// count the user clicks is the count the list has to account for.
+ int pendingEditCount() const;
+
+ QVector<PendingChange> pendingChangeSnapshot() const;
+
/// Whether the undo stack still holds anything. Exposed so a test can show
/// that a rejected write did not take unrelated history down with it.
bool canUndo() const { return m_undoStack.canUndo(); }
@@ -853,11 +871,14 @@ private:
/// Records one confirmed (message, tag) change, cancelling it against an
/// opposite change already outstanding for the same pair.
+ ///
+ /// `action` is the name the user would recognise, carried through so the
+ /// list behind the count can say what each change was. It is the
+ /// TagChange's own description rather than anything derived from the tag.
void recordPendingEdit(const QString &messageId, const QString &tag,
- bool added);
+ bool added, const QString &action);
+
- /// Net changes the index holds that a sync has not carried over.
- int pendingEditCount() const;
/// Shows or hides the "syncing" state: the progress bar and a disabled
/// Sync button.
@@ -1539,7 +1560,20 @@ private:
/// value true for added and false for removed; a pair that reverts is
/// erased rather than stored, so an edit and its inverse leave nothing
/// behind and the map cannot grow without bound.
- QHash<QString, bool> m_pendingTagEdits;
+ /// What one pending (message, tag) edit is: its direction, and the name of
+ /// the action that made it.
+ ///
+ /// The direction alone was enough while this only had to be counted. The
+ /// list behind the count (item 119) has to SAY what each change was, and
+ /// only the action that made it knows: `+deleted` is a Delete and
+ /// `-unread` is a Mark read, but deriving that here would be a second
+ /// table of tag names to labels, drifting from the one the actions already
+ /// pass as TagChange::description.
+ struct PendingEdit {
+ bool added = false;
+ QString action; ///< Translated, from TagChange::description.
+ };
+ QHash<QString, PendingEdit> m_pendingTagEdits;
/// Marks the open thread read once it has been on screen long enough.
///
diff --git a/src/types.h b/src/types.h
index a0f772b..c78ab76 100644
--- a/src/types.h
+++ b/src/types.h
@@ -264,6 +264,45 @@ struct TagChange
}
};
+/// One outstanding change, for the list behind the unsynced-changes count.
+///
+/// A SNAPSHOT taken when the user opens the list, then frozen: the count they
+/// clicked is the count the list accounts for, and a dialog left open for
+/// twenty minutes must not keep rewriting itself under them.
+///
+/// The scope follows the ACTION, never the storage. A thread action names its
+/// thread and reports how many messages it covered at snapshot time; a
+/// message action names its message. That distinction is already kept, since a
+/// held thread edit carries thread ids and everything else carries message
+/// ids, so nothing has to be expanded to reconstruct it.
+struct PendingChange
+{
+ /// The message or thread this change is about. Wire format, for resolving
+ /// a subject; never shown.
+ QString id;
+
+ /// True when `id` is a THREAD id and the change covers the conversation.
+ bool isThread = false;
+
+ /// What the user did, translated and ready to show ("Delete", "Mark
+ /// read"). Built where the change is recorded, since only there is the
+ /// direction of a tag write still known.
+ QString action;
+
+ /// The subject, filled by the resolve step. Empty until then, and left
+ /// empty for an id the index no longer holds: the row still appears, since
+ /// dropping it would make the list disagree with the count.
+ QString subject;
+
+ /// How many messages a thread change covered, at snapshot time. -1 for a
+ /// message change and for a thread whose resolve found nothing.
+ ///
+ /// At snapshot time and not at write time: 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.
+ int messageCount = -1;
+};
+
/// Database-level facts for the Maildir overview.
///
/// Every field is -1 until answered, so a dialog opened against a database that
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index a120ce6..be19652 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -416,6 +416,8 @@ private slots:
void anEditUndoneNettsBackToZero();
void aDifferentTagOnTheSameMessageStillCounts();
void everyPendingChangeCanNameItsMessages();
+ void theSnapshotGroupsActionsUnderTheirMessage();
+ void theSnapshotKeepsAThreadActionThreadScoped();
void anEditDuringABackgroundSyncIsNotSentYet();
void aHeldEditIsSentWhenTheBackgroundSyncEnds();
void aHeldEditCountsAsUnsynced();
@@ -6489,6 +6491,89 @@ void TestMainWindow::everyPendingChangeCanNameItsMessages()
"an edit and its inverse left the indicator claiming work");
}
+void TestMainWindow::theSnapshotGroupsActionsUnderTheirMessage()
+{
+ // The layout the user asked for: a message appears ONCE with its actions
+ // beneath it. That is a property of the row ORDER, so it is asserted on
+ // the snapshot rather than on a rendered dialog.
+ const Config config;
+ MainWindow window(config);
+
+ // Two actions on one message, one on another, interleaved so a snapshot
+ // that simply reported insertion order would fail.
+ const auto apply = [&window](const QString &id, const QString &tag,
+ const QString &description) {
+ TagChange change;
+ change.messageIds = { id };
+ change.added = { tag };
+ change.description = description;
+ QVERIFY(QMetaObject::invokeMethod(&window, "onTagsApplied",
+ Q_ARG(TagChange, change)));
+ };
+ apply(QStringLiteral("b@example.org"), QStringLiteral("flagged"),
+ QStringLiteral("Mark important"));
+ apply(QStringLiteral("a@example.org"), QStringLiteral("deleted"),
+ QStringLiteral("Delete"));
+ apply(QStringLiteral("b@example.org"), QStringLiteral("spam"),
+ QStringLiteral("Mark spam"));
+
+ const QVector<PendingChange> rows = window.pendingChangeSnapshot();
+ QCOMPARE(rows.size(), 3);
+
+ // One message per contiguous run: b's two actions are adjacent, so the
+ // dialog can draw the subject once and the actions under it.
+ QCOMPARE(rows.at(0).id, QStringLiteral("a@example.org"));
+ QCOMPARE(rows.at(1).id, QStringLiteral("b@example.org"));
+ QCOMPARE(rows.at(2).id, QStringLiteral("b@example.org"));
+
+ // Each row says what the user did, in the words the action itself used.
+ QCOMPARE(rows.at(0).action, QStringLiteral("Delete"));
+ QVERIFY(rows.at(1).action != rows.at(2).action);
+
+ // And every row here is message-scoped: none of these was a thread action.
+ for (const PendingChange &row : rows)
+ QVERIFY(!row.isThread);
+}
+
+void TestMainWindow::theSnapshotKeepsAThreadActionThreadScoped()
+{
+ // Scope follows the ACTION, not the storage. A held thread edit stays one
+ // thread row: reporting its messages instead would claim the user acted on
+ // each one, and the count they clicked would disagree with the list.
+ //
+ // Driven through the held queue because that is the only thing that
+ // carries thread ids; a confirmed edit is message-scoped by construction.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(model && view);
+ model->appendBatch({ makeThread(QStringLiteral("t1"), {}) });
+ selectThreadRow(view, 0);
+
+ // A cron sync takes the lock, which is what makes the edit HELD rather
+ // than sent, and a held edit is the only thing that carries thread ids.
+ QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
+ Q_ARG(SyncMonitor::State,
+ SyncMonitor::State::Running));
+
+ auto *action = window.findChild<QAction *>(QStringLiteral("flag_thread"));
+ QVERIFY(action);
+ action->trigger();
+ QVERIFY(window.hasEditAwaitingSend());
+
+ const QVector<PendingChange> rows = window.pendingChangeSnapshot();
+ QCOMPARE(rows.size(), 1);
+ QVERIFY2(rows.at(0).isThread,
+ "a thread action was reported as a message change");
+ QCOMPARE(rows.at(0).id, QStringLiteral("t1"));
+
+ // The count and the list agree, which is the property the whole dialog
+ // rests on.
+ QCOMPARE(rows.size(), window.pendingEditCount());
+}
+
// Item 37. A tag edit made while a background sync holds notmuch's write lock
// used to stall the worker: the read-write open BLOCKS until the lock frees
// (measured 9.158s against a 12s hold, returning NOTMUCH_STATUS_SUCCESS), so