Fix many-to-many writes

This was leaving orphaned records in the 'through' table, even though
everything worked as far as the API was concerned. The trick is to
ensure we hit the through table when destroying.

Author: Lee Richmond

lib/jsonapi_compliable/adapters/active_record.rb

@@ -73,11 +73,14 @@ def sideloading_module
       # @see Adapters::Abstract#associate
       def associate(parent, child, association_name, association_type)
         if association_type == :has_many
-          parent.association(association_name).loaded!
-          parent.association(association_name).add_to_target(child, :skip_callbacks)
+          associate_many(parent, child, association_name)
         elsif association_type == :habtm
-          parent.send(association_name) << child
-        else
+          if parent.send(association_name).exists?(child.id)
+            associate_many(parent, child, association_name)
+          else
+            parent.send(association_name) << child
+          end
+        elsif
           child.send("#{association_name}=", parent)
         end
       end
@@ -113,6 +116,13 @@ def destroy(model_class, id)
         instance.destroy
         instance
       end
+
+      private
+
+      def associate_many(parent, child, association_name)
+        parent.association(association_name).loaded!
+        parent.association(association_name).add_to_target(child, :skip_callbacks)
+      end
     end
   end
 end

lib/jsonapi_compliable/util/persistence.rb

@@ -95,13 +95,14 @@ def associate_parents(object, parents)
   end
 
   def associate_children(object, children)
-    # No need to associate destroyed objects
-    return if @meta[:method] == :destroy
-
     children.each do |x|
       if x[:object] && object
         if x[:meta][:method] == :disassociate
           x[:sideload].disassociate(object, x[:object])
+        elsif x[:meta][:method] == :destroy
+          if x[:sideload].type == :habtm
+            x[:sideload].disassociate(object, x[:object])
+          end # otherwise, no need to disassociate destroyed objects
         else
           x[:sideload].associate(object, x[:object])
         end

spec/integration/rails/persistence_spec.rb

@@ -154,6 +154,7 @@ def do_put(id)
       let(:prior_team) { Team.new(name: 'prior') }
       let(:disassociate_team) { Team.new(name: 'disassociate') }
       let(:destroy_team) { Team.new(name: 'destroy') }
+      let(:associate_team) { Team.create!(name: 'preexisting') }
 
       before do
         employee.teams << prior_team
@@ -172,7 +173,8 @@ def do_put(id)
                   { :'temp-id' => 'abc123', type: 'teams', method: 'create' },
                   { id: prior_team.id.to_s, type: 'teams', method: 'update' },
                   { id: disassociate_team.id.to_s, type: 'teams', method: 'disassociate' },
-                  { id: destroy_team.id.to_s, type: 'teams', method: 'destroy' }
+                  { id: destroy_team.id.to_s, type: 'teams', method: 'destroy' },
+                  { id: associate_team.id.to_s, type: 'teams', method: 'update' }
                 ]
               }
             }
@@ -187,22 +189,33 @@ def do_put(id)
               id: prior_team.id.to_s,
               type: 'teams',
               attributes: { name: 'Updated!' }
+            },
+            {
+              id: associate_team.id.to_s,
+              type: 'teams'
             }
           ]
         }
       end
 
-      it 'can create/update/disassociate/destroy' do
+      it 'can create/update/disassociate/associate/destroy' do
         expect(employee.teams).to include(destroy_team)
         expect(employee.teams).to include(disassociate_team)
         do_put(employee.id)
+
+        # Should properly delete/create from the through table
+        combos = EmployeeTeam.all.map { |et| [et.employee_id, et.team_id] }
+        expect(combos.uniq.length).to eq(combos.length)
+
         employee.reload
         expect(employee.teams).to_not include(disassociate_team)
         expect(employee.teams).to_not include(destroy_team)
         expect { disassociate_team.reload }.to_not raise_error
         expect { destroy_team.reload }.to raise_error(ActiveRecord::RecordNotFound)
         expect(prior_team.reload.name).to include('Updated!')
-        expect((employee.teams - [prior_team]).first.name).to eq('Team #1')
+        expect(employee.teams).to include(associate_team)
+        expect((employee.teams - [prior_team, associate_team]).first.name)
+          .to eq('Team #1')
       end
     end