aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-08 10:22:33 +0200
committerDanilo M. <danix@danix.xyz>2026-08-08 10:22:33 +0200
commitf0381d6095b1fce9b0d223208cb92552208b9e4c (patch)
tree03557567c1536b91b850019913ccd5cd31c915e7
parent02d218358347e5d7a29164bc70a346c57fe098ab (diff)
downloadqtmaildir-f0381d6095b1fce9b0d223208cb92552208b9e4c.tar.gz
qtmaildir-f0381d6095b1fce9b0d223208cb92552208b9e4c.zip
feat(model): expose a thread's replies as child rows
setThreadMessages drops the depth-0 message: it is the thread's first message and the root row already stands for it. Keeping it would show a thread of seven as one root and seven children, contradicting the reply count the row advertises. Calling again replaces rather than appends, so a thread reloaded after a sync does not list its replies twice. A message row reports its own sender and subject, not the thread's. That is the mistake worth guarding: the thread's author summary usually contains the first sender too, so reading it renders something plausible for the root's own reply and wrong for every other one. Mutation-checked, and the wrong version returns 'Alice' where 'Bob' belongs. Child rows carry no tag pills. The strip is a row-wide band of the thread's tags; one under each reply would stripe the list and repeat identical tags down the expansion.
-rw-r--r--src/threadlistmodel.cpp129
-rw-r--r--src/threadlistmodel.h29
-rw-r--r--tests/test_threadlistmodel.cpp115
3 files changed, 268 insertions, 5 deletions
diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp
index dabc447..3bc02d6 100644
--- a/src/threadlistmodel.cpp
+++ b/src/threadlistmodel.cpp
@@ -169,12 +169,60 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const
return {};
}
- // Message rows are handled in Task 4; until then only thread rows exist and
- // a child index cannot be produced. The bound is checked against the thread
- // list only after establishing this IS a thread row, since a child row's
+ // A message row. Handled before the bounds check below, since a child row's
// number indexes its siblings, not m_threads.
- if (index.parent().isValid())
- return {};
+ if (isMessageRow(index)) {
+ const MessageNode node = messageAt(index);
+ if (node.messageId.isEmpty())
+ return {};
+
+ switch (role) {
+ case IsMessageRole:
+ return true;
+ case MessageIdRole:
+ return node.messageId;
+ case MessageDepthRole:
+ return node.depth;
+ case ThreadIdRole:
+ // A message row still belongs to a thread, and a caller that only
+ // needs the containing thread must not have to walk up itself.
+ return node.threadId;
+ case TagsRole:
+ case PillTagsRole:
+ // No strip under a child row: the strip is a ROW-wide band carrying
+ // the thread's tags, and one under every reply would stripe the
+ // list and repeat the same tags down the whole expansion.
+ return QStringList();
+ case PillColoursRole:
+ return QVariantList();
+ case AccountLabelRole:
+ return QString();
+ case Qt::DisplayRole:
+ switch (index.column()) {
+ case AuthorsColumn:
+ // The REPLY's sender, not the thread's author summary. Reading
+ // the thread's fields here would look almost right, since the
+ // first sender usually appears in both.
+ return node.from;
+ case SubjectColumn:
+ return node.subject;
+ case DateColumn:
+ return node.date.toString(QStringLiteral("yyyy-MM-dd hh:mm"));
+ case AttachmentColumn:
+ return node.hasAttachment() ? attachmentGlyph() : QString();
+ case FlagColumn:
+ return node.isFlagged() ? flagGlyph() : QString();
+ default:
+ return {};
+ }
+ case Qt::ForegroundRole:
+ // Same rule as a thread row: read recedes, unread stays at the
+ // palette's own colour.
+ return node.isUnread() ? QVariant() : QVariant(readColour());
+ default:
+ return {};
+ }
+ }
if (index.row() >= m_threads.size())
return {};
@@ -184,6 +232,19 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const
if (role == ThreadIdRole)
return thread.threadId;
+ // Answered rather than left to fall through as an invalid QVariant. An
+ // invalid one converts to false and an empty string anyway, so the
+ // behaviour is the same, but a role the model never mentions is a latent
+ // bug the next reader has to prove is harmless.
+ if (role == IsMessageRole)
+ return false;
+
+ if (role == MessageIdRole)
+ return QString();
+
+ if (role == MessageDepthRole)
+ return 0;
+
if (role == TagsRole)
return thread.tags;
@@ -376,6 +437,64 @@ void ThreadListModel::clear()
endResetModel();
}
+void ThreadListModel::setThreadMessages(const QString &threadId,
+ const QVector<MessageNode> &nodes)
+{
+ for (int row = 0; row < m_threads.size(); ++row) {
+ if (m_threads.at(row).summary.threadId != threadId)
+ continue;
+
+ const QModelIndex parent = index(row, 0, QModelIndex());
+
+ // Replace, not append. A thread reloaded after a sync would otherwise
+ // list every reply twice.
+ if (!m_threads.at(row).children.isEmpty()) {
+ beginRemoveRows(parent, 0, m_threads.at(row).children.size() - 1);
+ m_threads[row].children.clear();
+ endRemoveRows();
+ }
+
+ QVector<MessageNode> children;
+ children.reserve(nodes.size());
+ for (const MessageNode &node : nodes) {
+ if (node.depth > 0)
+ children.append(node);
+ }
+
+ if (!children.isEmpty()) {
+ beginInsertRows(parent, 0, children.size() - 1);
+ m_threads[row].children = children;
+ endInsertRows();
+ }
+
+ // Set even when there are no replies: that is the difference between a
+ // single-message thread and one whose replies were never fetched.
+ m_threads[row].loaded = true;
+ return;
+ }
+}
+
+bool ThreadListModel::isMessageRow(const QModelIndex &index) const
+{
+ return index.isValid() && index.parent().isValid();
+}
+
+MessageNode ThreadListModel::messageAt(const QModelIndex &index) const
+{
+ if (!isMessageRow(index))
+ return {};
+
+ const int threadRow = index.parent().row();
+ if (threadRow < 0 || threadRow >= m_threads.size())
+ return {};
+
+ const QVector<MessageNode> &children = m_threads.at(threadRow).children;
+ if (index.row() < 0 || index.row() >= children.size())
+ return {};
+
+ return children.at(index.row());
+}
+
ThreadSummary ThreadListModel::threadAt(int row) const
{
if (row < 0 || row >= m_threads.size())
diff --git a/src/threadlistmodel.h b/src/threadlistmodel.h
index 1aa0271..a7ce5d3 100644
--- a/src/threadlistmodel.h
+++ b/src/threadlistmodel.h
@@ -84,6 +84,18 @@ public:
/// model because it owns the TagColors instance; a delegate reading
/// config itself would be a second source of truth.
PillColoursRole,
+
+ /// True when the row is a MESSAGE row rather than a thread root.
+ /// Drives both the action scope and whether the view paints a tag
+ /// strip under the row.
+ IsMessageRole,
+
+ /// The message id behind a message row. Empty on a thread root.
+ MessageIdRole,
+
+ /// The message's reply depth, for the view's indentation. 1 for a
+ /// direct reply, since depth 0 is the root row itself.
+ MessageDepthRole,
};
/// Row fill for a thread tagged `deleted`, and for one tagged `spam`.
@@ -131,6 +143,23 @@ public:
ThreadSummary threadAt(int row) const;
+ /// Fills in a thread's message rows once the worker has walked its tree.
+ ///
+ /// The depth-0 message is dropped: it is the thread's first message and the
+ /// ROOT row already stands for it. Keeping it would show a thread of seven
+ /// as one root and seven children, contradicting the reply count the row
+ /// advertises. Calling again replaces the rows rather than appending, so a
+ /// thread reloaded after a sync does not list its replies twice.
+ void setThreadMessages(const QString &threadId,
+ const QVector<MessageNode> &nodes);
+
+ /// True when the index is a message row rather than a thread root.
+ bool isMessageRow(const QModelIndex &index) const;
+
+ /// The message row's node, or a default-constructed one for any index that
+ /// is not a message row.
+ MessageNode messageAt(const QModelIndex &index) const;
+
/// The account keys behind a thread's account tags, for item 49's
/// per-account sync.
///
diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp
index 9684637..b4be527 100644
--- a/tests/test_threadlistmodel.cpp
+++ b/tests/test_threadlistmodel.cpp
@@ -29,6 +29,9 @@ class TestThreadListModel : public QObject
private slots:
void messageNodeHoldsDisplayFacts();
void rootRowsSurviveTheTreeConversion();
+ void repliesBecomeChildRowsUnderTheirThread();
+ void messageRowsShowTheirOwnSenderAndSubject();
+ void reloadingAThreadReplacesItsRepliesRatherThanRepeatingThem();
void startsEmpty();
void accountKeysComeFromTheAccountTags();
void accountKeysCoverAThreadSpanningTwoAccounts();
@@ -78,6 +81,118 @@ static ThreadSummary makeThread(const QString &id, const QString &subject)
return t;
}
+static MessageNode makeNode(const QString &id, int depth,
+ const QString &from = QStringLiteral("Alice"),
+ const QString &subject = QStringLiteral("Re: Hi"))
+{
+ MessageNode n;
+ n.messageId = id;
+ n.threadId = QStringLiteral("t1");
+ n.from = from;
+ n.subject = subject;
+ n.date = QDateTime::fromSecsSinceEpoch(1750000000);
+ n.depth = depth;
+ return n;
+}
+
+void TestThreadListModel::repliesBecomeChildRowsUnderTheirThread()
+{
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("A subject")) });
+
+ // Depth 0 is the thread's FIRST message and belongs on the root row, not in
+ // the children: the user's model is "N replies", so a thread of three shows
+ // one root and two children.
+ model.setThreadMessages(QStringLiteral("t1"),
+ { makeNode(QStringLiteral("m0@example.org"), 0),
+ makeNode(QStringLiteral("m1@example.org"), 1),
+ makeNode(QStringLiteral("m2@example.org"), 2) });
+
+ const QModelIndex root = model.index(0, 0, QModelIndex());
+ QCOMPARE(model.rowCount(root), 2);
+
+ const QModelIndex child =
+ model.index(0, ThreadListModel::SubjectColumn, root);
+ QVERIFY(child.isValid());
+ QCOMPARE(model.parent(child), model.index(0, 0, QModelIndex()));
+
+ QVERIFY(model.data(child, ThreadListModel::IsMessageRole).toBool());
+ QCOMPARE(model.data(child, ThreadListModel::MessageIdRole).toString(),
+ QStringLiteral("m1@example.org"));
+
+ // A message row still belongs to a thread, so a caller that only needs the
+ // containing thread does not have to walk up itself.
+ QCOMPARE(model.data(child, ThreadListModel::ThreadIdRole).toString(),
+ QStringLiteral("t1"));
+
+ // A thread root is not a message row and carries no message id.
+ QVERIFY(!model.data(root, ThreadListModel::IsMessageRole).toBool());
+ QVERIFY(model.data(root, ThreadListModel::MessageIdRole)
+ .toString().isEmpty());
+
+ QAbstractItemModelTester tester(
+ &model, QAbstractItemModelTester::FailureReportingMode::Warning);
+ Q_UNUSED(tester);
+}
+
+void TestThreadListModel::messageRowsShowTheirOwnSenderAndSubject()
+{
+ // A reply's row shows the REPLY's sender, not the thread's author summary.
+ // Reading the thread's fields for a child row is the obvious mistake and
+ // would look almost right, since the first sender is usually in both.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("A subject")) });
+ model.setThreadMessages(
+ QStringLiteral("t1"),
+ { makeNode(QStringLiteral("m0@example.org"), 0),
+ makeNode(QStringLiteral("m1@example.org"), 1,
+ QStringLiteral("Bob <bob@example.org>"),
+ QStringLiteral("Re: A subject")) });
+
+ const QModelIndex root = model.index(0, 0, QModelIndex());
+ const QModelIndex authors =
+ model.index(0, ThreadListModel::AuthorsColumn, root);
+ const QModelIndex subject =
+ model.index(0, ThreadListModel::SubjectColumn, root);
+
+ QCOMPARE(model.data(authors, Qt::DisplayRole).toString(),
+ QStringLiteral("Bob <bob@example.org>"));
+ QCOMPARE(model.data(subject, Qt::DisplayRole).toString(),
+ QStringLiteral("Re: A subject"));
+
+ // No tag strip under a child row. The strip is a row-wide band carrying the
+ // THREAD's tags; one under every reply would stripe the list and repeat the
+ // same tags down the whole expansion.
+ QVERIFY(model.data(subject, ThreadListModel::PillTagsRole)
+ .toStringList().isEmpty());
+}
+
+void TestThreadListModel::reloadingAThreadReplacesItsRepliesRatherThanRepeatingThem()
+{
+ // A thread reloaded after a sync must not end up listing its replies twice.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"),
+ QStringLiteral("A subject")) });
+
+ const QVector<MessageNode> nodes{
+ makeNode(QStringLiteral("m0@example.org"), 0),
+ makeNode(QStringLiteral("m1@example.org"), 1)
+ };
+
+ model.setThreadMessages(QStringLiteral("t1"), nodes);
+ const QModelIndex root = model.index(0, 0, QModelIndex());
+ QCOMPARE(model.rowCount(root), 1);
+
+ model.setThreadMessages(QStringLiteral("t1"), nodes);
+ QCOMPARE(model.rowCount(root), 1);
+
+ QAbstractItemModelTester tester(
+ &model, QAbstractItemModelTester::FailureReportingMode::Warning);
+ Q_UNUSED(tester);
+}
+
void TestThreadListModel::accountKeysComeFromTheAccountTags()
{
// Item 49 reads this to decide which mbsync channels a sync needs. Only