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

Expose curlify #5387

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Expose curlify #5387

wants to merge 3 commits into from

Conversation

ATeal
Copy link

@ATeal ATeal commented Jun 4, 2019

Adding the exposing of curlify

Description

By adding a base plugin, we can expose curlify. Additionally I added a wrapper around the swagger js client buildRequest function to making calling it a little saner.

Motivation and Context

In our project we're looking to include curl based examples, but not necessarily within the context of swagger-ui, and we typically have left the "Try it out" feature disabled. Rather than reinventing the wheel of writing a swagger -> curl converter, it feels like it could be exposed here.

Additionally it looks like asCurl used to be a function of the swagger-js library but was removed / moved to swagger-ui without a way of calling it besides in the UI itself. See this issue.

How Has This Been Tested?

Within npm run dev:

jsonified = window.ui.spec().toJS().json
{swagger: "2.0", info: {…}, host: "petstore.swagger.io", basePath: "/v2", tags: Array(3), …}
options = {
    spec: jsonified,
    operationId: "getPetById",
    parameters: {
        petId: "1"
    }
}
{spec: {…}, operationId: "getPetById", parameters: {…}}
window.ui.fn.buildCurl(options)
"curl -X GET "https://petstore.swagger.io/v2/pet/1""

If unit testing is required around this, I'll be happy to write some, but want to check opinions before I go further

Checklist

My PR contains...

  • No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fixes (non-breaking change which fixes an issue)
  • Improvements (misc. changes to existing features)
  • Features (non-breaking change which adds functionality)

My changes...

  • are breaking changes to a public API (config options, System API, major UI change, etc).
  • are breaking changes to a private API (Redux, component props, utility functions, etc.).
  • are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
  • are not breaking changes.

Documentation

  • My changes do not require a change to the project documentation.
  • My changes require a change to the project documentation.
  • If yes to above: I have updated the documentation accordingly.

Automated tests

  • My changes can not or do not need to be tested.
  • My changes can and should be tested by unit and/or integration tests.
  • If yes to above: I have added tests to cover my changes.
  • If yes to above: I have taken care to cover edge cases in my tests.
  • All new and existing tests passed.

@ATeal
Copy link
Author

ATeal commented Jun 6, 2019

@shockey Don't want to be a bother, but the project I'm on would be interested in using the above. Do you have any idea how long it may take to get a change in?

@tim-lai
Copy link
Contributor

tim-lai commented Jun 11, 2020

@ATeal Thanks for the PR! Working on catching up on long outstanding PRs... As a feature, this is something we think is a good feature to have. For the PR specifically, you should be aware of recent work around curlify, such as #6036, #5999. Building the request is non-trivial, so I think there should be some sanity-checks on the input argument. Tests will definitely help in this area, and I suggest leveraging existing tests for their sample api definitions.

@tim-lai tim-lai self-assigned this Jun 13, 2020
@ATeal
Copy link
Author

ATeal commented Jun 17, 2020

Phew thats been a while back I'll add that to my list of things to do, but it'll probably be a minute!

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.

None yet

2 participants