Search code examples
ruby-on-railsrubyblocksimple-formhas-and-belongs-to-many

Rails display HABTM works, edit HABTM shows multiple copies of last entry


Situation

I am trying to tie multiple phone numbers to a client with has_and_belongs_to_many. I am able to add a client and phone number. If I pull up a client where I have three phone numbers, it displays each number, but when I click on edit, it displays three input boxes with the same number in all three boxes instead of three unique numbers.

show.html.erb

This displays all three numbers.

<% @client.phones.each do |phone| %>
  <tr>
    <td><%= phone.number %></td>
<% end %>

_form.html.erb

Upon clicking edit, this displays three text input boxes with the same number in all three boxes.

  <% @client.phones.each do |phone| %>    
    <%=f.simple_fields_for @phone do |a| %>  
      <%=a.input :number %>
    <% end %>
  <% end %>

Below (same code as in show.html.erb) this will display all three unique numbers, but they are printed on the screen and obviously not in an input box where they can be changed.

  <% @client.phones.each do |phone| %>
    <tr>
      <td><%= phone.number %></td>
  <% end %>

clients_controller.erb

# GET /clients/1/edit
def edit
  #Find client by id
  @client = Client.find(params[:id])

  @phone = Phone.find(@client.phone_ids)

end


# PATCH/PUT /clients/1
# PATCH/PUT /clients/1.json
def update
  #Same as above, but if anything was changed, save the changes to the database
  @client = Client.find(params[:id])
  @phone = Phone.find(@client.phone_ids)
  @phone = Phone.where(number: params[:client][:phone][:number]).first_or_create
  @client.phones << @phone

  respond_to do |format|
    if @client.update(client_params)
      format.html { redirect_to @client, notice: 'Client was successfully updated.' }
      format.json { render :show, status: :ok, location: @client }
    else
      format.html { render :edit }
      format.json { render json: @client.errors, status: :unprocessable_entity }
    end
  end
end

My belief is that I am replacing my variable with each new number as it is not setup as an array. I started experimenting with map, but I haven't had any luck with it yet.

phone.map do |phone|
  Phone.find(@client.phone_ids)
end

Suggestions or insight if map is the right way to go or if there is something else that I am just not seeing?


Solution

  • This two lines cause the problem, because you add the found or created phone to the user's phones over and over again:

    @phone = Phone.where(number: params[:client][:phone][:number]).first_or_create
    @client.phones << @phone 
    

    Instead do:

    def update
      @client = Client.find(params[:id])
      @phone = @client.phones.where(number: params[:client][:phone][:number]).first_or_create
    
      respond_to do |format|
        ...
      end
    end
    

    That will prevent you from creating new duplicates in the future.

    Furthermore you have an typo in your _form.html.erb that leads to displaying the same phone number multiple times: Just remove the @ from <%= f.simple_fields_for @phone do |a| %>, it should be just

    <%= f.simple_fields_for phone do |a| %>
    

    To clean up duplicates in your db, you can run something like the following in your rails console:

    Client.all.each do |client|
      uniq_numbers = client.phones.map(&:number).uniq
    
      client.phones = numbers.map do |number|
        Phone.where(number: number).first
      end
    end