Page MenuHomePhabricator

Utilise subscription from ThickMemberInfo when building a notif
ClosedPublic

Authored by marcin on Jun 21 2024, 4:09 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 13, 3:28 AM
Unknown Object (File)
Wed, Nov 13, 3:28 AM
Unknown Object (File)
Mon, Nov 11, 12:46 PM
Unknown Object (File)
Thu, Nov 7, 2:50 PM
Unknown Object (File)
Tue, Nov 5, 12:27 AM
Unknown Object (File)
Fri, Nov 1, 6:42 PM
Unknown Object (File)
Wed, Oct 30, 8:37 AM
Unknown Object (File)
Oct 22 2024, 12:54 PM
Subscribers

Details

Summary

This differential extracts a common logic that uses thread subscription, user role and mentioning to decide whether to send a notif. Moreover this logic is used in the client-send code.

Test Plan
  1. Test that chat muting and user mentioning (also for ENS users) works on the keyserver from the notif perspective.
  2. To test the client, apply patch from D12477 and then:
    1. Without any changes there should be no notif payloads generated.
    2. Play around with hardcoding different configurations of thread subscriptions and examine content of generated notifs.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fix a bug: notifsAllowed returned variable was assigned to wrong condition

lib/push/utils.js
168–173 ↗(On Diff #41716)

This is too many parameters. Please replace with an object param, and keep this in mind going forward

Extract parameters to object

kamil added inline comments.
lib/push/send-utils.js
63 ↗(On Diff #41949)
80 ↗(On Diff #41949)
lib/push/utils.js
214–229 ↗(On Diff #41949)

for me, it's too complicated and can be simplified by moving e.g. result of some() to a descriptive variable but up to you

This revision is now accepted and ready to land.Jul 3 2024, 3:56 AM
lib/push/utils.js
214–229 ↗(On Diff #41949)

This code was copy pasted when it was extracted to lib from the keyserver therefore I would prefer to keep it untouched.

lib/push/send-utils.js
80 ↗(On Diff #41949)

Actually the only usage of this type looks like this:

const pushUserThreadInfos: { [userID: string]: PushUserThreadInfo } = {};
  for (const threadID of threadsToMessageIndices.keys()) {

so we need mutability here. I think it is better to just drop type declaration and type the variable inline with mutable type.

  1. Address nits regarding types
  2. Rebase before landing