diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb index 125671d02..45725c807 100644 --- a/app/controllers/form_controller.rb +++ b/app/controllers/form_controller.rb @@ -172,9 +172,9 @@ private end dynamic_duplicates = @log.lettings? ? current_user.lettings_logs.duplicate_logs(@log) : current_user.sales_logs.duplicate_logs(@log) - if dynamic_duplicates.count.positive? + if dynamic_duplicates.any? saved_duplicates = @log.duplicates - unless saved_duplicates == dynamic_duplicates + if saved_duplicates.none? || duplicates_changed?(dynamic_duplicates, saved_duplicates) @log.update!(duplicate_set_id: new_duplicate_set_id(@log)) if @log.duplicate_set_id.blank? dynamic_duplicates.each do |duplicate| duplicate.update!(duplicate_set_id: @log.duplicate_set_id) if duplicate.duplicate_set_id != @log.duplicate_set_id @@ -254,10 +254,21 @@ private class_name = @log.class.name.underscore original_log = current_user.send(class_name.pluralize).find_by(id: from_referrer_query("original_log_id")) + dynamic_duplicates = current_user.send(class_name.pluralize).duplicate_logs(@log) + + if dynamic_duplicates.any? + saved_duplicates = @log.duplicates + if 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) + saved_duplicates.first.update!(duplicate_set_id: nil) if saved_duplicates.count == 1 + end + else + remove_fixed_duplicate_set_ids(@log) + end - if original_log.present? && current_user.send(class_name.pluralize).duplicate_logs(original_log).count.positive? - unless current_user.send(class_name.pluralize).duplicate_logs(@log).count.positive? - remove_fixed_duplicate_set_ids(@log) + if original_log.present? && current_user.send(class_name.pluralize).duplicate_logs(original_log).any? + if dynamic_duplicates.none? flash[:notice] = deduplication_success_banner end send("#{class_name}_duplicate_logs_path", original_log, original_log_id: original_log.id, referrer: params[:referrer], organisation_id: params[:organisation_id]) @@ -300,4 +311,15 @@ private SalesLog.maximum(:duplicate_set_id).to_i + 1 end end + + def duplicates_changed?(dynamic_duplicates, saved_duplicates) + dynamic_duplicates.present? && saved_duplicates.present? && dynamic_duplicates.order(:id).pluck(:id) != saved_duplicates.order(:id).pluck(:id) + end + + def update_logs_with_duplicate_set_id(log, dynamic_duplicates, duplicate_set_id) + log.update!(duplicate_set_id:) + dynamic_duplicates.each do |duplicate| + duplicate.update!(duplicate_set_id: log.duplicate_set_id) if duplicate.duplicate_set_id != log.duplicate_set_id + end + end end diff --git a/spec/requests/form_controller_spec.rb b/spec/requests/form_controller_spec.rb index 54b52fc5a..4fe03baf3 100644 --- a/spec/requests/form_controller_spec.rb +++ b/spec/requests/form_controller_spec.rb @@ -896,14 +896,30 @@ RSpec.describe FormController, type: :request do } end - before do - post "/lettings-logs/#{lettings_log.id}/lead-tenant-age", params:, headers: headers.merge({ "HTTP_REFERER" => referrer }) + context "and the answer changes" do + before do + post "/lettings-logs/#{lettings_log.id}/lead-tenant-age", params:, headers: headers.merge({ "HTTP_REFERER" => referrer }) + end + + it "redirects back to the duplicates page for remaining duplicates" do + expect(response).to redirect_to("/lettings-logs/#{duplicate_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}") + expect(lettings_log.duplicates.count).to eq(0) + expect(duplicate_log.duplicates.count).to eq(0) + end end - it "redirects back to the duplicates page for remaining duplicates" do - expect(response).to redirect_to("/lettings-logs/#{duplicate_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}") - expect(lettings_log.duplicates.count).to eq(0) - expect(duplicate_log.duplicates.count).to eq(0) + context "and updating the answer creates a different set of duplicates" do + let!(:another_duplicate_log) { create(:lettings_log, :duplicate, created_by: user, age1_known: 1, age1: 20) } + + before do + post "/lettings-logs/#{lettings_log.id}/lead-tenant-age", params:, headers: headers.merge({ "HTTP_REFERER" => referrer }) + end + + it "correctly assigs duplicate set IDs" do + expect(lettings_log.reload.duplicates.count).to eq(1) + expect(lettings_log.duplicate_set_id).to eq(another_duplicate_log.reload.duplicate_set_id) + expect(duplicate_log.reload.duplicates.count).to eq(0) + end end context "and the answer didn't change" do @@ -918,6 +934,10 @@ RSpec.describe FormController, type: :request do } end + before do + post "/lettings-logs/#{lettings_log.id}/lead-tenant-age", params:, headers: headers.merge({ "HTTP_REFERER" => referrer }) + end + it "redirects back to the duplicates page for remaining duplicates" do expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}") expect(lettings_log.duplicates.count).to eq(1)