Page MenuHomePhabricator

[lib] Propagate thread infos to the notifs generating code
ClosedPublic

Authored by tomek on Thu, Oct 31, 8:20 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 16, 1:51 AM
Unknown Object (File)
Fri, Nov 15, 7:54 PM
Unknown Object (File)
Tue, Nov 12, 3:01 AM
Unknown Object (File)
Mon, Nov 11, 8:21 PM
Unknown Object (File)
Mon, Nov 11, 7:51 PM
Unknown Object (File)
Mon, Nov 11, 6:01 PM
Unknown Object (File)
Mon, Nov 11, 5:24 PM
Unknown Object (File)
Mon, Nov 11, 6:37 AM
Subscribers

Details

Summary

When a user performs an action, we generate an operation that is used to update Redux, send DM messages, and send the notifs. Updating Redux happens first. Generating notifs requires access to the thread. These cause a problem when leaving a thread, which might result in the thread being deleted from the store, making us unable to send the notifs.

Notif sending code needs threads for two main reasons: to determine the recipients and to create a notification text.

The solution is to pass a thread info inside notifs creation data so that we can use it instead of selecting from the store.

https://linear.app/comm/issue/ENG-9823/fix-sending-notifs-about-leaving-a-thick-thread

Depends on D13845

Test Plan

Created a thick thread with three users and verified that added users receive a thread, a message, and a notif about thread creation.
Left this thread as one of the users and checked that an exception isn't thrown. The notif wasn't generated because leaveThreadMessageSpec doesn't implement generatesNotifs function.
Added the user to the thread, which also doesn't generate notif for the same reason. Sending a text message to that thread generated the notifs.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Thu, Oct 31, 8:35 AM
Harbormaster failed remote builds in B32482: Diff 45515!

Include thread info in the notif data

kamil added inline comments.
lib/push/send-utils.js
125 ↗(On Diff #45584)
lib/types/notif-types.js
38 ↗(On Diff #45584)
This revision is now accepted and ready to land.Tue, Nov 5, 2:15 AM