From 98f97035817f17231b46d4d652e1b3e9e96cae15 Mon Sep 17 00:00:00 2001 From: Rachael Booth Date: Thu, 25 Jul 2024 17:30:40 +0100 Subject: [PATCH] CLDC-3566: Clear duplicate set ids when a log ceases to be a duplicate in normal updates --- app/controllers/form_controller.rb | 7 +- spec/requests/form_controller_spec.rb | 139 ++++++++++++++++++++++++-- 2 files changed, 136 insertions(+), 10 deletions(-) diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb index d84f8642a..95028c5e8 100644 --- a/app/controllers/form_controller.rb +++ b/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") diff --git a/spec/requests/form_controller_spec.rb b/spec/requests/form_controller_spec.rb index 020dba3a4..7d1933b30 100644 --- a/spec/requests/form_controller_spec.rb +++ b/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