diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index 45f25eca3..9ab6329d2 100644 --- a/app/models/lettings_log.rb +++ b/app/models/lettings_log.rb @@ -58,8 +58,10 @@ class LettingsLog < Log scope :age1_answered, -> { where.not(age1: nil).or(where(age1_known: 1)) } scope :tcharge_answered, -> { where.not(tcharge: nil).or(where(household_charge: 1)).or(where(is_carehome: 1)) } scope :chcharge_answered, -> { where.not(chcharge: nil).or(where(is_carehome: [nil, 0])) } - scope :location_answered, ->(log) { where(location_id: log.location_id).or(where(needstype: 1)) } - scope :postcode_answered, ->(log) { where(postcode_full: log.postcode_full).or(where(needstype: 2)) } + scope :location_for_log_answered, ->(log) { where(location_id: log.location_id).or(where(needstype: 1)) } + scope :postcode_for_log_answered, ->(log) { where(postcode_full: log.postcode_full).or(where(needstype: 2)) } + scope :location_answered, -> { where.not(location_id: nil).or(where(needstype: 1)) } + scope :postcode_answered, -> { where.not(postcode_full: nil).or(where(needstype: 2)) } scope :duplicate_logs, lambda { |log| visible .where.not(id: log.id) @@ -70,32 +72,31 @@ class LettingsLog < Log .age1_answered .tcharge_answered .chcharge_answered - .location_answered(log) - .postcode_answered(log) + .location_for_log_answered(log) + .postcode_for_log_answered(log) .where(log.slice(*DUPLICATE_LOG_ATTRIBUTES)) } - scope :duplicates, lambda { |created_by_id = nil| + scope :duplicate_sets, lambda { |created_by_id = nil| scope = visible - .group(*DUPLICATE_LOG_ATTRIBUTES) - .having("COUNT(*) > 1") + .group(*DUPLICATE_LOG_ATTRIBUTES, :postcode_full, :location_id) + .where.not(startdate: nil) + .where.not(sex1: nil) + .where.not(ecstat1: nil) + .where.not(needstype: nil) + .age1_answered + .tcharge_answered + .chcharge_answered + .location_answered + .postcode_answered + .having( + "COUNT(*) > 1", + ) if created_by_id - scope = scope.where("EXISTS ( - SELECT 1 - FROM lettings_logs AS inner_logs - WHERE lettings_logs.tenancycode = inner_logs.tenancycode - AND lettings_logs.startdate = inner_logs.startdate - AND lettings_logs.age1_known = inner_logs.age1_known - AND lettings_logs.age1 = inner_logs.age1 - AND lettings_logs.sex1 = inner_logs.sex1 - AND lettings_logs.ecstat1 = inner_logs.ecstat1 - AND lettings_logs.tcharge = inner_logs.tcharge - AND inner_logs.created_by_id = ? - )", created_by_id) + scope = scope.having("MAX(CASE WHEN created_by_id = ? THEN 1 ELSE 0 END) >= 1", created_by_id) end - - scope + scope.pluck("ARRAY_AGG(id)") } AUTOGENERATED_FIELDS = %w[id status created_at updated_at discarded_at].freeze diff --git a/app/models/organisation.rb b/app/models/organisation.rb index d776ed09f..77b5af50c 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -134,14 +134,10 @@ class Organisation < ApplicationRecord end def duplicate_lettings_logs_sets - duplicate_sets = lettings_logs.duplicates.pluck("ARRAY_AGG(id)") - - duplicate_sets.map { |array_str| array_str ? array_str.map(&:to_i) : [] } + lettings_logs.duplicate_sets.map { |array_str| array_str ? array_str.map(&:to_i) : [] } end def duplicate_sales_logs_sets - duplicate_sets = sales_logs.duplicates.pluck("ARRAY_AGG(id)") - - duplicate_sets.map { |array_str| array_str ? array_str.map(&:to_i) : [] } + sales_logs.duplicate_sets.map { |array_str| array_str ? array_str.map(&:to_i) : [] } end end diff --git a/app/models/sales_log.rb b/app/models/sales_log.rb index 240b38105..283485d11 100644 --- a/app/models/sales_log.rb +++ b/app/models/sales_log.rb @@ -54,27 +54,21 @@ class SalesLog < Log } scope :after_date, ->(date) { where("saledate >= ?", date) } - scope :duplicates, lambda { |created_by_id = nil| + scope :duplicate_sets, lambda { |created_by_id = nil| scope = visible .group(*DUPLICATE_LOG_ATTRIBUTES) - .having("COUNT(*) > 1") + .where.not(saledate: nil) + .where.not(sex1: nil) + .where.not(ecstat1: nil) + .where.not(postcode_full: nil) + .age1_answered + .having("COUNT(*) > 1") if created_by_id - scope = scope.where("EXISTS ( - SELECT 1 - FROM sales_logs AS inner_logs - WHERE sales_logs.purchid = inner_logs.purchid - AND sales_logs.saledate = inner_logs.saledate - AND sales_logs.age1_known = inner_logs.age1_known - AND sales_logs.age1 = inner_logs.age1 - AND sales_logs.sex1 = inner_logs.sex1 - AND sales_logs.ecstat1 = inner_logs.ecstat1 - AND sales_logs.postcode_full = inner_logs.postcode_full - AND inner_logs.created_by_id = ? - )", created_by_id) + scope = scope.having("MAX(CASE WHEN created_by_id = ? THEN 1 ELSE 0 END) >= 1", created_by_id) end - scope + scope.pluck("ARRAY_AGG(id)") } OPTIONAL_FIELDS = %w[purchid othtype].freeze diff --git a/app/models/user.rb b/app/models/user.rb index 0e3ed7578..5b7ec3e2f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -204,15 +204,11 @@ class User < ApplicationRecord end def duplicate_lettings_logs_sets - duplicate_sets = lettings_logs.duplicates(id).pluck("ARRAY_AGG(id)") - - duplicate_sets.map { |array_str| array_str ? array_str.map(&:to_i) : [] } + lettings_logs.duplicate_sets(id).map { |array_str| array_str ? array_str.map(&:to_i) : [] } end def duplicate_sales_logs_sets - duplicate_sets = sales_logs.duplicates(id).pluck("ARRAY_AGG(id)") - - duplicate_sets.map { |array_str| array_str ? array_str.map(&:to_i) : [] } + sales_logs.duplicate_sets(id).map { |array_str| array_str ? array_str.map(&:to_i) : [] } end protected diff --git a/spec/models/lettings_log_spec.rb b/spec/models/lettings_log_spec.rb index 489a31b9b..40f2ff355 100644 --- a/spec/models/lettings_log_spec.rb +++ b/spec/models/lettings_log_spec.rb @@ -2978,6 +2978,189 @@ RSpec.describe LettingsLog do end end end + + context "when getting list of duplicate logs" do + let(:organisation) { create(:organisation) } + let!(:log) { create(:lettings_log, :duplicate, owning_organisation: organisation) } + let!(:duplicate_log) { create(:lettings_log, :duplicate, owning_organisation: organisation) } + let(:duplicate_sets) { described_class.duplicate_sets } + + it "returns a list of duplicates in the same organisation" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + end + + context "when there is a deleted duplicate log" do + before do + create(:lettings_log, :duplicate, discarded_at: Time.zone.now, status: 4) + end + + it "does not return the deleted log as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + end + end + + context "when there is a log with a different start date" do + before do + create(:lettings_log, :duplicate, startdate: Time.zone.tomorrow) + end + + it "does not return a log with a different start date as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + end + end + + context "when there is a log with a different age1" do + before do + create(:lettings_log, :duplicate, age1: 50) + end + + it "does not return a log with a different age1 as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + end + end + + context "when there is a log with a different sex1" do + before do + create(:lettings_log, :duplicate, sex1: "F") + end + + it "does not return a log with a different sex1 as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + end + end + + context "when there is a log with a different ecstat1" do + before do + create(:lettings_log, :duplicate, ecstat1: 1) + end + + it "does not return a log with a different ecstat1 as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + end + end + + context "when there is a log with a different tcharge" do + before do + create(:lettings_log, :duplicate, brent: 100) + end + + it "does not return a log with a different tcharge as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(duplicate_log.id, log.id) + end + end + + context "when there is a log with a different tenancycode" do + before do + create(:lettings_log, :duplicate, tenancycode: "different") + end + + it "does not return a log with a different tenancycode as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + end + end + + context "when there is a log with a different postcode_full" do + before do + create(:lettings_log, :duplicate, postcode_full: "B1 1AA") + end + + it "does not return a log with a different postcode_full as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + end + end + + context "when there is a log with nil values for duplicate check fields" do + before do + create(:lettings_log, :duplicate, age1: nil, sex1: nil, ecstat1: nil, postcode_known: 2, postcode_full: nil) + end + + 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) + expect(duplicate_sets).to be_empty + end + end + + context "when there is a log with nil values for tenancycode" do + let!(:tenancycode_not_given) { create(:lettings_log, :duplicate, tenancycode: nil) } + + it "returns the log as a duplicate if tenancy code is nil" do + log.update!(tenancycode: nil) + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(log.id, tenancycode_not_given.id) + end + 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) } + + it "returns the log as a duplicate if age1 is not known" do + log.update!(age1_known: 1, age1: nil) + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(age1_not_known.id, log.id) + end + end + + context "when there is a duplicate supported housing log" 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:) } + + it "returns the log as a duplicate" do + expect(duplicate_sets.count).to eq(2) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + expect(duplicate_sets.second).to contain_exactly(duplicate_supported_housing_log.id, supported_housing_log.id) + end + + it "does not return the log if the locations are different" do + duplicate_supported_housing_log.update!(location: location_2) + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + end + + it "does not compare tcharge if there are no household charges" do + supported_housing_log.update!(household_charge: 1, supcharg: nil, brent: nil, scharge: nil, pscharge: nil, tcharge: nil) + duplicate_supported_housing_log.update!(household_charge: 1, supcharg: nil, brent: nil, scharge: nil, pscharge: nil, tcharge: nil) + expect(duplicate_sets.count).to eq(2) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + expect(duplicate_sets.second).to contain_exactly(supported_housing_log.id, duplicate_supported_housing_log.id) + end + + it "does not return logs not associated with the user if user is given" do + user = create(:user) + supported_housing_log.update!(created_by: user, managing_organisation: user.organisation) + duplicate_supported_housing_log.update!(owning_organisation: user.organisation) + duplicate_sets = described_class.duplicate_sets(user.id) + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(supported_housing_log.id, duplicate_supported_housing_log.id) + end + + it "compares chcharge if it's a carehome" do + supported_housing_log.update!(is_carehome: 1, chcharge: 100, supcharg: nil, brent: nil, scharge: nil, pscharge: nil, tcharge: nil) + duplicate_supported_housing_log.update!(is_carehome: 1, chcharge: 100, supcharg: nil, brent: nil, scharge: nil, pscharge: nil, tcharge: nil) + expect(duplicate_sets.count).to eq(2) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + expect(duplicate_sets.second).to contain_exactly(supported_housing_log.id, duplicate_supported_housing_log.id) + end + + it "does not return a duplicate if carehome charge is not given" do + supported_housing_log.update!(is_carehome: 1, chcharge: nil, supcharg: nil, brent: nil, scharge: nil, pscharge: nil, tcharge: nil) + duplicate_supported_housing_log.update!(is_carehome: 1, chcharge: nil, supcharg: nil, brent: nil, scharge: nil, pscharge: nil, tcharge: nil) + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + end + end + end end describe "#retirement_age_for_person" do diff --git a/spec/models/sales_log_spec.rb b/spec/models/sales_log_spec.rb index 1aa27cbd3..8e1b73b3b 100644 --- a/spec/models/sales_log_spec.rb +++ b/spec/models/sales_log_spec.rb @@ -279,6 +279,162 @@ RSpec.describe SalesLog, type: :model do end end + context "when getting list of duplicate logs" do + let(:organisation) { create(:organisation) } + let!(:log) { create(:sales_log, :duplicate) } + let!(:duplicate_log) { create(:sales_log, :duplicate) } + let(:duplicate_sets) { described_class.duplicate_sets } + + it "returns a list of duplicates in the same organisation" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + end + + context "when there is a deleted duplicate log" do + before do + create(:sales_log, :duplicate, discarded_at: Time.zone.now, status: 4) + end + + it "does not return the deleted log as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + end + end + + context "when there is a log with a different sale date" do + before do + create(:sales_log, :duplicate, saledate: Time.zone.tomorrow) + end + + it "does not return a log with a different sale date as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + end + end + + context "when there is a log with a different age1" do + before do + create(:sales_log, :duplicate, age1: 50) + end + + it "does not return a log with a different age1 as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + end + end + + context "when there is a log with a different sex1" do + before do + create(:sales_log, :duplicate, sex1: "X") + end + + it "does not return a log with a different sex1 as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + end + end + + context "when there is a log with a different ecstat1" do + before do + create(:sales_log, :duplicate, ecstat1: 9) + end + + it "does not return a log with a different ecstat1 as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + end + end + + context "when there is a log with a different purchid" do + before do + create(:sales_log, :duplicate, purchid: "different") + end + + it "does not return a log with a different purchid as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + end + end + + context "when there is a log with a different postcode_full" do + before do + create(:sales_log, :duplicate, postcode_full: "B1 1AA") + end + + it "does not return a log with a different postcode_full as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + end + end + + context "when there is a log with nil values for duplicate check fields" do + before do + create(:sales_log, :duplicate, age1: nil, sex1: nil, ecstat1: nil, pcodenk: 1, postcode_full: nil) + end + + 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) + expect(duplicate_sets).to be_empty + end + end + + context "when there is a log with nil values for purchid" do + let!(:purchid_not_given) { create(:sales_log, :duplicate, purchid: nil) } + + it "returns the log as a duplicate if tenancy code is nil" do + log.update!(purchid: nil) + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(log.id, purchid_not_given.id) + end + end + + context "when there is a log with age1 not known" do + let!(:age1_not_known) { create(:sales_log, :duplicate, age1_known: 1, age1: nil) } + + it "returns the log as a duplicate if age1 is not known" do + log.update!(age1_known: 1, age1: nil) + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(age1_not_known.id, log.id) + end + end + + context "when there is a log with age1 prefers not to say" do + let!(:age1_prefers_not_to_say) { create(:sales_log, :duplicate, age1_known: 2, age1: nil) } + + it "returns the log as a duplicate if age1 is prefers not to say" do + log.update!(age1_known: 2, age1: nil) + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(age1_prefers_not_to_say.id, log.id) + end + end + + context "when there is a log with age1 not known and prefers not to say" do + before do + create(:sales_log, :duplicate, age1_known: 2, age1: nil) + end + + it "doe not return the log as a duplicate" do + log.update!(age1_known: 1, age1: nil) + expect(duplicate_sets).to be_empty + end + end + + context "when user is given" do + let(:user) { create(:user) } + + before do + create_list(:sales_log, 2, :duplicate, purchid: "other duplicates") + log.update!(created_by: user, owning_organisation: user.organisation) + end + + it "does not return logs not associated with the given user" do + duplicate_sets = described_class.duplicate_sets(user.id) + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(log.id, duplicate_log.id) + end + end + end + describe "derived variables" do let(:sales_log) { create(:sales_log, :completed) }