Skip to content

Corrected the related parameter lookup on request params. #822

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

Merged
merged 2 commits into from
Mar 16, 2021

Conversation

Jack12816
Copy link
Contributor

- What is it good for

There is a nasty bug while using grape-swagger on request parameters that share the same base name in any way (eg. ids and user_ids) which results in a missing parameter and a badly named on (eg. ids is missing, while user_ids[][] is renamed to). An example Grape API class looks like this:

class Search < Grape::API
  params do
    with(allow_blank: false) do
      optional :ids, type: [String]
      optional :user_ids, type: [String]
    end
  end
  # [..]
end          

- What I did

I just changed the related parameter matching to use start_with? instead of include? to be sure that the related parameter is not accidentally matched while respecting actual matches. (eg. user_id and user_id[id])

- A picture of a cute animal (not mandatory but encouraged)

download

@LeFnord
Copy link
Member

LeFnord commented Mar 15, 2021

thank @Jack12816 … did it need an upgrade entry?

@LeFnord
Copy link
Member

LeFnord commented Mar 15, 2021

ah … one more thing, please add an CHANGELOG.md entry

@Jack12816
Copy link
Contributor Author

Jack12816 commented Mar 16, 2021

did it need an upgrade entry?

I don't think so, the expected behavior remains and this pr just fixes an edge case, but if you insist I'll can add an upgrade note :)

one more thing, please add an CHANGELOG.md entry

done

@LeFnord
Copy link
Member

LeFnord commented Mar 16, 2021

will update the branch, then doing the mörge

@LeFnord
Copy link
Member

LeFnord commented Mar 16, 2021

thanks :)

@LeFnord LeFnord merged commit ed07baf into ruby-grape:master Mar 16, 2021
@Jack12816
Copy link
Contributor Author

thank you, too! :) - what do you think about a release?

@Jack12816
Copy link
Contributor Author

@LeFnord ping 🏓

aka-momo pushed a commit to aka-momo/grape-swagger that referenced this pull request Feb 8, 2023
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.

2 participants