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.09k stars 383 forks source link

Issues using look-up or create with has_many through associations #240

Closed CEB-21 closed 9 years ago

CEB-21 commented 9 years ago

I have cocoon, simpleform, and rails4-autocomplete installed with all three working as intended. The project's users have and belong to many groups. This same association would apply to groups. What I'd like to do in my group's form partial is use the concept of look up or create, similar to what's shown in the wiki but instead of a select box there's an input box that suggests users present in the db.

Although its retrieving the name of users using the jquery autocomplete feature its treating that nested form solely as a post request and creates a brand new user with that name instead of adding that existing user to that particular group if there's a match. What would I need to do to get this to work?

Models

class User < ActiveRecord::Base
  has_many :memberships
  has_many :groups, through: :memberships
end

class Membership < ActiveRecord::Base
  belongs_to :user
  belongs_to :group
end

class Group < ActiveRecord::Base
  has_many :memberships
  has_many :users, through: :memberships
end

Routes

resources :groups do
  get :autocomplete_user_name, :on => :collection
end

Views

# _form.html.erb

<%= simple_form_for @group do |f| %>
  <%= f.input :name %>
  <h3>Users</h3>
  <div id="users">
    <%= f.simple_fields_for :users do |user| %>
      <%= render 'user_fields', :f => user %>
    <% end %>
    <div class="links">
      <%= link_to_add_association 'add user', f, :users %>
    </div>
  </div>
  <%= f.submit %>
<% end %>

# _user_fields.html.erb

<div class="nested-fields">
  <%= f.autocomplete_field :name, autocomplete_user_name_groups_path, :"data-autocomplete-label" => false %>
  <%= link_to_remove_association "remove user", f %>
</div>

Groups Controller

  ...
  def project_params
      params.require(:group).permit(:name, users_attributes: [:id, :name, :_destroy])
  end
nathanvda commented 9 years ago

I can easily explain the behaviour, offer alternative approaches, but not a clear solution.

When you fill in the user fields, you will create a new user. Always. To use an existing user, you will have to create a Membership instead, with a user_id pointing to an existing user. It is that simple.

If you look at the wiki, I switch between a lookup, using a dropdown, and the complete partial. But in your case, the lookup should be linked to MemberShip and you would need to create a linked membership instead, with a given (existing) user. Instead of using jquery type-ahead, you could use the select2 plugin, which supports typing in a dropdown selection.

Another alternative would be to keep your code as is, but handle this when creating/saving the users. Although imho this is more involved, and I believe possibly incorrect: a user will know when to select an exisiting user, or create a new user (I am guessing, but it all depends on your use-case).

Hope this helps a bit.

CEB-21 commented 9 years ago

Thanks so much for your suggestions. I decided to choose your first but feel like I'm taking a major shot in the dark. Would you be able show how this would look like based off what I've written so far? The error I'm now receiving is Association :membership not found

Models

class User < ActiveRecord::Base
  has_many :memberships
  has_many :groups, through: :memberships
end

class Membership < ActiveRecord::Base
  belongs_to :user
  belongs_to :group
end

class Group < ActiveRecord::Base
  has_many :memberships
  has_many :users, through: :memberships
  accepts_nested_attributes_for :memberships, reject_if: :all_blank, :allow_destroy => true
end

My groups controller now looks like this:

def group_params
  params.require(:group).permit(:name, memberships_attributes: [:user_id, :_destroy])
end

And changed my views to this:

# _form.html.erb

<%= simple_form_for @group do |f| %>
  <%= f.input :name %>
  <h3>Users</h3>
  <div id="memberships">
    <%= f.simple_fields_for :memberships do |membership| %>
      <%= render 'membership_fields', :f => membership %>
    <% end %>
    <div class="links">
      <%= link_to_add_association 'add user', f, :memberships %>
    </div>
  </div>
  <%= f.submit %>
<% end %>

# _membership_fields.html.erb  

  <div class="nested-fields">
    <%= f.association :membership, :collection => Membership.order(:user_id), :prompt => 'Choose an existing membership', class: 'membership-dropdown' %>
    <%= link_to_remove_association "remove membership", f %>
  </div>

I'm assuming my call for Select2 would look like this:

  $('.membership-drowdown').select2();
nathanvda commented 9 years ago

Mmmmmm you show me the _user_fields.html.erb and I was expecting the _membership_fields. In the membership-fields partial you would either show a f.association :user or create a new one.

CEB-21 commented 9 years ago

My apologies. I copied the code from my original post and forgot to change the title of that partial in the comment. The form seems to be free from error but now the select box isn't getting populated with any existing users nor is it saving. To give you a better idea I've pushed my project here: https://github.com/TKB21/test_project

nathanvda commented 9 years ago

Just had a quick look, your _membership_fields.html.erb looks like this:

<div class="nested-fields">
  <%= f.association :user, :collection => Membership.order(:user_id), :prompt => 'Choose an existing membership', class: 'membership-dropdown' %>
  <%= link_to_remove_association "remove membership", f %>
</div>

Weird thing you are doing: the collection are Memberships ? You should be selecting a user. Not sure if you know haml/slim, it is way cleaner, and less to type, so let me fix that:

 .nested-fields
   = f.association :user, :collection => User.order(:name), :prompt => 'Choose an existing user', class: 'membership-dropdown'
   = link_to_remove_association "remove membership", f
CEB-21 commented 9 years ago

Still a few small issues but making progress nonetheless. With your changes in place and select2 properly setup this is what I'm running into:

I've pushed my latest commit but to give you a clear idea of things here are the two pages where code was altered:

  .nested-fields
    = f.association :user, :collection => User.order(:name), :prompt => 'Choose an existing user', input_html: {class: 'user-dropdown'} # created a class for the select2 function call
    = link_to_remove_association "remove membership", f

Script

  $(function() {
    $('.user-dropdown').select2();
  });

Commit: https://github.com/TKB21/test_project/commit/057db316f6fe6a5aacbab1c759ce35d2611b2db3

nathanvda commented 9 years ago

Your jquery code is only applied to items already on the page. If you are using rails 4 and turbolinks you should listen to the page-ready event instead.

To enable the select2 on newly inserted items you should listen to the cocoon:before-insert or cocoon:after-insert callback and apply the select2 then.

Is that clear enough?

CEB-21 commented 9 years ago

Almost! What I'm still a little confused with is the page-ready event you mentioned. I thought this was accomplished already using $(document).ready(function({}); or $(function(){}); for short.

I managed to get select2 to work using cocoon:after-insert using the code below but I'd obviously would need it to be there once the page loads as well (most likely relates to your first suggestion I'm having trouble with). Using cocoon:before-insert adds functionality to all the select boxes but the existing one on the page if I'm in the edit view.

Also what's strange is that its still adding duplicate select boxes to the dom upon edit. These aren't new records keep in mind but just extra select boxes that shouldn't be there. Trying to remove them and saving doesn't work either.

  $(function() {
    $('form').on('cocoon:after-insert', function() {
      $('.user-dropdown').select2();
    });
  });
nathanvda commented 9 years ago

1) are you using rails4 and turbolinks or not? The page-change event is only relevant in that case (sorry, I used the wrong name). When using turbolinks the entire page is not refreshed, instead the page-ready event is triggered to indicate the page is refreshed.

For example:

$(window).bind 'page:change', () ->
  /* do select2 stuff here */

2) In your cocoon callback you should only update the newly inserted html, not the entire page. So rather do something like

$('form').on('cocoon:after-insert', function(e, added_user) {
    added_user.select2();
});

For more info read the documentation, which contains some more examples.

CEB-21 commented 9 years ago

Yes I'm using Rails 4 and turbolinks but wasn't able to get this to work. I tried searching through jquery's documentation and wasn't able to find anything about page:change so I'm not exactly sure if I wrote this correctly. With the following code I received the error Uncaught query function not defined for Select2 undefined when it came time to add a new user. I'm assuming its looking for that .user-dropdown class in place of added_user?

  $(window).bind('page:change', function() {
    $('form').on('cocoon:after-insert', function(e, added_user) {
      added_user.select2();
    });
  });
nathanvda commented 9 years ago

The page:change event is not jquery, but turbolinks. Documentation: turbolinks

I thought the added_user in the function-call is a jquery selector, so the select2 should just work. But checking the code, maybe you have to write added_user[0].select2.

I will try to verify that myself later, and adapt documentation accordingly (and check why that change was needed in the first place).

CEB-21 commented 9 years ago

Targeting the added_user based on its index still returned the error Uncaught query function not defined for Select2 with those duplicate select boxes also being created upon update. Please let me know what you find in your discoveries. So close!

nathanvda commented 9 years ago

Well, in my tests it just returns a jquery-selector, as in the documentation, so added_user.select2() should just work. In your error-message it is written with a capital S, so maybe it is just a typo? (according to the documentation it should a lowercase s).

CEB-21 commented 9 years ago

Just to clarify were you using this code?:

  $(window).bind('page:change', function() {
    $('form').on('cocoon:after-insert', function(e, added_user) {
      added_user.select2();
    });
  });
nathanvda commented 9 years ago

I verified that the second parameter given to the anonymous callback function is a jquery selector. I tested with jquery fadeIn method (as in the documentation). If a jquery method would work, I am assuming select2 would also work.

CEB-21 commented 9 years ago

Like you I was able to use the fadeIn method and was successful in doing so:

  $(function() {
    $('form').bind('cocoon:before-insert', function(e, inserted_item) {
      $(inserted_item).fadeIn();
    });
  });

The problem occurs when select2 is put into the equation. While troubleshooting I came across this post on StackOverflow that seems to identify this error. The issue seems to be similar but the wasn't able to apply their solution to my problem. It looks like they added the class select to the select box but didn't work when I tried:

  $(function() {
     $('form').bind('cocoon:before-insert', function(e, inserted_item)
       $(inserted_item).addClass('select').select2();
     });
  });
nathanvda commented 9 years ago

The inserted_item is a jquery selector, so you can just write inserted_item.fadeIn(..) (feels like I keep repeating myself here).

But sorry, I see the error now, you have to do

 inserted_item.find('.user_dropdown`).select2()

The inserted_item is everything, and then you need to select only the dropdown within the newly inserted block of html, and then apply select2.

CEB-21 commented 9 years ago

Great news. I managed to get the script to work with your suggestion. In addition to that I also got the pre-existing select boxes to load as well. Thanks for your patience as well. You'll have to excuse me, I'm still working on my js:

   $(function() {
     $('.user-dropdown').select2();

   $('form').bind('cocoon:after-insert', function(e, inserted_item) {
      inserted_item.find('.user-dropdown').select2();
    });
   });

I believe the duplicates being created and not being able to remove them has something to do with an error somewhere in my form that I just can't seem to figure out. I referred back to your post earlier in the thread suggesting that it might be due to how the script was written but that doesn't seem to be the case because after commenting out/removing the js the problem still persists. Do you think it has anything to do with using :memberships vs :users for my nested form? To test my theory I got the error: Association cannot be used in forms not associated with an object.

   = simple_form_for @group do |f|
     = f.input :name
     h3 Users
     #users
      = f.simple_fields_for :memberships do |membership|
        = render 'membership_fields', :f => membership
      = link_to_add_association 'add user', f, :memberships
     = f.submit
CEB-21 commented 9 years ago

Solved the duplication error. Turns out I needed to permit my join table's id in my group_params method in my groups controller. Thanks so much for your help:

   def group_params
     params.require(:group).permit(:name, memberships_attributes: [:id, :user_id, :_destroy])
   end
nathanvda commented 9 years ago

Ow I apologise, I seem to have misread your last comment. In my memory you had to get back to either choosing from a list or adding a new one, I completely missed the duplicates problem, somehow, I apologise, because indeed that is a known problem, and rather obvious once you think about it: without an existing ID all edited items seems like new items (since they cannot be found in the database). Awesome you found it!