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

Misleading / Incorrect documentation for webhook validation #1021

Closed
maxiimillian opened this issue May 16, 2024 · 5 comments · Fixed by #1022
Closed

Misleading / Incorrect documentation for webhook validation #1021

maxiimillian opened this issue May 16, 2024 · 5 comments · Fixed by #1022

Comments

@maxiimillian
Copy link

Issue Summary

The comments above the webhook middleware function say that the optional validate property will default to true, however this is only the case when no options are passed in,

Steps to Reproduce

  1. Create a webhook using the additional options parameter, but don't include the validate property.
    Make sure that the options will make the request invalid.
  2. Send a webhook request

Existing Code

if (!options) { options = { validate: true, }; }

I think the documentation should state this clearly or should actually set the validate to true if its not being set

@tiwarishubham635
Copy link
Contributor

HI @maxiimillian! I am able to reproduce this. Will create a PR for this

@maxiimillian
Copy link
Author

Thanks for having a look, just a heads up I believe this will set validate to true even when its been set in the options, since if validate is false, if (!options.validate) will be true.

Maybe a !('validate' in options) or (options.validate === undefined) would work better.

@tiwarishubham635
Copy link
Contributor

Yeah that makes sense, I'll make the change

@tiwarishubham635
Copy link
Contributor

@maxiimillian please check the PR now

@maxiimillian
Copy link
Author

Looks good

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 a pull request may close this issue.

2 participants