Update scopes

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)
-    end
-
-    scope
+      scope = scope.having("MAX(CASE WHEN created_by_id = ? THEN 1 ELSE 0 END) >= 1", created_by_id)
+    end
+    scope.pluck("ARRAY_AGG(id)")
   }
 
   AUTOGENERATED_FIELDS = %w[id status created_at updated_at discarded_at].freeze

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

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)
+    .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)
-    end
-
-    scope
+      scope = scope.having("MAX(CASE WHEN created_by_id = ? THEN 1 ELSE 0 END) >= 1", created_by_id)
+    end
+
+    scope.pluck("ARRAY_AGG(id)")
   }
 
   OPTIONAL_FIELDS = %w[purchid othtype].freeze

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

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

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) }