This project is archived and is in readonly mode.

#2704 ✓wontfix
cainlevy

allow calltime list of allowed attributes for mass assignment

Reported by cainlevy | May 24th, 2009 @ 04:02 AM | in 2.x

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.

Comments and changes to this ticket

  • Michael Koziarski

    Michael Koziarski May 24th, 2009 @ 07:26 AM

    • 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)
    
  • cainlevy

    cainlevy May 24th, 2009 @ 08:13 AM

    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) 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 is going too far that's fine with me -- I'm much more interested in this standalone change.

  • Michael Koziarski

    Michael Koziarski May 24th, 2009 @ 09:09 AM

    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.

  • cainlevy

    cainlevy May 24th, 2009 @ 09:43 AM

    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?

  • Michael Koziarski

    Michael Koziarski May 24th, 2009 @ 09:50 AM

    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.

  • cainlevy

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Attachments

Referenced by

Pages