Navigation Menu

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

fix: enable swagger generator to add referenced schema for an array of hashes param #689

Merged

Conversation

ftsanjuan
Copy link
Contributor

@ftsanjuan ftsanjuan commented Jul 21, 2020

Context
Currently, the swagger generator produces a schema that uses {type: "string"} for any param that is specified as an Array, even when the param is actually an Array of Hashes.

Example:
Currently, the following annotation for a param...

param :products, Array, of: Hash, required: true, desc: "Products list" do
  param :id, String, desc: "product id", required: true
  param :name, String, desc: "product name", required: true
end

would get rendered in a schema as:

{
  "name": "products",
  "type": "array",
  "items": {
    "type": "string"
  },
  "in": "formData",
  "required": true,
  "description": "Products list"
}

(note that the nested parameters for the Array are completely missing in the rendered schema)

Solution
The following fix looks into the param_desc to verify whether the param is a Hash, and if so, attempts to generate a referenced schema for its parameters.

The name of the reference is derived in a similar fashion to the naming for an operation id using the swagger_op_id_for_path as seen here, but the name is suffixed with _param_[name of the param]

@dmitry-solomadin
Copy link

Hey @iNecas any chance someone can review that, we are using a fork with @ftsanjuan's changes

@sebfie
Copy link

sebfie commented Apr 20, 2021

I also need it, can someone review it? He worked for this project, I think he deserves a review.

@byjus-taha-abbas
Copy link

byjus-taha-abbas commented Mar 30, 2022

@mathieujobin Please review and merge this one, thanks!

@mathieujobin
Copy link
Collaborator

I don't know... There are multiple PRs that attempt at fixing the swagger output.

Gotta be careful i think

And this PR had conflicts.

Needs to be rebased

@mathieujobin mathieujobin added the Next up consider for upcoming release or close label May 20, 2022
@mathieujobin mathieujobin merged commit be4b0fd into Apipie:master May 20, 2022
@ftsanjuan
Copy link
Contributor Author

Thank you @mathieujobin 🙏🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Next up consider for upcoming release or close
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants