Page MenuHomePhabricator

[lib][native][web] Stop running ID schema migration on notifs
ClosedPublic

Authored by ashoat on Oct 30 2024, 2:31 PM.
Tags
None
Referenced Files
F3529554: D13835.id45524.diff
Tue, Dec 24, 7:54 PM
F3529553: D13835.id45493.diff
Tue, Dec 24, 7:54 PM
F3529552: D13835.id.diff
Tue, Dec 24, 7:54 PM
F3529551: D13835.diff
Tue, Dec 24, 7:54 PM
Unknown Object (File)
Sun, Dec 22, 9:00 PM
Unknown Object (File)
Fri, Dec 20, 5:41 PM
Unknown Object (File)
Fri, Dec 20, 5:23 PM
Unknown Object (File)
Wed, Dec 18, 11:06 AM
Subscribers
None

Details

Summary

This logic was initially added in D8460:

A notif can be sent after the app updates, but before migration is run or the keyserver is notified of the new codeVersion. In this case we need to handle notifications with older ids.

We no longer support versions of the app from before the ID schema migration, and as such these concerns are no longer relevant.

This diff resolves ENG-9806.

Test Plan
  • I deployed a dev build to my physical iOS device
  • I logged in with a test user
  • I created a thick thread with another test user
  • I logged in as the other test user on a web device
  • I confirmed notifs were working for thick threads by sending a message to the thick thread from the web device
  • I applied this patch to prevent useHandleOlmMessageToDevice from working, such that messages would only appear if they are added to the store from a notif
  • I backgrounded the iOS app
  • I sent a message from the web device
  • I confirmed a notif was received
  • I foregrounded the iOS app and confirmed that the message appeared (before this diff, it did not)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

native/push/android.js
55 ↗(On Diff #45493)

If this approach is deemed too risky, something safer would be to gate the convertRawMessageInfoToNewIDSchema call in convertNotificationMessageInfoToNewIDSchema with thickThreadIDRegex, similar to how it's done here

But I don't think this is really necessary

This revision is now accepted and ready to land.Oct 31 2024, 8:40 AM