Rails core - Sep 2008 - why revert "adding accessible option to allow for allowing mass assignments"?

I noticed that Pratik just reverted the ":accessible option to allow
for allowing mass assignments" with the following commit:
http://github.com/rails/rails/commit/9994f0d90248db7d7eae36f0b597a15e8a427612

What was the reason behind this reversion?

I''ve been using the feature and it works great for my needs.

Best,

Zack Chandler
http://depixelate.com

--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

Hey Zach,

The :accessible change was not complete and still needs some more work to be
usable. There was no conclusion about how we want updates to work -
http://groups.google.com/group/rubyonrails-core/browse_thread/thread/4049b4b313fa8be2/730dbd7ad5e7ccc0

As 2.2 is very close, I didn''t want any half baked changes to be in
stable
release. But I''d be happy to apply the patch again, and work
incremently
after we have a 2.2 branch.

On Wed, Sep 10, 2008 at 8:36 PM, Zack Chandler
<zackchandler@gmail.com>wrote:
>
> I noticed that Pratik just reverted the ":accessible option to allow
> for allowing mass assignments" with the following commit:
>
>
http://github.com/rails/rails/commit/9994f0d90248db7d7eae36f0b597a15e8a427612
>
> What was the reason behind this reversion?
>
> I''ve been using the feature and it works great for my needs.
>
> Best,
>
> Zack Chandler
> http://depixelate.com
>
> >
>

-- 
Cheers!
- Pratik
http://m.onkey.org

--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

Pratik,

Got it - I figured it was prep time for 2.2...  I for one would like
to see this reapplied and iterated on after 2.2 is tagged as it really
helps out with multi-model forms.

Best,
Zack

On Wed, Sep 10, 2008 at 12:50 PM, Pratik <pratiknaik@gmail.com>
wrote:
> Hey Zach,
>
> The :accessible change was not complete and still needs some more work to
be
> usable. There was no conclusion about how we want updates to work -
>
http://groups.google.com/group/rubyonrails-core/browse_thread/thread/4049b4b313fa8be2/730dbd7ad5e7ccc0
>
> As 2.2 is very close, I didn''t want any half baked changes to be
in stable
> release. But I''d be happy to apply the patch again, and work
incremently
> after we have a 2.2 branch.
>
> On Wed, Sep 10, 2008 at 8:36 PM, Zack Chandler
<zackchandler@gmail.com>
> wrote:
>>
>> I noticed that Pratik just reverted the ":accessible option to
allow
>> for allowing mass assignments" with the following commit:
>>
>>
http://github.com/rails/rails/commit/9994f0d90248db7d7eae36f0b597a15e8a427612
>>
>> What was the reason behind this reversion?
>>
>> I''ve been using the feature and it works great for my needs.
>>
>> Best,
>>
>> Zack Chandler
>> http://depixelate.com
>>
>>
>
>
>
> --
> Cheers!
> - Pratik
> http://m.onkey.org
>
> >
>
--~--~---------~--~----~------------~-------~--~----~
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 like this feature too, but agree that it wasn''t ready for prime time
yet.  I just put up a patch for my own take on how to do this. It''s
not the whole story, but I wanted to get it up there so we can talk
about it:

http://rails.lighthouseapp.com/projects/8994/tickets/1031

--
Josh Susser
http://blog.hasmanythrough.com

On Sep 10, 1:23 pm, "Zack Chandler" <zackchand...@gmail.com>
wrote:
> Pratik,
>
> Got it - I figured it was prep time for 2.2...  I for one would like
> to see this reapplied and iterated on after 2.2 is tagged as it really
> helps out with multi-model forms.
>
> Best,
> Zack
>
> On Wed, Sep 10, 2008 at 12:50 PM, Pratik <pratikn...@gmail.com>
wrote:
> > Hey Zach,
>
> > The :accessible change was not complete and still needs some more work
to be
> > usable. There was no conclusion about how we want updates to work -
>
>http://groups.google.com/group/rubyonrails-core/browse_thread/thread/...
>
> > As 2.2 is very close, I didn''t want any half baked changes to
be in stable
> > release. But I''d be happy to apply the patch again, and work
incremently
> > after we have a 2.2 branch.
>
> > On Wed, Sep 10, 2008 at 8:36 PM, Zack Chandler
<zackchand...@gmail.com>
> > wrote:
>
> >> I noticed that Pratik just reverted the ":accessible option
to allow
> >> for allowing mass assignments" with the following commit:
>
>
>>http://github.com/rails/rails/commit/9994f0d90248db7d7eae36f0b597a15e...
>
> >> What was the reason behind this reversion?
>
> >> I''ve been using the feature and it works great for my
needs.
>
> >> Best,
>
> >> Zack Chandler
> >>http://depixelate.com
>
> > --
> > Cheers!
> > - Pratik
> >http://m.onkey.org
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

Here''s a plugin extracted from one of our apps and then extended for  
Ryan Bates''s complex-form-examples.
We only needed it for has_many and has_one associations atm, so I  
haven''t worked/tried it on others yet.

The plugin exists of 3 parts:
- AutosaveAssociation, which as the name implies automatically saves  
associations, not only on create as is the default.
- NestedParams, which is my take on this problem. It creates/updates/ 
destroys associations. This uses AutosaveAssociation.
- NestedParamsFormBuilder, which is a subclass of FormBuilder that  
adds code to #fields_for for handling NestedParams enabled models.

Obviously this is a wip, but it might be interesting for the  
discussion as well.

http://github.com/alloy/complex-form-examples/tree/alloy-nested_params/vendor/plugins/has_autosave_and_nested_params

Cheers,
Eloy

On 12 sep 2008, at 19:04, Josh Susser wrote:
>
> I like this feature too, but agree that it wasn''t ready for prime
time
> yet.  I just put up a patch for my own take on how to do this.
It''s
> not the whole story, but I wanted to get it up there so we can talk
> about it:
>
> http://rails.lighthouseapp.com/projects/8994/tickets/1031
>
> --
> Josh Susser
> http://blog.hasmanythrough.com
>
> On Sep 10, 1:23 pm, "Zack Chandler"
<zackchand...@gmail.com> wrote:
>> Pratik,
>>
>> Got it - I figured it was prep time for 2.2...  I for one would like
>> to see this reapplied and iterated on after 2.2 is tagged as it  
>> really
>> helps out with multi-model forms.
>>
>> Best,
>> Zack
>>
>> On Wed, Sep 10, 2008 at 12:50 PM, Pratik <pratikn...@gmail.com>  
>> wrote:
>>> Hey Zach,
>>
>>> The :accessible change was not complete and still needs some more  
>>> work to be
>>> usable. There was no conclusion about how we want updates to work -
>>>
http://groups.google.com/group/rubyonrails-core/browse_thread/thread/
>>> ...
>>
>>> As 2.2 is very close, I didn''t want any half baked changes
to be
>>> in stable
>>> release. But I''d be happy to apply the patch again, and
work
>>> incremently
>>> after we have a 2.2 branch.
>>
>>> On Wed, Sep 10, 2008 at 8:36 PM, Zack Chandler  
>>> <zackchand...@gmail.com>
>>> wrote:
>>
>>>> I noticed that Pratik just reverted the ":accessible
option to
>>>> allow
>>>> for allowing mass assignments" with the following commit:
>>
>>>>
http://github.com/rails/rails/commit/9994f0d90248db7d7eae36f0b597a15e
>>>> ...
>>
>>>> What was the reason behind this reversion?
>>
>>>> I''ve been using the feature and it works great for my
needs.
>>
>>>> Best,
>>
>>>> Zack Chandler
>>>> http://depixelate.com
>>
>>> --
>>> Cheers!
>>> - Pratik
>>> http://m.onkey.org
> >

--~--~---------~--~----~------------~-------~--~----~
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 one of the problem''s is that there are so many ways the
interface can be implemented. That is, the interface for setting the
associated attributes. I''m starting to document them here.

http://gist.github.com/10793

Josh, I haven''t added yours yet as I don''t fully understand
it. Is it
close to Approach 2? Let me know and I''ll add it.

We should weigh the pros and cons of each approach.

Regards,

Ryan


On Sep 12, 11:33 am, Eloy Duran <eloy.de.en...@gmail.com>
wrote:
> Here''s a plugin extracted from one of our apps and then extended
for  
> Ryan Bates''s complex-form-examples.
> We only needed it for has_many and has_one associations atm, so I  
> haven''t worked/tried it on others yet.
>
> The plugin exists of 3 parts:
> - AutosaveAssociation, which as the name implies automatically saves  
> associations, not only on create as is the default.
> - NestedParams, which is my take on this problem. It creates/updates/
> destroys associations. This uses AutosaveAssociation.
> - NestedParamsFormBuilder, which is a subclass of FormBuilder that  
> adds code to #fields_for for handling NestedParams enabled models.
>
> Obviously this is a wip, but it might be interesting for the  
> discussion as well.
>
> http://github.com/alloy/complex-form-examples/tree/alloy-nested_param...
>
> Cheers,
> Eloy
>
> On 12 sep 2008, at 19:04, Josh Susser wrote:
>
>
>
> > I like this feature too, but agree that it wasn''t ready for
prime time
> > yet.  I just put up a patch for my own take on how to do this.
It''s
> > not the whole story, but I wanted to get it up there so we can talk
> > about it:
>
> >http://rails.lighthouseapp.com/projects/8994/tickets/1031
>
> > --
> > Josh Susser
> >http://blog.hasmanythrough.com
>
> > On Sep 10, 1:23 pm, "Zack Chandler"
<zackchand...@gmail.com> wrote:
> >> Pratik,
>
> >> Got it - I figured it was prep time for 2.2...  I for one would
like
> >> to see this reapplied and iterated on after 2.2 is tagged as it  
> >> really
> >> helps out with multi-model forms.
>
> >> Best,
> >> Zack
>
> >> On Wed, Sep 10, 2008 at 12:50 PM, Pratik
<pratikn...@gmail.com>  
> >> wrote:
> >>> Hey Zach,
>
> >>> The :accessible change was not complete and still needs some
more  
> >>> work to be
> >>> usable. There was no conclusion about how we want updates to
work -
>
>>>http://groups.google.com/group/rubyonrails-core/browse_thread/thread/
> >>> ...
>
> >>> As 2.2 is very close, I didn''t want any half baked
changes to be  
> >>> in stable
> >>> release. But I''d be happy to apply the patch again,
and work  
> >>> incremently
> >>> after we have a 2.2 branch.
>
> >>> On Wed, Sep 10, 2008 at 8:36 PM, Zack Chandler  
> >>> <zackchand...@gmail.com>
> >>> wrote:
>
> >>>> I noticed that Pratik just reverted the ":accessible
option to  
> >>>> allow
> >>>> for allowing mass assignments" with the following
commit:
>
>
>>>>http://github.com/rails/rails/commit/9994f0d90248db7d7eae36f0b597a15e
> >>>> ...
>
> >>>> What was the reason behind this reversion?
>
> >>>> I''ve been using the feature and it works great
for my needs.
>
> >>>> Best,
>
> >>>> Zack Chandler
> >>>>http://depixelate.com
>
> >>> --
> >>> Cheers!
> >>> - Pratik
> >>>http://m.onkey.org
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

Are there any plans to allow this type of attribute nesting to go
beyond just 2 levels deep? For example, ":blog => {:posts =>[1 =>
{:comments => [1,2,3]}, 2 => {:comments => [4,5,6]}, ... ]}"
instead
of just ":blog => {:posts => [1,2,3]}"

Brad

On Sep 14, 8:56 pm, Ryan Bates <r...@railscasts.com>
wrote:
> I think one of the problem''s is that there are so many ways the
> interface can be implemented. That is, the interface for setting the
> associated attributes. I''m starting to document them here.
>
> http://gist.github.com/10793
>
> Josh, I haven''t added yours yet as I don''t fully
understand it. Is it
> close to Approach 2? Let me know and I''ll add it.
>
> We should weigh the pros and cons of each approach.
>
> Regards,
>
> Ryan
>
> On Sep 12, 11:33 am, Eloy Duran <eloy.de.en...@gmail.com> wrote:
>
> > Here''s a plugin extracted from one of our apps and then
extended for  
> > Ryan Bates''s complex-form-examples.
> > We only needed it for has_many and has_one associations atm, so I  
> > haven''t worked/tried it on others yet.
>
> > The plugin exists of 3 parts:
> > - AutosaveAssociation, which as the name implies automatically saves  
> > associations, not only on create as is the default.
> > - NestedParams, which is my take on this problem. It creates/updates/
> > destroys associations. This uses AutosaveAssociation.
> > - NestedParamsFormBuilder, which is a subclass of FormBuilder that  
> > adds code to #fields_for for handling NestedParams enabled models.
>
> > Obviously this is a wip, but it might be interesting for the  
> > discussion as well.
>
>
>http://github.com/alloy/complex-form-examples/tree/alloy-nested_param...
>
> > Cheers,
> > Eloy
>
> > On 12 sep 2008, at 19:04, Josh Susser wrote:
>
> > > I like this feature too, but agree that it wasn''t ready
for prime time
> > > yet.  I just put up a patch for my own take on how to do this.
It''s
> > > not the whole story, but I wanted to get it up there so we can
talk
> > > about it:
>
> > >http://rails.lighthouseapp.com/projects/8994/tickets/1031
>
> > > --
> > > Josh Susser
> > >http://blog.hasmanythrough.com
>
> > > On Sep 10, 1:23 pm, "Zack Chandler"
<zackchand...@gmail.com> wrote:
> > >> Pratik,
>
> > >> Got it - I figured it was prep time for 2.2...  I for one
would like
> > >> to see this reapplied and iterated on after 2.2 is tagged as
it  
> > >> really
> > >> helps out with multi-model forms.
>
> > >> Best,
> > >> Zack
>
> > >> On Wed, Sep 10, 2008 at 12:50 PM, Pratik
<pratikn...@gmail.com>  
> > >> wrote:
> > >>> Hey Zach,
>
> > >>> The :accessible change was not complete and still needs
some more  
> > >>> work to be
> > >>> usable. There was no conclusion about how we want updates
to work -
> >
>>>http://groups.google.com/group/rubyonrails-core/browse_thread/thread/
> > >>> ...
>
> > >>> As 2.2 is very close, I didn''t want any half
baked changes to be  
> > >>> in stable
> > >>> release. But I''d be happy to apply the patch
again, and work  
> > >>> incremently
> > >>> after we have a 2.2 branch.
>
> > >>> On Wed, Sep 10, 2008 at 8:36 PM, Zack Chandler  
> > >>> <zackchand...@gmail.com>
> > >>> wrote:
>
> > >>>> I noticed that Pratik just reverted the
":accessible option to  
> > >>>> allow
> > >>>> for allowing mass assignments" with the
following commit:
>
> >
>>>>http://github.com/rails/rails/commit/9994f0d90248db7d7eae36f0b597a15e
> > >>>> ...
>
> > >>>> What was the reason behind this reversion?
>
> > >>>> I''ve been using the feature and it works
great for my needs.
>
> > >>>> Best,
>
> > >>>> Zack Chandler
> > >>>>http://depixelate.com
>
> > >>> --
> > >>> Cheers!
> > >>> - Pratik
> > >>>http://m.onkey.org
--~--~---------~--~----~------------~-------~--~----~
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 Sep 15, 11:38 am, Brad Gessler <bradgess...@gmail.com>
wrote:
> Are there any plans to allow this type of attribute nesting to go
> beyond just 2 levels deep?
Yes, in a sense that comes "for free" because we''re just
doing mass
assignment in the nesting. You can go as deep as you want as long as
it''s supported through mass assignment. This is one reason why this
feature should need to be enabled manually instead of always on by
default (for security).
--~--~---------~--~----~------------~-------~--~----~
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''ve been giving some thought to the interface. One question I keep
coming back to is: how much do we want to support multi-model forms
through this? I think that is the primary use case, but this has other
uses as well. Active Record is not specific to web interfaces and
therefore shouldn''t be tied too heavily to them.

IMO Approach 1 (at http://gist.github.com/10793) is the cleanest
approach if we''re just doing this all in Ruby. This is also fairly
easy to use in a web form. However, there are a few major drawbacks
when doing so:

1. The resulting HTML is not validatable due to the duplicate form
field names.
2. It''s more difficult to work with the fields in Javascript because
of the duplicate form field names.
3. It''s impossible to nest associations multiple layers deep because
it gets confused when there are multiple "[]" in the field name.
4. Checkboxes (and I think radio buttons?) are nearly impossible to
get working.

Each of these problems stem from the fact that not each field has a
unique name/identifier. Therefore when it comes to multi-model form
fields, I''m more inclined to go with Approach 3 because each record
has its own unique key/identifier. Theoretically that will solve all
of the problems above.

That said, if you ever need to set associated attributes in other
scenarios (maybe preparing data in test cases), then Approach 3 is not
clean or optimal.
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

For what it''s worth, in my project I''ve been using something
like a
combination of Approaches 1 & 3 (from http://gist.github.com/10793):

{
  tasks =>
    {
      ''1'' => { :id => ''3'', :name =>
''Foo'' },
      ''2'' => { :name => ''Bar'' },
      ''3'' => { :name => ''Baz'' },
    }
}

As with Approach 1, you can tell new records from existing records by
the lack of an :id attribute.

As with Approach 3, this supports nesting associations multiple levels
deep because it uses a hash rather than arrays.

The keys in the tasks array (1, 2 & 3) come from the counter variable
created by render :partial, :collection (or from an each_with_index
loop).

Regards
Dave.


On Sep 15, 4:43 pm, Ryan Bates <r...@railscasts.com>
wrote:
> I''ve been giving some thought to the interface. One question I
keep
> coming back to is: how much do we want to support multi-model forms
> through this? I think that is the primary use case, but this has other
> uses as well. Active Record is not specific to web interfaces and
> therefore shouldn''t be tied too heavily to them.
>
> IMO Approach 1 (athttp://gist.github.com/10793) is the cleanest
> approach if we''re just doing this all in Ruby. This is also fairly
> easy to use in a web form. However, there are a few major drawbacks
> when doing so:
>
> 1. The resulting HTML is not validatable due to the duplicate form
> field names.
> 2. It''s more difficult to work with the fields in Javascript
because
> of the duplicate form field names.
> 3. It''s impossible to nest associations multiple layers deep
because
> it gets confused when there are multiple "[]" in the field name.
> 4. Checkboxes (and I think radio buttons?) are nearly impossible to
> get working.
>
> Each of these problems stem from the fact that not each field has a
> unique name/identifier. Therefore when it comes to multi-model form
> fields, I''m more inclined to go with Approach 3 because each
record
> has its own unique key/identifier. Theoretically that will solve all
> of the problems above.
>
> That said, if you ever need to set associated attributes in other
> scenarios (maybe preparing data in test cases), then Approach 3 is not
> clean or optimal.
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

Cool, Dave, nice approach. I added it to the gist.
http://gist.github.com/10793

Out of curiosity, do you have a "new task" link on the form which
dynamically adds new fields using Javascript? Is it difficult to
generate the auto-incrementing number with this?

Regards,

Ryan


On Sep 16, 8:01 am, Dave Rothlisberger <droth...@gmail.com>
wrote:
> For what it''s worth, in my project I''ve been using
something like a
> combination of Approaches 1 & 3 (fromhttp://gist.github.com/10793):
>
> {
>   tasks =>
>     {
>       ''1'' => { :id => ''3'', :name
=> ''Foo'' },
>       ''2'' => { :name => ''Bar'' },
>       ''3'' => { :name => ''Baz'' },
>     }
>
> }
>
> As with Approach 1, you can tell new records from existing records by
> the lack of an :id attribute.
>
> As with Approach 3, this supports nesting associations multiple levels
> deep because it uses a hash rather than arrays.
>
> The keys in the tasks array (1, 2 & 3) come from the counter variable
> created by render :partial, :collection (or from an each_with_index
> loop).
>
> Regards
> Dave.
>
> On Sep 15, 4:43 pm, Ryan Bates <r...@railscasts.com> wrote:
>
> > I''ve been giving some thought to the interface. One question
I keep
> > coming back to is: how much do we want to support multi-model forms
> > through this? I think that is the primary use case, but this has other
> > uses as well. Active Record is not specific to web interfaces and
> > therefore shouldn''t be tied too heavily to them.
>
> > IMO Approach 1 (athttp://gist.github.com/10793) is the cleanest
> > approach if we''re just doing this all in Ruby. This is also
fairly
> > easy to use in a web form. However, there are a few major drawbacks
> > when doing so:
>
> > 1. The resulting HTML is not validatable due to the duplicate form
> > field names.
> > 2. It''s more difficult to work with the fields in Javascript
because
> > of the duplicate form field names.
> > 3. It''s impossible to nest associations multiple layers deep
because
> > it gets confused when there are multiple "[]" in the field
name.
> > 4. Checkboxes (and I think radio buttons?) are nearly impossible to
> > get working.
>
> > Each of these problems stem from the fact that not each field has a
> > unique name/identifier. Therefore when it comes to multi-model form
> > fields, I''m more inclined to go with Approach 3 because each
record
> > has its own unique key/identifier. Theoretically that will solve all
> > of the problems above.
>
> > That said, if you ever need to set associated attributes in other
> > scenarios (maybe preparing data in test cases), then Approach 3 is not
> > clean or optimal.
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

Using the presence of the :id attribute to distinguish old vs new  
records seems ok, but it might get wonky for edge cases that use a  
different primary key.  I think I do like that approach.  The thing  
that is important that some of the gisted approaches miss is being  
able to order new records in the form.  It can get seriously confusing  
for users if data shifts around form one place to another if they  
failed a validation and need to correct something.

I''m not particularly attached to using two different faux accessors  
for create vs update.  It''s nice and clean, but really any way that  
lets you tell create vs update is fine by me.

--josh

On Sep 16, 2008, at 11:27 AM, Ryan Bates wrote:
>
> Cool, Dave, nice approach. I added it to the gist.
> http://gist.github.com/10793
>
> Out of curiosity, do you have a "new task" link on the form which
> dynamically adds new fields using Javascript? Is it difficult to
> generate the auto-incrementing number with this?
>
> Regards,
>
> Ryan
>
>
> On Sep 16, 8:01 am, Dave Rothlisberger <droth...@gmail.com> wrote:
>> For what it''s worth, in my project I''ve been using
something like a
>> combination of Approaches 1 & 3 (fromhttp://gist.github.com/10793):
>>
>> {
>>   tasks =>
>>     {
>>       ''1'' => { :id => ''3'',
:name => ''Foo'' },
>>       ''2'' => { :name => ''Bar''
},
>>       ''3'' => { :name => ''Baz''
},
>>     }
>>
>> }
>>
>> As with Approach 1, you can tell new records from existing records by
>> the lack of an :id attribute.
>>
>> As with Approach 3, this supports nesting associations multiple  
>> levels
>> deep because it uses a hash rather than arrays.
>>
>> The keys in the tasks array (1, 2 & 3) come from the counter
variable
>> created by render :partial, :collection (or from an each_with_index
>> loop).
>>
>> Regards
>> Dave.
>>
>> On Sep 15, 4:43 pm, Ryan Bates <r...@railscasts.com> wrote:
>>
>>> I''ve been giving some thought to the interface. One
question I keep
>>> coming back to is: how much do we want to support multi-model forms
>>> through this? I think that is the primary use case, but this has  
>>> other
>>> uses as well. Active Record is not specific to web interfaces and
>>> therefore shouldn''t be tied too heavily to them.
>>
>>> IMO Approach 1 (athttp://gist.github.com/10793) is the cleanest
>>> approach if we''re just doing this all in Ruby. This is
also fairly
>>> easy to use in a web form. However, there are a few major drawbacks
>>> when doing so:
>>
>>> 1. The resulting HTML is not validatable due to the duplicate form
>>> field names.
>>> 2. It''s more difficult to work with the fields in
Javascript because
>>> of the duplicate form field names.
>>> 3. It''s impossible to nest associations multiple layers
deep because
>>> it gets confused when there are multiple "[]" in the
field name.
>>> 4. Checkboxes (and I think radio buttons?) are nearly impossible to
>>> get working.
>>
>>> Each of these problems stem from the fact that not each field has a
>>> unique name/identifier. Therefore when it comes to multi-model form
>>> fields, I''m more inclined to go with Approach 3 because
each record
>>> has its own unique key/identifier. Theoretically that will solve
all
>>> of the problems above.
>>
>>> That said, if you ever need to set associated attributes in other
>>> scenarios (maybe preparing data in test cases), then Approach 3 is
>>> not
>>> clean or optimal.
> >
--
Josh Susser
http://blog.hasmanythrough.com



--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

Oh yeah, I should add a refinement of "Approach 4":

{
   :tasks_params =>
     {
       ''1'' => { :id => ''3'', :name
=> ''Foo'' },
       ''2'' => { :id => ''17'', :_delete
=> ''1'', :name => ''Bob'' },
       ''3'' => { :name => ''Bar'' },
       ''4'' => { :name => ''Baz'' }
     }
}

The presence of a :_delete param is used to indicate that an old  
record should be deleted.  This can be done simply with a checkbox or  
a JavaScript control.  I think this works much better than trying to  
delete records that just happen to be missing from the hash, since  
that can get ugly if you are paginating a bunch of records or  
otherwise subsetting e the list.
And I do think the _params (or _attributes) suffix on the name is  
important.

Anyway, I''ll try and get my proposed patch mashed around for this new  
arrangement, and see if we can get a good form_for mod to support this  
as well.

--josh

On Sep 16, 2008, at 12:12 PM, Josh Susser wrote:
>
> Using the presence of the :id attribute to distinguish old vs new
> records seems ok, but it might get wonky for edge cases that use a
> different primary key.  I think I do like that approach.  The thing
> that is important that some of the gisted approaches miss is being
> able to order new records in the form.  It can get seriously confusing
> for users if data shifts around form one place to another if they
> failed a validation and need to correct something.
>
> I''m not particularly attached to using two different faux
accessors
> for create vs update.  It''s nice and clean, but really any way
that
> lets you tell create vs update is fine by me.
>
> --josh
>
> On Sep 16, 2008, at 11:27 AM, Ryan Bates wrote:
>
>>
>> Cool, Dave, nice approach. I added it to the gist.
>> http://gist.github.com/10793
>>
>> Out of curiosity, do you have a "new task" link on the form
which
>> dynamically adds new fields using Javascript? Is it difficult to
>> generate the auto-incrementing number with this?
>>
>> Regards,
>>
>> Ryan
>>
>>
>> On Sep 16, 8:01 am, Dave Rothlisberger <droth...@gmail.com>
wrote:
>>> For what it''s worth, in my project I''ve been
using something like a
>>> combination of Approaches 1 & 3
(fromhttp://gist.github.com/10793):
>>>
>>> {
>>>  tasks =>
>>>    {
>>>      ''1'' => { :id => ''3'',
:name => ''Foo'' },
>>>      ''2'' => { :name =>
''Bar'' },
>>>      ''3'' => { :name =>
''Baz'' },
>>>    }
>>>
>>> }
>>>
>>> As with Approach 1, you can tell new records from existing records
>>> by
>>> the lack of an :id attribute.
>>>
>>> As with Approach 3, this supports nesting associations multiple
>>> levels
>>> deep because it uses a hash rather than arrays.
>>>
>>> The keys in the tasks array (1, 2 & 3) come from the counter  
>>> variable
>>> created by render :partial, :collection (or from an each_with_index
>>> loop).
>>>
>>> Regards
>>> Dave.
>>>
>>> On Sep 15, 4:43 pm, Ryan Bates <r...@railscasts.com> wrote:
>>>
>>>> I''ve been giving some thought to the interface. One
question I keep
>>>> coming back to is: how much do we want to support multi-model
forms
>>>> through this? I think that is the primary use case, but this
has
>>>> other
>>>> uses as well. Active Record is not specific to web interfaces
and
>>>> therefore shouldn''t be tied too heavily to them.
>>>
>>>> IMO Approach 1 (athttp://gist.github.com/10793) is the cleanest
>>>> approach if we''re just doing this all in Ruby. This is
also fairly
>>>> easy to use in a web form. However, there are a few major
drawbacks
>>>> when doing so:
>>>
>>>> 1. The resulting HTML is not validatable due to the duplicate
form
>>>> field names.
>>>> 2. It''s more difficult to work with the fields in
Javascript
>>>> because
>>>> of the duplicate form field names.
>>>> 3. It''s impossible to nest associations multiple
layers deep
>>>> because
>>>> it gets confused when there are multiple "[]" in the
field name.
>>>> 4. Checkboxes (and I think radio buttons?) are nearly
impossible to
>>>> get working.
>>>
>>>> Each of these problems stem from the fact that not each field
has a
>>>> unique name/identifier. Therefore when it comes to multi-model
form
>>>> fields, I''m more inclined to go with Approach 3
because each record
>>>> has its own unique key/identifier. Theoretically that will
solve
>>>> all
>>>> of the problems above.
>>>
>>>> That said, if you ever need to set associated attributes in
other
>>>> scenarios (maybe preparing data in test cases), then Approach 3
is
>>>> not
>>>> clean or optimal.
>>>
>
> --
> Josh Susser
> http://blog.hasmanythrough.com
>
>
>
> >
--
Josh Susser
http://blog.hasmanythrough.com



--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

> Oh yeah, I should add a refinement of "Approach 4":
>
> {
>   :tasks_params =>
>     {
>       ''1'' => { :id => ''3'', :name
=> ''Foo'' },
>       ''2'' => { :id => ''17'',
:_delete => ''1'', :name => ''Bob'' },
>       ''3'' => { :name => ''Bar'' },
>       ''4'' => { :name => ''Baz'' }
>     }
> }
>
> The presence of a :_delete param is used to indicate that an old
> record should be deleted.  This can be done simply with a checkbox or
> a JavaScript control.  I think this works much better than trying to
> delete records that just happen to be missing from the hash, since
> that can get ugly if you are paginating a bunch of records or
> otherwise subsetting e the list.
I really can''t think of a common use case for this scenario.
It seems to me that when you''re paginating a form, or any other edge  
case,
you shouldn''t rely on any naive solution, but rather solve it yourself
in your controller/model.

This is why I personally feel that destroying a record should be off  
by default.

But for simple common use cases, where you might have removed
a child from the DOM, the naive/simple approach I took in my plugin is  
good enough IMO.
>> Using the presence of the :id attribute to distinguish old vs new
>> records seems ok, but it might get wonky for edge cases that use a
>> different primary key.  I think I do like that approach.  The thing
>> that is important that some of the gisted approaches miss is being
>> able to order new records in the form.  It can get seriously  
>> confusing
>> for users if data shifts around form one place to another if they
>> failed a validation and need to correct something.
This is a very valid point which I have fixed in:
http://github.com/alloy/complex-form-examples/commit/4c740d5d8f5f99c0bfa5c3f38162ef26329c7cf0
Thanks for bringing that up.
>> I''m not particularly attached to using two different faux
accessors
>> for create vs update.  It''s nice and clean, but really any way
that
>> lets you tell create vs update is fine by me.
I''m sorry, but my English is not good enough to follow this bit.
Could you maybe explain it some more or in other words?

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-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 Sep 17, 2008, at 2:13 AM, Eloy Duran wrote:
>> Oh yeah, I should add a refinement of "Approach 4":
>>
>> {
>>  :tasks_params =>
>>    {
>>      ''1'' => { :id => ''3'',
:name => ''Foo'' },
>>      ''2'' => { :id => ''17'',
:_delete => ''1'', :name => ''Bob'' },
>>      ''3'' => { :name => ''Bar''
},
>>      ''4'' => { :name => ''Baz''
}
>>    }
>> }
>>
>> The presence of a :_delete param is used to indicate that an old
>> record should be deleted.  This can be done simply with a checkbox or
>> a JavaScript control.  I think this works much better than trying to
>> delete records that just happen to be missing from the hash, since
>> that can get ugly if you are paginating a bunch of records or
>> otherwise subsetting e the list.
>
> I really can''t think of a common use case for this scenario.
> It seems to me that when you''re paginating a form, or any other
edge
> case,
> you shouldn''t rely on any naive solution, but rather solve it
yourself
> in your controller/model.
>
> This is why I personally feel that destroying a record should be off
> by default.
>
> But for simple common use cases, where you might have removed
> a child from the DOM, the naive/simple approach I took in my plugin is
> good enough IMO.
This isn''t a naive solution, but one that worked well for us in a  
rather complicated workflow.  Making the controller do extra stuff to  
handle deletes outside of the save transaction is annoying and  
problematic.  Letting the model handle deletes as part of the same  
operation means the entire change can be handled in one transaction,  
which simplifies things a lot.  We found that using a _delete field  
worked best for us.  Noticing that all fields were blank was a lot of  
work for both the user and our code, and didn''t work at all if the  
model included radio buttons or select boxes without the blank option.
>>> Using the presence of the :id attribute to distinguish old vs new
>>> records seems ok, but it might get wonky for edge cases that use a
>>> different primary key.  I think I do like that approach.  The thing
>>> that is important that some of the gisted approaches miss is being
>>> able to order new records in the form.  It can get seriously
>>> confusing
>>> for users if data shifts around form one place to another if they
>>> failed a validation and need to correct something.
>
> This is a very valid point which I have fixed in:
>
http://github.com/alloy/complex-form-examples/commit/4c740d5d8f5f99c0bfa5c3f38162ef26329c7cf0
> Thanks for bringing that up.
>>> I''m not particularly attached to using two different faux
accessors
>>> for create vs update.  It''s nice and clean, but really any
way that
>>> lets you tell create vs update is fine by me.
>
> I''m sorry, but my English is not good enough to follow this bit.
> Could you maybe explain it some more or in other words?
Sorry about that, I was using non-standard jargon.  I use "faux  
accessor" to describe a model method that pretends to be an attribute  
accessor for the purpose of using a form to edit something that isn''t  
a primitive attribute.  And in my proposed patch for parameterized  
associations I use two different faux accessors for the API, e.g.  
create_tasks_params and update_tasks_params.

--
Josh Susser
http://blog.hasmanythrough.com



--~--~---------~--~----~------------~-------~--~----~
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 Sep 17, 2:13 am, Eloy Duran <eloy.de.en...@gmail.com>
wrote:
> >> I''m not particularly attached to using two different faux
accessors
> >> for create vs update.  It''s nice and clean, but really
any way that
> >> lets you tell create vs update is fine by me.
>
> I''m sorry, but my English is not good enough to follow this bit.
> Could you maybe explain it some more or in other words?
I think he''s referring to Approach 2 (of
http://gist.github.com/gists/10793
) which has both a new_task_attributes and existing_task_attributes
accessor. I''m not very fond of this either, especially when
you''re
setting it directly through Ruby and not using form params.

BTW Eloy, I saw your idea of using the timestamp when adding new task
records (through javascript). Genius!

What if we support both Approach 1 and Approach 4? Here''s the logic in
pseudo code.

--
def tasks=(new_tasks)
  if new_tasks is a hash
    grab values (as array) and sort by keys
    call self.tasks= with new values array
  else
    for each task in new_tasks
      if task is hash
        if task has id key
          update existing task matching id
        else
          add new task with hash attributes
        end
      else
        add task
      end
    end
  end
end
--

Supporting both hashes and arrays opens up a lot of flexibility.
Generally you''d use an array when dealing directly in Ruby, but a hash
when going through a form.

Regards,

Ryan
--~--~---------~--~----~------------~-------~--~----~
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 really can''t think of a common use case for this scenario.
>> It seems to me that when you''re paginating a form, or any
other edge
>> case,
>> you shouldn''t rely on any naive solution, but rather solve it
>> yourself
>> in your controller/model.
>>
>> This is why I personally feel that destroying a record should be off
>> by default.
>>
>> But for simple common use cases, where you might have removed
>> a child from the DOM, the naive/simple approach I took in my plugin  
>> is
>> good enough IMO.
>
> This isn''t a naive solution, but one that worked well for us in a
> rather complicated workflow.
I was referring to my implementation as being naive, not yours :)
> Making the controller do extra stuff to
> handle deletes outside of the save transaction is annoying and
> problematic.  Letting the model handle deletes as part of the same
> operation means the entire change can be handled in one transaction,
> which simplifies things a lot. We found that using a _delete field
> worked best for us.
Indeed I would solve this in the model as well.

I still don''t see the pagination etc as a common use case.
However, adding the option to check for the presence of "_delete"
=>
"1",
seems an easy/clear enough option as well.
> Noticing that all fields were blank was a lot of
> work for both the user and our code, and didn''t work at all if the
> model included radio buttons or select boxes without the blank option.
Just to be sure that we are talking about the same, and not different  
approaches from the gist :)
My implementation has 2 distinct options;
- :reject_empty : Which doesn''t create new records for empty hashes.
- :destroy_missing : Which destroys records if they''re missing in the  
attributes hash.
Both of these are off by default.
> Sorry about that, I was using non-standard jargon.  I use "faux
> accessor" to describe a model method that pretends to be an attribute
> accessor for the purpose of using a form to edit something that
isn''t
> a primitive attribute.  And in my proposed patch for parameterized
> associations I use two different faux accessors for the API, e.g.
> create_tasks_params and update_tasks_params.
Thanks for the clarification. I agree.

As I always prefer comparing implementation over discussion,
I will take some time one of the next few days to turn my plugin into  
a patch.
I will also include the option to mark a record to be destroyed with  
"_delete".

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-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 he''s referring to Approach 2 (of
http://gist.github.com/gists/10793
> ) which has both a new_task_attributes and existing_task_attributes
> accessor. I''m not very fond of this either, especially when
you''re
> setting it directly through Ruby and not using form params.
Thanks for clarifying.
> BTW Eloy, I saw your idea of using the timestamp when adding new task
> records (through javascript). Genius!
Thanks :)
> What if we support both Approach 1 and Approach 4? Here''s the
logic in
> pseudo code.
>
> --
> def tasks=(new_tasks)
>  if new_tasks is a hash
>    grab values (as array) and sort by keys
>    call self.tasks= with new values array
>  else
>    for each task in new_tasks
>      if task is hash
>        if task has id key
>          update existing task matching id
>        else
>          add new task with hash attributes
>        end
>      else
>        add task
>      end
>    end
>  end
> end
> --
>
> Supporting both hashes and arrays opens up a lot of flexibility.
> Generally you''d use an array when dealing directly in Ruby, but a
hash
> when going through a form.
I agree. I think I already support what you mean.
But maybe it would be an idea if you could send me a test which tests  
the difference
in behaviour between a Hash and a Array so I can verify?

Cheers,
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-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 Sep 17, 8:39 am, Eloy Duran <eloy.de.en...@gmail.com>
wrote:
> > I think he''s referring to Approach 2
(ofhttp://gist.github.com/gists/10793
> > ) which has both a new_task_attributes and existing_task_attributes
> > accessor. I''m not very fond of this either, especially when
you''re
> > setting it directly through Ruby and not using form params.
>
> Thanks for clarifying.
>
> > BTW Eloy, I saw your idea of using the timestamp when adding new task
> > records (through javascript). Genius!
>
> Thanks :)
>
>
>
> > What if we support both Approach 1 and Approach 4? Here''s the
logic in
> > pseudo code.
>
> > --
> > def tasks=(new_tasks)
> >  if new_tasks is a hash
> >    grab values (as array) and sort by keys
> >    call self.tasks= with new values array
> >  else
> >    for each task in new_tasks
> >      if task is hash
> >        if task has id key
> >          update existing task matching id
> >        else
> >          add new task with hash attributes
> >        end
> >      else
> >        add task
> >      end
> >    end
> >  end
> > end
> > --
>
> > Supporting both hashes and arrays opens up a lot of flexibility.
> > Generally you''d use an array when dealing directly in Ruby,
but a hash
> > when going through a form.
>
> I agree. I think I already support what you mean.
> But maybe it would be an idea if you could send me a test which tests  
> the difference
> in behaviour between a Hash and a Array so I can verify?
Sure thing, I''ll work on getting some real code together. I think the
primary difference between this approach and yours is that it uses
the :id key to distinguish existing records. I like this because it
removes the need for the magic "new_" hash key and is almost identical
to passing an array. The only thing the keys are used for is sorting
the values.

Regarding deleting. It depends on whether or not we use the "tasks="
accessor method. Rails already has this implemented, along with logic
on how deleting happens. AFAIK, it currently "resets" the tasks
association. That is, tasks which aren''t mentioned there are removed
from the association automatically. Whether they are deleted entirely
or not depends on the :dependent option in has_many (I''m assuming). I
think we should follow that same logic.

Look at it this way, when you''re setting "tasks=", the values
could be
represented as either hashes or actual Task instances (which are
currently supported). It makes sense that you can mix and match them
and not get strange behavior in how deleting happens.
--~--~---------~--~----~------------~-------~--~----~
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 was leaning toward Approach 1 & 4 because of the much cleaner
interface. However, it just hit me that this feature will not be
enabled by default (for security), so that kind of puts a damper on
using it directly in Ruby code (such as in test cases to create
records). I''m convinced that 99% of the time this will only be used in
multi-model forms so we should cater to that.

On that note, I''ve added Josh''s approach (from his patch) to
the gist.
See Approach 5.
http://gist.github.com/10793

I like this because the implementation is very simple. It does not
override the "tasks=" method or add any extra magic there. New records
are created with a hash so they can be sorted and have a unique
identifier when passing through the form.

Regards,

Ryan


On Sep 17, 8:50 am, Ryan Bates <r...@railscasts.com>
wrote:
> On Sep 17, 8:39 am, Eloy Duran <eloy.de.en...@gmail.com> wrote:
>
>
>
> > > I think he''s referring to Approach 2
(ofhttp://gist.github.com/gists/10793
> > > ) which has both a new_task_attributes and
existing_task_attributes
> > > accessor. I''m not very fond of this either, especially
when you''re
> > > setting it directly through Ruby and not using form params.
>
> > Thanks for clarifying.
>
> > > BTW Eloy, I saw your idea of using the timestamp when adding new
task
> > > records (through javascript). Genius!
>
> > Thanks :)
>
> > > What if we support both Approach 1 and Approach 4?
Here''s the logic in
> > > pseudo code.
>
> > > --
> > > def tasks=(new_tasks)
> > >  if new_tasks is a hash
> > >    grab values (as array) and sort by keys
> > >    call self.tasks= with new values array
> > >  else
> > >    for each task in new_tasks
> > >      if task is hash
> > >        if task has id key
> > >          update existing task matching id
> > >        else
> > >          add new task with hash attributes
> > >        end
> > >      else
> > >        add task
> > >      end
> > >    end
> > >  end
> > > end
> > > --
>
> > > Supporting both hashes and arrays opens up a lot of flexibility.
> > > Generally you''d use an array when dealing directly in
Ruby, but a hash
> > > when going through a form.
>
> > I agree. I think I already support what you mean.
> > But maybe it would be an idea if you could send me a test which tests
 
> > the difference
> > in behaviour between a Hash and a Array so I can verify?
>
> Sure thing, I''ll work on getting some real code together. I think
the
> primary difference between this approach and yours is that it uses
> the :id key to distinguish existing records. I like this because it
> removes the need for the magic "new_" hash key and is almost
identical
> to passing an array. The only thing the keys are used for is sorting
> the values.
>
> Regarding deleting. It depends on whether or not we use the
"tasks="
> accessor method. Rails already has this implemented, along with logic
> on how deleting happens. AFAIK, it currently "resets" the tasks
> association. That is, tasks which aren''t mentioned there are
removed
> from the association automatically. Whether they are deleted entirely
> or not depends on the :dependent option in has_many (I''m
assuming). I
> think we should follow that same logic.
>
> Look at it this way, when you''re setting "tasks=", the
values could be
> represented as either hashes or actual Task instances (which are
> currently supported). It makes sense that you can mix and match them
> and not get strange behavior in how deleting happens.
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

Ryan,
> Out of curiosity, do you have a "new task" link on the form which
> dynamically adds new fields using Javascript? Is it difficult to
> generate the auto-incrementing number with this?
I do have a "new task" link but it doesn''t use javascript
(I''m
somewhat new to web development so I''m keeping this as simple as
possible, which means no javascript).

Eloy''s idea of using a timestamp for the id is, as you said, genius.

Re. Josh''s comment on ordering, as long as you use a hash
you''re safe,
right? You can order by the hash key -- I''m not missing anything?
(perhaps around javascript drag-and-drop reordering?)

Re. approach 5 (in the gist), I prefer approach 4 because in your view
you can say:
  fields_for "tasks[#{position}]"
instead of having to remember "update_tasks_params[]" and
"create_tasks_params[]".

Like Eloy''s implementation, in my project I specify whether or not I
want to destroy records that are missing from the hash, by
a :destroy_missing option on the has_many declaration. But I
definitely agree that the best implementation would be to match what
ActiveRecord already does for tasks= when taking an array of objects
rather than a params hash -- which is, according to the docs:
"Replaces the collection''s content by deleting and adding objects
as
appropriate."

Re. Eloy''s :reject_empty option: I never create records if all the
attributes are blank (i.e. the user presses the "add another task"
button but leaves the task completely blank). Is there really a use
case for that? In fact, if a user deletes all the content from the
fields of an existing task, my implementation would delete that
record.

Cheers
Dave.

>
> Regards,
>
> Ryan
>
> On Sep 16, 8:01 am, Dave Rothlisberger <droth...@gmail.com> wrote:
>
> > For what it''s worth, in my project I''ve been using
something like a
> > combination of Approaches 1 & 3
(fromhttp://gist.github.com/10793):
>
> > {
> >   tasks =>
> >     {
> >       ''1'' => { :id => ''3'',
:name => ''Foo'' },
> >       ''2'' => { :name =>
''Bar'' },
> >       ''3'' => { :name =>
''Baz'' },
> >     }
>
> > }
>
> > As with Approach 1, you can tell new records from existing records by
> > the lack of an :id attribute.
>
> > As with Approach 3, this supports nesting associations multiple levels
> > deep because it uses a hash rather than arrays.
>
> > The keys in the tasks array (1, 2 & 3) come from the counter
variable
> > created by render :partial, :collection (or from an each_with_index
> > loop).
>
> > Regards
> > Dave.
>
> > On Sep 15, 4:43 pm, Ryan Bates <r...@railscasts.com> wrote:
>
> > > I''ve been giving some thought to the interface. One
question I keep
> > > coming back to is: how much do we want to support multi-model
forms
> > > through this? I think that is the primary use case, but this has
other
> > > uses as well. Active Record is not specific to web interfaces and
> > > therefore shouldn''t be tied too heavily to them.
>
> > > IMO Approach 1 (athttp://gist.github.com/10793) is the cleanest
> > > approach if we''re just doing this all in Ruby. This is
also fairly
> > > easy to use in a web form. However, there are a few major
drawbacks
> > > when doing so:
>
> > > 1. The resulting HTML is not validatable due to the duplicate
form
> > > field names.
> > > 2. It''s more difficult to work with the fields in
Javascript because
> > > of the duplicate form field names.
> > > 3. It''s impossible to nest associations multiple layers
deep because
> > > it gets confused when there are multiple "[]" in the
field name.
> > > 4. Checkboxes (and I think radio buttons?) are nearly impossible
to
> > > get working.
>
> > > Each of these problems stem from the fact that not each field has
a
> > > unique name/identifier. Therefore when it comes to multi-model
form
> > > fields, I''m more inclined to go with Approach 3 because
each record
> > > has its own unique key/identifier. Theoretically that will solve
all
> > > of the problems above.
>
> > > That said, if you ever need to set associated attributes in other
> > > scenarios (maybe preparing data in test cases), then Approach 3
is not
> > > clean or optimal.
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

Great to hear! I brought the issue up because the params parser can
only parse 2 levels deep before it gets screwed up and generates the
wrong hash (I guess this all depends on the hash structure that is
ultimately decided on for nested mass-assignment).

I also agree that enabling this manually makes a lot of sense for
security purposes.

Brad

On Sep 15, 4:22 pm, Ryan Bates <r...@railscasts.com>
wrote:
> On Sep 15, 11:38 am, Brad Gessler <bradgess...@gmail.com> wrote:
>
> > Are there any plans to allow this type of attribute nesting to go
> > beyond just 2 levels deep?
>
> Yes, in a sense that comes "for free" because we''re just
doing mass
> assignment in the nesting. You can go as deep as you want as long as
> it''s supported through mass assignment. This is one reason why
this
> feature should need to be enabled manually instead of always on by
> default (for security).
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

> Great to hear! I brought the issue up because the params parser can
> only parse 2 levels deep before it gets screwed up and generates the
> wrong hash (I guess this all depends on the hash structure that is
> ultimately decided on for nested mass-assignment).
Yes, that''s why whatever implementation is chosen *must* use hashes
rather than arrays.

--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

> Re. Josh''s comment on ordering, as long as you use a hash
you''re safe,
> right? You can order by the hash key -- I''m not missing anything?
> (perhaps around javascript drag-and-drop reordering?)
Indeed. That''s actually what he opted for with that comment, if I  
understood correctly.
> Like Eloy''s implementation, in my project I specify whether or not
I
> want to destroy records that are missing from the hash, by
> a :destroy_missing option on the has_many declaration. But I
> definitely agree that the best implementation would be to match what
> ActiveRecord already does for tasks= when taking an array of objects
> rather than a params hash -- which is, according to the docs:
> "Replaces the collection''s content by deleting and adding
objects as
> appropriate."
Agreed, it seems to follow the principle of least surprise.
However, I wonder if that might have to do with the fact that
using a has_many writer method is usually not used in combination with  
mass assignment....?

I guess for me it just doesn''t feel right if by default stuff will get
deleted,
where one might expect that you are simply updating the attributes.
But this may very well just be me being "paranoia" :)
Comments?
> Re. Eloy''s :reject_empty option: I never create records if all the
> attributes are blank (i.e. the user presses the "add another
task"
> button but leaves the task completely blank). Is there really a use
> case for that? In fact, if a user deletes all the content from the
> fields of an existing task, my implementation would delete that
> record.
Using the :reject_empty option as a default might indeed be sensible.

Deleting records for which we get a empty hash however is a different  
case.
The problem is that you might not always be sending
all attributes of a record. Thus assuming that a empty hash also means  
that
all the attributes of a record are empty is a bit dangerous.
There are ways to solve this of course, but that is not something I  
would
like to have by default.

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-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
-~----------~----~----~----~------~----~------~--~---

> Agreed, it seems to follow the principle of least surprise.
> However, I wonder if that might have to do with the fact that
> using a has_many writer method is usually not used in combination with  
> mass assignment....?
>
> I guess for me it just doesn''t feel right if by default stuff will
get  
> deleted,
> where one might expect that you are simply updating the attributes.
> But this may very well just be me being "paranoia" :)
> Comments?
You''re right: "I submitted a form to edit one of my comments on a
blog
post, and all the other comments got deleted" is way more surprising
than "when Post#comments= takes a params hash it acts differently than
when it takes an array of Comment objects."

--~--~---------~--~----~------------~-------~--~----~
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 Sep 17, 2008, at 3:10 PM, Dave Rothlisberger wrote:
>
>> Agreed, it seems to follow the principle of least surprise.
>> However, I wonder if that might have to do with the fact that
>> using a has_many writer method is usually not used in combination  
>> with
>> mass assignment....?
>>
>> I guess for me it just doesn''t feel right if by default stuff
will
>> get
>> deleted,
>> where one might expect that you are simply updating the attributes.
>> But this may very well just be me being "paranoia" :)
>> Comments?
>
> You''re right: "I submitted a form to edit one of my comments
on a blog
> post, and all the other comments got deleted" is way more surprising
> than "when Post#comments= takes a params hash it acts differently than
> when it takes an array of Comment objects."
This is why I don''t like overloading the comments= setter to handle  
hashes, but prefer having a comments_params= setter instead.  The  
basic setter deals with models, the params setter is the convenience  
interface for controllers to use for multi-model form support.  It''s a
much simpler implementation too.


--
Josh Susser
http://blog.hasmanythrough.com



--~--~---------~--~----~------------~-------~--~----~
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 Sep 17, 3:15 pm, Josh Susser <j...@hasmanythrough.com>
wrote:
> This is why I don''t like overloading the comments= setter to
handle  
> hashes, but prefer having a comments_params= setter instead.  The  
> basic setter deals with models, the params setter is the convenience  
> interface for controllers to use for multi-model form support.
 It''s a  
> much simpler implementation too.
Agreed, the setter method should have a different name than the
existing "tasks=" method. The behavior is different enough to warrant
its own method. Otherwise it will just be confusing on the
implementation side and the API side. Is there any good reason for
using the same setter method?

Ryan
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

Sorry I''m late to this discussion, I''ve been on holidays.

I''ve already spammed the list with this before, but I believe my plugin
(which screws with UrlEncodedParameterParser) could solve some of the 
issues you guys are running into with this.

http://www.vector-seven.com/git/rails/plugins/form_collections.git/

It''s not very well named, but it effectively uses Arrays instead of 
Hashes when numeric indices are used in the form. It also simplifies the 
implementation of UrlEncodedPairParser at the expense of slightly 
modified semantics in the parameter parsing (i.e. yes, it breaks a few 
tests -- some of which I''m convinced were not correct to begin with).

With such a patch in play, ordering issues would be resolved (thanks to 
the use of Arrays) and updates could be detected by effectively checking 
if the :id attribute is set on the nested form attributes. In the "Add 
Comment" case mentioned later in this discussion, I would assume one 
would be doing a many_assoc#<< rather than a many_assoc#= ...

The only thing really missing from the plugin is a way to handle sparse 
arrays. i.e. Assigning values only to indices 1, 200 and 999 of an array 
should not yield an array of 1000 elements, 997 of which are nil. This 
should be fairly trivial.

Cheers,
Tom

Ryan Bates wrote:
> I''ve been giving some thought to the interface. One question I
keep
> coming back to is: how much do we want to support multi-model forms
> through this? I think that is the primary use case, but this has other
> uses as well. Active Record is not specific to web interfaces and
> therefore shouldn''t be tied too heavily to them.
>
> IMO Approach 1 (at http://gist.github.com/10793) is the cleanest
> approach if we''re just doing this all in Ruby. This is also fairly
> easy to use in a web form. However, there are a few major drawbacks
> when doing so:
>
> 1. The resulting HTML is not validatable due to the duplicate form
> field names.
> 2. It''s more difficult to work with the fields in Javascript
because
> of the duplicate form field names.
> 3. It''s impossible to nest associations multiple layers deep
because
> it gets confused when there are multiple "[]" in the field name.
> 4. Checkboxes (and I think radio buttons?) are nearly impossible to
> get working.
>
> Each of these problems stem from the fact that not each field has a
> unique name/identifier. Therefore when it comes to multi-model form
> fields, I''m more inclined to go with Approach 3 because each
record
> has its own unique key/identifier. Theoretically that will solve all
> of the problems above.
>
> That said, if you ever need to set associated attributes in other
> scenarios (maybe preparing data in test cases), then Approach 3 is not
> clean or optimal.
> >
>   

--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

> Agreed, the setter method should have a different name than the
> existing "tasks=" method. The behavior is different enough to
warrant
> its own method. Otherwise it will just be confusing on the
> implementation side and the API side. Is there any good reason for
> using the same setter method?
The reason I had used the association setter is because in the case
of  a has_one association things like fields_for would work out of
the box and I only needed to create a special fields_for for a  
has_many association.

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-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
-~----------~----~----~----~------~----~------~--~---