Page MenuHomePhabricator

Move android rescinds and badge updates to lib
ClosedPublic

Authored by marcin on Jun 13 2024, 8:50 AM.
Tags
None
Referenced Files
F2829054: D12427.diff
Fri, Sep 27, 3:15 PM
Unknown Object (File)
Tue, Sep 3, 7:10 PM
Unknown Object (File)
Tue, Sep 3, 7:10 PM
Unknown Object (File)
Tue, Sep 3, 7:10 PM
Unknown Object (File)
Tue, Sep 3, 7:10 PM
Unknown Object (File)
Tue, Sep 3, 7:10 PM
Unknown Object (File)
Tue, Sep 3, 7:10 PM
Unknown Object (File)
Tue, Sep 3, 7:10 PM
Subscribers

Details

Summary

This differential move Android rescinds and badge updates to lib and makes the keyserver use shared code.

Test Plan
  1. Flow
  2. Test that rescind and badge updates work on Android

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

I strongly encourage you to rethink the amount of type spreads you're doing, especially when they're nested within other types

lib/push/notif-creators.js
329 ↗(On Diff #41290)

In general, I am skeptical of this approach of nesting unions inside a spread. In the past I've seen Flow not handle it well. I usually opt to make the upper type a union, and to include the shared properties in each branch (senderDeviceID, platformDetails) via a spread

333 ↗(On Diff #41290)
lib/types/notif-types.js
140 ↗(On Diff #41290)

What is this spread doing?

165 ↗(On Diff #41290)

What is this spread doing?

lib/push/android-notif-creators.js
266 ↗(On Diff #41556)

This is logic copy pasted from keyserver/push/rescind.js but extended to allow rescinds without notification id (thick thread rescinds).

337 ↗(On Diff #41556)

This logic is copy-pasted from keyserver/src/send.js but extended to allow thick thread badge updates.

You forgot to address @ashoat's comments

lib/types/notif-types.js
168–174 ↗(On Diff #41556)

This applies to types above

This revision is now accepted and ready to land.Jun 25 2024, 3:26 AM