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. * ? -- 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-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
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 On 29 dec 2009, at 12:30, graf.otodrakula wrote:> 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. > * ? > > -- > > 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- > core@googlegroups.com. > To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com > . > For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en > . > >-- 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-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
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. On Dec 29 2009, 5:54 pm, Eloy Duran <eloy.de.en...@gmail.com> wrote:> 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 > > On 29 dec 2009, at 12:30, graf.otodrakula wrote: > > > > > 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. > > * ? > > > -- > > > 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- > > core@googlegroups.com. > > To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com > > . > > For more options, visit this group athttp://groups.google.com/group/rubyonrails-core?hl=en > > .-- 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-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Nicolás Sanguinetti
2010-Jan-05 14:26 UTC
Re: Re: Proposition: Nested attributes and attr_accessible
On Tue, Jan 5, 2010 at 12:19 PM, José Valim <jose.valim@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> 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. > > On Dec 29 2009, 5:54 pm, Eloy Duran <eloy.de.en...@gmail.com> wrote: >> 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 >> >> On 29 dec 2009, at 12:30, graf.otodrakula wrote: >> >> >> >> > 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. >> > * ? >> >> > -- >> >> > 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- >> > core@googlegroups.com. >> > To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com >> > . >> > For more options, visit this group athttp://groups.google.com/group/rubyonrails-core?hl=en >> > . > > -- > > 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-core@googlegroups.com. > To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. > For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. > > >-- 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-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Eloy Duran
2010-Jan-05 14:28 UTC
Re: Re: Proposition: Nested attributes and attr_accessible
You''re spot on, +1 for being more whiny and guiding the user in dev. mode. Eloy On Jan 5, 2010, at 3:19 PM, José Valim 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. > > 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. > > On Dec 29 2009, 5:54 pm, Eloy Duran <eloy.de.en...@gmail.com> wrote: >> 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 >> >> On 29 dec 2009, at 12:30, graf.otodrakula wrote: >> >> >> >>> 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. >>> * ? >> >>> -- >> >>> 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- >>> core@googlegroups.com. >>> To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com >>> . >>> For more options, visit this group athttp://groups.google.com/group/rubyonrails-core?hl=en >>> . > > -- > > 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-core@googlegroups.com. > To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. > For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. > >-- 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-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
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. -- 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-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.