feat: add fcm_options property to webpush config by Odonno · Pull Request #64 · firebase/firebase-admin-dotnet · GitHub

No description provided.

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

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.

I signed it!

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

Looks good. Please update the tests, and add an entry to the CHANGELOG file.

2019

Add newline at eof

Thanks @Odonno. Just a couple of updates needed to the validation logic, and rename the property to FcmOptions.

Create the copy first and then run validation on that.

var copy = ...;

if (copy.Link != null && !copy.Link.StartsWuth("https://")
{

}

Can we also check the well-formedness of the URL by running Uri.IsWellFormedUriString(copy.Link, UriKind.Absolute)?

I missed this in my previous review. Lets rename this property to FcmOptions to be consistent with other languages (namely Node.js and Go).

@hiranya911 Thank you for your help on this.

Found a small bug in the new validation logic. I can merge once that's fixed.

…-admin-dotnet into feat/fcm_options

LGTM 👍

feat: add fcm_options property to webpush config

fb648a1

fix: fix xml documentation

4999df3

hiranya911 suggested changes Jun 3, 2019

View reviewed changes

Odonno added 3 commits June 3, 2019 20:10

fix

4428d33

docs: update changelog

fefb0da

tests: add tests and implement CopyAndValidate method

8c4aaa8

hiranya911 suggested changes Jun 3, 2019

View reviewed changes

hiranya911 self-assigned this Jun 3, 2019

Odonno and others added 2 commits June 3, 2019 22:51

refactor: rename FcmOptions property and verify well formed uri

90e57fa

Merge branch 'master' into feat/fcm_options

481c60e

hiranya911 suggested changes Jun 3, 2019

View reviewed changes

Comment thread FirebaseAdmin/FirebaseAdmin/Messaging/WebpushFcmOptions.cs Outdated

Odonno added 2 commits June 3, 2019 23:32

fix: correctly check error on link

3650f93

Merge branch 'feat/fcm_options' of https://github.com/Odonno/firebase…

6cfdad8

…-admin-dotnet into feat/fcm_options

hiranya911 approved these changes Jun 3, 2019

View reviewed changes

hiranya911 merged commit b54e7ea into firebase:master Jun 3, 2019

Odonno deleted the feat/fcm_options branch June 4, 2019 06:52

+                      /// Gets or sets the Webpush options that will be included in the message.
+                      /// </summary>
+                      [JsonProperty("fcm_options")]
+                      public WebpushFcmOptions Options { get; set; }

		@@ -0,0 +1,32 @@
		// Copyright 2018, Google Inc. All rights reserved.

+                      [JsonProperty("link")]
+                      public string Link { get; set; }
+                  }
+              }

                
                    No newline at end of file

+                      /// </summary>
+                      internal WebpushFcmOptions CopyAndValidate()
+                      {
+                          if (this.Link != null && !this.Link.StartsWith("https"))

feat: add fcm_options property to webpush config by Odonno · Pull Request #64 · firebase/firebase-admin-dotnet · GitHub

Uh oh!

feat: add fcm_options property to webpush config - #64

feat: add fcm_options property to webpush config#64hiranya911 merged 9 commits intofirebase:masterfirebase/firebase-admin-dotnet:masterfrom Odonno:feat/fcm_optionsCopy head branch name to clipboard

Conversation

Odonno commented Jun 3, 2019

Uh oh!

googlebot commented Jun 3, 2019

What to do if you already signed the CLA

Individual signers

Corporate signers

Uh oh!

Odonno commented Jun 3, 2019

Uh oh!

googlebot commented Jun 3, 2019

Uh oh!

hiranya911 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hiranya911 Jun 3, 2019

Choose a reason for hiding this comment

Uh oh!

hiranya911 Jun 3, 2019

Choose a reason for hiding this comment

Uh oh!

hiranya911 left a comment

Choose a reason for hiding this comment

Uh oh!

hiranya911 Jun 3, 2019

Choose a reason for hiding this comment

Uh oh!

hiranya911 Jun 3, 2019

Choose a reason for hiding this comment

Uh oh!

hiranya911 Jun 3, 2019

Choose a reason for hiding this comment

Uh oh!

Odonno commented Jun 3, 2019

Uh oh!

hiranya911 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hiranya911 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

feat: add fcm_options property to webpush config#64
hiranya911 merged 9 commits into
firebase:masterfrom
Odonno:feat/fcm_options

hiranya911 left a comment •
edited
Loading