zendesk / arturo

Feature Sliders for Rails
http://github.com/zendesk/arturo
Other
206 stars 24 forks source link

Rails 4 compatibility issue ActiveModel::MassAssignmentSecurity::Error: Can't mass-assign protected attributes for Arturo::Feature: deployment_percentage #83

Open bstrech opened 10 years ago

bstrech commented 10 years ago

Creating a new feature doesn't work.

feature = Arturo::Feature.create(:symbol=>:new_one, :deployment_percentage=>0) ActiveModel::MassAssignmentSecurity::Error: Can't mass-assign protected attributes for Arturo::Feature: deployment_percentage, symbol

In feature.rb attr_accessible :symbol, :deployment_percentage if ActiveRecord::VERSION::MAJOR < 4

We don't have attr_accessible in rails 4 :(

jamesarosen commented 10 years ago

We'll need to use params.require and .permit in the controller. It'll add some annoying complexity, but it shouldn't be too hard.

bstrech commented 10 years ago

We often create our features via a migration or seed so that we have them across all of our environments rather than create them via the controller. We only use the controller to change the deployment percentage.

I forgot to mention that we are trying to go with config.active_record.whitelist_attributes = true

jamesarosen commented 10 years ago

I didn't realize Rails 4 had any support for model-level protection. I thought it had totally moved to the controller in the form of StrongParams. I guess we could try to figure out a better condition than if ActiveRecord::VERSION::MAJOR < 4. Perhaps we can detect whitelist_attributes.

jamesarosen commented 10 years ago

Are you including the protected_attributes gem? I can't find anywhere in Rails 4 proper that actually does anything with whitelist_attributes other than just emit warnings.

bstrech commented 10 years ago

Yes I am using the protected_attributes gem

jamesarosen commented 10 years ago

Unfortunately, Rails 4.0 still defines attr_accessible:

def attr_accessible(*args)
  raise "`attr_accessible` is extracted out of Rails into a gem. " \
    "Please use new recommended protection model for params" \
    "(strong_parameters) or add `protected_attributes` to your " \
    "Gemfile to use old one."
end

That means we can't use introspection like

attr_accessible :symbol, :deployment_percentage if respond_to?(:attr_accessible)

The only thing I can think of would be

begin
  attr_accessible :symbol, :deployment_percentage
rescue
  # Rails 4.0 doesn't support attr_accessible, but does define it
end

or the equally gross

if ActiveRecord::VERSION::MAJOR < 4 || Object.const_defined?(:ProtectedAttributes)
  attr_accessible :symbol, :deployment_percentage
end
bstrech commented 10 years ago

Yes, both of those solutions are less than ideal, but turns out I don't need it for now.

After removing the usage of the protected_attributes gem, my project is now ok. I never made the conversion, I am going to just use strong attributes. I am ok, but imagine someone out there is going to upgrade to 4.0 and need to keep protected_attributes.