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.

Sign up for GitHub

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

Jump to bottom

feat: add webhooks support #1304

Merged
merged 5 commits into from
Aug 14, 2020
Merged

feat: add webhooks support #1304

merged 5 commits into from
Aug 14, 2020

Conversation

stasiukanya
Copy link
Contributor

This PR implements webhooks support.

Example of usage:

x-webhooks:
  newPet:
    post:
      summary: New pet
      description: Information about a new pet in the systems
      operationId: newPet
      tags: 
        - pet
      requestBody:
        content:
          application/json:
            schema:
              $ref: "#/components/schemas/Pet"
      responses:
        "200":
          description: Return a 200 status to indicate that the data was received successfully

Layout:

image

adamaltman
adamaltman reviewed Jun 23, 2020
View reviewed changes
src/components/SideMenu/MenuItem.tsx Outdated
@@ -90,7 +90,11 @@ export class OperationMenuItemContent extends React.Component<OperationMenuItemC
deprecated={item.deprecated}
ref={this.ref}
>
<OperationBadge type={item.httpVerb}>{shortenHTTPVerb(item.httpVerb)}</OperationBadge>
{item.isWebhook ? (
<OperationBadge type="hook">hook</OperationBadge>
Copy link
Member

Choose a reason for hiding this comment

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

I wonder why not webhook?

Copy link
Member

Choose a reason for hiding this comment

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

Check out the screenshot above.webhook will be wider than the rest badges and will look ugly.

Copy link
Member

@adamaltman adamaltman Jun 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This label/badge needs to be configurable. "hook" is a pejorative word in various vernacular (can mean to cheat or prostitution).

Copy link
Member

Choose a reason for hiding this comment

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

@adamaltman Do you have any other ideas? Maybe whook or webh? We use opts for options and del for delete.

lesyk-lesyk
lesyk-lesyk reviewed Jun 23, 2020
View reviewed changes
src/services/SpecStore.ts Outdated
@@ -15,6 +16,7 @@ export class SpecStore {
externalDocs?: OpenAPIExternalDocumentation;
contentItems: ContentItemModel[];
securitySchemes: SecuritySchemesModel;
'x-webhooks'?: WebhookModel;
Copy link

Choose a reason for hiding this comment

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

What about xWebhooks or xWebHooks?

Copy link
Member

@RomanHotsiy RomanHotsiy Jun 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This is by OpenAPI spec, all vendor extensions should be prefixed by x-.

Copy link
Member

Choose a reason for hiding this comment

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

Oops, my bad. This is our code not typing for OAS. @stasiukanya could you update it to xWebhooks? And everywhere else.

Copy link
Member

Choose a reason for hiding this comment

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

Why not webhooks then if it is our code?

Copy link
Member

Choose a reason for hiding this comment

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

@adamaltman good catch!

RomanHotsiy
RomanHotsiy reviewed Jun 23, 2020
View reviewed changes
src/services/SpecStore.ts Outdated
@@ -26,5 +28,6 @@ export class SpecStore {
this.externalDocs = this.parser.spec.externalDocs;
this.contentItems = MenuBuilder.buildStructure(this.parser, this.options);
this.securitySchemes = new SecuritySchemesModel(this.parser);
this['x-webhooks'] = new WebhookModel(this.parser, options, this.parser.spec['x-webhooks']);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
this['x-webhooks'] = new WebhookModel(this.parser, options, this.parser.spec['x-webhooks']);
this.xWebhooks = new WebhookModel(this.parser, options, this.parser.spec['x-webhooks']);

@adamaltman adamaltman mentioned this pull request Jul 24, 2020
Closed
@Herrick19 Herrick19 mentioned this pull request Jul 24, 2020
Open
@mathewsonke mathewsonke mentioned this pull request Jul 30, 2020
Merged
@RomanHotsiy RomanHotsiy merged commit 41f81b4 into master Aug 14, 2020
@RomanHotsiy
Copy link
Member

🎉 👏

@RomanHotsiy
Copy link
Member

Hey @lornajane! We've already released x-webhooks support in the latest 2.0.0-rc.37 and redoc-cli@0.9.11.

Could you take a look and let us know what you think? We still haven't announced it, wanted to get your feedback before!

@lornajane
Copy link
Contributor

I love this! The feature looks exactly what I expected (basically callbacks, but in a different context), and feels familiar. I think this is the catalyst I needed to go ahead and start updating our specs within the 3.0 versions to include this, ready for an official OpenAPI 3.1 release!

@andriyl andriyl deleted the webhooks-support branch April 8, 2021 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adamaltman adamaltman adamaltman left review comments

@lesyk-lesyk lesyk-lesyk lesyk-lesyk left review comments

@RomanHotsiy RomanHotsiy RomanHotsiy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@stasiukanya @RomanHotsiy @lornajane @adamaltman @lesyk-lesyk