Page MenuHomePhabricator
Differential D10326

Don't add nullish id field to badgeonly notification payload
ClosedPublic
Actions

Authored by marcin on Dec 13 2023, 7:17 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 23, 1:55 AM
Unknown Object (File)
Wed, Nov 13, 7:41 AM
Unknown Object (File)
Wed, Nov 13, 7:41 AM
Unknown Object (File)
Wed, Nov 13, 7:41 AM
Unknown Object (File)
Wed, Nov 13, 7:41 AM
Unknown Object (File)
Wed, Nov 13, 7:41 AM
Unknown Object (File)
Oct 12 2024, 4:53 AM
Unknown Object (File)
Oct 12 2024, 4:53 AM
View All 63 Files
Subscribers
ashoat

Details

Reviewers
tomek
michal
Commits
rCOMM320040675330: Don't add nullish id field to badgeonly notification payload
Summary

The id field of Android notification must be either string or be absent at all. Currently if notification doesn't have id field
encryptAndroidNotification function puts nullish value to its encrypted form. This was the casue of
https://linear.app/comm/issue/ENG-6051/encrypted-badge-updates-on-android-are-broken.

Test Plan
  1. Add console.log to fcmSInglePush method to log if sending notifications fails.
  2. Build Android app and send badge only notification.
  3. Ensure no error is logged

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

marcin created this revision.Dec 13 2023, 7:17 AM
Herald added a subscriber: ashoat. · View Herald TranscriptDec 13 2023, 7:17 AM
Harbormaster completed remote builds in B25020: Diff 34566.Dec 13 2023, 7:35 AM
marcin requested review of this revision.Dec 13 2023, 7:35 AM
tomek accepted this revision.Dec 13 2023, 9:46 AM
tomek added inline comments.
keyserver/src/push/crypto.js
201–204 ↗(On Diff #34566)

plaintext is a single word. Regardless, I think unencryptedData might be less confusing.

This revision is now accepted and ready to land.Dec 13 2023, 9:46 AM
marcin updated this revision to Diff 34620.Dec 14 2023, 12:27 AM
  1. Address comment about inaccurate name for data that is not supposed to undergo encryption.
  2. Rebase before landing.
Harbormaster completed remote builds in B25058: Diff 34620.Dec 14 2023, 12:46 AM
Closed by commit rCOMM320040675330: Don't add nullish id field to badgeonly notification payload (authored by marcin). · Explain WhyDec 14 2023, 1:08 AM
This revision was automatically updated to reflect the committed changes.
marcin added a commit: rCOMM320040675330: Don't add nullish id field to badgeonly notification payload.

Revision Contents
Changeset List

PathSize
keyserver/
src/
push/
12 lines

Diff 34622

View Options

keyserver/src/push/crypto.js

Loading...