This project is archived and is in readonly mode.

#2705 ✓wontfix
cainlevy

deprecate attr_accessible, attr_protected

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

This is a follow-up to #2704, which revisits mass assignment and one-ups attr_accessible and attr_protected. If that patch is accepted, then I don't see enough remaining use for these two methods. They would only apply in a situation where a developer had forgotten to specify the allowed attributes at calltime. But this developer, presumably the forgetful sort, is unlikely to have remembered to set attr_accessible or attr_protected in the first place.

Furthermore, encouraging the usage of allowed_attributes at calltime (see #2704) makes it all the more likely that attr_accessible and attr_protected declarations will be spotty and poorly maintained.

Rather than support two systems for protecting methods and deal with the resulting confusion, I'd vote we deprecate attr_accessible and attr_protected and promote the new system. This is my attempt at fighting API bloat. :-)

The only other use I can see for keeping attr_accessible and attr_protected is to ease the upgrade process for older applications. But Rails 3.0 seems like the best time to go for a clean break!

NOTE: This patch also deprecates attributes= since it doesn't easily support the allowed_attributes syntax. I could imagine keeping it as a shortcut for the default-protection mass assignment use case, but I would rather remove it for a leaner API.

Comments and changes to this ticket

  • Michael Koziarski

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

    • State changed from “new” to “wontfix”

    While there are several cases where a more complicated mass assignment permissions model is needed, the base case is incredibly useful for a huge number of users.

    Requiring everyone to specify every attribute every time they assign them sits well on the other side of the 'reasonable complexity' line.

  • cainlevy

    cainlevy May 24th, 2009 @ 08:25 AM

    True. The base case does cover a lot of ground. And until I finally did away with ActiveScaffold (the beast) and went back to a more light-weight admin interface approach (http://github.com/cainlevy/presenting), it was enough for me.

    I suspect, though, that moving the assignable attribute list from the model to the controller would be a nearly net zero change in terms of complexity, except where the complexity is useful.

    Thanks to these lovely RESTful resource controllers, my application rarely mass assigns to some model class in more than one location. And when it does, it's nearly always because it's a different permissions context. So in the former case I'd remove a line of code from my User model and add it to my UsersController ... no more or less complex. And the latter case is, well, exactly where I'd want to specify the assignable columns in the controller anyway!

    Hopefully I'm not committing the fallacy of assuming everyone's application works like mine.

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>

People watching this ticket

Attachments

Tags

Referenced by

Pages