When using rails 3.2, the generator adds 'attr_accessible' to the model.... #2522
Conversation
| end | ||
|
|
||
| def needs_attr_accessible? | ||
| Rails::VERSION::MAJOR == 3 |
There was a problem hiding this comment.
Maybe we should simply check if ActionController::StrongParameters is defined or not?
|
Thanks @jcoyne! I have added one small comment and your pull request fails the test suite, could you please take a look? Thanks! |
|
@josevalim I've fixed the build and now I'm checking for |
| end | ||
|
|
||
| def needs_attr_accessible? | ||
| !ActionController.const_defined? :StrongParameters |
There was a problem hiding this comment.
I think you can use defined?(ActionController::StrongParameters) here
There was a problem hiding this comment.
Does it make a difference?
|
As I stated in the original issue, you should include the attr_accessible call if they're on Rails 3.2 and don't have strong_parameters included OR if they're on Rails 4 and have the protected_attributes gem included. Checking for StrongParameters will yield a false positive in Rails 4 if using the protected_attributes gem.
We should do the most correct thing that we possibly can and in this case, we have enough information to make a good decision about including this line. |
|
Okay, I didn't exactly understand all the possible permutations. I though @nashby was just arguing the merits of |
|
@latortuga I'm still not sure I have all the permutations straight. Something like this? def needs_attr_accessible?
case Rails::VERSION::MAJOR
when 3
!defined?(ActionController::StrongParameters)
when 4
defined?(ActiveModel::MassAssignmentSecurity)
end
end |
|
@jcoyne I like that implementation 👍. I would like some others' thoughts on perhaps checking the Rails config option |
|
There was an error in the build ("Could not find devise-3.0.0.rc in any of the sources") can someone with permissions restart it? |
When using rails 3.2, the generator adds 'attr_accessible' to the model....
... Fixes #2515