From 53ae216bae9f7d7be5b563fe9f02e8634f61c3a8 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Fri, 9 Aug 2024 15:59:12 +0100 Subject: [PATCH] Revert "CLDC-3566: Clear duplicate set ids when a log ceases to be a duplicate through normal update flow (#2534)" (#2563) This reverts commit e66d5ea1d0c380c7f7a550becec43b96518df762. --- app/controllers/form_controller.rb | 74 +++--- app/services/feature_toggle.rb | 4 + spec/requests/form_controller_spec.rb | 311 +++----------------------- 3 files changed, 78 insertions(+), 311 deletions(-) diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb index 8ed367e93..d84f8642a 100644 --- a/app/controllers/form_controller.rb +++ b/app/controllers/form_controller.rb @@ -26,8 +26,6 @@ class FormController < ApplicationController flash[:notice] = "You have successfully updated #{updated_question_string}" end - update_duplication_tracking - pages_requiring_update = pages_requiring_update(shown_page_ids_with_unanswered_questions_before_update) redirect_to(successful_redirect_path(pages_requiring_update)) else @@ -194,43 +192,24 @@ private params[@log.model_name.param_key]["interruption_page_referrer_type"].presence end - def update_duplication_tracking - class_name = @log.class.name.underscore - dynamic_duplicates = current_user.send(class_name.pluralize).duplicate_logs(@log) - - 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) - saved_duplicates.first.update!(duplicate_set_id: nil) if saved_duplicates.count == 1 - end - else - remove_fixed_duplicate_set_ids(@log) - end - end - def successful_redirect_path(pages_to_check) - class_name = @log.class.name.underscore - - if is_referrer_type?("duplicate_logs") || is_referrer_type?("duplicate_logs_banner") - original_log = current_user.send(class_name.pluralize).find_by(id: from_referrer_query("original_log_id")) + if FeatureToggle.deduplication_flow_enabled? + if is_referrer_type?("duplicate_logs") || is_referrer_type?("duplicate_logs_banner") + return correcting_duplicate_logs_redirect_path + end - if original_log.present? && current_user.send(class_name.pluralize).duplicate_logs(original_log).any? - if @log.duplicates.none? - flash[:notice] = deduplication_success_banner + dynamic_duplicates = @log.lettings? ? current_user.lettings_logs.duplicate_logs(@log) : current_user.sales_logs.duplicate_logs(@log) + 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) + saved_duplicates.first.update!(duplicate_set_id: nil) if saved_duplicates.count == 1 end - return send("#{class_name}_duplicate_logs_path", original_log, original_log_id: original_log.id, referrer: params[:referrer], organisation_id: params[:organisation_id]) - else - flash[:notice] = deduplication_success_banner - return send("#{class_name}_duplicate_logs_path", "#{class_name}_id".to_sym => from_referrer_query("first_remaining_duplicate_id"), original_log_id: from_referrer_query("original_log_id"), referrer: params[:referrer], organisation_id: params[:organisation_id]) + return send("#{@log.class.name.underscore}_duplicate_logs_path", @log, original_log_id: @log.id) end end - if @log.duplicates.any? - return send("#{@log.class.name.underscore}_duplicate_logs_path", @log, original_log_id: @log.id) - end - if is_referrer_type?("check_answers") next_page_id = form.next_page_id(@page, @log, current_user) next_page = form.get_page(next_page_id) @@ -336,6 +315,35 @@ private CONFIRMATION_PAGE_IDS = %w[uprn_confirmation uprn_selection].freeze + def correcting_duplicate_logs_redirect_path + 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).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]) + else + remove_fixed_duplicate_set_ids(original_log) + flash[:notice] = deduplication_success_banner + send("#{class_name}_duplicate_logs_path", "#{class_name}_id".to_sym => from_referrer_query("first_remaining_duplicate_id"), original_log_id: from_referrer_query("original_log_id"), referrer: params[:referrer], organisation_id: params[:organisation_id]) + end + end + def deduplication_success_banner deduplicated_log_link = "Log #{@log.id}" changed_labels = { diff --git a/app/services/feature_toggle.rb b/app/services/feature_toggle.rb index 91989ff86..810bfb845 100644 --- a/app/services/feature_toggle.rb +++ b/app/services/feature_toggle.rb @@ -11,6 +11,10 @@ class FeatureToggle !Rails.env.development? end + def self.deduplication_flow_enabled? + true + end + def self.duplicate_summary_enabled? true end diff --git a/spec/requests/form_controller_spec.rb b/spec/requests/form_controller_spec.rb index 80157e992..020dba3a4 100644 --- a/spec/requests/form_controller_spec.rb +++ b/spec/requests/form_controller_spec.rb @@ -763,162 +763,39 @@ RSpec.describe FormController, type: :request do } end - context "when the log will not be a duplicate" do - before do - post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params: - end - - it "re-renders the same page with errors if validation fails" do - expect(response).to have_http_status(:redirect) - end - - it "only updates answers that apply to the page being submitted" do - lettings_log.reload - expect(lettings_log.age1).to eq(answer) - expect(lettings_log.age2).to be nil - end - - it "tracks who updated the record" do - lettings_log.reload - whodunnit_actor = lettings_log.versions.last.actor - expect(whodunnit_actor).to be_a(User) - expect(whodunnit_actor.id).to eq(user.id) - end + before do + post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params: end - context "when 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(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 - - 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 + it "re-renders the same page with errors if validation fails" do + expect(response).to have_http_status(:redirect) + 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 + it "only updates answers that apply to the page being submitted" do + lettings_log.reload + expect(lettings_log.age1).to eq(answer) + expect(lettings_log.age2).to be nil end - context "when 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) } + it "tracks who updated the record" do + lettings_log.reload + whodunnit_actor = lettings_log.versions.last.actor + expect(whodunnit_actor).to be_a(User) + expect(whodunnit_actor.id).to eq(user.id) + end - before do - allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.none) - post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params: - end + context "and duplicate logs" do + let(:duplicate_logs) { create_list(:lettings_log, 2) } - 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 + before do + allow(LettingsLog).to receive(:duplicate_logs).and_return(duplicate_logs) + post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params: 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 + 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 end @@ -939,141 +816,19 @@ RSpec.describe FormController, type: :request do }, } end - let(:page_id) { "buyer_1_age" } - - context "when the answer makes the log a duplicate" do - context "with one other log" do - let(:new_duplicate) { create(:sales_log) } - - before do - allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.where(id: new_duplicate.id)) - post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params: - end - - it "sets a new duplicate set id on both logs" do - sales_log.reload - new_duplicate.reload - expect(sales_log.duplicate_set_id).not_to be_nil - expect(sales_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("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}") - follow_redirect! - expect(page).to have_content("These logs are duplicates") - end - end - - context "with a set of other logs" do - let(:duplicate_set_id) { 100 } - let(:new_duplicates) { create_list(:sales_log, 2, duplicate_set_id:) } - - before do - allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.where(id: new_duplicates.pluck(:id))) - post "/sales-logs/#{sales_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 - sales_log.reload - expect(sales_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("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_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(:sales_log, duplicate_set_id: old_duplicate_set_id) } - let(:sales_log) { create(:sales_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) } - let(:new_duplicate) { create(:sales_log) } - - before do - allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.where(id: new_duplicate.id)) - post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params: - end - - it "updates the relevant duplicate set ids" do - sales_log.reload - old_duplicate.reload - new_duplicate.reload - expect(old_duplicate.duplicate_set_id).to be_nil - expect(sales_log.duplicate_set_id).not_to be_nil - expect(sales_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(:sales_log, 2, duplicate_set_id: old_duplicate_set_id) } - let(:sales_log) { create(:sales_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) } - let(:new_duplicate) { create(:sales_log) } - - before do - allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.where(id: new_duplicate.id)) - post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params: - end - - it "updates the relevant duplicate set ids" do - sales_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(sales_log.duplicate_set_id).not_to be_nil - expect(sales_log.duplicate_set_id).not_to eql(old_duplicate_set_id) - expect(sales_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id) - end - end - end - end - - context "when 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(:sales_log, duplicate_set_id: old_duplicate_set_id) } - let(:sales_log) { create(:sales_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) } - before do - allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.none) - post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params: - end + context "and duplicate logs" do + let!(:duplicate_logs) { create_list(:sales_log, 2) } - it "updates the relevant duplicate set ids" do - sales_log.reload - old_duplicate.reload - expect(old_duplicate.duplicate_set_id).to be_nil - expect(sales_log.duplicate_set_id).to be_nil - end + before do + allow(SalesLog).to receive(:duplicate_logs).and_return(duplicate_logs) + post "/sales-logs/#{sales_log.id}/buyer-1-age", params: end - context "when the log had multiple duplicates" do - let(:old_duplicate_set_id) { 140 } - let!(:old_duplicates) { create_list(:sales_log, 2, duplicate_set_id: old_duplicate_set_id) } - let(:sales_log) { create(:sales_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) } - - before do - allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.none) - post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params: - end - - it "updates the relevant duplicate set ids" do - sales_log.reload - old_duplicates.each do |log| - log.reload - expect(log.duplicate_set_id).to eql(old_duplicate_set_id) - end - expect(sales_log.duplicate_set_id).to be_nil - end + it "redirects to the duplicate logs page" do + expect(response).to redirect_to("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}") + follow_redirect! + expect(page).to have_content("These logs are duplicates") end end end