I have struggled much of the day today to do something that on the
surface seems like it should be a piece of cake. But either it''s not,
or I''m too dense to see the easy way to do it.
I''m leaning toward modifying the rack parameter parser to (a) make it
far cleaner and more intelligible than it currently is (including
documenting exactly what it does!), and (b) make it easy to do what
I''m after. I think I can do this in a mostly backward-compatible
fashion, and could provide a compatibility mode if needed for anyone
who needs the old behavior.
I''d appreciate any feedback the experts may have before I embark on
this.
Here''s a simplified version of what I''m trying to do that
ended up
being (I think) impossible without resorting to some javascript that
really should not be required:
Say I''ve got a nested model structure that looks like this:
class Project < ActiveRecord::Base
has_many :tasks
accepts_nested_parameters_for :tasks
end
class Task < ActiveRecord::Base
belongs_to :project
end
I''d like to be able create an edit form for :project that includes a
dynamically varying list of :task subforms. I''ll have a delete button
next to each :task subform, and an "Add a Task" button at the bottom
of the last. All fairly standard and not very hard, especially with
the new accepts_nested_parameters_for feature.
But now here''s the kicker: I also want to use the
"sortable_element"
helper so that the user can reorder the tasks - including newly added
tasks - within the project. In order to save the final ordering, this
means I need to be able to detect, from the request parameters, the
order that the :task subforms appeared in the overall form at the time
it was submitted. This should be easy, because browsers are required
to send form parameters in the order in which they appear at the time
of submission, and sortable_element works by physically reordering the
subforms in the overall DOM structure.
The problem is that rack loses this ordering information, and I''m
pretty much convinced that it can''t be made to keep it. You can ALMOST
get this to work by specifying :index=>'''' and
:child_index=>'''' in all
your fields_for calls. But this results in illegal HTML (strictly
speaking, though it works, at least in the browser I''ve tried), and
it''s also slightly risky. The illegal part comes from the fact that
you''ll end up with multiple input elements in your form that have the
same id value. E.g. a text field for task ''name'' would appear
in
every :task subform, and in each appearance it would carry the same id
value. And not only would that be illegal, it would also make it
impossible to have per-subform radio button groups. The radio button
input elements would all have the same name across all subforms, so
they''d all be linked, rather than only within a subform.
Even without those problems, the strategy is dangerous because of the
way rack detects when it''s time to start a new hash, rather than
continuing to fill a current hash. This comes into play for a
parameter with a name like
''project[tasks_attributes][][name]''. The
the empty brackets call for an array of hashes at that point in the
structure, and rack will look at whether ''name'' appears as a
key in
the current last element of that array. If so, it opens up a new
element in the array, otherwise it adds to the current last element.
This mechanism will fail if not all your subforms offer the same set
of fields. E.g you might have separate buttons for "add quick task"
and "add task," both of which add the same sort of object, but
"add
quick task" provides an abbreviated form for easier entry. You''d
have
to add the other fields as hidden fields in the abbreviated subform in
order to be safe, and it''d probably be a hellish debugging exercise to
figure that out!
OK, so here''s my idea:
1. When "[]" appears in a parameter name it always indicates an array
at that point in the structure. What fallows goes into a new element
at the end of the array, using logic equivalent to what is currently
provided by rack.
2. When "[.index]" appears, what follows will be be added to the hash
that is the current last element of the array. The leading dot
prevents this index from itself being a hash key in what would then
have to be a containing hash, rather than a containing array, so
ordering of the substructures is preserved.
4. When "[+index]" appears, a new hash is added to the end of the
array, and what follows goes into that hash.
5. When [index] appears in a parameter name (without leading dot or
plus), the index is treated as a hash key for what follows same as
current behavior.
Here''s an example:
project[title]=''My Project''
project[tasks_attributes][+0][id]=1
project[tasks_attributes][.0][name]=Sleep
project[tasks_attributes][+1][id]=5
project[tasks_attributes][.1][id]=Sleep more
project[id]=3
The params structure would look like this:
{:title=>"My Project", :id=>"3",
:tasks_attributes=>[{:id=>"1",
:name=>"Sleep"},
{:id=>"5", :name=>"Sleep more"}]
}
The current rake parameter processor would yield (assuming dots and
pluses are removed from the above):
{:title=>"My Project, :id=>"3".
:tasks_attributes=>{"1"=>{:id=>"5",
:name=>"Sleep more"}, "0"=>
{:id=>"1", :name=>"Sleep"}}
}
With the new structure it would be simple matter for the controller to
take a pass through params[:project][:tasks_attributes] and
assign :position values for the benefit of acts_as_list in order to
retain the final subform ordering. With the current structure, one
would need to use a javascript callback before submit to add those
values to the DOM, or AJAX calls to update positions every time
something gets reordered. Neither would be all that difficult, but
it''s infuriating, all the required information is already available
for free in the raw form submission.
To make this really work well, the form helpers would also need to be
modified to make use of the enhanced naming scheme. If I do this, my
goal will be to do both the enhanced rack parameter parser and the
form helper modifications to take advantage.
Thanks for any comments or recommendations.
-andy
I''ve got a slightly modified form of this working.
The rules for parameter names in my scheme are:
- may optionally begin with an unbracketed name
- thereafter must be a sequence of bracketed names
- any bracketed name starting with ''.'' must be immediately
followed by
one that does not
- the whole thing may optionally be terminated with ''[]''
A "bracketed name" in the above is just any string that
doesn''t
include ''['' or '']''
characters, surrounded by ''['' and '']''
My replacement normalize_params implementation in rack/utils.rb checks
the parameter name it''s initially called with to see if it meets these
criteria.
Moreover, I''m pretty certain that any name that follows the rules but
does not
have any bracketed names beginning with ''.'' will yield the
same
results via
either the old or the new function, so what I''ve done should be
backward
compatible.
The new scheme uses the dotted names to introduce arrays of hashes.
The
part of the name following the ''.'' is saved discretely in the
last
hash of that
array (under a key named ''.index''), and a new hash is
allocated
whenever
the current dotted name is different from the one that was most
recently
stashed. The stashed .index keys are all removed before the parameter
structure is finally returned, so they won''t screw up attributes
setters.
This scheme means you don''t need to worry about using
''+xxx'' for the
first
parameter in a group, the rest of which will use ''.xxx''
instead, as
per my
original proposal.
With these changes in place (plus a small change to acts_as_list to
avoid
assigning a position value to a new record if one has already been
assigned), my original problem is very easily solved, as follows:
In my :task partial template, I use:
<%= project_form.fields_for :tasks, task,
:child_index=>".#{task.id ||
''REPLACE_ME''}" do |tf| %>
My "add a task" helper needs to replace ''REPLACE_ME''
with a unique
value via javascript (as is already common practice in these dynamic
multiform scenarios).
This yields parameter names in the subform like:
project[tasks_attributes][.13][name]
Then in my projects controller, my update action is defined like this:
def update
params[:project][:tasks_attributes].each_with_index { |ta,i| ta
[:position] = i }
@project = Project.find(params[:id])
...
end
That''s it. I''m actually thinking now that, since
it''s so
straightforward
to elect to use this feature (you just need to supply a :child_index
that
starts with a dot in the nested fields_for call), it may not require
any
changes to the form builders as I had originally expected.
I haven''t yet tried this with more deeply nested forms, but I will do
so
shortly.
If anyone has any comments or recommendations, please post. I''d like,
soon, to publish a patch for this to be incorporated into the rails
distro.
Thanks
-andy
On Aug 6, 11:06 pm, andy <andyrlo...@gmail.com>
wrote:> I have struggled much of the day today to do something that on the
> surface seems like it should be a piece of cake. But either it''s
not,
> or I''m too dense to see the easy way to do it.
>
> I''m leaning toward modifying the rack parameter parser to (a) make
it
> far cleaner and more intelligible than it currently is (including
> documenting exactly what it does!), and (b) make it easy to do what
> I''m after. I think I can do this in a mostly backward-compatible
> fashion, and could provide a compatibility mode if needed for anyone
> who needs the old behavior.
>
> I''d appreciate any feedback the experts may have before I embark
on
> this.
>
> Here''s a simplified version of what I''m trying to do that
ended up
> being (I think) impossible without resorting to some javascript that
> really should not be required:
>
> Say I''ve got a nested model structure that looks like this:
>
> class Project < ActiveRecord::Base
> has_many :tasks
> accepts_nested_parameters_for :tasks
> end
>
> class Task < ActiveRecord::Base
> belongs_to :project
> end
>
> I''d like to be able create an edit form for :project that includes
a
> dynamically varying list of :task subforms. I''ll have a delete
button
> next to each :task subform, and an "Add a Task" button at the
bottom
> of the last. All fairly standard and not very hard, especially with
> the new accepts_nested_parameters_for feature.
>
> But now here''s the kicker: I also want to use the
"sortable_element"
> helper so that the user can reorder the tasks - including newly added
> tasks - within the project. In order to save the final ordering, this
> means I need to be able to detect, from the request parameters, the
> order that the :task subforms appeared in the overall form at the time
> it was submitted. This should be easy, because browsers are required
> to send form parameters in the order in which they appear at the time
> of submission, and sortable_element works by physically reordering the
> subforms in the overall DOM structure.
>
> The problem is that rack loses this ordering information, and I''m
> pretty much convinced that it can''t be made to keep it. You can
ALMOST
> get this to work by specifying :index=>'''' and
:child_index=>'''' in all
> your fields_for calls. But this results in illegal HTML (strictly
> speaking, though it works, at least in the browser I''ve tried),
and
> it''s also slightly risky. The illegal part comes from the fact
that
> you''ll end up with multiple input elements in your form that have
the
> same id value. E.g. a text field for task ''name'' would
appear in
> every :task subform, and in each appearance it would carry the same id
> value. And not only would that be illegal, it would also make it
> impossible to have per-subform radio button groups. The radio button
> input elements would all have the same name across all subforms, so
> they''d all be linked, rather than only within a subform.
>
> Even without those problems, the strategy is dangerous because of the
> way rack detects when it''s time to start a new hash, rather than
> continuing to fill a current hash. This comes into play for a
> parameter with a name like
''project[tasks_attributes][][name]''. The
> the empty brackets call for an array of hashes at that point in the
> structure, and rack will look at whether ''name'' appears
as a key in
> the current last element of that array. If so, it opens up a new
> element in the array, otherwise it adds to the current last element.
>
> This mechanism will fail if not all your subforms offer the same set
> of fields. E.g you might have separate buttons for "add quick
task"
> and "add task," both of which add the same sort of object, but
"add
> quick task" provides an abbreviated form for easier entry.
You''d have
> to add the other fields as hidden fields in the abbreviated subform in
> order to be safe, and it''d probably be a hellish debugging
exercise to
> figure that out!
>
> OK, so here''s my idea:
>
> 1. When "[]" appears in a parameter name it always indicates an
array
> at that point in the structure. What fallows goes into a new element
> at the end of the array, using logic equivalent to what is currently
> provided by rack.
> 2. When "[.index]" appears, what follows will be be added to the
hash
> that is the current last element of the array. The leading dot
> prevents this index from itself being a hash key in what would then
> have to be a containing hash, rather than a containing array, so
> ordering of the substructures is preserved.
> 4. When "[+index]" appears, a new hash is added to the end of
the
> array, and what follows goes into that hash.
> 5. When [index] appears in a parameter name (without leading dot or
> plus), the index is treated as a hash key for what follows same as
> current behavior.
>
> Here''s an example:
>
> project[title]=''My Project''
> project[tasks_attributes][+0][id]=1
> project[tasks_attributes][.0][name]=Sleep
> project[tasks_attributes][+1][id]=5
> project[tasks_attributes][.1][id]=Sleep more
> project[id]=3
>
> The params structure would look like this:
> {:title=>"My Project", :id=>"3",
> :tasks_attributes=>[{:id=>"1",
:name=>"Sleep"},
> {:id=>"5", :name=>"Sleep more"}]
> }
>
> The current rake parameter processor would yield (assuming dots and
> pluses are removed from the above):
> {:title=>"My Project, :id=>"3".
> :tasks_attributes=>{"1"=>{:id=>"5",
:name=>"Sleep more"}, "0"=>
> {:id=>"1", :name=>"Sleep"}}
> }
>
> With the new structure it would be simple matter for the controller to
> take a pass through params[:project][:tasks_attributes] and
> assign :position values for the benefit of acts_as_list in order to
> retain the final subform ordering. With the current structure, one
> would need to use a javascript callback before submit to add those
> values to the DOM, or AJAX calls to update positions every time
> something gets reordered. Neither would be all that difficult, but
> it''s infuriating, all the required information is already
available
> for free in the raw form submission.
>
> To make this really work well, the form helpers would also need to be
> modified to make use of the enhanced naming scheme. If I do this, my
> goal will be to do both the enhanced rack parameter parser and the
> form helper modifications to take advantage.
>
> Thanks for any comments or recommendations.
>
> -andy
Hi Andy, I have in the past built an extension to the form_for helper family to accept a :element_id_prefix option that handles the "multiple html elements with the same ID" problem. It''s been sitting on my to-do list for awhile to convert it to a rails patch and submit it for consideration. I like that you want to take advantage of the standards already defined for html form submission and POST data to achieve ordering, but your solution seems needlessly roundabout for the end goal. You seem to actually be addressing two distinct issues here: 1) Clean up implementation of parameter parsing so that it maintains the order in the originally POSTed data string. 2) Introduce a :child_index value to fields_for to avoid the "multiple html elements with the same ID" problem. In regards to #1, what you get by doing this is your ability to concisely auto-assign position values to the children objects in your controller (which still requires you to iterate over the passed in params). This seems to me to not be a necessarily common need. So I''m inclined to ask, what else do we get out of the redesign of the parameter parsing? And do we really need to change the scheme for encoding field names to achieve this? Can we not achieve it by only changing the parsing code? #2 seems completely useful to me. That said, I would also like it on the form_* family of functions as well. If it''s a problem with fields_for, it''s also a problem with form_for (and one I''ve obviously encountered before). Stepping back a moment and looking at your specific problem, I would have simply solved it by outputting a hidden :position field as part of the form, and then setting the :onUpdate parameter of sortable_element to set those hidden fields (only a couple of lines of javascript). This in conjunction with a solution for "multiple html elements with the same ID" would seem to completely serve your needs, and you wouldn''t need any additional controller code to actually set the position field on the child elements. -John On Fri, Aug 7, 2009 at 12:57 PM, andy <andyrlowry@gmail.com> wrote:> > I''ve got a slightly modified form of this working. > > The rules for parameter names in my scheme are: > > - may optionally begin with an unbracketed name > - thereafter must be a sequence of bracketed names > - any bracketed name starting with ''.'' must be immediately followed by > one that does not > - the whole thing may optionally be terminated with ''[]'' > > A "bracketed name" in the above is just any string that doesn''t > include ''['' or '']'' > characters, surrounded by ''['' and '']'' > > My replacement normalize_params implementation in rack/utils.rb checks > the parameter name it''s initially called with to see if it meets these > criteria. > Moreover, I''m pretty certain that any name that follows the rules but > does not > have any bracketed names beginning with ''.'' will yield the same > results via > either the old or the new function, so what I''ve done should be > backward > compatible. > > The new scheme uses the dotted names to introduce arrays of hashes. > The > part of the name following the ''.'' is saved discretely in the last > hash of that > array (under a key named ''.index''), and a new hash is allocated > whenever > the current dotted name is different from the one that was most > recently > stashed. The stashed .index keys are all removed before the parameter > structure is finally returned, so they won''t screw up attributes > setters. > > This scheme means you don''t need to worry about using ''+xxx'' for the > first > parameter in a group, the rest of which will use ''.xxx'' instead, as > per my > original proposal. > > With these changes in place (plus a small change to acts_as_list to > avoid > assigning a position value to a new record if one has already been > assigned), my original problem is very easily solved, as follows: > > In my :task partial template, I use: > > <%= project_form.fields_for :tasks, task, > :child_index=>".#{task.id || ''REPLACE_ME''}" do |tf| %> > > My "add a task" helper needs to replace ''REPLACE_ME'' with a unique > value via javascript (as is already common practice in these dynamic > multiform scenarios). > > This yields parameter names in the subform like: > > project[tasks_attributes][.13][name] > > Then in my projects controller, my update action is defined like this: > > def update > params[:project][:tasks_attributes].each_with_index { |ta,i| ta > [:position] = i } > @project = Project.find(params[:id]) > ... > end > > That''s it. I''m actually thinking now that, since it''s so > straightforward > to elect to use this feature (you just need to supply a :child_index > that > starts with a dot in the nested fields_for call), it may not require > any > changes to the form builders as I had originally expected. > > I haven''t yet tried this with more deeply nested forms, but I will do > so > shortly. > > If anyone has any comments or recommendations, please post. I''d like, > soon, to publish a patch for this to be incorporated into the rails > distro. > > Thanks > > -andy > > On Aug 6, 11:06 pm, andy <andyrlo...@gmail.com> wrote: > > I have struggled much of the day today to do something that on the > > surface seems like it should be a piece of cake. But either it''s not, > > or I''m too dense to see the easy way to do it. > > > > I''m leaning toward modifying the rack parameter parser to (a) make it > > far cleaner and more intelligible than it currently is (including > > documenting exactly what it does!), and (b) make it easy to do what > > I''m after. I think I can do this in a mostly backward-compatible > > fashion, and could provide a compatibility mode if needed for anyone > > who needs the old behavior. > > > > I''d appreciate any feedback the experts may have before I embark on > > this. > > > > Here''s a simplified version of what I''m trying to do that ended up > > being (I think) impossible without resorting to some javascript that > > really should not be required: > > > > Say I''ve got a nested model structure that looks like this: > > > > class Project < ActiveRecord::Base > > has_many :tasks > > accepts_nested_parameters_for :tasks > > end > > > > class Task < ActiveRecord::Base > > belongs_to :project > > end > > > > I''d like to be able create an edit form for :project that includes a > > dynamically varying list of :task subforms. I''ll have a delete button > > next to each :task subform, and an "Add a Task" button at the bottom > > of the last. All fairly standard and not very hard, especially with > > the new accepts_nested_parameters_for feature. > > > > But now here''s the kicker: I also want to use the "sortable_element" > > helper so that the user can reorder the tasks - including newly added > > tasks - within the project. In order to save the final ordering, this > > means I need to be able to detect, from the request parameters, the > > order that the :task subforms appeared in the overall form at the time > > it was submitted. This should be easy, because browsers are required > > to send form parameters in the order in which they appear at the time > > of submission, and sortable_element works by physically reordering the > > subforms in the overall DOM structure. > > > > The problem is that rack loses this ordering information, and I''m > > pretty much convinced that it can''t be made to keep it. You can ALMOST > > get this to work by specifying :index=>'''' and :child_index=>'''' in all > > your fields_for calls. But this results in illegal HTML (strictly > > speaking, though it works, at least in the browser I''ve tried), and > > it''s also slightly risky. The illegal part comes from the fact that > > you''ll end up with multiple input elements in your form that have the > > same id value. E.g. a text field for task ''name'' would appear in > > every :task subform, and in each appearance it would carry the same id > > value. And not only would that be illegal, it would also make it > > impossible to have per-subform radio button groups. The radio button > > input elements would all have the same name across all subforms, so > > they''d all be linked, rather than only within a subform. > > > > Even without those problems, the strategy is dangerous because of the > > way rack detects when it''s time to start a new hash, rather than > > continuing to fill a current hash. This comes into play for a > > parameter with a name like ''project[tasks_attributes][][name]''. The > > the empty brackets call for an array of hashes at that point in the > > structure, and rack will look at whether ''name'' appears as a key in > > the current last element of that array. If so, it opens up a new > > element in the array, otherwise it adds to the current last element. > > > > This mechanism will fail if not all your subforms offer the same set > > of fields. E.g you might have separate buttons for "add quick task" > > and "add task," both of which add the same sort of object, but "add > > quick task" provides an abbreviated form for easier entry. You''d have > > to add the other fields as hidden fields in the abbreviated subform in > > order to be safe, and it''d probably be a hellish debugging exercise to > > figure that out! > > > > OK, so here''s my idea: > > > > 1. When "[]" appears in a parameter name it always indicates an array > > at that point in the structure. What fallows goes into a new element > > at the end of the array, using logic equivalent to what is currently > > provided by rack. > > 2. When "[.index]" appears, what follows will be be added to the hash > > that is the current last element of the array. The leading dot > > prevents this index from itself being a hash key in what would then > > have to be a containing hash, rather than a containing array, so > > ordering of the substructures is preserved. > > 4. When "[+index]" appears, a new hash is added to the end of the > > array, and what follows goes into that hash. > > 5. When [index] appears in a parameter name (without leading dot or > > plus), the index is treated as a hash key for what follows same as > > current behavior. > > > > Here''s an example: > > > > project[title]=''My Project'' > > project[tasks_attributes][+0][id]=1 > > project[tasks_attributes][.0][name]=Sleep > > project[tasks_attributes][+1][id]=5 > > project[tasks_attributes][.1][id]=Sleep more > > project[id]=3 > > > > The params structure would look like this: > > {:title=>"My Project", :id=>"3", > > :tasks_attributes=>[{:id=>"1", :name=>"Sleep"}, > > {:id=>"5", :name=>"Sleep more"}] > > } > > > > The current rake parameter processor would yield (assuming dots and > > pluses are removed from the above): > > {:title=>"My Project, :id=>"3". > > :tasks_attributes=>{"1"=>{:id=>"5", :name=>"Sleep more"}, "0"=> > > {:id=>"1", :name=>"Sleep"}} > > } > > > > With the new structure it would be simple matter for the controller to > > take a pass through params[:project][:tasks_attributes] and > > assign :position values for the benefit of acts_as_list in order to > > retain the final subform ordering. With the current structure, one > > would need to use a javascript callback before submit to add those > > values to the DOM, or AJAX calls to update positions every time > > something gets reordered. Neither would be all that difficult, but > > it''s infuriating, all the required information is already available > > for free in the raw form submission. > > > > To make this really work well, the form helpers would also need to be > > modified to make use of the enhanced naming scheme. If I do this, my > > goal will be to do both the enhanced rack parameter parser and the > > form helper modifications to take advantage. > > > > Thanks for any comments or recommendations. > > > > -andy > > >--~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Thanks for your comments, John. Re the need for changing the parameter parser to achieve #1, I think it''s absolutely necessary. I''m pretty convinced that the existing parser is simply incapable of retaining ordering of record structures unless those record structures are populated via *identical* parameter names. Anything you do to disambiguate parameter names will force the parameter parser to treat those disambiguating chunks as keys in a hash, and their relative order of appearance will thereby be lost. The key thing I''ve done with my reimplementation is to come up with a syntax that tells the parameter parser to omit certain "words" in the parameter name (i.e. strings of non-bracket characters) from the final structure, using those words solely to control allocations of new hashes in the surrounding array. The syntax I''ve currently got for this is an initial dot. I''m thinking of switching to double brackets instead (e.g. "[[index]]" instead of "[.index]" because it wouldn''t preclude developers using dotted index values if they really want to. (The corresponding argument to not take away developers'' ability to use double-bracketed indexes is weaker because doing so with the current parser accomplishes nothing - the double brackets are processed identically as the same indexes with single bracketing.) Also, keep in mind that the current parameter parser is actually potentially dangerous, in that values from different subforms can end up in the same record. My "quick edit" vs "full edit" example is one dangerous case, but another is when your subform collection is actually polymorphic (the situation I''m working on right now and which spawned all this). The current parser relies on seeing a repeated attribute to know when to start filling in a new record, so polymorphism in the subforms has the potential to screw things up! I think my new parser would be able to completely reliably handle all parameter names produced by rails'' form helpers, using a much more restrictive syntax than is accepted by the current parser (that parser is EXTREMELY lax and will parse totally screwy parameter names). With the backward compatibility features, this shouldn''t affect non-rails uses of rack. Re :child_index, I didn''t actually introduce that - it''s part of the existing nested attributes support, I assume introduced as part of the new accepts_nested_attributes_for feature. Normally you don''t need to supply a :child_index value, and record position will be used automatically in most cases. But here I want to use a value that will trigger the proper parameter ordering. Ultimately, I''m thinking that this sort of handling would be a better default than what''s currently done - i.e. there''s really no benefit that I can see from having the records come back in the params structure in an arbitrary order, and with this change it''d be quite easy to make as-submitted order be a "sensible" default. Then imagine altering acts_as_list so that the attributes= method would fix positions when handed an array of records, and suddenly my controller cruft could disappear, and this order maintenance would be in the model, where it belongs, rather than in the controller or in javascript. So in summary, you''re right that the javascript approach could work and would be simple, but I''d argue that the parameter parsing change would open the door to a far more "sensible" default in any case. Then I have the option of fixing positions with a one-liner in the controller, and you could still use your javascript approach if you prefer it. Eventually acts_as_list can handle it and we can both retire our extra logic. -andy On Aug 7, 4:38 pm, John Trupiano <jtrupi...@gmail.com> wrote:> Hi Andy, > > I have in the past built an extension to the form_for helper family to > accept a :element_id_prefix option that handles the "multiple html elements > with the same ID" problem. It''s been sitting on my to-do list for awhile to > convert it to a rails patch and submit it for consideration. > > I like that you want to take advantage of the standards already defined for > html form submission and POST data to achieve ordering, but your solution > seems needlessly roundabout for the end goal. You seem to actually be > addressing two distinct issues here: > > 1) Clean up implementation of parameter parsing so that it maintains the > order in the originally POSTed data string. > 2) Introduce a :child_index value to fields_for to avoid the "multiple html > elements with the same ID" problem. > > In regards to #1, what you get by doing this is your ability to concisely > auto-assign position values to the children objects in your controller > (which still requires you to iterate over the passed in params). This seems > to me to not be a necessarily common need. So I''m inclined to ask, what > else do we get out of the redesign of the parameter parsing? And do we > really need to change the scheme for encoding field names to achieve this? > Can we not achieve it by only changing the parsing code? > > #2 seems completely useful to me. That said, I would also like it on the > form_* family of functions as well. If it''s a problem with fields_for, it''s > also a problem with form_for (and one I''ve obviously encountered before). > > Stepping back a moment and looking at your specific problem, I would have > simply solved it by outputting a hidden :position field as part of the form, > and then setting the :onUpdate parameter of sortable_element to set those > hidden fields (only a couple of lines of javascript). This in conjunction > with a solution for "multiple html elements with the same ID" would seem to > completely serve your needs, and you wouldn''t need any additional controller > code to actually set the position field on the child elements. > > -John > > On Fri, Aug 7, 2009 at 12:57 PM, andy <andyrlo...@gmail.com> wrote: > > > I''ve got a slightly modified form of this working. > > > The rules for parameter names in my scheme are: > > > - may optionally begin with an unbracketed name > > - thereafter must be a sequence of bracketed names > > - any bracketed name starting with ''.'' must be immediately followed by > > one that does not > > - the whole thing may optionally be terminated with ''[]'' > > > A "bracketed name" in the above is just any string that doesn''t > > include ''['' or '']'' > > characters, surrounded by ''['' and '']'' > > > My replacement normalize_params implementation in rack/utils.rb checks > > the parameter name it''s initially called with to see if it meets these > > criteria. > > Moreover, I''m pretty certain that any name that follows the rules but > > does not > > have any bracketed names beginning with ''.'' will yield the same > > results via > > either the old or the new function, so what I''ve done should be > > backward > > compatible. > > > The new scheme uses the dotted names to introduce arrays of hashes. > > The > > part of the name following the ''.'' is saved discretely in the last > > hash of that > > array (under a key named ''.index''), and a new hash is allocated > > whenever > > the current dotted name is different from the one that was most > > recently > > stashed. The stashed .index keys are all removed before the parameter > > structure is finally returned, so they won''t screw up attributes > > setters. > > > This scheme means you don''t need to worry about using ''+xxx'' for the > > first > > parameter in a group, the rest of which will use ''.xxx'' instead, as > > per my > > original proposal. > > > With these changes in place (plus a small change to acts_as_list to > > avoid > > assigning a position value to a new record if one has already been > > assigned), my original problem is very easily solved, as follows: > > > In my :task partial template, I use: > > > <%= project_form.fields_for :tasks, task, > > :child_index=>".#{task.id || ''REPLACE_ME''}" do |tf| %> > > > My "add a task" helper needs to replace ''REPLACE_ME'' with a unique > > value via javascript (as is already common practice in these dynamic > > multiform scenarios). > > > This yields parameter names in the subform like: > > > project[tasks_attributes][.13][name] > > > Then in my projects controller, my update action is defined like this: > > > def update > > params[:project][:tasks_attributes].each_with_index { |ta,i| ta > > [:position] = i } > > @project = Project.find(params[:id]) > > ... > > end > > > That''s it. I''m actually thinking now that, since it''s so > > straightforward > > to elect to use this feature (you just need to supply a :child_index > > that > > starts with a dot in the nested fields_for call), it may not require > > any > > changes to the form builders as I had originally expected. > > > I haven''t yet tried this with more deeply nested forms, but I will do > > so > > shortly. > > > If anyone has any comments or recommendations, please post. I''d like, > > soon, to publish a patch for this to be incorporated into the rails > > distro. > > > Thanks > > > -andy > > > On Aug 6, 11:06 pm, andy <andyrlo...@gmail.com> wrote: > > > I have struggled much of the day today to do something that on the > > > surface seems like it should be a piece of cake. But either it''s not, > > > or I''m too dense to see the easy way to do it. > > > > I''m leaning toward modifying the rack parameter parser to (a) make it > > > far cleaner and more intelligible than it currently is (including > > > documenting exactly what it does!), and (b) make it easy to do what > > > I''m after. I think I can do this in a mostly backward-compatible > > > fashion, and could provide a compatibility mode if needed for anyone > > > who needs the old behavior. > > > > I''d appreciate any feedback the experts may have before I embark on > > > this. > > > > Here''s a simplified version of what I''m trying to do that ended up > > > being (I think) impossible without resorting to some javascript that > > > really should not be required: > > > > Say I''ve got a nested model structure that looks like this: > > > > class Project < ActiveRecord::Base > > > has_many :tasks > > > accepts_nested_parameters_for :tasks > > > end > > > > class Task < ActiveRecord::Base > > > belongs_to :project > > > end > > > > I''d like to be able create an edit form for :project that includes a > > > dynamically varying list of :task subforms. I''ll have a delete button > > > next to each :task subform, and an "Add a Task" button at the bottom > > > of the last. All fairly standard and not very hard, especially with > > > the new accepts_nested_parameters_for feature. > > > > But now here''s the kicker: I also want to use the "sortable_element" > > > helper so that the user can reorder the tasks - including newly added > > > tasks - within the project. In order to save the final ordering, this > > > means I need to be able to detect, from the request parameters, the > > > order that the :task subforms appeared in the overall form at the time > > > it was submitted. This should be easy, because browsers are required > > > to send form parameters in the order in which they appear at the time > > > of submission, and sortable_element works by physically reordering the > > > subforms in the overall DOM structure. > > > > The problem is that rack loses this ordering information, and I''m > > > pretty much convinced that it can''t be made to keep it. You can ALMOST > > > get this to work by specifying :index=>'''' and :child_index=>'''' in all > > > your fields_for calls. But this results in illegal HTML (strictly > > > speaking, though it works, at least in the browser I''ve tried), and > > > it''s also slightly risky. The illegal part comes from the fact that > > > you''ll end up with multiple input elements in your form that have the > > > same id value. E.g. a text field for task ''name'' would appear in > > > every :task subform, and in each appearance it would carry the same id > > > value. And not only would that be illegal, it would also make it > > > impossible to have per-subform radio button groups. The radio button > > > input elements would all have the same name across all subforms, so > > > they''d all be linked, rather than only within a subform. > > > > Even without those problems, the strategy is dangerous because of the > > > way rack detects when it''s time to start a new hash, rather than > > > continuing to fill a current hash. This comes into play for a > > > parameter with a name like ''project[tasks_attributes][][name]''. The > > > the empty brackets call for an array of hashes at that point in the > > > structure, and rack will look at whether ''name'' appears as a key in > > > the current last element of that array. If so, it opens up a new > > > element in the array, otherwise it adds to the current last element. > > > > This mechanism will fail if not all your subforms offer the same set > > > of fields. E.g you might have separate buttons for "add quick task" > > > and "add task," both of which add the same sort of object, but "add > > > quick task" provides an abbreviated form for easier entry. You''d have > > > to add the other fields as hidden fields in the abbreviated subform in > > > order to be safe, and it''d probably be a hellish debugging exercise to > > > figure that out! > > > > OK, so here''s my idea: > > > > 1. When "[]" appears in a parameter name it always indicates an array > > > at that point in the structure. What fallows goes into a new element > > > at the end of the array, using logic equivalent to what is currently > > > provided by rack. > > > 2. When "[.index]" appears, what follows will be be added to the hash > > > that is the current last element of the array. The leading dot > > > prevents this index from itself being a hash key in what would then > > > have to be a containing hash, rather than a containing array, > > ... > > read more »
Correct me if I''m wrong, but couldn''t this be solved by having rack return ordered hashes? On 6-Aug-09, at 11:06 PM, andy wrote:> The problem is that rack loses this ordering information, and I''m > pretty much convinced that it can''t be made to keep it.
Hmm... I think you''re right, and especially with ordered hashes being
standard in ruby 1.9 it''s a very natural choice.
It will still leave a pretty ugly and ill-defined piece of code in
rack, and maybe that should be addressed, but it''s not needed for this
if we adopt your approach. By ill-defined I mean that trying to
describe what it actually does in any sensible fashion is extremely
difficult. Descriptions of what it''s intended to do are generally
quite succinct, but in fact, it''ll take a parameter assignment like
''a
[[[[[][][b][][c]][][[[[[[[[x]=1'' and yield
{"a"=>{"b"=>[{"c"=>
[{"x"=>1}]}]}}, and assigning "a]b[=3" yields
{"a"=>{"b"=>nil}} and
other such bizarre cases. And even seemingly well-formed cases can
yield unexpected results, like the situations I''ve already talked
about where a nonuniform set of subforms might, with rack''s current
behavior for embedded "[]", end up mixing attributes from different
subforms into each others'' records.
So I know rack is a separate offering from rails, but it seems the
rack parameter parser was largely adopted from rails, and the
connection is pretty tight. If a general clean-up of the rack
parameter parser is worth doing, would that likely be driven by this
community?
-andy
On Aug 7, 9:17 pm, Mateo Murphy <mateo.mur...@gmail.com>
wrote:> Correct me if I''m wrong, but couldn''t this be solved by
having rack
> return ordered hashes?
>
> On 6-Aug-09, at 11:06 PM, andy wrote:
>
>
>
> > The problem is that rack loses this ordering information, and
I''m
> > pretty much convinced that it can''t be made to keep it.
As for doing a cleanup in the first place, unexpected results would imply that there are bugs On 8-Aug-09, at 8:08 AM, andy wrote:> even seemingly well-formed cases can > yield unexpected results, like the situations I''ve already talked > about where a nonuniform set of subforms might, with rack''s current > behavior for embedded "[]", end up mixing attributes from different > subforms into each others'' records.If this is the case, then I a gree that a refactoring is in order...> So I know rack is a separate offering from rails, but it seems the > rack parameter parser was largely adopted from rails, and the > connection is pretty tight. If a general clean-up of the rack > parameter parser is worth doing, would that likely be driven by this > community?Considering rack seems to have its own fairly active community and mailing list, I think it would be best to approach them first. Have you tried the rack-devel list?
Unless I''m missing something, why wouldn''t you just use hidden fields that indicate the order? On Aug 6, 11:06 pm, andy <andyrlo...@gmail.com> wrote:> I have struggled much of the day today to do something that on the > surface seems like it should be a piece of cake. But either it''s not, > or I''m too dense to see the easy way to do it. > > I''m leaning toward modifying the rack parameter parser to (a) make it > far cleaner and more intelligible than it currently is (including > documenting exactly what it does!), and (b) make it easy to do what > I''m after. I think I can do this in a mostly backward-compatible > fashion, and could provide a compatibility mode if needed for anyone > who needs the old behavior. > > I''d appreciate any feedback the experts may have before I embark on > this. > > Here''s a simplified version of what I''m trying to do that ended up > being (I think) impossible without resorting to some javascript that > really should not be required: > > Say I''ve got a nested model structure that looks like this: > > class Project < ActiveRecord::Base > has_many :tasks > accepts_nested_parameters_for :tasks > end > > class Task < ActiveRecord::Base > belongs_to :project > end > > I''d like to be able create an edit form for :project that includes a > dynamically varying list of :task subforms. I''ll have a delete button > next to each :task subform, and an "Add a Task" button at the bottom > of the last. All fairly standard and not very hard, especially with > the new accepts_nested_parameters_for feature. > > But now here''s the kicker: I also want to use the "sortable_element" > helper so that the user can reorder the tasks - including newly added > tasks - within the project. In order to save the final ordering, this > means I need to be able to detect, from the request parameters, the > order that the :task subforms appeared in the overall form at the time > it was submitted. This should be easy, because browsers are required > to send form parameters in the order in which they appear at the time > of submission, and sortable_element works by physically reordering the > subforms in the overall DOM structure. > > The problem is that rack loses this ordering information, and I''m > pretty much convinced that it can''t be made to keep it. You can ALMOST > get this to work by specifying :index=>'''' and :child_index=>'''' in all > your fields_for calls. But this results in illegal HTML (strictly > speaking, though it works, at least in the browser I''ve tried), and > it''s also slightly risky. The illegal part comes from the fact that > you''ll end up with multiple input elements in your form that have the > same id value. E.g. a text field for task ''name'' would appear in > every :task subform, and in each appearance it would carry the same id > value. And not only would that be illegal, it would also make it > impossible to have per-subform radio button groups. The radio button > input elements would all have the same name across all subforms, so > they''d all be linked, rather than only within a subform. > > Even without those problems, the strategy is dangerous because of the > way rack detects when it''s time to start a new hash, rather than > continuing to fill a current hash. This comes into play for a > parameter with a name like ''project[tasks_attributes][][name]''. The > the empty brackets call for an array of hashes at that point in the > structure, and rack will look at whether ''name'' appears as a key in > the current last element of that array. If so, it opens up a new > element in the array, otherwise it adds to the current last element. > > This mechanism will fail if not all your subforms offer the same set > of fields. E.g you might have separate buttons for "add quick task" > and "add task," both of which add the same sort of object, but "add > quick task" provides an abbreviated form for easier entry. You''d have > to add the other fields as hidden fields in the abbreviated subform in > order to be safe, and it''d probably be a hellish debugging exercise to > figure that out! > > OK, so here''s my idea: > > 1. When "[]" appears in a parameter name it always indicates an array > at that point in the structure. What fallows goes into a new element > at the end of the array, using logic equivalent to what is currently > provided by rack. > 2. When "[.index]" appears, what follows will be be added to the hash > that is the current last element of the array. The leading dot > prevents this index from itself being a hash key in what would then > have to be a containing hash, rather than a containing array, so > ordering of the substructures is preserved. > 4. When "[+index]" appears, a new hash is added to the end of the > array, and what follows goes into that hash. > 5. When [index] appears in a parameter name (without leading dot or > plus), the index is treated as a hash key for what follows same as > current behavior. > > Here''s an example: > > project[title]=''My Project'' > project[tasks_attributes][+0][id]=1 > project[tasks_attributes][.0][name]=Sleep > project[tasks_attributes][+1][id]=5 > project[tasks_attributes][.1][id]=Sleep more > project[id]=3 > > The params structure would look like this: > {:title=>"My Project", :id=>"3", > :tasks_attributes=>[{:id=>"1", :name=>"Sleep"}, > {:id=>"5", :name=>"Sleep more"}] > } > > The current rake parameter processor would yield (assuming dots and > pluses are removed from the above): > {:title=>"My Project, :id=>"3". > :tasks_attributes=>{"1"=>{:id=>"5", :name=>"Sleep more"}, "0"=> > {:id=>"1", :name=>"Sleep"}} > } > > With the new structure it would be simple matter for the controller to > take a pass through params[:project][:tasks_attributes] and > assign :position values for the benefit of acts_as_list in order to > retain the final subform ordering. With the current structure, one > would need to use a javascript callback before submit to add those > values to the DOM, or AJAX calls to update positions every time > something gets reordered. Neither would be all that difficult, but > it''s infuriating, all the required information is already available > for free in the raw form submission. > > To make this really work well, the form helpers would also need to be > modified to make use of the enhanced naming scheme. If I do this, my > goal will be to do both the enhanced rack parameter parser and the > form helper modifications to take advantage. > > Thanks for any comments or recommendations. > > -andy
That could be part of a solution, but to make it dynamic would mean some javascript to update the position fields as subforms are moved around by the user, or just prior to submit. What I''m looking for is a "dead-simple" way of doing what seems a pretty obvious use-case, namely: the user gets to reorder subforms via some drag-and-drop feature like the sortable_element helper, and when the form is submitted, the order of the subforms at the time of submission gets reflected in the database. Using hidden fields and javascript to manipulate them seems unreasonably complicated when the browsers are required to send form values in the order of appearance anyway, since that''s the order we want for this use case. But the current rack parameter parser either destroys the relative ordering of the subforms or requires that the field names & ids be identical from one subform to the next. The latter results in illegal HTML, potential mixing of subform data, and an inability to use checkboxes and radio groups properly in the subforms. Hence you can''t really take advantage of this very convenient browser behavior. I''ve been away and hence quiet for a few days, but per recommendations earlier in this thread, I''ll take it up with the rack community to see about making their parameter parser use ordered hashes (shouldn''t be a hard sell, since it''ll happen with ruby 1.9 anyway), and also a general cleanup of the current parser. I may also look into updates to the rails acts_as_list plugin, which I think could make the whole thing fully automatic (so no need for a controller hack to update subform positions). -andy On Sat, Aug 8, 2009 at 10:06 PM, Allen <blatyo@gmail.com> wrote:> > Unless I''m missing something, why wouldn''t you just use hidden fields > that indicate the order? > > On Aug 6, 11:06 pm, andy <andyrlo...@gmail.com> wrote: > > I have struggled much of the day today to do something that on the > > surface seems like it should be a piece of cake. But either it''s not, > > or I''m too dense to see the easy way to do it. > > > > I''m leaning toward modifying the rack parameter parser to (a) make it > > far cleaner and more intelligible than it currently is (including > > documenting exactly what it does!), and (b) make it easy to do what > > I''m after. I think I can do this in a mostly backward-compatible > > fashion, and could provide a compatibility mode if needed for anyone > > who needs the old behavior. > > > > I''d appreciate any feedback the experts may have before I embark on > > this. > > > > Here''s a simplified version of what I''m trying to do that ended up > > being (I think) impossible without resorting to some javascript that > > really should not be required: > > > > Say I''ve got a nested model structure that looks like this: > > > > class Project < ActiveRecord::Base > > has_many :tasks > > accepts_nested_parameters_for :tasks > > end > > > > class Task < ActiveRecord::Base > > belongs_to :project > > end > > > > I''d like to be able create an edit form for :project that includes a > > dynamically varying list of :task subforms. I''ll have a delete button > > next to each :task subform, and an "Add a Task" button at the bottom > > of the last. All fairly standard and not very hard, especially with > > the new accepts_nested_parameters_for feature. > > > > But now here''s the kicker: I also want to use the "sortable_element" > > helper so that the user can reorder the tasks - including newly added > > tasks - within the project. In order to save the final ordering, this > > means I need to be able to detect, from the request parameters, the > > order that the :task subforms appeared in the overall form at the time > > it was submitted. This should be easy, because browsers are required > > to send form parameters in the order in which they appear at the time > > of submission, and sortable_element works by physically reordering the > > subforms in the overall DOM structure. > > > > The problem is that rack loses this ordering information, and I''m > > pretty much convinced that it can''t be made to keep it. You can ALMOST > > get this to work by specifying :index=>'''' and :child_index=>'''' in all > > your fields_for calls. But this results in illegal HTML (strictly > > speaking, though it works, at least in the browser I''ve tried), and > > it''s also slightly risky. The illegal part comes from the fact that > > you''ll end up with multiple input elements in your form that have the > > same id value. E.g. a text field for task ''name'' would appear in > > every :task subform, and in each appearance it would carry the same id > > value. And not only would that be illegal, it would also make it > > impossible to have per-subform radio button groups. The radio button > > input elements would all have the same name across all subforms, so > > they''d all be linked, rather than only within a subform. > > > > Even without those problems, the strategy is dangerous because of the > > way rack detects when it''s time to start a new hash, rather than > > continuing to fill a current hash. This comes into play for a > > parameter with a name like ''project[tasks_attributes][][name]''. The > > the empty brackets call for an array of hashes at that point in the > > structure, and rack will look at whether ''name'' appears as a key in > > the current last element of that array. If so, it opens up a new > > element in the array, otherwise it adds to the current last element. > > > > This mechanism will fail if not all your subforms offer the same set > > of fields. E.g you might have separate buttons for "add quick task" > > and "add task," both of which add the same sort of object, but "add > > quick task" provides an abbreviated form for easier entry. You''d have > > to add the other fields as hidden fields in the abbreviated subform in > > order to be safe, and it''d probably be a hellish debugging exercise to > > figure that out! > > > > OK, so here''s my idea: > > > > 1. When "[]" appears in a parameter name it always indicates an array > > at that point in the structure. What fallows goes into a new element > > at the end of the array, using logic equivalent to what is currently > > provided by rack. > > 2. When "[.index]" appears, what follows will be be added to the hash > > that is the current last element of the array. The leading dot > > prevents this index from itself being a hash key in what would then > > have to be a containing hash, rather than a containing array, so > > ordering of the substructures is preserved. > > 4. When "[+index]" appears, a new hash is added to the end of the > > array, and what follows goes into that hash. > > 5. When [index] appears in a parameter name (without leading dot or > > plus), the index is treated as a hash key for what follows same as > > current behavior. > > > > Here''s an example: > > > > project[title]=''My Project'' > > project[tasks_attributes][+0][id]=1 > > project[tasks_attributes][.0][name]=Sleep > > project[tasks_attributes][+1][id]=5 > > project[tasks_attributes][.1][id]=Sleep more > > project[id]=3 > > > > The params structure would look like this: > > {:title=>"My Project", :id=>"3", > > :tasks_attributes=>[{:id=>"1", :name=>"Sleep"}, > > {:id=>"5", :name=>"Sleep more"}] > > } > > > > The current rake parameter processor would yield (assuming dots and > > pluses are removed from the above): > > {:title=>"My Project, :id=>"3". > > :tasks_attributes=>{"1"=>{:id=>"5", :name=>"Sleep more"}, "0"=> > > {:id=>"1", :name=>"Sleep"}} > > } > > > > With the new structure it would be simple matter for the controller to > > take a pass through params[:project][:tasks_attributes] and > > assign :position values for the benefit of acts_as_list in order to > > retain the final subform ordering. With the current structure, one > > would need to use a javascript callback before submit to add those > > values to the DOM, or AJAX calls to update positions every time > > something gets reordered. Neither would be all that difficult, but > > it''s infuriating, all the required information is already available > > for free in the raw form submission. > > > > To make this really work well, the form helpers would also need to be > > modified to make use of the enhanced naming scheme. If I do this, my > > goal will be to do both the enhanced rack parameter parser and the > > form helper modifications to take advantage. > > > > Thanks for any comments or recommendations. > > > > -andy > > >--~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---