Paths

Table of Contentst

Differential D13495

Implement sending large thick thread notifs
ClosedPublic
Actions

Authored by marcin on Thu, Sep 26, 1:07 PM.

Tags

None

Referenced Files

	F2838907: D13495.id44630.diff
	Sun, Sep 29, 12:21 AM

	F2838522: D13495.diff
	Sat, Sep 28, 10:26 PM

	F2835069: D13495.id44630.diff
	Sat, Sep 28, 12:37 PM

	F2834277: D13495.diff
	Sat, Sep 28, 10:39 AM

	F2833770: D13495.diff
	Sat, Sep 28, 8:52 AM

	Unknown Object (File)
	Fri, Sep 27, 9:11 AM

	Unknown Object (File)
	Fri, Sep 27, 9:11 AM

	Unknown Object (File)
	Fri, Sep 27, 9:11 AM

View All 13 Files

Subscribers

Details

Reviewers

tomek
kamil
bartek

Commits

rCOMM39553e6dfb3a: Implement sending large thick thread notifs

Summary

This differential uploads large e2ee notifs to blob service

Test Plan

Modify app to log all blob that it sends (JS) and decrypted blobs that it receives (native code). Send large and small notifs around. Ensure that only large notifs are logged in native code. Ensure that if there are multiple devices of the same platform as receivers only one distinct blob is uploaded to blob service.

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

marcin created this revision.Thu, Sep 26, 1:07 PM

Herald added a subscriber: ashoat. · View Herald TranscriptThu, Sep 26, 1:07 PM

marcin added a parent revision: D13469: Create large notifications in thick threads.Thu, Sep 26, 1:08 PM

Harbormaster completed remote builds in B31889: Diff 44612.Thu, Sep 26, 1:24 PM

marcin requested review of this revision.Thu, Sep 26, 1:24 PM

bartek added inline comments.Fri, Sep 27, 2:28 AM

lib/push/send-hooks.react.js
295–296 ↗	(On Diff #44612)	This function is sync and does both
308–309 ↗	(On Diff #44612)	I'd move this directly into `fetch` call inside `assignMultipleHolders`
lib/utils/blob-service.js
88–91 ↗	(On Diff #44612)	In case we're going to leave this hack, we need to leave in-code comment here. We're relying on undocumented behavior of React Native on iOS

tomek added inline comments.Fri, Sep 27, 4:55 AM

lib/push/android-notif-creators.js
172 ↗	(On Diff #44612)	What is the reason behind this change?
225–228 ↗	(On Diff #44612)	It might be worth hiding less obvious code into a function with a readable name, e.g. `generateUUIDs`.

tomek added inline comments.Fri, Sep 27, 4:56 AM

lib/push/send-hooks.react.js
265 ↗	(On Diff #44612)	It is confusing to see a notifs related argument in a general-purpose function. Can we rename it?

Send string on native and blob on web.

tomek accepted this revision.Fri, Sep 27, 6:37 AM

tomek added 1 blocking reviewer(s): bartek.

Harbormaster completed remote builds in B31899: Diff 44628.Fri, Sep 27, 6:37 AM

marcin added inline comments.Fri, Sep 27, 6:50 AM

lib/push/android-notif-creators.js
172 ↗	(On Diff #44612)	Debuging artifact.
lib/utils/blob-service.js
88–91 ↗	(On Diff #44612)	Hack was deleted.

bartek accepted this revision.Fri, Sep 27, 6:51 AM

This revision is now accepted and ready to land.Fri, Sep 27, 6:51 AM

address review

Rebase before landing

marcin edited the test plan for this revision. (Show Details)Fri, Sep 27, 7:12 AM

Harbormaster completed remote builds in B31903: Diff 44636.Fri, Sep 27, 7:15 AM

bartek mentioned this in D13503: [blob] Support uploading base64-encoded blobs.Fri, Sep 27, 7:32 AM

rebase

This revision was landed with ongoing or failed builds.Fri, Sep 27, 7:48 AM

Closed by commit rCOMM39553e6dfb3a: Implement sending large thick thread notifs (authored by marcin). · Explain Why

This revision was automatically updated to reflect the committed changes.

marcin added a commit: rCOMM39553e6dfb3a: Implement sending large thick thread notifs.

Harbormaster completed remote builds in B31910: Diff 44643.Fri, Sep 27, 8:03 AM

bartek mentioned this in rCOMMb11c17237618: [blob] Support uploading base64-encoded blobs.Sat, Sep 28, 12:49 AM

Revision Contents
Changeset List

Path

Size

keyserver/

src/

creators/

farcaster-channel-tag-creator.js

6 lines

invite-link-creator.js

6 lines

push/

encrypted-notif-utils-api.js

2 lines

services/

67 lines

lib/

facts/

blob-service.js

6 lines

push/

android-notif-creators.js

10 lines

apns-notif-creators.js

10 lines

23 lines

send-hooks.react.js

98 lines

12 lines

types/

1 line

utils/

__mocks__/

1 line

blob-service.js

92 lines

native/

push/

encrypted-notif-utils-api.js

2 lines

web/

push-notif/

encrypted-notif-utils-api.js

2 lines

Diff 44646

keyserver/src/creators/farcaster-channel-tag-creator.js

Loading...

keyserver/src/creators/invite-link-creator.js

Loading...

keyserver/src/push/encrypted-notif-utils-api.js

Loading...

keyserver/src/services/blob.js

Loading...

lib/facts/blob-service.js

Loading...

lib/push/android-notif-creators.js

Loading...

lib/push/apns-notif-creators.js

Loading...

lib/push/crypto.js

Loading...

lib/push/send-hooks.react.js

Loading...

lib/push/send-utils.js

Loading...

lib/types/notif-types.js

Loading...

lib/utils/mocks/config.js

Loading...

lib/utils/blob-service.js

Loading...

native/push/encrypted-notif-utils-api.js

Loading...

web/push-notif/encrypted-notif-utils-api.js

Loading...