fix(llc): update watcher count and watchers from realtime events#2837
fix(llc): update watcher count and watchers from realtime events#2837VelikovPetar wants to merge 4 commits into
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughChangesWatcher count synchronization
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
| watchers: [...?existingWatchers?.where((user) => user.id != watcher.id)], | ||
| ), | ||
| final existingWatchers = channelState.watchers ?? const <User>[]; | ||
| _channelState = channelState.copyWith( |
There was a problem hiding this comment.
Any particular reason to not use updateChannelState here?
There was a problem hiding this comment.
The problem is that updateChannelState does a "merge" operation over the watchers:
final watchers = _channelState.watchers ?? const <User>[];
final newWatchers = watchers.merge(
updatedState.watchers,
key: (watcher) => watcher.id,
);So basically, if the initial watcher state is: ["a", "b", "c"]
And we receive a user.stop.watching for c, we would merge ["a", "b"] into ["a", "b", "c"] which wouldn't actually update the state.
We could introduce some new flags to the updateChannelState method to handle such scenarios, but I think that it would make things more complicated.
Let me know what do you think!
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2837 +/- ##
==========================================
+ Coverage 71.48% 71.53% +0.05%
==========================================
Files 426 426
Lines 26892 26897 +5
==========================================
+ Hits 19224 19242 +18
+ Misses 7668 7655 -13 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Submit a pull request
Linear: FLU-619
Github Issue: #
CLA
Description of the pull request
ChannelClientState.watcherCountandChannelClientState.watchersnever reflected realtime events during a session:watcher_countonuser.watching.start/user.watching.stop/message.newwas dropped intoEvent.extraData, so the count stayed frozen at the last fullchannel.watch()/queryChannelssnapshot.watchersonuser.watching.stop— the additive keyed merge inupdateChannelState(needed for watcher pagination) can only upsert, so the departing watcher lingered until the next full query.iOS and Android already trust the server-provided
watcher_countper event, so Flutter was the outlier.Changes
Event.watcherCount, mapping the serverwatcher_countfield (promoted to a first-class field instead ofextraData).user.watching.start,user.watching.stop, andmessage.new.ChannelClientState.watchersonuser.watching.stop(assign state directly, bypassing the additive merge).notification.message_new/notification.thread_message_neware intentionally excluded: the backend fetches the watcher count (viaGetChannelWatchersCount) only after the notification fan-out inPostMessageInsert, so these events structurally carrywatcher_count: 0.Tests
Watching Eventstest group inchannel_test.dart: start/stop update the count, stop removes the watcher, a null count preserves the existing value,message.newupdates the count, andnotification.message_newdoes not overwrite it.melos run format+melos run analyzeclean; watching-event tests pass.Screenshots / Videos
No UI changes.
Summary by CodeRabbit
New Features
Bug Fixes