nathanvda / cocoon

Dynamic nested forms using jQuery made easy; works with formtastic, simple_form or default forms
http://github.com/nathanvda/cocoon
MIT License
3.08k stars 384 forks source link

Associations only saving the last one added #376

Closed drusepth closed 8 years ago

drusepth commented 8 years ago

Hi! I see you've got a slur of issues already but I was hoping you might have some insight on what I (probably) am doing wrong in using it.

I have an app in which objects have a lot of named, self-referential (and normal) associations. For example, the Character model has a best_friends association (pointing to at least 1 Character), a siblings association (pointing to at least 1 Character), a hometowns association (pointing to at least 1 Location), etc. In the Character#edit page, I allow users to create these associations using Cocoon, and observe the following:

Just to give you a sense of context, here's what the UI looks like adding relations: 2016-08-02-165143_964x569_scrot (Ugly I know, but just trying to get it working right now! :) )

Here's the relevant code:

CharactersController

  ...
  def edit
    @content = Character.find(params[:id])
  end
  ...

relevant edit.html.erb (in the Content class, which Character inherits from)

        <% content.class.attribute_categories.each do |category, data| %>
          <div id="<%= category %>" class="row">
            <% data[:attributes].each do |attribute| %>
              <div class="col s10 m8 l4">
                <% value = content.send(attribute) %>
                <% if value.is_a?(ActiveRecord::Associations::CollectionProxy) %>
                  <% through_class = content.class.reflect_on_association(attribute).options[:through].to_s %>
                  <%= render 'content/form/relation_input', f: f, attribute: attribute, relation: through_class %>
                <% else %>
                  <%= render 'content/form/text_input', f: f, attribute: attribute %>
                <% end %>
              </div>
            <% end %>
          </div>
        <% end %>

content/form/_relation_input.html.erb (rendered in the case of associations, like this question):

<div>
  <%= f.label attribute, attribute.humanize.capitalize %>
</div>

<div id="<%= relation %>">
  <%= f.fields_for relation do |builder| %>
    <%= render 'content/form/groupship_fields', f: builder, attribute: attribute.singularize, parent: f.object %>
  <% end %>
  <div class="links">
    <% color = f.object.send(attribute).build.class.color %>
    <%= link_to_add_association "add #{attribute.to_s.singularize}", f, relation,
        class: "btn #{color}",
        partial: 'content/form/groupship_fields',
        render_options: { locals: {
            attribute: attribute.singularize,
            parent: f.object
        }} %>
  </div>
</div>

content/form/_groupship_fields.html.erb

<div class="nested-fields">
  <% uuid = SecureRandom.uuid %>

  <% if f.object && f.object.send(attribute) %>
    <% klass = f.object.send(attribute).class %>

    <div class="chip">
      <%= link_to f.object.send(attribute) do %>
        <span class="<%= klass.color %>-text">
          <i class="material-icons left"><%= klass.icon %></i>
        </span>
        <%= f.object.send(attribute).name %>
      <% end %>
    </div>

    <%= link_to_remove_association 'remove', f %>

  <% else %>
    <% klass = parent.send(attribute.pluralize).build.class.name.downcase %>

    <div class="input-field">
      <%= f.label attribute %>
      <%= f.autocomplete_field "#{attribute}_id", send("autocomplete_name_#{klass.pluralize}_path"), 'data-auto-focus' => true, id_element: "##{uuid}" %>
      <%= f.hidden_field "#{attribute}_id", id: uuid %>

      <%= link_to_remove_association 'remove', f %>
    </div>

  <% end %>
</div>

And, of course, back to the CharacterController#update that handles processing:

  def update
    @content = Character.find(params[:id])

    if @content.update_attributes(content_params)
      successful_response(@content, t(:update_success, model_name: humanized_model_name))
    else
      failed_response('edit', :unprocessable_entity)
    end
  end

If there's any insight you can give into what I might be doing wrong, it'd be much appreciated. I assume I'm just messing up something small to make it only process the last of each association sent.

Please let me know if there's any other information you need to take a look. The relevant code (though not simplified) is all in https://github.com/indentlabs/Indent/tree/master/app/views/content if you want to look at the source directly.

nathanvda commented 8 years ago

First thing: check what is actually posted to the controller? Sometimes due to an error in your html (e.g. using id's instead of classes) it will only send one of a repeating group. But from what I could see, that seems ok (still check to be safe).

I saw in your code (thanks for sharing it), that you use a handy shortcut to define your relations: relate :siblings, with: :siblingships and in the concern you ony add the accepts_nested_attributes_for for the connecting relation. Imho you also have to add it for the actual relation you are editing. A bit weird that it partially works, so not sure if that is the case.

Also, you might have a problem: all your "related"-classes are in the folder content_groupers but you do not prefix the class with ContentGroupers so not sure if the rails autoloading will actually find the class correctly (but maybe you solved that otherwise --I didn't go looking to far).

drusepth commented 8 years ago

Thanks for the insight. Upon inspecting what is actually posted to the controller, you may be right: there's only one ID being passed through: when adding two mothers (in addition to one existing one), the relevant posted params look like:

 "motherships_attributes"=>
  {"0"=>{"id"=>"1", "_destroy"=>"false"},
   "1470283435380"=>{"mother_id"=>"2", "_destroy"=>"false"},
   "1470283438992"=>{"mother_id"=>"", "_destroy"=>"false"}},

And after wiping them all (successfully, in one submit), trying to add 3 at the same time:

 "motherships_attributes"=>
  {"1470285441878"=>{"mother_id"=>"3", "_destroy"=>"false"},
   "1470285445040"=>{"mother_id"=>"", "_destroy"=>"false"},
   "1470285447940"=>{"mother_id"=>"", "_destroy"=>"false"}},

Pretty sure those empty mother_id params are the culprit here.

After inspecting the HTML further, it looks like any of the autocomplete fields are updating just the first hidden id element in the form, because I'm setting the hidden field ID to SecureRandom.uuid:

    <% uuid = SecureRandom.uuid %>
    <% klass = parent.send(attribute.pluralize).build.class.name.downcase %>

    <div class="input-field">
      <%= f.label attribute %>
      <%= f.autocomplete_field "#{attribute}_id", send("autocomplete_name_#{klass.pluralize}_path"), 'data-auto-focus' => true, id_element: "##{uuid}" %>
      <%= f.hidden_field "#{attribute}_id", id: uuid %>

      <%= link_to_remove_association 'remove', f %>
    </div>

Each additional field has the same ID (for some reason, it looks like it might be copying the HTML instead of rendering the partial again (and therefore calling SecureRandom.uuid again)), so each dropdown is only updating the hidden field for the first dropdown.

For example, selecting a Character from the second dropdown updates the first hidden element's value (even if it already had a value from the first dropdown): 2016-08-04-002701_265x280_scrot

2016-08-04-002751_683x593_scrot

Are a dynamic number of fields a usual case for Cocoon? What do you recommend for IDs here? Setting them with JS instead, perhaps?

Also FWIW on the relate shortcut, I tried adding in the actual relation as well, but it didn't seem to make a difference here. I'll take a look around whether I need it in the long run though, thanks for the heads up.

And yep! Adding config.autoload_paths += Dir[Rails.root.join('app', 'models', '{*/}')] to config/application.rb should be recursively autoloading everything in the models directory, at least.

Thanks again for your help. I'll keep digging, but anything you can offer would be much appreciated. And thanks for Cocoon!

nathanvda commented 8 years ago

It is a little over my head, just gave it a quick cursory read, but please note that all views are server-side-rendered, so every time you press link_to_add_association it just inserts a pre-rendered partial. If you need to set a specific value upon insertion, you will have to use js and the callbacks.

Also, not sure but you seem to make it harder on yourself. To be clear:

(I will read it in more detail later, if needed)

drusepth commented 8 years ago

Just an FYI, the complications seemed to stem from the autocomplete field I was using (rails-jquery-autocomplete) that required me to specify the ID. I swapped it out for a native rails f.select and everything works perfectly now!

Thank you for your all your help! :)