Page MenuHomePhabricator
Differential D12539

Utilise subscription from ThickMemberInfo when building a notif
Needs ReviewPublic
Actions

Authored by marcin on Fri, Jun 21, 4:09 AM.
Tags
None
Referenced Files
F2169242: D12539.id41716.diff
Tue, Jul 2, 10:49 AM
F2165329: D12539.id.diff
Tue, Jul 2, 1:32 AM
Unknown Object (File)
Sun, Jun 30, 9:34 PM
Unknown Object (File)
Sun, Jun 30, 2:41 PM
Unknown Object (File)
Thu, Jun 27, 12:50 PM
Unknown Object (File)
Wed, Jun 26, 5:03 PM
Unknown Object (File)
Wed, Jun 26, 1:46 PM
Unknown Object (File)
Tue, Jun 25, 4:43 PM
View All 14 Files
Subscribers
ashoat

Details

Reviewers
kamil
tomek
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
Branch
marcin/eng-8410
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

marcin created this revision.Fri, Jun 21, 4:09 AM
Herald added a subscriber: ashoat. · View Herald TranscriptFri, Jun 21, 4:09 AM
Harbormaster completed remote builds in B29866: Diff 41601.Fri, Jun 21, 4:25 AM
marcin requested review of this revision.Fri, Jun 21, 4:25 AM
marcin updated this revision to Diff 41602.Fri, Jun 21, 4:44 AM

Fix android bug

marcin added a parent revision: D12477: Implement building encrypted notifications on the client.Fri, Jun 21, 4:44 AM
marcin added a parent revision: D12496: [lib] Fix up client ThreadStore validation.
marcin mentioned this in D8566: Upload/download large notification payload from the keyserver/Android NSE if its size exceeds FCM limits.Fri, Jun 21, 4:46 AM
Harbormaster completed remote builds in B29867: Diff 41602.Fri, Jun 21, 5:00 AM
marcin added a child revision: D12572: Add encryptedNotifUtilsAPI on client platforms with mock encryption..Tue, Jun 25, 5:33 AM
marcin updated this revision to Diff 41716.Wed, Jun 26, 9:47 AM

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

Harbormaster completed remote builds in B29939: Diff 41716.Wed, Jun 26, 10:13 AM
ashoat added inline comments.Mon, Jul 1, 11:38 AM
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

marcin mentioned this in D12464: Implement notification texts generation on the client.Tue, Jul 2, 12:30 PM

Revision Contents
Changeset List

PathSize
keyserver/
src/
creators/
4 lines
push/
46 lines
lib/
push/
123 lines
60 lines
shared/
5 lines

Diff 41601

View Options

keyserver/src/creators/message-creator.js

Loading...
View Options

keyserver/src/push/send.js

Loading...
View Options

lib/push/send-utils.js

Loading...
View Options

lib/push/utils.js

Loading...
View Options

lib/shared/thread-utils.js

Loading...