From f6dafd31c62c1c92ab61e3ed874139433c22a0af Mon Sep 17 00:00:00 2001 From: Kat Date: Wed, 17 Jan 2024 08:39:26 +0000 Subject: [PATCH] Add missing sales deduplication steps --- app/controllers/delete_logs_controller.rb | 5 +++++ app/controllers/form_controller.rb | 22 +++++++----------- spec/features/sales_log_spec.rb | 27 +++++++++++++++++++++++ 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/app/controllers/delete_logs_controller.rb b/app/controllers/delete_logs_controller.rb index 2a56944dc..fd3a0ee3b 100644 --- a/app/controllers/delete_logs_controller.rb +++ b/app/controllers/delete_logs_controller.rb @@ -61,6 +61,11 @@ class DeleteLogsController < ApplicationController logs = SalesLog.find(params.require(:ids)) discard logs if request.referer&.include?("delete-duplicates") + logs.each do |log| + log.duplicate_log_references.destroy_all + end + SalesLog.find(params["remaining_log_id"]).duplicate_log_references.destroy_all + redirect_to sales_log_duplicate_logs_path(sales_log_id: params["remaining_log_id"], original_log_id: params["original_log_id"], referrer: params[:referrer], organisation_id: params[:organisation_id]), notice: I18n.t("notification.duplicate_logs_deleted", count: logs.count, log_ids: duplicate_log_ids(logs)) else redirect_to sales_logs_path, notice: I18n.t("notification.logs_deleted", count: logs.count) diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb index 38db72aa5..4733fbd49 100644 --- a/app/controllers/form_controller.rb +++ b/app/controllers/form_controller.rb @@ -171,23 +171,17 @@ private return correcting_duplicate_logs_redirect_path end - if @log.lettings? - dynamic_duplicates = current_user.lettings_logs.duplicate_logs(@log) - if dynamic_duplicates.count.positive? - saved_duplicates = @log.duplicates - unless saved_duplicates == dynamic_duplicates - duplicate_log_reference = DuplicateLogReference.find_or_create_by!(log_id: @log.id, log_type: "LettingsLog") - dynamic_duplicates.each do |duplicate| - DuplicateLogReference.find_or_create_by!(log_id: duplicate.id, log_type: "LettingsLog", duplicate_set_id: duplicate_log_reference.duplicate_set_id) - end - return send("lettings_log_duplicate_logs_path", @log, original_log_id: @log.id) + dynamic_duplicates = @log.lettings? ? current_user.lettings_logs.duplicate_logs(@log) : current_user.sales_logs.duplicate_logs(@log) + if dynamic_duplicates.count.positive? + saved_duplicates = @log.duplicates + unless saved_duplicates == dynamic_duplicates + duplicate_log_reference = DuplicateLogReference.find_or_create_by!(log_id: @log.id, log_type: @log.class.name) + dynamic_duplicates.each do |duplicate| + DuplicateLogReference.find_or_create_by!(log_id: duplicate.id, log_type: @log.class.name, duplicate_set_id: duplicate_log_reference.duplicate_set_id) end + return send("#{@log.class.name.underscore}_duplicate_logs_path", @log, original_log_id: @log.id) end end - - if @log.sales? && current_user.sales_logs.duplicate_logs(@log).count.positive? - return send("sales_log_duplicate_logs_path", @log, original_log_id: @log.id) - end end if is_referrer_type?("check_answers") diff --git a/spec/features/sales_log_spec.rb b/spec/features/sales_log_spec.rb index bac9da761..3bfaf3705 100644 --- a/spec/features/sales_log_spec.rb +++ b/spec/features/sales_log_spec.rb @@ -198,6 +198,11 @@ RSpec.describe "Sales Log Features" do end it "allows keeping the original log and deleting duplicates" do + expect(DuplicateLogReference.count).to eq(2) + sales_log.reload + expect(sales_log.duplicates.count).to eq(1) + expect(sales_log.duplicates).to include(duplicate_log) + expect(page).to have_current_path("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}") click_link("Keep this log and delete duplicates", href: "/sales-logs/#{sales_log.id}/delete-duplicates?original_log_id=#{sales_log.id}") expect(page).to have_current_path("/sales-logs/#{sales_log.id}/delete-duplicates?original_log_id=#{sales_log.id}") @@ -210,6 +215,9 @@ RSpec.describe "Sales Log Features" do expect(page).not_to have_content("These logs are duplicates") expect(page).not_to have_link("Keep this log and delete duplicates") expect(page).to have_link("Back to Log #{sales_log.id}", href: "/sales-logs/#{sales_log.id}") + + expect(DuplicateLogReference.count).to eq(0) + expect(sales_log.duplicates.count).to eq(0) end it "allows changing answer on remaining original log" do @@ -222,6 +230,11 @@ RSpec.describe "Sales Log Features" do end it "allows keeping the duplicate log and deleting the original one" do + expect(DuplicateLogReference.count).to eq(2) + duplicate_log.reload + expect(duplicate_log.duplicates.count).to eq(1) + expect(duplicate_log.duplicates).to include(sales_log) + expect(page).to have_current_path("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}") click_link("Keep this log and delete duplicates", href: "/sales-logs/#{duplicate_log.id}/delete-duplicates?original_log_id=#{sales_log.id}") expect(page).to have_current_path("/sales-logs/#{duplicate_log.id}/delete-duplicates?original_log_id=#{sales_log.id}") @@ -234,6 +247,9 @@ RSpec.describe "Sales Log Features" do expect(page).not_to have_content("These logs are duplicates") expect(page).not_to have_link("Keep this log and delete duplicates") expect(page).to have_link("Back to sales logs", href: "/sales-logs") + + expect(DuplicateLogReference.count).to eq(0) + expect(duplicate_log.duplicates.count).to eq(0) end it "allows changing answers on remaining duplicate log" do @@ -246,6 +262,11 @@ RSpec.describe "Sales Log Features" do end it "allows deduplicating logs by changing the answers on the duplicate log" do + expect(DuplicateLogReference.count).to eq(2) + sales_log.reload + expect(sales_log.duplicates.count).to eq(1) + expect(sales_log.duplicates).to include(duplicate_log) + expect(page).to have_current_path("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}") click_link("Change", href: "/sales-logs/#{duplicate_log.id}/purchaser-code?first_remaining_duplicate_id=#{sales_log.id}&original_log_id=#{sales_log.id}&referrer=duplicate_logs") fill_in("sales-log-purchid-field", with: "something else") @@ -255,6 +276,9 @@ RSpec.describe "Sales Log Features" do expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success") expect(page).to have_content("Log #{duplicate_log.id} is no longer a duplicate and has been removed from the list") expect(page).to have_content("You changed the purchaser code.") + + expect(DuplicateLogReference.count).to eq(0) + expect(duplicate_log.duplicates.count).to eq(0) end it "allows deduplicating logs by changing the answers on the original log" do @@ -266,6 +290,9 @@ RSpec.describe "Sales Log Features" do expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success") expect(page).to have_content("Log #{sales_log.id} is no longer a duplicate and has been removed from the list") expect(page).to have_content("You changed the purchaser code.") + + expect(DuplicateLogReference.count).to eq(0) + expect(duplicate_log.duplicates.count).to eq(0) end end end