I'm still struggling a bit for clear reasons why it might be better to filter the attributes controller-side or model-side. The only functional advantage I can think of either way is that if you want a system with overridable defaults, you want to build the filtering into the model. For example, if you had a baseline attr_accessible declaration and overrode it at calltime with a different set. But eh, combining declarations like that sounds messy and prone to failure.
Or perhaps some security plugin for people who want to start with fewer (or no) assignable fields could create default blacklists for any /is_.*/ and /.*_id/ fields, to be overridden as needed. Hmm, that actually sounds interesting. Perhaps I'll add that optional module into my plugin. :-) Other than that, is the difference entirely aesthetic? I think the "assign(params[:user], ...)" syntax reads better than "attributes = params[:user].slice(...)", but that alone isn't enough reason to change an API. On May 26, 3:42 am, Hongli Lai <[email protected]> wrote: > On May 24, 8:25 am, Michael Koziarski <[email protected]> wrote: > > > > > cainlevy wrote: > > > Which points to an interesting question -- should the model or the > > > controller be responsible for filtering the attributes? That is, > > > should the burden be on the model to only assign allowed parameters, > > > or the controller to only pass allowed parameters? It certainly seems > > > simple to do it from the controller using something like your > > > Hash#pick method, but I think it's safer to do it from the model. For > > > example, if the model is responsible for filtering assignable > > > attributes, it may create an intelligent default blacklist for cases > > > where the developer has paid no attention. > > > > I've just about finished a patch to implement AR::Base#assign > > > (attributes, allowed_attributes). In the process I've realized that > > > allowed_attributes can simply be an override to attr_accessible/ > > > attr_protected, which makes for an easily backwards compatible API > > > update. So that'll be my first ticket. > > > 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) > > > or > > > @user.attributes = params[:user].except(:admin) > > > > I'd really prefer to remove attr_accessible/attr_protected altogether > > > as I believe they are in all ways inferior to the new approach and > > > would only serve to clutter the API in the name of backwards > > > compatibility. But that's a secondary concern, and will be in a second > > > ticket that may be evaluated independently. > > > attr_accessible and friends are a great simple solution for a really > > common case. We shouldn't lose sight of that just because there are > > some cases where they're not perfect. > > I agree. Lance raises very valid points, but what would what's wrong > with using Hash#slice and Hash#except to solve these problems? > Especially if the documentation for attr_accessible/attr_protected can > be updated to refer to the usage of Hash#slice and Hash#except for > more complex use cases? --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
