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

option to disable all scopes that `ActiveRecord.enum` generates #34605

Merged
merged 1 commit into from Dec 4, 2018

Conversation

Projects
None yet
4 participants
@alfie-max
Copy link
Contributor

commented Dec 3, 2018

Summary

Fixes #34599

By default ActiveRecord.enum generates several default scopes based on the values. This PR adds an option to disable those scopes which could possibly cause conflicts with existing methods for the model.

Other Information

@rails-bot rails-bot bot added the activerecord label Dec 3, 2018

@alfie-max

This comment has been minimized.

Copy link
Contributor Author

commented Dec 3, 2018

i just gave this a try.. @rafaelfranca kindly check this out and maybe i can work on making some changes if necessary

@SamSaffron

This comment has been minimized.

Copy link
Contributor

commented Dec 3, 2018

This looks correct to me but we should have a test I think, that way we can confirm it will not regress, params with the name _enum etc. end up being prime candidates long term for removal cause they do not feel quite right starting with _ is usually reserved for ignored vars.

@rafaelfranca rafaelfranca added this to the 6.0.0 milestone Dec 3, 2018

@alfie-max alfie-max force-pushed the alfie-max:disable_enum_scopes branch from a7de8af to 2e5e5f5 Dec 3, 2018

@alfie-max

This comment has been minimized.

Copy link
Contributor Author

commented Dec 3, 2018

@SamSaffron added a test.. Not sure if that would suffice. Actually caught a case which i missed which would be the default case while adding the test.

Show resolved Hide resolved activerecord/lib/active_record/enum.rb Outdated
Show resolved Hide resolved activerecord/lib/active_record/enum.rb Outdated
Show resolved Hide resolved activerecord/test/cases/enum_test.rb Outdated

@alfie-max alfie-max force-pushed the alfie-max:disable_enum_scopes branch from 2e5e5f5 to fa92fa1 Dec 3, 2018

@gmcgibbon
Copy link
Member

left a comment

👍 LGTM. Please squash your commits

@alfie-max

This comment has been minimized.

Copy link
Contributor Author

commented Dec 3, 2018

Sure.. just a min

@alfie-max alfie-max force-pushed the alfie-max:disable_enum_scopes branch from fa92fa1 to 3362877 Dec 3, 2018

@alfie-max

This comment has been minimized.

Copy link
Contributor Author

commented Dec 3, 2018

should i add this to the CHANGELOG?

@alfie-max alfie-max force-pushed the alfie-max:disable_enum_scopes branch from 3362877 to 3670d60 Dec 3, 2018

@rafaelfranca

This comment has been minimized.

Copy link
Member

commented Dec 3, 2018

👍 for a changelog entry

@alfie-max alfie-max force-pushed the alfie-max:disable_enum_scopes branch from 3670d60 to f8abaf6 Dec 3, 2018

@alfie-max alfie-max force-pushed the alfie-max:disable_enum_scopes branch from f8abaf6 to 6267ee9 Dec 4, 2018

@alfie-max alfie-max force-pushed the alfie-max:disable_enum_scopes branch from 6267ee9 to c4d157a Dec 4, 2018

@rafaelfranca rafaelfranca merged commit e517226 into rails:master Dec 4, 2018

2 checks passed

codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@rafaelfranca

This comment has been minimized.

Copy link
Member

commented Dec 4, 2018

Thank you so much!

@alfie-max alfie-max deleted the alfie-max:disable_enum_scopes branch Dec 4, 2018

@SamSaffron

This comment has been minimized.

Copy link
Contributor

commented Dec 4, 2018

congrats @alfie-max on getting your first Rails PR done! Thanks

@alfie-max

This comment has been minimized.

Copy link
Contributor Author

commented Dec 5, 2018

thanks @SamSaffron 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.