Page MenuHomePhabricator

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

Authored by marcin on Oct 9 2023, 9:19 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 4, 8:19 AM
Unknown Object (File)
Fri, Nov 1, 12:28 PM
Unknown Object (File)
Fri, Nov 1, 12:28 PM
Unknown Object (File)
Fri, Nov 1, 12:27 PM
Unknown Object (File)
Fri, Nov 1, 12:27 PM
Unknown Object (File)
Fri, Nov 1, 12:26 PM
Unknown Object (File)
Fri, Nov 1, 12:04 PM
Unknown Object (File)
Sat, Oct 19, 4:20 AM
Subscribers

Details

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
  1. Build iOS app.
  2. Background it.
  3. Send multiple messages.
  4. Kill the keyserver.
  5. Bring the app to the foreground again.
  6. Ensure all messages are available in the relevant thread(s) even the keyserver is not running.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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 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
  1. Make messageInfosArray not optional since it is actually not
  2. Use flatMap

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