CLDC-4028: Ensure logs will update when an organisation name change is created (#3109)

* CLDC-4028: Ensure logs will update when an organisation name change is created, as they draw the org name

this could lead to redundant exports if the name change does not impact the log at its startdate (which we put into the log), but it'd be more difficult to query this way so fine to be safe here

also query name changes by updated_at. shouldn't really matter as we don't expect name changes to be updated but worth being safe in case we need to update a name change for maintenance reasons

* CLDC-4028: Fix created at name change in lettings log

always safer to use updated at for checking whether the record has changed

app/services/exports/lettings_log_export_service.rb

@@ -30,14 +30,41 @@ module Exports
       "core_#{year}_#{year + 1}_apr_mar_#{base_number_str}_#{increment_str}".downcase
     end
 
-    def retrieve_resources(recent_export, full_update, year)
-      if !full_update && recent_export
-        params = { from: recent_export.started_at, to: @start_time }
-        LettingsLog.exportable.where("(updated_at >= :from AND updated_at <= :to) OR (values_updated_at IS NOT NULL AND values_updated_at >= :from AND values_updated_at <= :to)", params).filter_by_year(year)
-      else
-        params = { to: @start_time }
-        LettingsLog.exportable.where("updated_at <= :to", params).filter_by_year(year)
-      end
+    def retrieve_resources_from_range(range, year)
+      relation = LettingsLog.exportable.filter_by_year(year)
+                            .left_joins(:created_by, :updated_by, :assigned_to, :owning_organisation, :managing_organisation)
+
+      ids = relation
+        .where({ updated_at: range })
+        .or(
+          relation.where.not(values_updated_at: nil).where(values_updated_at: range),
+        )
+        .or(
+          relation.where.not({ created_by: { updated_at: nil } }).where({ created_by: { updated_at: range } }),
+        )
+        .or(
+          relation.where.not({ updated_by: { updated_at: nil } }).where({ updated_by: { updated_at: range } }),
+        )
+        .or(
+          relation.where.not({ assigned_to: { updated_at: nil } }).where({ assigned_to: { updated_at: range } }),
+        )
+        .or(
+          relation.where.not({ owning_organisation: { updated_at: nil } }).where({ owning_organisation: { updated_at: range } }),
+        )
+        .or(
+          relation.where.not({ managing_organisation: { updated_at: nil } }).where({ managing_organisation: { updated_at: range } }),
+        )
+        .pluck(:id)
+
+      # these must be separate as activerecord struggles to join to two different name change tables in the same query
+      ids.concat(
+        relation.left_joins(owning_organisation: :organisation_name_changes).where(owning_organisation: { organisation_name_changes: { updated_at: range } }).pluck(:id),
+      )
+      ids.concat(
+        relation.left_joins(managing_organisation: :organisation_name_changes).where(managing_organisation: { organisation_name_changes: { updated_at: range } }).pluck(:id),
+      )
+
+      LettingsLog.where(id: ids)
     end
 
     def apply_cds_transformation(lettings_log, export_mode)

app/services/exports/organisation_export_service.rb

@@ -25,24 +25,19 @@ module Exports
       "organisations_2024_2025_apr_mar_#{base_number_str}_#{increment_str}".downcase
     end
 
-    def retrieve_resources(recent_export, full_update, _year)
-      if !full_update && recent_export
-        params = { from: recent_export.started_at, to: @start_time }
+    def retrieve_resources_from_range(range, _year)
+      relation = Organisation.left_joins(:users, :organisation_name_changes)
+      ids = relation
+              .where({ updated_at: range })
+              .or(
+                relation.where(organisation_name_changes: { created_at: range }),
+              )
+              .or(
+                relation.where(users: { is_dpo: true, updated_at: range }),
+              )
+              .pluck(:id)
 
-        Organisation
-          .where(updated_at: params[:from]..params[:to])
-          .or(
-            Organisation.where(id: OrganisationNameChange.where(created_at: params[:from]..params[:to]).select(:organisation_id)),
-          )
-      else
-        params = { to: @start_time }
-
-        Organisation
-          .where("updated_at <= :to", params)
-          .or(
-            Organisation.where(id: OrganisationNameChange.where("created_at <= :to", params).select(:organisation_id)),
-          )
-      end
+      Organisation.where(id: ids)
     end
 
     def build_export_xml(organisations)

app/services/exports/sales_log_export_service.rb

@@ -30,14 +30,40 @@ module Exports
       "core_sales_#{year}_#{year + 1}_apr_mar_#{base_number_str}_#{increment_str}".downcase
     end
 
-    def retrieve_resources(recent_export, full_update, year)
-      if !full_update && recent_export
-        params = { from: recent_export.started_at, to: @start_time }
-        SalesLog.exportable.where("(updated_at >= :from AND updated_at <= :to) OR (values_updated_at IS NOT NULL AND values_updated_at >= :from AND values_updated_at <= :to)", params).filter_by_year(year)
-      else
-        params = { to: @start_time }
-        SalesLog.exportable.where("updated_at <= :to", params).filter_by_year(year)
-      end
+    def retrieve_resources_from_range(range, year)
+      relation = SalesLog.exportable.filter_by_year(year).left_joins(:created_by, :updated_by, :assigned_to, :owning_organisation, :managing_organisation)
+
+      ids = relation
+              .where({ updated_at: range })
+              .or(
+                relation.where.not(values_updated_at: nil).where(values_updated_at: range),
+              )
+              .or(
+                relation.where.not({ created_by: { updated_at: nil } }).where({ created_by: { updated_at: range } }),
+              )
+              .or(
+                relation.where.not({ updated_by: { updated_at: nil } }).where({ updated_by: { updated_at: range } }),
+              )
+              .or(
+                relation.where.not({ assigned_to: { updated_at: nil } }).where({ assigned_to: { updated_at: range } }),
+              )
+              .or(
+                relation.where.not({ owning_organisation: { updated_at: nil } }).where({ owning_organisation: { updated_at: range } }),
+              )
+              .or(
+                relation.where.not({ managing_organisation: { updated_at: nil } }).where({ managing_organisation: { updated_at: range } }),
+              )
+              .pluck(:id)
+
+      # these must be separate as activerecord struggles to join to two different name change tables in the same query
+      ids.concat(
+        relation.left_joins(owning_organisation: :organisation_name_changes).where(owning_organisation: { organisation_name_changes: { updated_at: range } }).pluck(:id),
+      )
+      ids.concat(
+        relation.left_joins(managing_organisation: :organisation_name_changes).where(managing_organisation: { organisation_name_changes: { updated_at: range } }).pluck(:id),
+      )
+
+      SalesLog.where(id: ids)
     end
 
     def apply_cds_transformation(sales_log, _export_mode)

app/services/exports/user_export_service.rb

@@ -25,14 +25,19 @@ module Exports
       "users_2024_2025_apr_mar_#{base_number_str}_#{increment_str}".downcase
     end
 
-    def retrieve_resources(recent_export, full_update, _year)
-      if !full_update && recent_export
-        params = { from: recent_export.started_at, to: @start_time }
-        User.where("(updated_at >= :from AND updated_at <= :to) OR (values_updated_at IS NOT NULL AND values_updated_at >= :from AND values_updated_at <= :to)", params)
-      else
-        params = { to: @start_time }
-        User.where("updated_at <= :to", params)
-      end
+    def retrieve_resources_from_range(range, _year)
+      relation = User.left_joins(organisation: :organisation_name_changes)
+      ids = relation
+              .where({ updated_at: range })
+              .or(
+                relation.where.not(organisations: { updated_at: nil }).where(organisations: { updated_at: range }),
+              )
+              .or(
+                relation.where(organisation_name_changes: { updated_at: range }),
+              )
+              .pluck(:id)
+
+      User.where(id: ids)
     end
 
     def build_export_xml(users)

app/services/exports/xml_export_service.rb

@@ -31,6 +31,16 @@ module Exports
       Export.new(collection:, year:, started_at: @start_time, base_number:, increment_number:)
     end
 
+    def retrieve_resources(recent_export, full_update, year)
+      range = if !full_update && recent_export
+                recent_export.started_at..@start_time
+              else
+                ..@start_time
+              end
+
+      retrieve_resources_from_range(range, year)
+    end
+
     def write_export_archive(export, year, recent_export, full_update)
       archive = get_archive_name(year, export.base_number, export.increment_number)
 

spec/services/exports/lettings_log_export_service_spec.rb

@@ -1,6 +1,8 @@
 require "rails_helper"
 
 RSpec.describe Exports::LettingsLogExportService do
+  include CollectionTimeHelper
+
   subject(:export_service) { described_class.new(storage_service, start_time) }
 
   let(:storage_service) { instance_double(Storage::S3Service) }
@@ -16,7 +18,7 @@ RSpec.describe Exports::LettingsLogExportService do
   let(:expected_manifest_filename) { "manifest.xml" }
   let(:start_time) { Time.zone.local(2022, 5, 1) }
   let(:organisation) { create(:organisation, name: "MHCLG", housing_registration_no: 1234) }
-  let(:user) { FactoryBot.create(:user, email: "test1@example.com", organisation:) }
+  let(:user) { create(:user, email: "test1@example.com", organisation:) }
 
   def replace_entity_ids(lettings_log, export_template)
     export_template.sub!(/\{id\}/, (lettings_log["id"] + Exports::LettingsLogExportService::LOG_ID_OFFSET).to_s)
@@ -480,6 +482,91 @@ RSpec.describe Exports::LettingsLogExportService do
         end
       end
     end
+
+    context "and one lettings log has not been updated in the time range" do
+      let(:expected_zip_filename) { "core_#{current_collection_start_year}_#{current_collection_end_year}_apr_mar_f0001_inc0001.zip" }
+      let(:start_time) { current_collection_start_date }
+      let!(:owning_organisation) { create(:organisation, name: "MHCLG owning", housing_registration_no: 1234) }
+      let!(:managing_organisation) { create(:organisation, name: "MHCLG managing", housing_registration_no: 1234) }
+      let!(:created_by_user) { create(:user, email: "test-created-by@example.com", organisation: managing_organisation) }
+      let!(:updated_by_user) { create(:user, email: "test-updated-by@example.com", organisation: managing_organisation) }
+      let!(:assigned_to_user) { create(:user, email: "test-assigned-to@example.com", organisation: managing_organisation) }
+      let!(:lettings_log) { create(:lettings_log, :completed, startdate: current_collection_start_date, created_by: created_by_user, updated_by: updated_by_user, assigned_to: assigned_to_user, owning_organisation:, managing_organisation:) }
+
+      before do
+        # touch all the related records to ensure their updated_at value is outside the export range
+        Timecop.freeze(start_time + 1.month)
+        owning_organisation.touch
+        managing_organisation.touch
+        created_by_user.touch
+        updated_by_user.touch
+        assigned_to_user.touch
+        lettings_log.touch
+        Timecop.freeze(start_time)
+      end
+
+      it "does not export the lettings log" do
+        expect(storage_service).not_to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_lettings_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the lettings log if created_by_user is updated" do
+        created_by_user.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_lettings_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the lettings log if updated_by_user is updated" do
+        updated_by_user.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_lettings_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the lettings log if assigned_to_user is updated" do
+        assigned_to_user.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_lettings_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the lettings log if owning_organisation is updated" do
+        owning_organisation.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_lettings_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the lettings log if managing_organisation is updated" do
+        managing_organisation.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_lettings_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the lettings log if owning_organisation name change is created" do
+        create(:organisation_name_change, organisation: owning_organisation)
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_lettings_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the lettings log if managing_organisation name change is created" do
+        create(:organisation_name_change, organisation: managing_organisation)
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_lettings_logs(collection_year: current_collection_start_year)
+      end
+    end
   end
 
   context "when exporting a supported housing lettings logs in XML" do

spec/services/exports/organisation_export_service_spec.rb

@@ -1,6 +1,8 @@
 require "rails_helper"
 
 RSpec.describe Exports::OrganisationExportService do
+  include CollectionTimeHelper
+
   subject(:export_service) { described_class.new(storage_service, start_time) }
 
   let(:storage_service) { instance_double(Storage::S3Service) }
@@ -283,5 +285,40 @@ RSpec.describe Exports::OrganisationExportService do
         expect(export_service.export_xml_organisations).to eq({ expected_zip_filename.gsub(".zip", "") => start_time })
       end
     end
+
+    context "and one organisation has not been updated in the time range" do
+      let(:dpo_user) { create(:user, email: "dpo@example.com", is_dpo: true, organisation:) }
+      let(:organisation) { create(:organisation, with_dsa: false) }
+
+      before do
+        # touch all the related records to ensure their updated_at value is outside the export range
+        Timecop.freeze(start_time + 1.month)
+        organisation.touch
+        dpo_user.touch
+        Timecop.freeze(start_time)
+      end
+
+      it "does not export the organisation" do
+        expect(storage_service).not_to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_organisations
+      end
+
+      it "does export the organisation if an organisation name change is made" do
+        FactoryBot.create(:organisation_name_change, organisation:)
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_organisations
+      end
+
+      it "does export the organisation if dpo_user is updated" do
+        dpo_user.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_organisations
+      end
+    end
   end
 end

spec/services/exports/sales_log_export_service_spec.rb

@@ -1,6 +1,8 @@
 require "rails_helper"
 
 RSpec.describe Exports::SalesLogExportService do
+  include CollectionTimeHelper
+
   subject(:export_service) { described_class.new(storage_service, start_time) }
 
   let(:storage_service) { instance_double(Storage::S3Service) }
@@ -421,5 +423,90 @@ RSpec.describe Exports::SalesLogExportService do
         end
       end
     end
+
+    context "and one sales log has not been updated in the time range" do
+      let(:expected_zip_filename) { "core_sales_#{current_collection_start_year}_#{current_collection_end_year}_apr_mar_f0001_inc0001.zip" }
+      let(:start_time) { current_collection_start_date }
+      let!(:owning_organisation) { create(:organisation, name: "MHCLG owning", housing_registration_no: 1234) }
+      let!(:managing_organisation) { create(:organisation, name: "MHCLG managing", housing_registration_no: 1234) }
+      let!(:created_by_user) { create(:user, email: "test-created-by@example.com", organisation: managing_organisation) }
+      let!(:updated_by_user) { create(:user, email: "test-updated-by@example.com", organisation: managing_organisation) }
+      let!(:assigned_to_user) { create(:user, email: "test-assigned-to@example.com", organisation: managing_organisation) }
+      let!(:sales_log) { create(:sales_log, :export, saledate: start_time, assigned_to: assigned_to_user, created_by: created_by_user, updated_by: updated_by_user, owning_organisation:, managing_organisation:) }
+
+      before do
+        # touch all the related records to ensure their updated_at value is outside the export range
+        Timecop.freeze(start_time + 1.month)
+        owning_organisation.touch
+        managing_organisation.touch
+        created_by_user.touch
+        updated_by_user.touch
+        assigned_to_user.touch
+        sales_log.touch
+        Timecop.freeze(start_time)
+      end
+
+      it "does not export the sales log" do
+        expect(storage_service).not_to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_sales_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the sales log if created_by_user is updated" do
+        created_by_user.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_sales_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the sales log if updated_by_user is updated" do
+        updated_by_user.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_sales_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the sales log if assigned_to_user is updated" do
+        assigned_to_user.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_sales_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the sales log if owning_organisation is updated" do
+        owning_organisation.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_sales_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the sales log if managing_organisation is updated" do
+        managing_organisation.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_sales_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the sales log if owning_organisation name change is created" do
+        create(:organisation_name_change, organisation: owning_organisation)
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_sales_logs(collection_year: current_collection_start_year)
+      end
+
+      it "does export the sales log if managing_organisation name change is created" do
+        create(:organisation_name_change, organisation: managing_organisation)
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_sales_logs(collection_year: current_collection_start_year)
+      end
+    end
   end
 end

spec/services/exports/user_export_service_spec.rb

@@ -1,6 +1,8 @@
 require "rails_helper"
 
 RSpec.describe Exports::UserExportService do
+  include CollectionTimeHelper
+
   subject(:export_service) { described_class.new(storage_service, start_time) }
 
   let(:storage_service) { instance_double(Storage::S3Service) }
@@ -235,5 +237,40 @@ RSpec.describe Exports::UserExportService do
         expect(export_service.export_xml_users).to eq({ expected_zip_filename.gsub(".zip", "") => start_time })
       end
     end
+
+    context "and one user has not been updated in the time range" do
+      let(:start_time) { current_collection_start_date }
+      let!(:user) { create(:user, organisation:) }
+
+      before do
+        # touch all the related records to ensure their updated_at value is outside the export range
+        Timecop.freeze(start_time + 1.month)
+        organisation.touch
+        user.touch
+        Timecop.freeze(start_time)
+      end
+
+      it "does not export the user" do
+        expect(storage_service).not_to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_users
+      end
+
+      it "does export the user if organisation is updated" do
+        organisation.touch
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_users
+      end
+
+      it "does export the user if an organisation name change is made" do
+        FactoryBot.create(:organisation_name_change, organisation:)
+
+        expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args)
+
+        export_service.export_xml_users
+      end
+    end
   end
 end