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

Add exclude? method to ActionController::Parameters #45887

Merged

Conversation

ianneub
Copy link
Contributor

@ianneub ianneub commented Aug 25, 2022

Summary

This pull request adds the exclude? method in ActionController::Parameters. This method is delegated to the hash that backs the parameter data. It is the inverse of include?.

Other Information

Adding this method keeps things a little more consistent with a Hash.

@rails-bot rails-bot bot added the actionpack label Aug 25, 2022
@p8
Copy link
Member

p8 commented Aug 25, 2022

Hi @ianneub, thanks for contributing to Rails!

There are more methods from Hash that ActionController::Parameters doesn't support.
Could you provide a use-case for this method that would justify adding the method?

@ianneub
Copy link
Contributor Author

ianneub commented Aug 25, 2022

Could you provide a use-case for this method that would justify adding the method?

Of course. I have a controller that receives an array as a parameter. It loops through this array and will skip an entry if an id was not provided. Currently I use a negated include? to check this, but recently noticed Rubocop suggesting I change that to .exclude?. Rubocop doesn't know that is an instance of ActionController::Parameters.

I could change this to use an empty id parameter, but in this case that would break the API. Not the end of the world. Ultimately this is just a nicety.

Thanks!

@p8 p8 added the ready PRs ready to merge label Aug 30, 2022
@rafaelfranca rafaelfranca merged commit 37492e2 into rails:main Sep 9, 2022
@ianneub ianneub deleted the add-exclude-to-action_controller-parameters branch September 10, 2022 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
actionpack ready PRs ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants