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

v3.0.0 #714

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

v3.0.0 #714

wants to merge 29 commits into from

Conversation

romanblanco
Copy link
Member

@romanblanco romanblanco commented Dec 16, 2023

Problem

Draft for a PR to release changes for v3.0.0.

This concerns this parts of the OpenAPI Specification:

The changes I made are compatible with:

  • OAS3
  • OAS3.1

Related Issues

Checklist

  • Added tests
  • Changelog updated
  • Added documentation to README.md
  • Added example of using the enhancement into test-app

romanblanco and others added 22 commits November 11, 2023 21:48
Co-authored-by: Matt Polito <matt.polito@hashrocket.com>
`#send` is dangerous to call on objects passed into your library.

Instead, this will take a page from normal RSpec request specs and allow
developers to specify a "request_params" hash and a "request_headers" hash in let blocks
that this lib can then access.

I had to use request_ because example#headers already exists in request
specs as a shorthand for integration_session.headers.

I think this also cleans up some of the logic nicely.

I moved extracting the params & headers into initializer to avoid
duplicating effort.

This will obviously break peoples' pre-existing specs, and needs to be
part of a major version change.
fix README for new request_* pattern
@romanblanco romanblanco added this to the Gem 3.0.0 milestone Dec 16, 2023
@romanblanco
Copy link
Member Author

@jtannas @mattpolito @BookOfGreg, I've tried to test the changes for v3.0.0 and they seem to work.

@romanblanco romanblanco marked this pull request as ready for review December 16, 2023 22:12
@romanblanco romanblanco linked an issue Dec 23, 2023 that may be closed by this pull request
3 tasks
@philsturgeon
Copy link

I'll be happy to test this if you can shout when alpha version is tagged.

# OAS 3: https://swagger.io/docs/specification/serialization/
if swagger_doc && doc_version(swagger_doc).start_with?('3') && param[:schema]
# NOTE: https://swagger.io/docs/specification/serialization/
if param[:schema]
Copy link

Choose a reason for hiding this comment

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

I can't add a comment down there, but shouldn't the else case be removed? Since it relies on type and collectionFormat which just do not exist anymore in the new OpenAPI version.

Copy link

Choose a reason for hiding this comment

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

I also saw that RSwag explicitly supports using type: ... instead of the "correct" versionschema: { type: ... }. (in this commit).

This actually leads to some confusing behaviour because in the current RSwag version: the only place where using type: does not work is when trying to implement a specific serialization strategy (because then there is no schema key of course).

So I would move the

type = type = param[:type] || param.dig(:schema, :type)

to the top of the file instead, remove this condition altogether and then use that type for serialization.

@@ -4,11 +4,11 @@
# This is used by the Swagger middleware to serve requests for API descriptions
# NOTE: If you're using rswag-specs to generate Swagger, you'll need to ensure
# that it's configured to generate files in the same folder
c.openapi_root = Rails.root.to_s + '/swagger'
c.openapi_root = Rails.root.to_s + '/openapi'

Choose a reason for hiding this comment

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

Rails.root is a Pathname, so it's better to do this:

c.openapi_root = Rails.root.join('openapi').to_s

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.

[BUG] DEPRECATION WARNING: Rswag::Ui
7 participants