feat: add new feature to app identifier to push notification payload #35686
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
|
|
AtharvShelke
changed the title
AtharvShelke
changed the title
AtharvShelke
changed the title
There was a problem hiding this comment.
The type GatewayNotification should be updated to reflect the new appName property.
I also found that the type GatewayNotification (that probably was copy pasted from the push gateway code) can be misleading because it diverges from the data that is sent to the endpoint which is essentially just PendingPushNotification.
This might be a more fitting type description but this change should probably be put in another issue/PR:
type GatewayNotification = Omit<PendingPushNotification, 'priority'> & {
uniqueId: string
appName: string
topic?: string
}
apps/meteor/app/push/server/push.ts
Outdated
| if ('apn' in app.token && app.token.apn) { | ||
| countApn.push(app._id); | ||
| return this.sendGatewayPush(gateway, 'apn', app.token.apn, { | ||
| topic: app.appName, | ||
| ...gatewayNotification | ||
| }); | ||
| } | ||
|
|
||
| if ('gcm' in app.token && app.token.gcm) { | ||
| countGcm.push(app._id); | ||
| return this.sendGatewayPush(gateway, 'gcm', app.token.gcm, { | ||
| appName: app.appName, // Add appName field | ||
| ...gatewayNotification | ||
| }); | ||
| } |
There was a problem hiding this comment.
I would update getGatewayNotificationData, pass app: IAppsTokens as argument and move the entire logic to create the payload there:
- set
topicfor apn - set
appNamefor both apn and gcm for consistency
feat: add app identifier to push notification payload (#35685)
Proposed changes
This PR ensures that the app identifier (appName) is included in the push notification payload for the GCM endpoint. Currently, the identifier is only included in the APN payload as topic, but it is missing from GCM.
To improve app-specific handling and allow the push gateway to correctly distinguish between different apps (official Rocket.Chat app and white-labeled versions), this update adds appName to the GCM payload while keeping topic available for its intended purpose.
Changes made:
Added appName to the GCM push notification payload.
Ensured consistency between APN and GCM by including the app identifier in both.
Issue(s)
Closes #35685
Steps to test or reproduce
Set up a Rocket.Chat instance with push notifications enabled for both APN and GCM.
Send a push notification and inspect the payload.
Verify that the appName field is present in the GCM payload and correctly identifies the app.
Further comments
This change improves push notification handling for both the official Rocket.Chat app and custom white-labeled versions. Feedback is welcome, and I’m happy to make any necessary adjustments!