FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add support for "webpush.fcm_options" (WebpushFcmOptions) by robertu7 · Pull Request #203 · firebase/firebase-admin-python · GitHub

Add support for "webpush.fcm_options" (WebpushFcmOptions) - #203

Closed
robertu7 wants to merge 2 commits into
firebase:masterfrom
robertu7:master
Closed

Add support for "webpush.fcm_options" (WebpushFcmOptions)#203
robertu7 wants to merge 2 commits into
firebase:masterfrom
robertu7:master

Conversation

robertu7 commented Oct 5, 2018

Copy link
Copy Markdown

Resolved #202

Copy link
Copy Markdown

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. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

Copy link
Copy Markdown

CLAs look good, thanks!

hiranya911 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks @robermac for the PR. A few changes required before we can merge this.

'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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

WebpushFcmOptions

raise ValueError('WebpushConfig.fcm_options must be an instance of '
'WebFcmOptions class.')
result = {
'link': _Validators.check_string(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Also check for https:// prefix.

Comment thread tests/test_messaging.py
check_encoding(messaging.Message(
topic='topic', webpush=messaging.WebpushConfig(fcm_options=fcm_options)))
return excinfo

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Also add a test case to verify the happy path.

Copy link
Copy Markdown
Contributor

Closing this in favor of the changes being implemented in https://github.com/firebase/firebase-admin-python/tree/hkj-fcm-options

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL