Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

allow calltime list of allowed attributes for mass assignment

#2704

As described and discussed in http://groups.google.com/group/rubyonrails-core/browse_thread/threa....

This ticket adds an ActiveRecord::Base#assign method that mimics #attributes= but with one difference -- it accepts a list of assignable attributes. This effectively moves the attr_protected/attr_accessible paradigm into a context that is naturally aware of the current user's role and permissions.

The list of assignable attributes act as an override to any existing attr_accessible or attr_protected declarations. This keeps it backwards compatible; if the list of assignable attributes is not specified, then assign(hash) acts exactly the same as attributes=(hash).

All methods that accept mass assignment hashes are upgraded to also accept a list of allowed attributes. This only created a backwards-compatibility conflict in one spot: the replace parameter for build_association() and create_association() has been pushed into the third position.

Reported by cainlevy · May 24th, 2009 @ 04:02 AM

State: wontfix
Milestone: 2.x
Assigned to: nobody
Importance: none

Activity

  1. Michael Koziarski
    Michael Koziarski
    • State changed from new to wontfix

    I don't think this massive change to the api is justified. You're introducing complexity for all users to support a few cases which, while hardly rare, aren't 100% of user's requirements.

    It should be trivial for you to implement this as a plugin to see if people prefer this approach to specifying assignable attributes. If that picks up momentum we can look at pulling it in to rails.

    In the meantime users can already do:

    
    @user.attributes = params[:user].slice(:email, :password, :password_confirmation)
    @user.attributes = params[:user].except(:admin)
    

    May 24th, 2009 @ 07:26 AM

  2. cainlevy
    cainlevy

    Hey Koz, thanks for the reply --

    I certainly hope I'm not just creating complexity for everyone. How does this ticket do that? This change (ignoring #2705 deprecate attr_accessible, attr_protected) is nearly 100% backwards compatible, isn't it? Most of the diff deals with changing method signatures for association builders to pass through the extra optional parameter, which really isn't a fun thing to do in a plugin.

    I do think that Hash#slice and Hash#except can help, but I also think there's value in the mass assignment method of choice knowing whether the attributes have already been filtered or not. For example, I like the fallback behavior to an intelligent default attribute blacklist for the primary key and other special columns.

    Again, I'm curious how this adds needless complexity, as it's a completely optional parameter that only appears when you need it. If you think that ticket #2705 deprecate attr_accessible, attr_protected is going too far that's fine with me -- I'm much more interested in this standalone change.

    May 24th, 2009 @ 08:13 AM

  3. Michael Koziarski
    Michael Koziarski

    I certainly don't want to come accross like I'm opposed to changing the
    attribute assignment code. What I'm trying to get at is that we should
    test something drastic like this through plugins.

    This lets us see how people adopt it, bed down any issues etc. If the
    plugin gains critical mass, then that's a great sign that the
    functionality should be rolled into the core of rails itself.

    The plugin would be incredibly simple, a single method for AR::Base and
    can therefore focus on gathering API and usability feedback from users.

    May 24th, 2009 @ 09:09 AM

  4. cainlevy
    cainlevy

    Yep. It would certainly be easy to distribute the assign() method via a plugin. It'd be much harder to extend the change to the other mass assignment methods for consistency, but perhaps you're saying that's not necessary to field test the concept?

    Hmm, when can we say that a plugin has critical mass?

    May 24th, 2009 @ 09:43 AM

  5. Michael Koziarski
    Michael Koziarski

    Right, there's no reason to remove or deprecate the existing method.
    This is about experimenting with an alternative implementation.

    As for when it has critical mass, it should be 'obvious' and 'everyone'
    should be using it. These are obviously slippery definitions but in the
    past we've been well-served by it.

    Even if it remains a plugin forever, we'll have an alternative
    mass-assignment method available for users who want it.

    May 24th, 2009 @ 09:50 AM

  6. cainlevy