Paths

Table of Contentst

Differential D9429

Send all notifications received by backgrounded app to JS at once
ClosedPublic
Actions

Authored by marcin on Oct 9 2023, 9:19 AM.

Tags

None

Referenced Files

	F2775731: D9429.id31872.diff
	Fri, Sep 20, 5:21 AM

	F2775012: D9429.diff
	Fri, Sep 20, 3:53 AM

	Unknown Object (File)
	Sun, Sep 15, 1:15 PM

	Unknown Object (File)
	Sun, Sep 15, 1:15 PM

	Unknown Object (File)
	Sun, Sep 15, 1:15 PM

	Unknown Object (File)
	Sun, Sep 15, 1:15 PM

	Unknown Object (File)
	Sun, Sep 15, 1:13 PM

	Unknown Object (File)
	Sun, Sep 15, 12:53 PM

View All 93 Files

Subscribers

Details

Reviewers

tomek
bartek
michal

Commits

rCOMM19e5f0ff3d84: Send all notifications received by backgrounded app to JS at once

Summary

Previously for each notification received by backgrounded app we sent separate JS event so that it could be persisted in redux and SQLite. However there was
no reason not to send one JS event with array of all messages. It can potentially help the issue:
https://linear.app/comm/issue/ENG-5147/messageinfos-from-notifs-take-too-long-to-import-on-ios-start#comment-a2fb0774

Test Plan

Build iOS app.
Background it.
Send multiple messages.
Kill the keyserver.
Bring the app to the foreground again.
Ensure all messages are available in the relevant thread(s) even the keyserver is not running.

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

marcin created this revision.Oct 9 2023, 9:19 AM

Herald added subscribers: will, ashoat. · View Herald TranscriptOct 9 2023, 9:19 AM

Harbormaster completed remote builds in B23110: Diff 31812.Oct 9 2023, 9:37 AM

marcin requested review of this revision.Oct 9 2023, 9:37 AM

Looks good to me but letting others review as well

native/push/push-handler.react.js
548–550 ↗	(On Diff #31812)	This should also work

tomek accepted this revision.Oct 10 2023, 4:23 AM

tomek added inline comments.

native/push/ios.js
85 ↗	(On Diff #31812)	Is it really optional?

This revision is now accepted and ready to land.Oct 10 2023, 4:23 AM

Make messageInfosArray not optional since it is actually not
Use flatMap

Ensure in Objective-C code that messageInfosArray is not optional.

Harbormaster completed remote builds in B23143: Diff 31870.Oct 10 2023, 7:44 AM

Harbormaster completed remote builds in B23145: Diff 31872.

Closed by commit rCOMM19e5f0ff3d84: Send all notifications received by backgrounded app to JS at once (authored by marcin). · Explain WhyOct 10 2023, 8:36 AM

This revision was automatically updated to reflect the committed changes.

marcin added a commit: rCOMM19e5f0ff3d84: Send all notifications received by backgrounded app to JS at once.

Revision Contents
Changeset List

Path

Size

native/

ios/

Comm/

10 lines

push/

4 lines

push-handler.react.js

8 lines

Diff 31877

native/ios/Comm/AppDelegate.mm

Loading...

native/push/ios.js

Loading...

native/push/push-handler.react.js

Loading...