Add support for "webpush.fcm_options" (WebpushFcmOptions) #203
Conversation
|
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 (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
|
|
CLAs look good, thanks! |
There was a problem hiding this comment.
Thanks @robermac for the PR. A few changes required before we can merge this.
| @@ -622,6 +636,7 @@ def encode_webpush(cls, webpush): | |||
| 'headers': _Validators.check_string_dict( | |||
| 'WebpushConfig.headers', webpush.headers), | |||
| 'notification': cls.encode_webpush_notification(webpush.notification), | |||
| 'fcm_options': cls.encode_webpush_fcm_options(webpush.fcm_options) | |||
There was a problem hiding this comment.
The json key should be called fcmOptions. The REST API documentation mentions fcm_options by error and the FCM team is going to fix it.
| return None | ||
| if not isinstance(fcm_options, WebpushFcmOptions): | ||
| raise ValueError('WebpushConfig.fcm_options must be an instance of ' | ||
| 'WebFcmOptions class.') |
| raise ValueError('WebpushConfig.fcm_options must be an instance of ' | ||
| 'WebFcmOptions class.') | ||
| result = { | ||
| 'link': _Validators.check_string( |
There was a problem hiding this comment.
Also check for https:// prefix.
| check_encoding(messaging.Message( | ||
| topic='topic', webpush=messaging.WebpushConfig(fcm_options=fcm_options))) | ||
| return excinfo | ||
|
|
There was a problem hiding this comment.
Also add a test case to verify the happy path.
|
Closing this in favor of the c 8E16 hanges being implemented in https://github.com/firebase/firebase-admin-python/tree/hkj-fcm-options |
Resolved #202