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

feat: add callback accessors into operation class #515

Merged
merged 5 commits into from
Oct 26, 2021

Conversation

julshotal
Copy link
Contributor

@julshotal julshotal commented Oct 21, 2021

🧰 Changes

Adds support for RM-2271

The nested loops and array concatenation in this are super gross - they exist mostly due to callbacks being able to have multiple expressions and each expression being aPath Item Object which supports multiple methods and needing to concat these returned arrays to prevent nested arrays (we want just one array of multiple objects)

The operation class now has the following support for callbacks:

  • hasCallbacks() returns a boolean based on whether or not the schema has any callbacks
  • getCallback(identifier, expression, method) returns an operation created from a specific callback (found via the identifier, expression, and method)
  • getCallbacks() returns an array of operations created via getCallback() for all existing callbacks
  • getCallbackExamples() returns an object of the identifier, expression, method, and example (which and array of objects containing status and mediaTypes from getRequestExamples()) An array is used here instead of just an object in case the response for the example contains multiple status codes

🧬 QA & Testing

see tests*
callbacks.json is just the /callbacks route of kitchen-sink right now with an example added + callbacks with multiple expressions and methods for testing

*most of the testing for get-callback-examples is covered by the tests for get-response-examples due to the use of getResponseExamples(), but a few tests were duplicated for callbacks just to double check all info is coming back as intended!

@julshotal julshotal marked this pull request as ready for review October 25, 2021 21:07
@julshotal julshotal requested review from erunion and Dashron October 25, 2021 21:07
@Dashron
Copy link
Contributor

Dashron commented Oct 26, 2021

Tests look fine, what is getCallbackExamples used for? I don't remember a UI element that iterates over the callback examples but I could be misremembering.

@julshotal
Copy link
Contributor Author

julshotal commented Oct 26, 2021

Tests look fine, what is getCallbackExamples used for? I don't remember a UI element that iterates over the callback examples but I could be misremembering.

The callback examples are going to be used to show a callback tab in the response component - this is the ticket that references it

@julshotal julshotal added the enhancement New feature or request label Oct 26, 2021
src/operation.js Outdated

const callback = this.schema.callbacks[identifier] ? this.schema.callbacks[identifier][expression] : false;
if (!callback || !callback[method]) return false;
return new Operation(this.oas, expression, method, callback[method]);
Copy link
Member

Choose a reason for hiding this comment

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

Want to make this return a Callback instance? You can copy these couple lines from my webhook PR:

Should also update your tests to assert that this returns instances of that too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to disable an eslint rule but it should use a fresh Callback class now 👌

Copy link
Member

Choose a reason for hiding this comment

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

yeah it's ok to disable that multiple-class-per-file rule because it's not worth the trouble of having a 2 line file just to export a child class

@Dashron
Copy link
Contributor

Dashron commented Oct 26, 2021

That link cleared up callback examples for me. Thanks! 👍 after Jon's comment is addressed!

@julshotal julshotal merged commit c1759bd into main Oct 26, 2021
@julshotal julshotal deleted the feat/operation-callback-accessors branch October 26, 2021 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants