feat: add fcm_options property to webpush config #64
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
ℹ️ Googlers: Go here for more info. |
|
I signed it! |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
| @@ -0,0 +1,32 @@ | |||
| // Copyright 2018, Google Inc. All rights reserved. | |||
| [JsonProperty("link")] | ||
| public string Link { get; set; } | ||
| } | ||
| } |
There was a problem hiding this comment.
Thanks @Odonno. Just a couple of updates needed to the validation logic, and rename the property to FcmOptions.
| /// </summary> | ||
| internal WebpushFcmOptions CopyAndValidate() | ||
| { | ||
| if (this.Link != null && !this.Link.StartsWith("https")) |
There was a problem hiding this comment.
Create the copy first and then run validation on that.
var copy = ...;
if (copy.Link != null && !copy.Link.StartsWuth("https://")
{
}
| /// </summary> | ||
| internal WebpushFcmOptions CopyAndValidate() | ||
| { | ||
| if (this.Link != null && !this.Link.StartsWith("https")) |
There was a problem hiding this comment.
Can we also check the well-formedness of the URL by running Uri.IsWellFormedUriString(copy.Link, UriKind.Absolute)?
| /// Gets or sets the Webpush options that will be included in the message. | ||
| /// </summary> | ||
| [JsonProperty("fcm_options")] | ||
| public WebpushFcmOptions Options { get; set; } |
There was a problem hiding this comment.
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. |
No description provided.