Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

alias attr_accessible to mass_assignable #7241

Closed
wants to merge 1 commit into from

5 participants

Alex Chaffee Rafael Mendonça França Santiago Pastorino Guillermo Iguaran José Valim
Alex Chaffee

attr_accessible is a very confusing name. 

First, it has nothing to do with the other attr_* macros, which deal with Ruby instance-variable-based getters and setters, not Rails attributes. 

Second, the term "access" is generally understood to refer to reading a value, viz. accessor vs. mutator. Yet this macro is about the opposite of reading; instead, it determines whether an attribute can be written, aka assigned.

Third, this is in a module called Mass Assignment, and this macro is literally specifying which values can be mass assigned. So why be so mysterious? Let's just call it mass_assignable and get on with our lives. It will make this feature more understandable to every Rails programmer who has had to figure out what that line is doing there, especially now that it's in every generated model class -- and rightly so, since it's an important and useful feature and deserves to be embraced and understood by all.

This patch is just to show the (trivial) implementation; changes to documentation and to the generators will follow if people agree it's worth doing.

Alex Chaffee alexch alias attr_accessible to mass_assignable
attr_accessible is a very confusing name. 




First, it has nothing to do with the other attr_* macros, which deal with Ruby instance-variable-based getters and setters, not Rails attributes. 




Second, the term "access" is generally understood to refer to reading a value, viz. accessor vs. mutator. Yet this macro is about the *opposite* of reading; instead, it determines whether an attribute can be *written*, aka assigned.




Third, this is in a module called Mass Assignment, and this macro is literally specifying which values can be mass assigned. So why be so mysterious? Let's just call it mass_assignable and get on with our lives. It will make this feature more understandable to every Rails programmer who has had to struggle to understand just what that line is doing there, especially now that it's in every single generated model class -- and rightly so, since it's an important and useful feature and deserves to be embraced and understood by all.




This patch is just to show the (trivial) implementation; documentation and a fix to the generators will follow.
ab6bfa5
Rafael Mendonça França
Owner

Thank you for the pull request.

We are planing to remove attr_accesible in Rails 4 and extract it to a gem, so I'll close this pull request. Feel free to open a new one to the extracted gem when it is ready.

Alex Chaffee
Rafael Mendonça França
Owner

Right.

Rafael Mendonça França rafaelfranca reopened this
Rafael Mendonça França
Owner

In summary, I'm :-1: for this change. Right now the API is very clear and attr_accessible is well documented.

A explanation more

1) It has nothing to do with the attr_* macros, but it is still related with attributes. attr seems a good prefix to something that deal with attributes.

2) You are right about this one.

3) Now that it will be removed and we don't will add it in every model by default I don't think that we should deal with this rename. We will remove attr_accessible either from the oficial documentation so new Rails developers should only know about this from legacy code. So we don't need to make it more "understandable". I think that adding an alias will make the things more confusing to newcomers that can see projects using attr_accessible and using mass_assignable. Even in the same project this can occur.

Lets wait to see what the others think.

Alex Chaffee
Rafael Mendonça França
Owner

But we don't want to make the community adapt attr_accessible or any new name for this. We are going to educate the community to use strong_parameters.

New Rails projects using 4.0.0 will not have mass assignment protection at the model level. We are going to extract to a plugin only make the upgrade path to 4.0.0 easier.

Santiago Pastorino
Owner

:-1: because of what @rafaelfranca said in the previous comment

Guillermo Iguaran

Don't add more code related to attr_accessible in Rails edge, you can see that is already removed in this branch that is pending for merge in master: https://github.com/rails/rails/tree/integrate-strong_parameters

Santiago Pastorino
Owner

I'm closing this issue for the commented reasons, thanks for helping anyway :).

Santiago Pastorino spastorino closed this
Alex Chaffee
José Valim
Owner

Since we are on the subject, @guilleiguaran can you send a pull request from your branch so we can review it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 2, 2012
  1. Alex Chaffee

    alias attr_accessible to mass_assignable

    alexch authored
    attr_accessible is a very confusing name. 
    
    
    
    
    First, it has nothing to do with the other attr_* macros, which deal with Ruby instance-variable-based getters and setters, not Rails attributes. 
    
    
    
    
    Second, the term "access" is generally understood to refer to reading a value, viz. accessor vs. mutator. Yet this macro is about the *opposite* of reading; instead, it determines whether an attribute can be *written*, aka assigned.
    
    
    
    
    Third, this is in a module called Mass Assignment, and this macro is literally specifying which values can be mass assigned. So why be so mysterious? Let's just call it mass_assignable and get on with our lives. It will make this feature more understandable to every Rails programmer who has had to struggle to understand just what that line is doing there, especially now that it's in every single generated model class -- and rightly so, since it's an important and useful feature and deserves to be embraced and understood by all.
    
    
    
    
    This patch is just to show the (trivial) implementation; documentation and a fix to the generators will follow.
This page is out of date. Refresh to see the latest.
Showing with 1 addition and 0 deletions.
  1. +1 −0  activemodel/lib/active_model/mass_assignment_security.rb
1  activemodel/lib/active_model/mass_assignment_security.rb
View
@@ -184,6 +184,7 @@ def attr_accessible(*args)
self._active_authorizer = self._accessible_attributes
end
+ alias mass_assignable attr_accessible
def protected_attributes(role = :default)
protected_attributes_configs[role]
Something went wrong with that request. Please try again.