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

do not use mass assignment anymore for active record adapter #171

Merged
merged 1 commit into from
Oct 11, 2016

Conversation

AlexWheeler
Copy link
Collaborator

fixes #161

  • rails 3 does not allow mass assignment. This makes sure the active record adapter does not mass assign attributes

Copy link
Collaborator

@jnunemaker jnunemaker left a comment

Choose a reason for hiding this comment

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

Looks great! Left one small note, but if you don't have time I can fix it later. Feel free to merge away.

# race condition, but add is only used by enable/disable which happen
# super rarely, so it shouldn't matter in practice
@feature_class.where(attributes).first || @feature_class.create!(attributes)
unless @feature_class.find_by_key(feature.key)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think dynamic finders are deprecated find_by_key which is why I went with the less aesthetically pleasing where(...).first. Would you mind swapping this back to the where so it is more future proof?

@AlexWheeler AlexWheeler force-pushed the activerecord-adapter-dont-mass-assign branch from 8010637 to ead8b6b Compare October 10, 2016 22:35
* breaks rails 3 due to mass assignment violations
@AlexWheeler AlexWheeler merged commit d36b0d3 into master Oct 11, 2016
@AlexWheeler AlexWheeler deleted the activerecord-adapter-dont-mass-assign branch October 11, 2016 00:57
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.

Can't mass-assign protected attributes: key
2 participants