Proposition: Nested attributes and attr_accessible

41 views
Skip to first unread message

graf.otodrakula

unread,
Dec 29, 2009, 6:30:12 AM12/29/09
to Ruby on Rails: Core
Do you guys think accepts_nested_attributes should alter
attr_accessible if it's not nil?

== Pros:
* Nested attributes will work out of the box with models that have
attr_accessible defined.
* In most cases, user wants accepts_nested_attributes to work with
controllers/views, so he adds :comments_attributes to Post's
attr_accessible anyway.
* ?

== Cons:
* Modifying user's whitelist.
* ?

Eloy Duran

unread,
Dec 29, 2009, 11:54:48 AM12/29/09
to rubyonra...@googlegroups.com
Like discussed on IRC, in general I'm in favor of this change as well.
My biggest concerns are indeed about touching the user's whitelist.
And the user might use the nested attributes accessor directly,
however, I don't know anyone that does this…

Eg: parent.children_attributes = { … }

In case nobody indicates they use it this way, I'd say lets do it.

Eloy

> --
>
> 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 rubyonrails-
> co...@googlegroups.com.
> To unsubscribe from this group, send email to rubyonrails-co...@googlegroups.com
> .
> For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en
> .
>
>

José Valim

unread,
Jan 5, 2010, 9:19:20 AM1/5/10
to Ruby on Rails: Core
I think that the main reason to do so by default, is to avoid issues
like people setting nested attributes, but the attribute is protected
and then the assignment never work.

If this is the case, I would prefer to have a more whiny behavior when
a protected attribute is assigned.

Besides, if if we check that attr_protected or attr_accessible was
set, there are always the case the user puts attr_accessible after the
accept_nested_attributes_for and then those two models would have
different behavior:

class Model < ActiveRecord::Base
attr_accessible :foo
accepts_nested_attributes_for :bar
end

class OtherModel < ActiveRecord::Base
accepts_nested_attributes_for :bar
attr_accessible :foo
end

And then just the first would whilte list :bar_attributes, never the
second, and this would be even stranger.

Nicolás Sanguinetti

unread,
Jan 5, 2010, 9:26:32 AM1/5/10
to rubyonra...@googlegroups.com
On Tue, Jan 5, 2010 at 12:19 PM, José Valim <jose....@gmail.com> wrote:
> I think that the main reason to do so by default, is to avoid issues
> like people setting nested attributes, but the attribute is protected
> and then the assignment never work.
>
> If this is the case, I would prefer to have a more whiny behavior when
> a protected attribute is assigned.

Indeed, attr_accessible and attr_protected should raise and be noisy
in development/test. I'm fine with them swallowing stuff in
production, but I've been bitten several times by this.

-foca

> To post to this group, send email to rubyonra...@googlegroups.com.

Eloy Duran

unread,
Jan 5, 2010, 9:28:43 AM1/5/10
to rubyonra...@googlegroups.com
You're spot on, +1 for being more whiny and guiding the user in dev. mode.

Eloy

> To post to this group, send email to rubyonra...@googlegroups.com.

byrnejb

unread,
Jan 29, 2010, 12:01:00 PM1/29/10
to Ruby on Rails: Core

On Dec 29 2009, 6:30 am, "graf.otodrakula" <graf.otodrak...@gmail.com>
wrote:


> Do you guys think accepts_nested_attributes should alter
> attr_accessible if it's not nil?

I am working with nested attributes at the moment on Rails 2.3.5 and I
have a concern over attr_accessible. I am unable to discover a method
to protect an attribute once it has been added to the
accessible_attributes collection.

If there is, in fact, no way to remove the accessible setting from an
attribute then this seems to me a serious security flaw. One can
conceive of a complex system involving many forms and pages wherein,
over time, the effect of the above condition would be to force all
significant attributes to become accessible eventually.

Is there a method to explicitly set attributes in a nested_attribute
update? That would address the problem.

Reply all
Reply to author
Forward
0 new messages