Allow fcmOptions in payload keys

[parndt]

In #597 support was added for fcmOptions for specifying the analytics label for notifications.

Unfortunately, the list of valid payload keys wasn't extended to support it.

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it!) and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[parndt]

@hiranya911 I tried to add all the right specs for this to ensure that this change is correct and avoids regressions, which pass locally, but I note that gulp build fails because of a missing index signature. Are you able to provide guidance here for me, please?

src/messaging/messaging.ts(775,33): error TS7017: Element implicitly has an 'any' type because type 'DataMessagePayload | FcmOptionsPayload | NotificationMessagePayload' has no index signature.

[parndt]

I opted for this implementation for the index signature:

/* Payload for fcmOptions messages */
export interface FcmOptionsPayload {
  analyticsLabel?: string;
  [key: string]: string;
}

you're welcome to edit the PR to adjust if this isn't suitable 😄 or just let me know what to change.

Thanks!

[hiranya911]

I have a few concerns with this change.

1. I don't think the legacy FCM API supports fcmOptions (see https://firebase.google.com/docs/cloud-messaging/http-server-ref).
2. Even if it did, I don't know if we should be adding new features to the legacy API.

@chong-shao wdyt?

[parndt]

Thanks @hiranya911 for the review.

For some extra context which may help with deciding about this PR, here's how we're using this code inside a cloud function using messaging.sendToDevice:

const admin = require('firebase-admin')

admin.messaging().sendToDevice(
  pushToken,
  data: {
    custom_notification: JSON.stringify(
      Object.assign({ deepLink, show_in_foreground: true }, notification)
    ),
    deepLink,
  },
  fcmOptions: {
    analyticsLabel: 'test.analytics.label',
  },
  notification,
)

We saw this error:

{ Error: Messaging payload contains an invalid "fcmOptions" property. Valid properties are "data" and "notification".

I guess that is triggering usage of the legacy APIs when using MessagingPayload; is there another way to do this that uses the non-legacy API and allows us to use fcmOptions with analyticsLabel?
Does it involve this migration process ?

Thanks for your time! 😄

[hiranya911]

You're using the legacy FCM APIs (sendToToken(), sendToDevice(), sendToDeviceGroup()). Those APIs don't support fcmOptions. You have to use the send() API for that. We have already added fcmOptions support there.

[hiranya911]

@parndt please take a look at the Node.js examples on this page and update your code to use the new API: https://firebase.google.com/docs/cloud-messaging/send-message

The old API doesn't support fcmOptions.

[parndt]

Thanks a lot! 🙇‍♂️ that's exactly what I needed.