From 8be383e6844466748059d1c57abf36ebf3ef7f71 Mon Sep 17 00:00:00 2001 From: Kat Date: Wed, 9 Aug 2023 08:27:27 +0100 Subject: [PATCH] Add owning organisation to deduplication fields, only return back to the original log if it still belongs to the org --- app/controllers/form_controller.rb | 13 +++++++++++-- app/models/lettings_log.rb | 2 +- app/models/sales_log.rb | 2 +- spec/models/lettings_log_spec.rb | 31 +++++++++++++++--------------- spec/models/sales_log_spec.rb | 29 ++++++++++++++-------------- 5 files changed, 44 insertions(+), 33 deletions(-) diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb index 2062cf406..3df834add 100644 --- a/app/controllers/form_controller.rb +++ b/app/controllers/form_controller.rb @@ -247,7 +247,7 @@ private original_log = current_user.send(class_name.pluralize).find_by(id: from_referrer_query("original_log_id")) - if current_user.send(class_name.pluralize).duplicate_logs(original_log).count.positive? + if original_log.present? && current_user.send(class_name.pluralize).duplicate_logs(original_log).count.positive? flash[:notice] = deduplication_success_banner unless current_user.send(class_name.pluralize).duplicate_logs(@log).count.positive? send("#{class_name}_duplicate_logs_path", original_log, original_log_id: original_log.id) else @@ -258,7 +258,16 @@ private def deduplication_success_banner deduplicated_log_link = "Log #{@log.id}" - changed_question_label = (@page.questions.first.check_answer_label.to_s.presence || @page.questions.first.header.to_s).downcase + changed_labels = { + property_postcode: "postcode", + lead_tenant_age: "lead tenant’s age", + rent_4_weekly: "household rent and charges", + rent_bi_weekly: "household rent and charges", + rent_monthly: "household rent and charges", + rent_or_other_charges: "household rent and charges", + address: "postcode", + } + changed_question_label = changed_labels[@page.id.to_sym] || (@page.questions.first.check_answer_label.to_s.presence || @page.questions.first.header.to_s).downcase I18n.t("notification.duplicate_logs.deduplication_success_banner", log_link: deduplicated_log_link, changed_question_label:).html_safe end diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index 2f11e31ff..6104ee3d5 100644 --- a/app/models/lettings_log.rb +++ b/app/models/lettings_log.rb @@ -79,7 +79,7 @@ class LettingsLog < Log NUM_OF_WEEKS_FROM_PERIOD = { 2 => 26, 3 => 13, 4 => 12, 5 => 50, 6 => 49, 7 => 48, 8 => 47, 9 => 46, 1 => 52, 10 => 53 }.freeze SUFFIX_FROM_PERIOD = { 2 => "every 2 weeks", 3 => "every 4 weeks", 4 => "every month" }.freeze RETIREMENT_AGES = { "M" => 67, "F" => 60, "X" => 67 }.freeze - DUPLICATE_LOG_ATTRIBUTES = %w[tenancycode startdate age1_known age1 sex1 ecstat1 tcharge household_charge chcharge].freeze + DUPLICATE_LOG_ATTRIBUTES = %w[owning_organisation_id tenancycode startdate age1_known age1 sex1 ecstat1 tcharge household_charge chcharge].freeze def form FormHandler.instance.get_form(form_name) || FormHandler.instance.current_lettings_form diff --git a/app/models/sales_log.rb b/app/models/sales_log.rb index 2a0a6d5c4..4e829d760 100644 --- a/app/models/sales_log.rb +++ b/app/models/sales_log.rb @@ -55,7 +55,7 @@ class SalesLog < Log OPTIONAL_FIELDS = %w[purchid othtype].freeze RETIREMENT_AGES = { "M" => 65, "F" => 60, "X" => 65 }.freeze - DUPLICATE_LOG_ATTRIBUTES = %w[purchid saledate age1_known age1 sex1 ecstat1 postcode_full].freeze + DUPLICATE_LOG_ATTRIBUTES = %w[owning_organisation_id purchid saledate age1_known age1 sex1 ecstat1 postcode_full].freeze def lettings? false diff --git a/spec/models/lettings_log_spec.rb b/spec/models/lettings_log_spec.rb index b34377543..7eceb9b5d 100644 --- a/spec/models/lettings_log_spec.rb +++ b/spec/models/lettings_log_spec.rb @@ -2831,8 +2831,9 @@ RSpec.describe LettingsLog do end context "when filtering duplicate logs" do - let(:log) { create(:lettings_log, :duplicate) } - let!(:duplicate_log) { create(:lettings_log, :duplicate) } + let(:organisation) { create(:organisation) } + let(:log) { create(:lettings_log, :duplicate, owning_organisation: organisation) } + let!(:duplicate_log) { create(:lettings_log, :duplicate, owning_organisation: organisation) } it "returns all duplicate logs for given log" do expect(described_class.duplicate_logs(log).count).to eq(1) @@ -2847,7 +2848,7 @@ RSpec.describe LettingsLog do end context "when there is a deleted duplicate log" do - let!(:deleted_duplicate_log) { create(:lettings_log, :duplicate, discarded_at: Time.zone.now) } + let!(:deleted_duplicate_log) { create(:lettings_log, :duplicate, discarded_at: Time.zone.now, owning_organisation: organisation) } it "does not return the deleted log as a duplicate" do expect(described_class.duplicate_logs(log)).not_to include(deleted_duplicate_log) @@ -2855,7 +2856,7 @@ RSpec.describe LettingsLog do end context "when there is a log with a different start date" do - let!(:different_start_date_log) { create(:lettings_log, :duplicate, startdate: Time.zone.tomorrow) } + let!(:different_start_date_log) { create(:lettings_log, :duplicate, startdate: Time.zone.tomorrow, owning_organisation: organisation) } it "does not return a log with a different start date as a duplicate" do expect(described_class.duplicate_logs(log)).not_to include(different_start_date_log) @@ -2863,7 +2864,7 @@ RSpec.describe LettingsLog do end context "when there is a log with a different age1" do - let!(:different_age1) { create(:lettings_log, :duplicate, age1: 50) } + let!(:different_age1) { create(:lettings_log, :duplicate, age1: 50, owning_organisation: organisation) } it "does not return a log with a different age1 as a duplicate" do expect(described_class.duplicate_logs(log)).not_to include(different_age1) @@ -2871,7 +2872,7 @@ RSpec.describe LettingsLog do end context "when there is a log with a different sex1" do - let!(:different_sex1) { create(:lettings_log, :duplicate, sex1: "F") } + let!(:different_sex1) { create(:lettings_log, :duplicate, sex1: "F", owning_organisation: organisation) } it "does not return a log with a different sex1 as a duplicate" do expect(described_class.duplicate_logs(log)).not_to include(different_sex1) @@ -2879,7 +2880,7 @@ RSpec.describe LettingsLog do end context "when there is a log with a different ecstat1" do - let!(:different_ecstat1) { create(:lettings_log, :duplicate, ecstat1: 1) } + let!(:different_ecstat1) { create(:lettings_log, :duplicate, ecstat1: 1, owning_organisation: organisation) } it "does not return a log with a different ecstat1 as a duplicate" do expect(described_class.duplicate_logs(log)).not_to include(different_ecstat1) @@ -2887,7 +2888,7 @@ RSpec.describe LettingsLog do end context "when there is a log with a different tcharge" do - let!(:different_tcharge) { create(:lettings_log, :duplicate, brent: 100) } + let!(:different_tcharge) { create(:lettings_log, :duplicate, brent: 100, owning_organisation: organisation) } it "does not return a log with a different tcharge as a duplicate" do expect(described_class.duplicate_logs(log)).not_to include(different_tcharge) @@ -2895,7 +2896,7 @@ RSpec.describe LettingsLog do end context "when there is a log with a different tenancycode" do - let!(:different_tenancycode) { create(:lettings_log, :duplicate, tenancycode: "different") } + let!(:different_tenancycode) { create(:lettings_log, :duplicate, tenancycode: "different", owning_organisation: organisation) } it "does not return a log with a different tenancycode as a duplicate" do expect(described_class.duplicate_logs(log)).not_to include(different_tenancycode) @@ -2903,7 +2904,7 @@ RSpec.describe LettingsLog do end context "when there is a log with a different postcode_full" do - let!(:different_postcode_full) { create(:lettings_log, :duplicate, postcode_full: "B1 1AA") } + let!(:different_postcode_full) { create(:lettings_log, :duplicate, postcode_full: "B1 1AA", owning_organisation: organisation) } it "does not return a log with a different postcode_full as a duplicate" do expect(described_class.duplicate_logs(log)).not_to include(different_postcode_full) @@ -2911,7 +2912,7 @@ RSpec.describe LettingsLog do end context "when there is a log with nil values for duplicate check fields" do - let!(:duplicate_check_fields_not_given) { create(:lettings_log, :duplicate, age1: nil, sex1: nil, ecstat1: nil, postcode_known: 2, postcode_full: nil) } + let!(:duplicate_check_fields_not_given) { create(:lettings_log, :duplicate, age1: nil, sex1: nil, ecstat1: nil, postcode_known: 2, postcode_full: nil, owning_organisation: organisation) } it "does not return a log with nil values as a duplicate" do log.update!(age1: nil, sex1: nil, ecstat1: nil, postcode_known: 2, postcode_full: nil) @@ -2920,7 +2921,7 @@ RSpec.describe LettingsLog do end context "when there is a log with nil values for tenancycode" do - let!(:tenancycode_not_given) { create(:lettings_log, :duplicate, tenancycode: nil) } + let!(:tenancycode_not_given) { create(:lettings_log, :duplicate, tenancycode: nil, owning_organisation: organisation) } it "returns the log as a duplicate if tenancy code is nil" do log.update!(tenancycode: nil) @@ -2929,7 +2930,7 @@ RSpec.describe LettingsLog do end context "when there is a log with age1 not known" do - let!(:age1_not_known) { create(:lettings_log, :duplicate, age1_known: 1, age1: nil) } + let!(:age1_not_known) { create(:lettings_log, :duplicate, age1_known: 1, age1: nil, owning_organisation: organisation) } it "returns the log as a duplicate if age1 is not known" do log.update!(age1_known: 1, age1: nil) @@ -2941,8 +2942,8 @@ RSpec.describe LettingsLog do let(:scheme) { create(:scheme) } let(:location) { create(:location, scheme:) } let(:location_2) { create(:location, scheme:) } - let(:supported_housing_log) { create(:lettings_log, :duplicate, needstype: 2, location:, scheme:) } - let!(:duplicate_supported_housing_log) { create(:lettings_log, :duplicate, needstype: 2, location:, scheme:) } + let(:supported_housing_log) { create(:lettings_log, :duplicate, needstype: 2, location:, scheme:, owning_organisation: organisation) } + let!(:duplicate_supported_housing_log) { create(:lettings_log, :duplicate, needstype: 2, location:, scheme:, owning_organisation: organisation) } it "returns the log as a duplicate" do expect(described_class.duplicate_logs(supported_housing_log)).to include(duplicate_supported_housing_log) diff --git a/spec/models/sales_log_spec.rb b/spec/models/sales_log_spec.rb index d032293c8..1aa27cbd3 100644 --- a/spec/models/sales_log_spec.rb +++ b/spec/models/sales_log_spec.rb @@ -161,8 +161,9 @@ RSpec.describe SalesLog, type: :model do end context "when filtering duplicate logs" do - let(:log) { create(:sales_log, :duplicate) } - let!(:duplicate_log) { create(:sales_log, :duplicate) } + let(:organisation) { create(:organisation) } + let(:log) { create(:sales_log, :duplicate, owning_organisation: organisation) } + let!(:duplicate_log) { create(:sales_log, :duplicate, owning_organisation: organisation) } it "returns all duplicate logs for given log" do expect(described_class.duplicate_logs(log).count).to eq(1) @@ -177,7 +178,7 @@ RSpec.describe SalesLog, type: :model do end context "when there is a deleted duplicate log" do - let!(:deleted_duplicate_log) { create(:sales_log, :duplicate, discarded_at: Time.zone.now) } + let!(:deleted_duplicate_log) { create(:sales_log, :duplicate, discarded_at: Time.zone.now, owning_organisation: organisation) } it "does not return the deleted log as a duplicate" do expect(described_class.duplicate_logs(log)).not_to include(deleted_duplicate_log) @@ -185,7 +186,7 @@ RSpec.describe SalesLog, type: :model do end context "when there is a log with a different sale date" do - let!(:different_sale_date_log) { create(:sales_log, :duplicate, saledate: Time.zone.tomorrow) } + let!(:different_sale_date_log) { create(:sales_log, :duplicate, saledate: Time.zone.tomorrow, owning_organisation: organisation) } it "does not return a log with a different sale date as a duplicate" do expect(described_class.duplicate_logs(log)).not_to include(different_sale_date_log) @@ -193,7 +194,7 @@ RSpec.describe SalesLog, type: :model do end context "when there is a log with a different age1" do - let!(:different_age1) { create(:sales_log, :duplicate, age1: 50) } + let!(:different_age1) { create(:sales_log, :duplicate, age1: 50, owning_organisation: organisation) } it "does not return a log with a different age1 as a duplicate" do expect(described_class.duplicate_logs(log)).not_to include(different_age1) @@ -201,7 +202,7 @@ RSpec.describe SalesLog, type: :model do end context "when there is a log with a different sex1" do - let!(:different_sex1) { create(:sales_log, :duplicate, sex1: "M") } + let!(:different_sex1) { create(:sales_log, :duplicate, sex1: "M", owning_organisation: organisation) } it "does not return a log with a different sex1 as a duplicate" do expect(described_class.duplicate_logs(log)).not_to include(different_sex1) @@ -209,7 +210,7 @@ RSpec.describe SalesLog, type: :model do end context "when there is a log with a different ecstat1" do - let!(:different_ecstat1) { create(:sales_log, :duplicate, ecstat1: 0) } + let!(:different_ecstat1) { create(:sales_log, :duplicate, ecstat1: 0, owning_organisation: organisation) } it "does not return a log with a different ecstat1 as a duplicate" do expect(described_class.duplicate_logs(log)).not_to include(different_ecstat1) @@ -217,7 +218,7 @@ RSpec.describe SalesLog, type: :model do end context "when there is a log with a different purchid" do - let!(:different_purchid) { create(:sales_log, :duplicate, purchid: "different") } + let!(:different_purchid) { create(:sales_log, :duplicate, purchid: "different", owning_organisation: organisation) } it "does not return a log with a different purchid as a duplicate" do expect(described_class.duplicate_logs(log)).not_to include(different_purchid) @@ -225,7 +226,7 @@ RSpec.describe SalesLog, type: :model do end context "when there is a log with a different postcode_full" do - let!(:different_postcode_full) { create(:sales_log, :duplicate, postcode_full: "B1 1AA") } + let!(:different_postcode_full) { create(:sales_log, :duplicate, postcode_full: "B1 1AA", owning_organisation: organisation) } it "does not return a log with a different postcode_full as a duplicate" do expect(described_class.duplicate_logs(log)).not_to include(different_postcode_full) @@ -233,7 +234,7 @@ RSpec.describe SalesLog, type: :model do end context "when there is a log with nil values for duplicate check fields" do - let!(:duplicate_check_fields_not_given) { create(:sales_log, :duplicate, age1: nil, sex1: nil, ecstat1: nil, pcodenk: 1, postcode_full: nil) } + let!(:duplicate_check_fields_not_given) { create(:sales_log, :duplicate, age1: nil, sex1: nil, ecstat1: nil, pcodenk: 1, postcode_full: nil, owning_organisation: organisation) } it "does not return a log with nil values as a duplicate" do log.update!(age1: nil, sex1: nil, ecstat1: nil, pcodenk: 1, postcode_full: nil) @@ -242,7 +243,7 @@ RSpec.describe SalesLog, type: :model do end context "when there is a log with nil values for purchid" do - let!(:purchid_not_given) { create(:sales_log, :duplicate, purchid: nil) } + let!(:purchid_not_given) { create(:sales_log, :duplicate, purchid: nil, owning_organisation: organisation) } it "returns the log as a duplicate if purchid is nil" do log.update!(purchid: nil) @@ -251,7 +252,7 @@ RSpec.describe SalesLog, type: :model do end context "when there is a log age not known" do - let!(:age1_not_known) { create(:sales_log, :duplicate, age1_known: 1, age1: nil) } + let!(:age1_not_known) { create(:sales_log, :duplicate, age1_known: 1, age1: nil, owning_organisation: organisation) } it "returns the log as a duplicate if age is not known" do log.update!(age1_known: 1, age1: nil) @@ -260,7 +261,7 @@ RSpec.describe SalesLog, type: :model do end context "when there is a log age pefers not to say" do - let!(:age1_prefers_not_to_say) { create(:sales_log, :duplicate, age1_known: 2, age1: nil) } + let!(:age1_prefers_not_to_say) { create(:sales_log, :duplicate, age1_known: 2, age1: nil, owning_organisation: organisation) } it "returns the log as a duplicate if age is prefers not to say" do log.update!(age1_known: 2, age1: nil) @@ -269,7 +270,7 @@ RSpec.describe SalesLog, type: :model do end context "when there is a log age pefers not to say and not known" do - let!(:age1_prefers_not_to_say) { create(:sales_log, :duplicate, age1_known: 2, age1: nil) } + let!(:age1_prefers_not_to_say) { create(:sales_log, :duplicate, age1_known: 2, age1: nil, owning_organisation: organisation) } it "does not return the log as a duplicate if age is prefers not to say" do log.update!(age1_known: 1, age1: nil)