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.

deprecate attr_accessible, attr_protected

#2705

This is a follow-up to #2704 allow calltime list of allowed attributes for mass assignment, 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 allow calltime list of allowed attributes for mass assignment) 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.

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

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

Activity

  1. Michael Koziarski
    Michael Koziarski
    • 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.

    May 24th, 2009 @ 07:18 AM

  2. cainlevy
    cainlevy

    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.

    May 24th, 2009 @ 08:25 AM