From 3c16ed06fdc97e04b452f62b596b8cd69105e1ea Mon Sep 17 00:00:00 2001 From: jolavillette Date: Wed, 29 Jul 2026 12:40:07 +0200 Subject: [PATCH] gxsforums: document why getForumStatistics cannot use the generic statistic RsGxsDataAccess::getGroupStatistic() looks like a drop-in replacement that would avoid rebuilding the whole post hierarchy, but its obsolete-version filter only ever marks the original message obsolete, so a post edited N times is counted N times and superseded versions keep their unread flag forever. Tried in the field: the unread counter stuck at a non-zero value with everything read, and jumped by the number of stored versions when one post was marked unread. Leave a warning in place so the shortcut is not attempted again, and state what the real optimisation is: factoring the version collapsing out of computeMessagesHierarchy() so both paths share it. Co-Authored-By: Claude Opus 5 (1M context) --- src/services/p3gxsforums.cc | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/services/p3gxsforums.cc b/src/services/p3gxsforums.cc index d46f133b5..a9a86119c 100644 --- a/src/services/p3gxsforums.cc +++ b/src/services/p3gxsforums.cc @@ -1077,6 +1077,25 @@ bool p3GxsForums::getForumGroupStatistics(const RsGxsGroupId& ForumId,GxsGroupSt bool p3GxsForums::getForumStatistics(const RsGxsGroupId& forumId,RsGxsForumStatistics& stat) { + // NOTE: this rebuilds the whole post hierarchy just to increment three + // counters, which is expensive on a large forum. It cannot be replaced by + // the generic RsGxsDataAccess::getGroupStatistic(): that one collapses post + // versions with + // + // if(!mOrigMsgId.isNull() && mOrigMsgId != mMsgId) obsolete.insert(mOrigMsgId) + // + // which only ever marks the *original* message obsolete. Every edit of a + // post carries mOrigMsgId = the original, so a post edited N times is + // counted N times instead of once, and superseded versions that were never + // displayed keep their unread flag forever. computeMessagesHierarchy() below + // does it properly: it groups the versions, merges chains, checks that the + // editor is the original author or a moderator, and keeps only the most + // recent one. + // + // Making this cheap means factoring that version collapsing out of + // computeMessagesHierarchy() so both share it -- not reimplementing the rule + // a second time. + // 1 - get group data std::vector groups;