ckdake / setler

Setler lets you use the 'Feature Flags' pattern or add settings to models in Rails
MIT License
84 stars 21 forks source link

Mass-Assignment Error #22

Closed virtualstaticvoid closed 11 years ago

virtualstaticvoid commented 11 years ago

In Rails > 3.0 assigning settings results in a mass-assignment error.

alindeman commented 11 years ago

Given that attr_accessible is going away in Rails 4, we might want to do

attr_accessible :value if respond_to?(:attr_accessible)

Thoughts?

virtualstaticvoid commented 11 years ago

Good point. I've revised the pull request accordingly. Also, the var attribute needs to be specified for the tests to pass.

ckdake commented 11 years ago

Thanks Andy! for the Rails 4 feedback.

Something about this still feels strange to me, how are you using Setler when setting things? Maybe this is a documentation fix instead of a code change? e.g. in apps where I use Setler, I only change settings individually.

virtualstaticvoid commented 11 years ago

That's just it, I am assigning using the method described in the README.rdoc.

I.e.

Settings[:feature] = true

In my Rails application, the configuration for whitelisting attributes is set to true, config.active_record.whitelist_attributes = true, which is the default for new applications, so my models need to include attr_accessible directives.

I created a sample application to illustrate the issue. See my setler_example repository. There is a rake task which is used to toggle features. You can replicate the issue by commenting out the attr_accessible directive in the Settings model and then running the app:toggle_feature_flag[feature] rake task.

E.g.

bundle exec rake app:toggle_feature_flag[chunky_bacon]
alindeman commented 11 years ago

I think in Rails 3.1 or 3.2, the default is that models are required to use whitelisting or blacklisting. So because we call #update_attributes here, we need something like attr_accessible.

ckdake commented 11 years ago

Ah right, my brain is stuck thinking that this is a Controller specific thing, and all my 3.x apps are upgrades from 3.0 or earlier ;) Merging!

ckdake commented 11 years ago

0.0.11 released including this fix. Thanks!

virtualstaticvoid commented 11 years ago

Awesome, thanks!