Fix DM activity handler not to send unnecessary read status update
Needs ReviewPublic
Actions

Authored by marcin on Wed, Sep 18, 7:36 AM.

Details

Reviewers

ashoat
tomek
kamil

Summary

This differential prevents DM activity handler from spamming unread status unpdates by sending updates only for threads that are unread when we enter them and using debouncing when receiving messages to currently active thread.

Test Plan

Add console log to each portion of sent dm's
Ensure that when entering thick thread updates are sent once only if the thread was unread.
Ensuure that after receiving stream of messages to thick thread, unread updates are sent only once 5 seconds after the last message.

Diff Detail

Repository

rCOMM Comm

Branch

marcin/eng-9267

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

marcin created this revision.Wed, Sep 18, 7:36 AM

marcin added inline comments.Wed, Sep 18, 7:43 AM

lib/handlers/dm-activity-handler.js
89 ↗	(On Diff #44314)	If there is pending debounced call from previous active thread we want to call it right now.
110 ↗	(On Diff #44314)	Checking for change in latest message makes sense only if we are in the same thread.
web/redux/redux-setup.js
460 ↗	(On Diff #44314)	This code was changing thick thread read status before dm activity handler could detect change and send necessary updates.

Harbormaster completed remote builds in B31700: Diff 44314.Wed, Sep 18, 7:53 AM

marcin requested review of this revision.Wed, Sep 18, 7:53 AM

This solution feels a lot more complicated than it should. Maybe it has to be that complicated, with canceling and flushing, but I'm not sure.

Would it be possible to have a memo, with activeThread as a dependency, where we store the denounced function and then simply call this function in an effect, without canceling and flushing?

This revision now requires changes to proceed.Thu, Sep 19, 3:02 AM

Address review

Harbormaster completed remote builds in B31715: Diff 44345.Thu, Sep 19, 6:10 AM