Paths

Table of Contentst

Differential D13495

Implement sending large thick thread notifs
ClosedPublic
Actions

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

Tags

None

Referenced Files

	Unknown Object (File)
	Wed, Dec 18, 10:02 AM

	Unknown Object (File)
	Wed, Dec 18, 10:02 AM

	Unknown Object (File)
	Wed, Dec 18, 10:02 AM

	Unknown Object (File)
	Wed, Dec 18, 10:02 AM

	Unknown Object (File)
	Wed, Dec 18, 10:02 AM

	Unknown Object (File)
	Wed, Dec 18, 10:02 AM

	Unknown Object (File)
	Wed, Dec 18, 10:02 AM

	Unknown Object (File)
	Wed, Dec 18, 10:01 AM

View All 78 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

Branch

marcin/eng-8709

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

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

Herald added a subscriber: ashoat. · View Herald TranscriptSep 26 2024, 1:07 PM

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

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

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

bartek added inline comments.Sep 27 2024, 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.Sep 27 2024, 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.Sep 27 2024, 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.Sep 27 2024, 6:37 AM

tomek added 1 blocking reviewer(s): bartek.

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

marcin added inline comments.Sep 27 2024, 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.Sep 27 2024, 6:51 AM

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

address review

Rebase before landing

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

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

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

rebase

This revision was landed with ongoing or failed builds.Sep 27 2024, 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.Sep 27 2024, 8:03 AM

bartek mentioned this in rCOMMb11c17237618: [blob] Support uploading base64-encoded blobs.Sep 28 2024, 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

16 lines

apns-notif-creators.js

16 lines

19 lines

send-hooks.react.js

102 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 44628

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...