Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Creating an array in CreateWebHookRequest instead of a JSon object #24

Open
aspnet-hello opened this issue Mar 13, 2018 · 2 comments
Open
Milestone

Comments

@aspnet-hello
Copy link

From @divad4686 on Monday, February 13, 2017 7:31:15 AM

I'm building a Webhook provider library, and one of the requirements I have is to change the default Webhook request body. I can do this by overriding the CreateWebHookRequestBody in the WebHookSender class.

The problem is that my request body have to be wrapper in an array ( it is a requirement for Rest Hooks in Zapier) but the functions CreateWebHookRequest, CreateWebHookRequestBody and SignWebHookRequest all work with JObject class, making it impossible to wrap the body in an array.

I fixed this by overriding the 3 functions in my own class, but most of the work was copy-pasting the default implementation, replacing JObject for JContainer and using JArray.FromObject in CreateWebHookRequestBody.

I think I could do a pull request here in the project with this commit so people will have it easy later to use JArray or JObject in their own implementations. This should not have any side effect, since in the end, outside CreateWebHookRequestBody, all it is done with this object is a. .ToString()

The only problem I could see is that it would not compile the project for people that already override CreateWebHookRequestBody

Copied from original issue: aspnet/WebHooks#115

@aspnet-hello aspnet-hello added this to the Backlog milestone Mar 13, 2018
@aspnet-hello
Copy link
Author

From @hybridtechie on Thursday, August 3, 2017 10:25:29 PM

It's much easier to use Zapier scripting and wrapping the response in an array.

@aspnet-hello
Copy link
Author

From @dougbu on Tuesday, January 16, 2018 6:36:51 PM

We've moved this issue to the Backlog milestone. This means that it is not going to happen for the coming release. We will reassess the backlog following the current release and consider this item at that time. However, keep in mind that there are many other high priority features with which it will be competing for resources.

Have also marked this issue as up-for-grabs. This means we will review a PR filling this gap if someone submits one.

In this particular case, the PR should avoid breaking changes and continue to support those who have overridden the existing methods. Add new wrapper methods e.g. CreateWebHookRequestBodyContainer(...) and call the existing overloads. Then use the new wrappers in CreateWebHookRequest(...).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant