Pass custom APNs sounds through for FCM notifications - #747
Open
IshanA2007 wants to merge 1 commit into
Open
IshanA2007 wants to merge 1 commit into
IshanA2007 wants to merge 1 commit into
Conversation
apns_config only ever set aps['sound'] when the notification's sound attribute was exactly 'default', so any custom sound value configured by the user was silently omitted from the APNs payload sent via FCM. Pass the sound through when set, and keep omitting the key entirely when no sound is configured (so notifications stay silent, matching the existing behavior of android_notification and the plain APNs client). Fixes rpush#715
IshanA2007
force-pushed
the
fix/issue-715
branch
from
September 2, 2026 20:08
ecfc125 to
e248b3b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #715
Since the move to the FCM v1 API,
apns_configinRpush::Client::ActiveModel::Fcm::Notificationonly setaps.soundwhen the notification'ssoundwas exactly'default':Any custom sound (e.g.
'alert.caf') was silently dropped from the APNs payload, so iOS devices receiving FCM notifications fell back to no sound. The Android payload built a few lines above already passes the value through (json['sound'] = sound if sound), as does the APNs client.This makes the APNs branch do the same: pass
soundthrough whenever it is set, and keep omitting the key when it isnilso notifications without a sound stay silent, matching the existing behaviour and the APNs client's own specs.Added regression specs to the shared FCM notification examples for both the custom-sound and the nil cases.