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

Define x-webhooks in API definitions #156

Merged
merged 35 commits into from
Aug 26, 2020
Merged

Define x-webhooks in API definitions #156

merged 35 commits into from
Aug 26, 2020

Conversation

mathewsonke
Copy link
Contributor

  • Adds webhook definitions for each eventType
  • Updates the GlobalWebhookEventTypes list
  • Adds Embeds and Links for some resources
  • Defines request bodies for webhooks
  • Sorts the EventType list

Our webhooks can use any HTTP verb and they will provide the same information. The webhooks spec however does not appear to allow us to define webhooks which are verb-agnostic. For that reason I have defined all the webhooks as post.

@mathewsonke mathewsonke requested a review from a team July 30, 2020 13:56
@mathewsonke mathewsonke self-assigned this Jul 30, 2020
@mathewsonke mathewsonke requested review from a team July 30, 2020 13:56
@mathewsonke
Copy link
Contributor Author

The preview build check was (and is) failing, but for some reason it is not being counted as a check anymore, and I can't trigger a preview build manually either. I'm still investigating.

openapi/storefront.yaml Outdated Show resolved Hide resolved
requestBody:
$ref: ../components/schema/requestBodies/Webhooks/Customer.yaml
responses:
'200':
Copy link
Member

Choose a reason for hiding this comment

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

We accept any 200-series response. I think you can use 2XX with uppercase X:

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.3.md#patterned-fields-1

Copy link
Member

Choose a reason for hiding this comment

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

Same for all of the other webhook responses... (I won't leave the same comment everywhere)

@mathewsonke
Copy link
Contributor Author

I believe the build failure is unavoidable until this PR in Redoc is merged: Redocly/redoc#1304

Right now, openapi-cli won't properly bundle the external files referenced from x-webhooks which results in a 404 when trying to preview the docs.

@adamaltman to avoid build problems, should I hold off merging this PR until x-webhooks support is deployed by Redoc?

@adamaltman
Copy link
Member

That shouldn’t be tied to build failure.

@mathewsonke
Copy link
Contributor Author

Update- I reached out to Roman a couple days ago to help me investigate the build issues and the bug Adam reported, but he's got a lot of work right now and hasn't had a chance to get to it yet.

@adamaltman
Copy link
Member

@mathewsonke the issue is openapi-cli doesn't bundle the specification extensions. I opened this bug: Redocly/redocly-cli#174 (feel free to contribute there if you want to fix it ASAP... if not, it's okay. The solution may be in adjusting here (somewhere): https://github.com/Redocly/openapi-cli/blob/master/src/typings/openapi.ts

@mathewsonke mathewsonke requested review from a team August 21, 2020 15:36
@adamaltman adamaltman merged commit 16f9c13 into master Aug 26, 2020
@adamaltman adamaltman deleted the define-x-webhooks branch August 26, 2020 13:45
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.

2 participants