Page MenuHomePhabricator

[lib] Fix messages disappearing from thick sidebars
ClosedPublic

Authored by inka on Tue, Oct 1, 2:45 AM.
Tags
None
Referenced Files
F2876689: D13554.diff
Thu, Oct 3, 5:26 AM
F2868920: D13554.id.diff
Wed, Oct 2, 3:59 PM
F2866385: D13554.id.diff
Wed, Oct 2, 11:05 AM
Unknown Object (File)
Wed, Oct 2, 3:47 AM
Unknown Object (File)
Wed, Oct 2, 3:09 AM
Unknown Object (File)
Wed, Oct 2, 2:10 AM
Unknown Object (File)
Wed, Oct 2, 1:31 AM
Unknown Object (File)
Wed, Oct 2, 1:11 AM
Subscribers

Details

Summary

issue: ENG-9403
I considered using threadWatcher, but it seems like it has a different purpose:
Overall threadWatcher represents threads that are bing watched by the user. For thick sidebars we want to always receive all updates.
More datailed explanation:
threadWatcher is currently only used in the context of keyservers. It is used to listen for chat updates when the user is either

  1. viewing a chat they are not a member of (this is not possible in thick threads, other than sidebars. But for sidebars with this code we will be getting all updates anyway)
  2. viewing a list of sidebars / subchannels of a chat they are not a mamber of (this is not possible in thick threads)
  3. viewing settings of a chat they are not a member of (this is not possible in thick threads)

Then, keyserver specific actions, keyserver specific selector, and this reducer use the ids from thread watcher.

Test Plan

Tested that if a user is a member of a thick thread but not a member of its sidebar, they receive this sidebars messages. Tested they receive updates (tested name update)
Tested that if a user is mamber of a thick sidebar and leaves it, the messages are not removed from the store.
Tested that if the user leaves the parent chat, the messages from the thick sidebar ARE removed (in the function, !!threadInfo condition is not satisfied)

Checked all places where threadWatcher is used and verified that none of them require changes for thick sidebars.

Diff Detail

Repository
rCOMM Comm
Branch
inka/fix_sidebars
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

inka requested review of this revision.Tue, Oct 1, 5:27 AM
tomek added inline comments.
lib/reducers/message-reducer.js
148

Can we make this more generic and test whether threadTypeIsThick?

This revision is now accepted and ready to land.Tue, Oct 1, 5:36 AM
This revision was landed with ongoing or failed builds.Tue, Oct 1, 12:49 PM
This revision was automatically updated to reflect the committed changes.