CLDC-4090: Document linked validations

* 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

Gemfile.lock

@@ -343,7 +343,7 @@ GEM
       activesupport (>= 3.0.0)
     raabro (1.4.0)
     racc (1.8.1)
-    rack (3.1.17)
+    rack (3.1.18)
     rack-attack (6.7.0)
       rack (>= 1.0, < 4)
     rack-mini-profiler (3.3.1)

app/models/validations/property_validations.rb

@@ -32,6 +32,7 @@ module Validations::PropertyValidations
     end
   end
 
+  # see also: this validation in sales/property_validations.rb
   def validate_uprn(record)
     return unless record.uprn
 
@@ -40,6 +41,7 @@ module Validations::PropertyValidations
     record.errors.add :uprn, I18n.t("validations.lettings.property.uprn.invalid")
   end
 
+  # see also: this validation in sales/property_validations.rb
   def validate_property_postcode(record)
     postcode = record.postcode_full
     return unless postcode
@@ -50,6 +52,7 @@ module Validations::PropertyValidations
     end
   end
 
+  # see also: this validation in sales/property_validations.rb
   def validate_la_in_england(record)
     return unless record.form.start_year_2025_or_later?
 
@@ -76,4 +79,40 @@ module Validations::PropertyValidations
       record.errors.add :startdate, I18n.t("validations.lettings.property.startdate.location_not_in_england")
     end
   end
+
+  # see also: this validation in sales/property_validations.rb
+  def validate_la_is_active(record)
+    return unless record.form.start_year_2025_or_later? && record.startdate.present?
+
+    if record.is_general_needs?
+      return unless record.la
+
+      la = LocalAuthority.england.find_by(code: record.la)
+
+      # will be caught by the not in england validation
+      return if la.nil?
+      # only compare end date if it exists
+      return if record.startdate >= la.start_date && (la.end_date.nil? || record.startdate <= la.end_date)
+
+      record.errors.add :la, I18n.t("validations.lettings.property.la.la_not_valid_for_date", la: la.name)
+      record.errors.add :postcode_full, I18n.t("validations.lettings.property.postcode_full.la_not_valid_for_date", la: la.name)
+      record.errors.add :uprn, I18n.t("validations.lettings.property.uprn.la_not_valid_for_date", la: la.name)
+      record.errors.add :uprn_confirmation, I18n.t("validations.lettings.property.uprn_confirmation.la_not_valid_for_date", la: la.name)
+      record.errors.add :uprn_selection, I18n.t("validations.lettings.property.uprn_selection.la_not_valid_for_date", la: la.name)
+      record.errors.add :startdate, I18n.t("validations.lettings.property.startdate.la_not_valid_for_date", la: la.name)
+    elsif record.is_supported_housing?
+      return unless record.location
+
+      la = LocalAuthority.england.find_by(code: record.location.location_code)
+
+      # will be caught by the not in england validation
+      return if la.nil?
+      # only compare end date if it exists
+      return if record.startdate >= la.start_date && (la.end_date.nil? || record.startdate <= la.end_date)
+
+      record.errors.add :location_id, I18n.t("validations.lettings.property.location_id.la_not_valid_for_date", la: la.name)
+      record.errors.add :scheme_id, I18n.t("validations.lettings.property.scheme_id.la_not_valid_for_date", la: la.name)
+      record.errors.add :startdate, I18n.t("validations.lettings.property.startdate.la_not_valid_for_date", la: la.name)
+    end
+  end
 end

app/models/validations/sales/property_validations.rb

@@ -21,6 +21,7 @@ module Validations::Sales::PropertyValidations
     end
   end
 
+  # see also: this validation in validations/property_validations.rb
   def validate_uprn(record)
     return unless record.uprn
 
@@ -29,6 +30,7 @@ module Validations::Sales::PropertyValidations
     record.errors.add :uprn, I18n.t("validations.sales.property_information.uprn.invalid")
   end
 
+  # see also: this validation in validations/property_validations.rb
   def validate_property_postcode(record)
     postcode = record.postcode_full
     return unless postcode
@@ -39,6 +41,7 @@ module Validations::Sales::PropertyValidations
     end
   end
 
+  # see also: this validation in validations/property_validations.rb
   def validate_la_in_england(record)
     return unless record.form.start_year_2025_or_later? && record.la.present?
     return if record.la.in?(LocalAuthority.england.pluck(:code))
@@ -54,4 +57,23 @@ module Validations::Sales::PropertyValidations
       record.errors.add :saledate, :skip_bu_error, message: I18n.t("validations.sales.property_information.saledate.postcode_not_in_england")
     end
   end
+
+  # see also: this validation in validations/property_validations.rb
+  def validate_la_is_active(record)
+    return unless record.form.start_year_2025_or_later? && record.la.present? && record.startdate.present?
+
+    la = LocalAuthority.england.find_by(code: record.la)
+
+    # will be caught by the not in england validation
+    return if la.nil?
+    # only compare end date if it exists
+    return if record.startdate >= la.start_date && (la.end_date.nil? || record.startdate <= la.end_date)
+
+    record.errors.add :la, I18n.t("validations.sales.property_information.la.la_not_valid_for_date", la: la.name)
+    record.errors.add :postcode_full, I18n.t("validations.sales.property_information.postcode_full.la_not_valid_for_date", la: la.name)
+    record.errors.add :uprn, I18n.t("validations.sales.property_information.uprn.la_not_valid_for_date", la: la.name)
+    record.errors.add :uprn_confirmation, I18n.t("validations.sales.property_information.uprn_confirmation.la_not_valid_for_date", la: la.name)
+    record.errors.add :uprn_selection, I18n.t("validations.sales.property_information.uprn_selection.la_not_valid_for_date", la: la.name)
+    record.errors.add :saledate, :skip_bu_error, message: I18n.t("validations.sales.property_information.saledate.la_not_valid_for_date", la: la.name)
+  end
 end

app/services/bulk_upload/sales/year2025/row_parser.rb

@@ -12,7 +12,7 @@ class BulkUpload::Sales::Year2025::RowParser
     field_5: "Which organisation is reporting this sale?",
     field_6: "Username",
     field_7: "What is the purchaser code?",
-    field_8: "What is the sale type?",
+    field_8: "Is this a shared ownership or discounted ownership sale?",
     field_9: "What is the type of shared ownership sale?",
     field_10: "Is this a staircasing transaction?",
     field_11: "What is the type of discounted ownership sale?",
@@ -311,7 +311,7 @@ class BulkUpload::Sales::Year2025::RowParser
 
   validates :field_8,
             presence: {
-              message: I18n.t("#{ERROR_BASE_KEY}.not_answered", question: "shared ownership sale type."),
+              message: I18n.t("#{ERROR_BASE_KEY}.not_answered", question: "sale type."),
               category: :setup,
             },
             on: :after_log

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)
 

app/services/merge/merge_organisations_service.rb

@@ -4,6 +4,8 @@ class Merge::MergeOrganisationsService
     @merging_organisations = Organisation.find(merging_organisation_ids)
     @merge_date = merge_date || Time.zone.today
     @absorbing_organisation_active_from_merge_date = absorbing_organisation_active_from_merge_date
+    @pre_to_post_merge_scheme_ids = {}
+    @pre_to_post_merge_location_ids = {}
   end
 
   def call
@@ -70,12 +72,14 @@ private
     merging_organisation.owned_schemes.each do |scheme|
       new_scheme = Scheme.new(scheme.attributes.except("id", "owning_organisation_id", "old_id", "old_visible_id").merge(owning_organisation: @absorbing_organisation, startdate: [scheme&.startdate, @merge_date].compact.max))
       new_scheme.save!(validate: false)
+      @pre_to_post_merge_scheme_ids[scheme.id] = new_scheme.id
       scheme.scheme_deactivation_periods.each do |deactivation_period|
         split_scheme_deactivation_period_between_organisations(deactivation_period, new_scheme)
       end
       scheme.locations.each do |location|
         new_location = Location.new(location.attributes.except("id", "scheme_id", "old_id", "old_visible_id").merge(scheme: new_scheme, startdate: [location&.startdate, @merge_date].compact.max))
         new_location.save!(validate: false)
+        @pre_to_post_merge_location_ids[location.id] = new_location.id
         location.location_deactivation_periods.each do |deactivation_period|
           split_location_deactivation_period_between_organisations(deactivation_period, new_location)
         end
@@ -98,8 +102,8 @@ private
         lettings_log.managing_organisation = @absorbing_organisation
       end
       if lettings_log.scheme.present?
-        scheme_to_set = @absorbing_organisation.owned_schemes.find_by(service_name: lettings_log.scheme.service_name)
-        location_to_set = scheme_to_set.locations.find_by(name: lettings_log.location&.name, postcode: lettings_log.location&.postcode)
+        scheme_to_set = @absorbing_organisation.owned_schemes.find_by(id: @pre_to_post_merge_scheme_ids[lettings_log.scheme.id])
+        location_to_set = scheme_to_set.locations.find_by(id: @pre_to_post_merge_location_ids[lettings_log.location&.id])
 
         lettings_log.scheme = scheme_to_set if scheme_to_set.present?
         # in some cases the lettings_log location is nil even if scheme is present (they're two different questions).

app/views/layouts/_feedback.html.erb

@@ -10,7 +10,7 @@
           </div>
         </div>
         <div class="gem-c-feedback__prompt-questions">
-          <a class="govuk-button gem-c-feedback__prompt-link" target="_blank" href="https://forms.office.com/Pages/ResponsePage.aspx?id=EGg0v32c3kOociSi7zmVqC4YDsCJ3llAvEZelBFBLUBURFVUTzFDTUJPQlM4M0laTE5DTlNFSjJBQi4u">
+          <a class="govuk-button gem-c-feedback__prompt-link" target="_blank" href="https://forms.office.com/e/thT1Vm7edm">
             Give feedback (opens in a new tab)
           </a>
         </div>

config/locales/en.yml

@@ -31,7 +31,7 @@
 
 en:
   service_name: "Submit social housing lettings and sales data (CORE)"
-  feedback_form: "https://forms.office.com/Pages/ResponsePage.aspx?id=EGg0v32c3kOociSi7zmVqC4YDsCJ3llAvEZelBFBLUBURFVUTzFDTUJPQlM4M0laTE5DTlNFSjJBQi4u"
+  feedback_form: "https://forms.office.com/e/thT1Vm7edm"
   organisation:
     created: "%{organisation} was created."
     updated: "Organisation details updated."

config/locales/validations/lettings/property_information.en.yml

@@ -5,6 +5,7 @@ en:
         postcode_full:
           invalid: "Enter a postcode in the correct format, for example AA1 1AA."
           not_in_england: "It looks like you have an entered a postcode outside of England. Only create logs for lettings in England."
+          la_not_valid_for_date: "%{la} does not exist on the tenancy start date, due to a change in local authority names and boundaries. Please enter the local authority name in use on the tenancy start date"
         rsnvac:
           non_temp_accommodation: "Answer cannot be re-let to tenant who occupied the same property as temporary accommodation as this accommodation is not temporary."
           referral_invalid: "Answer cannot be re-let to tenant who occupied the same property as temporary accommodation as a different source of referral for this letting."
@@ -22,17 +23,24 @@ en:
         uprn:
           invalid: "UPRN must be 12 digits or less."
           not_in_england: "It looks like you have entered an address outside of England. Only create logs for lettings in England."
+          la_not_valid_for_date: "%{la} does not exist on the tenancy start date, due to a change in local authority names and boundaries. Please enter the local authority name in use on the tenancy start date"
         uprn_confirmation:
           not_in_england: "It looks like you have entered an address outside of England. Only create logs for lettings in England."
+          la_not_valid_for_date: "%{la} does not exist on the tenancy start date, due to a change in local authority names and boundaries. Please enter the local authority name in use on the tenancy start date"
         uprn_selection:
           not_in_england: "It looks like you have entered an address outside of England. Only create logs for lettings in England."
+          la_not_valid_for_date: "%{la} does not exist on the tenancy start date, due to a change in local authority names and boundaries. Please enter the local authority name in use on the tenancy start date"
         la:
           not_in_england: "It looks like you have entered an address outside of England. Only create logs for lettings in England."
+          la_not_valid_for_date: "%{la} does not exist on the tenancy start date, due to a change in local authority names and boundaries. Please enter the local authority name in use on the tenancy start date"
         scheme_id:
           not_in_england: "This scheme’s only location is outside of England. Only create logs for lettings in England."
+          la_not_valid_for_date: "%{la} does not exist on the tenancy start date, due to a change in local authority names and boundaries. Please enter the local authority name in use on the tenancy start date"
         location_id:
           not_in_england: "It looks like you have selected a location outside of England. Only create logs for lettings in England."
+          la_not_valid_for_date: "%{la} does not exist on the tenancy start date, due to a change in local authority names and boundaries. Please enter the local authority name in use on the tenancy start date"
         startdate:
           postcode_not_in_england: "It looks like you have an entered a postcode outside of England. Only create logs for lettings in England."
           address_not_in_england: "It looks like you have entered an address outside of England. Only create logs for lettings in England."
           location_not_in_england: "It looks like you have selected a location outside of England. Only create logs for lettings in England."
+          la_not_valid_for_date: "%{la} does not exist on the tenancy start date, due to a change in local authority names and boundaries. Please enter the local authority name in use on the tenancy start date"

config/locales/validations/sales/property_information.en.yml

@@ -8,6 +8,7 @@ en:
             not_joint_purchase: "Buyer’s last accommodation and discounted ownership postcodes must match."
           invalid: "Enter a postcode in the correct format, for example AA1 1AA."
           not_in_england: "It looks like you have entered a postcode outside of England. Only create logs for sales in England."
+          la_not_valid_for_date: "%{la} does not exist on the property sale date, due to a change in local authority names and boundaries. Please enter the local authority name in use on the sale date"
         ppostcode_full:
           postcode_must_match_previous:
             joint_purchase: "Buyers’ last accommodation and discounted ownership postcodes must match."
@@ -22,6 +23,7 @@ en:
             not_joint_purchase: "Buyer’s last accommodation and discounted ownership postcodes must match."
           invalid: "UPRN must be 12 digits or less."
           not_in_england: "It looks like you have entered an address outside of England. Only create logs for sales in England."
+          la_not_valid_for_date: "%{la} does not exist on the property sale date, due to a change in local authority names and boundaries. Please enter the local authority name in use on the sale date"
         beds:
           bedsits_have_max_one_bedroom: "Number of bedrooms must be 1 if the property is a bedsit."
         proptype:
@@ -30,10 +32,14 @@ en:
           invalid: "You must answer UPRN known?"
         la:
           not_in_england: "It looks like you have entered an address outside of England. Only create logs for sales in England."
+          la_not_valid_for_date: "%{la} does not exist on the property sale date, due to a change in local authority names and boundaries. Please enter the local authority name in use on the sale date"
         uprn_confirmation:
           not_in_england: "It looks like you have entered an address outside of England. Only create logs for sales in England."
+          la_not_valid_for_date: "%{la} does not exist on the property sale date, due to a change in local authority names and boundaries. Please enter the local authority name in use on the sale date"
         uprn_selection:
           not_in_england: "It looks like you have entered an address outside of England. Only create logs for sales in England."
+          la_not_valid_for_date: "%{la} does not exist on the property sale date, due to a change in local authority names and boundaries. Please enter the local authority name in use on the sale date"
         saledate:
           postcode_not_in_england: "It looks like you have entered a postcode outside of England. Only create logs for sales in England."
           address_not_in_england: "It looks like you have entered an address outside of England. Only create logs for sales in England."
+          la_not_valid_for_date: "%{la} does not exist on the property sale date, due to a change in local authority names and boundaries. Please enter the local authority name in use on the sale date"

docs/setup.md

@@ -22,6 +22,8 @@ We recommend using [nvm](https://github.com/nvm-sh/nvm) to manage NodeJS version
 
 1. Install PostgreSQL
 
+   If you already have a valid Postgres installation you can skip this step.
+
    macOS:
 
    ```bash
@@ -38,6 +40,8 @@ We recommend using [nvm](https://github.com/nvm-sh/nvm) to manage NodeJS version
 
 2. Create a Postgres user
 
+   If you already have a valid Postgres installation you can skip this step.
+
    ```bash
    sudo su - postgres -c "createuser <username> -s -P"
    ```
@@ -84,35 +88,22 @@ We recommend using [nvm](https://github.com/nvm-sh/nvm) to manage NodeJS version
    brew install yarn
    ```
 
-   or you could run it without specifying the version and it should use the version from .nvmrc
-
-   ```bash
-   nvm install
-   nvm use
-   brew install yarn
-   ```
-
    Linux (Debian):
 
    ```bash
-   curl -sL https://deb.nodesource.com/setup_20.x | sudo bash -
-   sudo apt -y install nodejs
-   mkdir -p ~/.npm-packages
-   npm config set prefix ~/.npm-packages
-   echo 'NPM_PACKAGES="~/.npm-packages"' >> ~/.bashrc
-   echo 'export PATH="$PATH:$NPM_PACKAGES/bin"' >> ~/.bashrc
-   source ~/.bashrc
-   npm install --location=global yarn
+   npm install --global yarn
    ```
 
 6. (For running tests) Install Gecko Driver
 
    Linux (Debian):
 
+   Find the latest version at https://github.com/mozilla/geckodriver/releases/, right click the linux64.tar.gz download and copy URL.
+
    ```bash
-   wget https://github.com/mozilla/geckodriver/releases/download/v0.31.0/geckodriver-v0.31.0-linux64.tar.gz
-   tar -xvzf geckodriver-v0.31.0-linux64.tar.gz
-   rm geckodriver-v0.31.0-linux64.tar.gz
+   wget <url copied>
+   tar -xvzf <file downloaded>
+   rm <file downloaded>
    chmod +x geckodriver
    sudo mv geckodriver /usr/local/bin/
    ```
@@ -147,22 +138,30 @@ Also ensure you have firefox installed
    bundle exec rake db:seed
    ```
 
-5. For Ordinance Survey related functionality, such as using the UPRN, you will need to set `OS_DATA_KEY` in your .env file. This key is shared across the team and can be found in AWS Secrets Manager.
-6. For email functionality, you will need a GOV.UK Notify API key, which is individual to you. Ask an existing team member to invite you to the "CORE Helpdesk" Notify service. Once invited, sign in and go to "API integration" to generate an API key, and set this as `GOVUK_NOTIFY_API_KEY` in your .env file.
+5. Build assets once before running the app for the first time:
+
+   ```bash
+   yarn build --mode=development
+   ```
+
+6. For Ordinance Survey related functionality, such as using the UPRN, you will need to set `OS_DATA_KEY` in your .env file. This key is shared across the team and can be found in AWS Secrets Manager.
+7. For email functionality, you will need a GOV.UK Notify API key, which is individual to you. Ask an existing team member to invite you to the "CORE Helpdesk" Notify service. Once invited, sign in and go to "API integration" to generate an API key, and set this as `GOVUK_NOTIFY_API_KEY` in your .env file.
 
 ## Running Locally
 
 ### Application
 
-Start the dev servers
+Start the dev servers via one of the following methods:
 
-a. Using Foreman:
+a. If using RubyMine, run the "submit-social-housing-lettings-and-sales-data" Rails configuration.
+
+b. Using Foreman:
 
 ```bash
 ./bin/dev
 ```
 
-b. Individually:
+c. Individually:
 
 Rails:
 
@@ -176,20 +175,18 @@ JavaScript (for hot reloading):
 yarn build --mode=development --watch
 ```
 
-If you’re not modifying front end assets you can bundle them as a one off task:
-
-```bash
-yarn build --mode=development
-```
-
 Development mode will target the latest versions of Chrome, Firefox and Safari for transpilation while production mode will target older browsers.
 
 The Rails server will start on <http://localhost:3000>.
 
 To sign in locally, you can use any username and password from your local database. The seed task in `seeds.rb` creates users in various roles all with the password `REVIEW_APP_USER_PASSWORD` from your .env file (which has default value `password`).
-To create any other users, you can edit the seed commands, or run similar commands in the rails console.
+To create any other users, you can log in as a support user and create new users via the admin interface. Or, you can edit the seed commands, or run similar commands in the rails console.
 You can also insert a new user row using SQL, but you will need to generate a correctly encrypted password. You can find the value to use for encrypted password which corresponds to the password `YOURPASSWORDHERE` using `User.new(:password => [YOURPASSWORDHERE]).encrypted_password`.
 
+### rbenv
+
+In general, whenever needing to run a Ruby command, use `bundle exec <command>` to ensure the correct Ruby version and gemset are used. Rbenv will automatically use the correct Ruby version.
+
 ### Debugging
 
 Add the line `binding.pry` to the code to pause the execution of the code at that line and open a powerful interactive console for debugging.
@@ -219,6 +216,8 @@ More details on debugging in RubyMine can be found at <https://www.jetbrains.com
 bundle exec rspec
 ```
 
+Or if using RubyMine, right click a spec file and select the 'Run' option.
+
 To run a specific folder use
 
 ```bash
@@ -233,6 +232,12 @@ bundle exec rspec ./spec/path/to/file.rb
 
 or run individual files/tests from your IDE
 
+If you have made database changes, you may need to run the migrations for the test database:
+
+```bash
+bundle exec rake db:migrate RAILS_ENV=test
+```
+
 ### Feature toggles
 
 Feature toggles can be found in `app/services/feature_toggle.rb`

spec/models/validations/property_validations_spec.rb

@@ -280,4 +280,100 @@ RSpec.describe Validations::PropertyValidations do
       end
     end
   end
+
+  describe "#validate_la_is_active" do
+    let(:la_ecode_active) { "E09000033" }
+    let(:la_ecode_inactive) { "E07000156" }
+    let(:local_authority_active) { LocalAuthority.find_by(code: la_ecode_active) }
+    let(:local_authority_inactive) { LocalAuthority.find_by(code: la_ecode_inactive) }
+
+    context "with a log on or after 2025" do
+      before do
+        allow(log.form).to receive(:start_year_2025_or_later?).and_return true
+      end
+
+      context "and the local authority is active for general needs log" do
+        let(:log) { build(:lettings_log, :completed, la: la_ecode_active, needstype: 1) }
+
+        it "does not add an error" do
+          property_validator.validate_la_is_active(log)
+          expect(log.errors["la"]).to be_empty
+          expect(log.errors["postcode_full"]).to be_empty
+          expect(log.errors["uprn"]).to be_empty
+          expect(log.errors["uprn_confirmation"]).to be_empty
+          expect(log.errors["uprn_selection"]).to be_empty
+          expect(log.errors["startdate"]).to be_empty
+        end
+      end
+
+      context "and the local authority is inactive for general needs log" do
+        let(:log) { build(:lettings_log, :completed, la: la_ecode_inactive, needstype: 1) }
+
+        it "adds an error" do
+          property_validator.validate_la_is_active(log)
+          expect(log.errors["la"]).to include(I18n.t("validations.lettings.property.la.la_not_valid_for_date", la: local_authority_inactive.name))
+          expect(log.errors["postcode_full"]).to include(I18n.t("validations.lettings.property.postcode_full.la_not_valid_for_date", la: local_authority_inactive.name))
+          expect(log.errors["uprn"]).to include(I18n.t("validations.lettings.property.uprn.la_not_valid_for_date", la: local_authority_inactive.name))
+          expect(log.errors["uprn_confirmation"]).to include(I18n.t("validations.lettings.property.uprn_confirmation.la_not_valid_for_date", la: local_authority_inactive.name))
+          expect(log.errors["uprn_selection"]).to include(I18n.t("validations.lettings.property.uprn_selection.la_not_valid_for_date", la: local_authority_inactive.name))
+          expect(log.errors["startdate"]).to include(I18n.t("validations.lettings.property.startdate.la_not_valid_for_date", la: local_authority_inactive.name))
+          expect(log.errors["scheme_id"]).to be_empty
+          expect(log.errors["location_id"]).to be_empty
+        end
+      end
+
+      context "and the local authority is active for supported housing log" do
+        let(:location) { create(:location, location_code: la_ecode_active) }
+        let(:log) { build(:lettings_log, :completed, needstype: 2, location:) }
+
+        it "does not add an error" do
+          property_validator.validate_la_is_active(log)
+          expect(log.errors["scheme_id"]).to be_empty
+          expect(log.errors["location_id"]).to be_empty
+          expect(log.errors["startdate"]).to be_empty
+          expect(log.errors["la"]).to be_empty
+          expect(log.errors["postcode_full"]).to be_empty
+          expect(log.errors["uprn"]).to be_empty
+          expect(log.errors["uprn_confirmation"]).to be_empty
+          expect(log.errors["uprn_selection"]).to be_empty
+        end
+      end
+
+      context "and the local authority is inactive for supported housing log" do
+        let(:location) { create(:location, location_code: la_ecode_inactive) }
+        let(:log) { build(:lettings_log, :completed, needstype: 2, location:) }
+
+        it "adds an error" do
+          property_validator.validate_la_is_active(log)
+          expect(log.errors["scheme_id"]).to include(I18n.t("validations.lettings.property.scheme_id.la_not_valid_for_date", la: local_authority_inactive.name))
+          expect(log.errors["location_id"]).to include(I18n.t("validations.lettings.property.location_id.la_not_valid_for_date", la: local_authority_inactive.name))
+          expect(log.errors["startdate"]).to include(I18n.t("validations.lettings.property.startdate.la_not_valid_for_date", la: local_authority_inactive.name))
+          expect(log.errors["la"]).to be_empty
+          expect(log.errors["postcode_full"]).to be_empty
+          expect(log.errors["uprn"]).to be_empty
+          expect(log.errors["uprn_confirmation"]).to be_empty
+          expect(log.errors["uprn_selection"]).to be_empty
+        end
+      end
+    end
+
+    context "with a log before 2025" do
+      before do
+        allow(log.form).to receive(:start_year_2025_or_later?).and_return false
+      end
+
+      context "and the local authority is inactive" do
+        let(:log) { build(:lettings_log, :completed, la: la_ecode_inactive) }
+
+        it "does not add an error" do
+          property_validator.validate_la_is_active(log)
+          expect(log.errors["la"]).to be_empty
+          expect(log.errors["postcode_full"]).to be_empty
+          expect(log.errors["uprn"]).to be_empty
+          expect(log.errors["uprn_confirmation"]).to be_empty
+          expect(log.errors["uprn_selection"]).to be_empty
+        end
+      end
+    end
+  end
 end

spec/models/validations/sales/property_validations_spec.rb

@@ -139,4 +139,65 @@ RSpec.describe Validations::Sales::PropertyValidations do
       end
     end
   end
+
+  describe "#validate_la_is_active" do
+    let(:la_ecode_active) { "E09000033" }
+    let(:la_ecode_inactive) { "E07000156" }
+    let(:local_authority_active) { LocalAuthority.find_by(code: la_ecode_active) }
+    let(:local_authority_inactive) { LocalAuthority.find_by(code: la_ecode_inactive) }
+
+    context "with a log on or after 2025" do
+      before do
+        allow(log.form).to receive(:start_year_2025_or_later?).and_return true
+      end
+
+      context "and the local authority is active" do
+        let(:log) { build(:sales_log, :completed, la: la_ecode_active) }
+
+        it "adds an error" do
+          property_validator.validate_la_is_active(log)
+          expect(log.errors["la"]).to be_empty
+          expect(log.errors["postcode_full"]).to be_empty
+          expect(log.errors["uprn"]).to be_empty
+          expect(log.errors["uprn_confirmation"]).to be_empty
+          expect(log.errors["uprn_selection"]).to be_empty
+          expect(log.errors["saledate"]).to be_empty
+        end
+      end
+
+      context "and the local authority is inactive" do
+        let(:log) { build(:sales_log, :completed, la: la_ecode_inactive) }
+
+        it "does not add an error" do
+          property_validator.validate_la_is_active(log)
+          expect(log.errors["la"]).to include(I18n.t("validations.sales.property_information.la.la_not_valid_for_date", la: local_authority_inactive.name))
+          expect(log.errors["postcode_full"]).to include(I18n.t("validations.sales.property_information.postcode_full.la_not_valid_for_date", la: local_authority_inactive.name))
+          expect(log.errors["uprn"]).to include(I18n.t("validations.sales.property_information.uprn.la_not_valid_for_date", la: local_authority_inactive.name))
+          expect(log.errors["uprn_confirmation"]).to include(I18n.t("validations.sales.property_information.uprn_confirmation.la_not_valid_for_date", la: local_authority_inactive.name))
+          expect(log.errors["uprn_selection"]).to include(I18n.t("validations.sales.property_information.uprn_selection.la_not_valid_for_date", la: local_authority_inactive.name))
+          expect(log.errors["saledate"]).to include(I18n.t("validations.sales.property_information.saledate.la_not_valid_for_date", la: local_authority_inactive.name))
+        end
+      end
+    end
+
+    context "with a log before 2025" do
+      before do
+        allow(log.form).to receive(:start_year_2025_or_later?).and_return false
+      end
+
+      context "and the local authority is inactive" do
+        let(:log) { build(:sales_log, :completed, la: la_ecode_inactive) }
+
+        it "does not add an error" do
+          property_validator.validate_la_is_active(log)
+          expect(log.errors["la"]).to be_empty
+          expect(log.errors["postcode_full"]).to be_empty
+          expect(log.errors["uprn"]).to be_empty
+          expect(log.errors["uprn_confirmation"]).to be_empty
+          expect(log.errors["uprn_selection"]).to be_empty
+          expect(log.errors["saledate"]).to be_empty
+        end
+      end
+    end
+  end
 end

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

spec/services/merge/merge_organisations_service_spec.rb

@@ -728,6 +728,31 @@ RSpec.describe Merge::MergeOrganisationsService do
             expect(incomplete_lettings_log.location).to be_nil
           end
 
+          it "associates correct lettings logs with correct locations which share a name and postcode" do
+            scheme = create(:scheme, owning_organisation: merging_organisation)
+            location_0 = create(:location, scheme:, name: "duplicate name", postcode: "EE1 1EE", location_admin_district: "dist1", startdate: Time.zone.today)
+            location_1 = create(:location, scheme:, name: "duplicate name", postcode: "EE1 1EE", location_admin_district: "dist2", startdate: Time.zone.today)
+            location_2 = create(:location, scheme:, name: "duplicate name", postcode: "EE1 1EE", location_admin_district: "dist3", startdate: Time.zone.today)
+
+            lettings_log_0 = build(:lettings_log, scheme:, owning_organisation: merging_organisation, startdate: Time.zone.today, location: location_0)
+            lettings_log_0.save!(validate: false)
+            lettings_log_1 = build(:lettings_log, scheme:, owning_organisation: merging_organisation, startdate: Time.zone.today, location: location_1)
+            lettings_log_1.save!(validate: false)
+            lettings_log_2 = build(:lettings_log, scheme:, owning_organisation: merging_organisation, startdate: Time.zone.today, location: location_2)
+            lettings_log_2.save!(validate: false)
+
+            expect(Rails.logger).not_to receive(:error)
+            merge_organisations_service.call
+
+            lettings_log_0.reload
+            lettings_log_1.reload
+            lettings_log_2.reload
+
+            expect(lettings_log_0.location.location_admin_district).to eq("dist1")
+            expect(lettings_log_1.location.location_admin_district).to eq("dist2")
+            expect(lettings_log_2.location.location_admin_district).to eq("dist3")
+          end
+
           context "with merge date in closed collection year" do
             subject(:merge_organisations_service) { described_class.new(absorbing_organisation_id: absorbing_organisation.id, merging_organisation_ids:, merge_date: Time.zone.local(2021, 3, 3)) }