fix: Discussion message count displays one less than actual#38719
fix: Discussion message count displays one less than actual#38719dodaa08 wants to merge 6 commits intoRocketChat:developfrom
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughThe discussion metadata propagation hook was changed to detect edited messages and, for edits, skip incrementing the discussion room's message count and skip updating its last-message timestamp; other propagation (refresh/notify parent room) is unchanged. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38719 +/- ##
===========================================
- Coverage 70.51% 70.50% -0.02%
===========================================
Files 3176 3176
Lines 111139 111139
Branches 20050 20041 -9
===========================================
- Hits 78367 78355 -12
- Misses 30721 30742 +21
+ Partials 2051 2042 -9
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Proposed changes (including videos or screenshots)
This PR fixes an issue where the discussion message count displayed in the parent room's system message was always one less than the actual count.
Fix by adding a 1 before updating the Parent Room data with the correct timestamp for the message, also a check if the message is Only a fresh one not edited for the count.
Before, The issue is still reproducible
Screencast.From.2026-02-16.22-21-45.mp4
After
Screencast.From.2026-02-16.00-51-07.mp4
Issue(s)
Fixes #35435
Steps to test or reproduce
Additional test cases:
Further comments
Considered using
setImmediate()but the current approach is more consistent and straightforward.I hope this approach fits the most, let me know if the code or tests needs an upgrade here.
Summary by CodeRabbit