CLDC-3566: Clear duplicate set ids when a log ceases to be a duplicate in normal updates

app/controllers/form_controller.rb

@@ -199,8 +199,8 @@ private
       end
 
       dynamic_duplicates = @log.lettings? ? current_user.lettings_logs.duplicate_logs(@log) : current_user.sales_logs.duplicate_logs(@log)
+      saved_duplicates = @log.duplicates
       if dynamic_duplicates.any?
-        saved_duplicates = @log.duplicates
         if saved_duplicates.none? || duplicates_changed?(dynamic_duplicates, saved_duplicates)
           duplicate_set_id = dynamic_duplicates.first.duplicate_set_id || new_duplicate_set_id(@log)
           update_logs_with_duplicate_set_id(@log, dynamic_duplicates, duplicate_set_id)
@@ -208,6 +208,11 @@ private
         end
         return send("#{@log.class.name.underscore}_duplicate_logs_path", @log, original_log_id: @log.id)
       end
+
+      if saved_duplicates.any?
+        @log.update!(duplicate_set_id: nil)
+        saved_duplicates.first.update!(duplicate_set_id: nil) if saved_duplicates.count == 1
+      end
     end
 
     if is_referrer_type?("check_answers")

spec/requests/form_controller_spec.rb

@@ -784,18 +784,139 @@ RSpec.describe FormController, type: :request do
             expect(whodunnit_actor.id).to eq(user.id)
           end
 
-          context "and duplicate logs" do
-            let(:duplicate_logs) { create_list(:lettings_log, 2) }
+          context "and the answer makes the log a duplicate" do
+            context "with one other log" do
+              let(:new_duplicate) { create(:lettings_log) }
 
-            before do
-              allow(LettingsLog).to receive(:duplicate_logs).and_return(duplicate_logs)
-              post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+              before do
+                allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.where(id: new_duplicate.id))
+                post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+              end
+
+              it "sets a new duplicate set id on both logs" do
+                lettings_log.reload
+                new_duplicate.reload
+                expect(lettings_log.duplicate_set_id).not_to be_nil
+                expect(lettings_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id)
+              end
+
+              it "redirects to the duplicate logs page" do
+                expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
+                follow_redirect!
+                expect(page).to have_content("These logs are duplicates")
+              end
             end
 
-            it "redirects to the duplicate logs page" do
-              expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
-              follow_redirect!
-              expect(page).to have_content("These logs are duplicates")
+            context "with a set of other logs" do
+              let(:duplicate_set_id) { 100 }
+              let(:new_duplicates) { create_list(:lettings_log, 2, duplicate_set_id:) }
+
+              before do
+                allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.where(id: new_duplicates.pluck(:id)))
+                post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+              end
+
+              it "sets the logs duplicate set id to that of the set it is now part of" do
+                lettings_log.reload
+                expect(lettings_log.duplicate_set_id).to eql(duplicate_set_id)
+                new_duplicates.each do |log|
+                  log.reload
+                  expect(log.duplicate_set_id).to eql(duplicate_set_id)
+                end
+              end
+
+              it "redirects to the duplicate logs page" do
+                expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
+                follow_redirect!
+                expect(page).to have_content("These logs are duplicates")
+              end
+            end
+
+            context "when the log was previously in a different duplicate set" do
+              context "with a single other log" do
+                let(:old_duplicate_set_id) { 110 }
+                let!(:old_duplicate) { create(:lettings_log, duplicate_set_id: old_duplicate_set_id) }
+                let(:lettings_log) { create(:lettings_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
+                let(:new_duplicate) { create(:lettings_log) }
+
+                before do
+                  allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.where(id: new_duplicate.id))
+                  post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+                end
+
+                it "updates the relevant duplicate set ids" do
+                  lettings_log.reload
+                  old_duplicate.reload
+                  new_duplicate.reload
+                  expect(old_duplicate.duplicate_set_id).to be_nil
+                  expect(lettings_log.duplicate_set_id).not_to be_nil
+                  expect(lettings_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id)
+                end
+              end
+
+              context "with multiple other logs" do
+                let(:old_duplicate_set_id) { 120 }
+                let!(:old_duplicates) { create_list(:lettings_log, 2, duplicate_set_id: old_duplicate_set_id) }
+                let(:lettings_log) { create(:lettings_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
+                let(:new_duplicate) { create(:lettings_log) }
+
+                before do
+                  allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.where(id: new_duplicate.id))
+                  post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+                end
+
+                it "updates the relevant duplicate set ids" do
+                  lettings_log.reload
+                  new_duplicate.reload
+                  old_duplicates.each do |log|
+                    log.reload
+                    expect(log.duplicate_set_id).to eql(old_duplicate_set_id)
+                  end
+                  expect(lettings_log.duplicate_set_id).not_to be_nil
+                  expect(lettings_log.duplicate_set_id).not_to eql(old_duplicate_set_id)
+                  expect(lettings_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id)
+                end
+              end
+            end
+          end
+
+          context "and the answer makes the log stop being a duplicate" do
+            context "when the log had one duplicate" do
+              let(:old_duplicate_set_id) { 130 }
+              let!(:old_duplicate) { create(:lettings_log, duplicate_set_id: old_duplicate_set_id) }
+              let(:lettings_log) { create(:lettings_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
+
+              before do
+                allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.none)
+                post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+              end
+
+              it "updates the relevant duplicate set ids" do
+                lettings_log.reload
+                old_duplicate.reload
+                expect(old_duplicate.duplicate_set_id).to be_nil
+                expect(lettings_log.duplicate_set_id).to be_nil
+              end
+            end
+
+            context "when the log had multiple duplicates" do
+              let(:old_duplicate_set_id) { 140 }
+              let!(:old_duplicates) { create_list(:lettings_log, 2, duplicate_set_id: old_duplicate_set_id) }
+              let(:lettings_log) { create(:lettings_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
+
+              before do
+                allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.none)
+                post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+              end
+
+              it "updates the relevant duplicate set ids" do
+                lettings_log.reload
+                old_duplicates.each do |log|
+                  log.reload
+                  expect(log.duplicate_set_id).to eql(old_duplicate_set_id)
+                end
+                expect(lettings_log.duplicate_set_id).to be_nil
+              end
             end
           end
         end