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

Show file upload for schema binary format #7325

Merged
merged 5 commits into from Jun 22, 2021

Conversation

neiser
Copy link
Contributor

@neiser neiser commented May 28, 2021

When the spec has format: binary as the schema for a request body, the UI should show a file upload as it already does for some specific content types. This enables support for basically any type of content, see also discussion in #5636.

Description

  • Checks for "format: binary" and then takes that if in request-body.jsx
  • Also fixes a little display bug actually showing the selected content type

Motivation and Context

Fixes #5636

How Has This Been Tested?

Tested locally with spec from #5636 (comment)

Screenshots (if appropriate):

File upload for application/zip:
image

Correct content type display (instead of hardcoded value):
image

Checklist

My PR contains...

  • Bug fixes (non-breaking change which fixes an issue)
  • Features (non-breaking change which adds functionality)

My changes...

  • are not breaking changes.

Documentation

  • My changes do not require a change to the project documentation.

Automated tests

  • My changes can and should be tested by unit and/or integration tests.

Can you maybe add a little test case? I'm not really familiar with the code base. Feel free to add commits to that PR!

@tim-lai
Copy link
Contributor

tim-lai commented Jun 9, 2021

@neiser Thanks for the PR! I think this solution is acceptable. I think we should also include a check for format: base64 as well. In addition, this PR should include tests, where Cypress is probably the easiest to implement.

@neiser
Copy link
Contributor Author

neiser commented Jun 10, 2021

@tim-lai Thanks for your response. If we want to support format: base64 as well, shouldn't Swagger UI also convert the payload which is uploaded accordingly? Or is this already happening in some other part of the code base?

I'm quite unexperienced in Swagger UI code base, but I'll see if I can manage to also write tests for it. I can't promise any timeline, so feel free to add it yourself if you find time.

@tim-lai
Copy link
Contributor

tim-lai commented Jun 15, 2021

@neiser I believe SwaggerUI generally expects/handles files as format: base64. I think this was due to original implementation support of application/octet-stream. So adding format:base64 makes it an explicit check that SwaggerUI should already support. 🤞 😄

@neiser neiser force-pushed the show-file-upload-for-binary-format branch from 66184ee to ffad1fa Compare June 16, 2021 15:32
neiser added a commit to neiser/swagger-ui that referenced this pull request Jun 16, 2021
According to
swagger-api#7325 (comment)
the uploaded file should be converted to base64 automatically.
@neiser
Copy link
Contributor Author

neiser commented Jun 16, 2021

@tim-lai I've added format: base64 support. I can't promise if I'll find time to write a cypress test case.

@neiser
Copy link
Contributor Author

neiser commented Jun 18, 2021

@tim-lai I've just added a cypress test verifying an upload button is displayed for the various options (either certain content types or schema formats). I didn't quite get your comment on base64 handling and I've also not a good clue how to test this properly (again, I'm quite unexperienced with e2e frontend testing).

Feel free to "edit" this PR and make corrections before merging. From my point of view, it should be done now.

@neiser neiser force-pushed the show-file-upload-for-binary-format branch from 05612eb to e1aa12b Compare June 18, 2021 09:44
@tim-lai tim-lai merged commit 13c110a into swagger-api:master Jun 22, 2021
@tim-lai
Copy link
Contributor

tim-lai commented Jun 22, 2021

@neiser The Cypress tests captured what I was looking for... that the file upload button is displayed. Thanks for feature, test, and follow-up. PR is merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Request bodies with content types such as application/zip do not have a file upload input
2 participants