CLDC-4033: Correct name of resource variable

this override function is passed a resource symbol, not a resource object

.github/workflows/review_pipeline.yml

@@ -52,6 +52,6 @@ jobs:
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         with:
-          msg: "Created review app at https://review.submit-social-housing-data.communities.gov.uk/${{ github.event.pull_request.number }}"
+          msg: "Created review app at https://review.submit-social-housing-data.communities.gov.uk/${{ github.event.pull_request.number }}. Note that the review app will be automatically deprovisioned after 30 days and will need the review app pipeline running again."
           check_for_duplicate_msg: true
           duplicate_msg_pattern: Created review app at*

Gemfile.lock

@@ -411,7 +411,7 @@ GEM
     responders (3.1.1)
       actionpack (>= 5.2)
       railties (>= 5.2)
-    rexml (3.3.9)
+    rexml (3.4.4)
     roo (2.10.1)
       nokogiri (~> 1)
       rubyzip (>= 1.3.0, < 3.0.0)

app/controllers/auth/passwords_controller.rb

@@ -4,6 +4,7 @@ class Auth::PasswordsController < Devise::PasswordsController
   def reset_confirmation
     self.resource = resource_class.new
     @email = params["email"]
+    @unconfirmed = params["unconfirmed"] == "true"
     if @email.blank?
       resource.errors.add :email, I18n.t("validations.email.blank")
       render "devise/passwords/new", status: :unprocessable_entity
@@ -65,8 +66,8 @@ protected
     resource.need_two_factor_authentication?(request) ? :updated_2FA : :updated
   end
 
-  def after_sending_reset_password_instructions_path_for(_resource)
+  def after_sending_reset_password_instructions_path_for(_resource_name)
-    account_password_reset_confirmation_path(email: params.dig("user", "email"))
+    account_password_reset_confirmation_path(email: params.dig("user", "email"), unconfirmed: resource.initial_confirmation_sent && !resource.confirmed?)
   end
 
   def after_resetting_password_path_for(resource)

app/models/form.rb

@@ -86,7 +86,11 @@ class Form
   end
 
   def next_page_id(page, log, current_user, ignore_answered: false)
-    return page.next_unresolved_page_id || :check_answers if log.unresolved
+    if log.unresolved
+      return log.scheme_has_large_number_of_locations? ? "location_search" : "location" if page.next_unresolved_page_id == :location_or_location_search
+
+      return page.next_unresolved_page_id || :check_answers
+    end
 
     page_ids = page.subsection.pages.map(&:id)
     page_index = page_ids.index(page.id)

app/models/form/lettings/pages/scheme.rb

@@ -6,7 +6,7 @@ class Form::Lettings::Pages::Scheme < ::Form::Page
         "needstype" => 2,
       },
     ]
-    @next_unresolved_page_id = "location"
+    @next_unresolved_page_id = :location_or_location_search
   end
 
   def questions

app/models/organisation.rb

@@ -38,7 +38,13 @@ class Organisation < ApplicationRecord
     Organisation.where(id: ids)
   end
 
-  scope :search_by_name, ->(name) { where("name ILIKE ?", "%#{name}%") }
+  scope :search_by_name, lambda { |name|
+    # we can't use the activerecord queries to query the DB as the `name` col doesn't update when the org changes name.
+    # this is due to the info being stored in the organisation_name_changes table.
+    # so we load all and filter in memory. the number of organisations is small so this should be fine.
+    matched_ids = all.filter { |org| org.name.downcase.include?(name.downcase) }.map(&:id)
+    where(id: matched_ids)
+  }
   scope :search_by, ->(param) { search_by_name(param) }
   scope :filter_by_active, -> { where(active: true) }
   scope :filter_by_inactive, -> { where(active: false) }

app/models/user.rb

@@ -358,6 +358,14 @@ class User < ApplicationRecord
     end
   end
 
+  def send_reset_password_instructions
+    if confirmed?
+      super
+    else
+      send_confirmation_instructions
+    end
+  end
+
 protected
 
   # Checks whether a password is needed or not. For validations only.

app/services/exports/lettings_log_export_service.rb

@@ -30,14 +30,32 @@ module Exports
       "core_#{year}_#{year + 1}_apr_mar_#{base_number_str}_#{increment_str}".downcase
     end
 
-    def retrieve_resources(recent_export, full_update, year)
+    def retrieve_resources_from_range(range, year)
-      if !full_update && recent_export
+      relation = LettingsLog.exportable.filter_by_year(year).left_joins(:created_by, :updated_by, :assigned_to, :owning_organisation, :managing_organisation)
-        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)
+      ids = relation
-      else
+        .where({ updated_at: range })
-        params = { to: @start_time }
+        .or(
-        LettingsLog.exportable.where("updated_at <= :to", params).filter_by_year(year)
+          relation.where.not(values_updated_at: nil).where(values_updated_at: range),
-      end
+        )
+        .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)
+
+      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)
+    def retrieve_resources_from_range(range, _year)
-      if !full_update && recent_export
+      relation = Organisation.left_joins(:users, :organisation_name_changes)
-        params = { from: recent_export.started_at, to: @start_time }
+      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
+      Organisation.where(id: ids)
-          .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
     end
 
     def build_export_xml(organisations)

app/services/exports/sales_log_export_service.rb

@@ -30,14 +30,32 @@ module Exports
       "core_sales_#{year}_#{year + 1}_apr_mar_#{base_number_str}_#{increment_str}".downcase
     end
 
-    def retrieve_resources(recent_export, full_update, year)
+    def retrieve_resources_from_range(range, year)
-      if !full_update && recent_export
+      relation = SalesLog.exportable.filter_by_year(year).left_joins(:created_by, :updated_by, :assigned_to, :owning_organisation, :managing_organisation)
-        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)
+      ids = relation
-      else
+              .where({ updated_at: range })
-        params = { to: @start_time }
+              .or(
-        SalesLog.exportable.where("updated_at <= :to", params).filter_by_year(year)
+                relation.where.not(values_updated_at: nil).where(values_updated_at: range),
-      end
+              )
+              .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)
+
+      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)
+    def retrieve_resources_from_range(range, _year)
-      if !full_update && recent_export
+      relation = User.left_joins(organisation: :organisation_name_changes)
-        params = { from: recent_export.started_at, to: @start_time }
+      ids = relation
-        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)
+              .where({ updated_at: range })
-      else
+              .or(
-        params = { to: @start_time }
+                relation.where.not(organisations: { updated_at: nil }).where(organisations: { updated_at: range }),
-        User.where("updated_at <= :to", params)
+              )
-      end
+              .or(
+                relation.where(organisation_name_changes: { created_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

@@ -26,7 +26,7 @@ class Merge::MergeOrganisationsService
       log_success_message
     rescue ActiveRecord::RecordInvalid => e
       Rails.logger.error("Organisation merge failed with: #{e.message}")
-      raise ActiveRecord::Rollback
+      raise
     end
   end
 

app/views/devise/passwords/reset_resend_confirmation.html.erb

@@ -6,7 +6,11 @@
       <%= content_for(:title) %>
     </h1>
 
-    <p class="govuk-body">We’ve sent a link to reset your password to <strong><%= @email %></strong>.</p>
+    <% if @unconfirmed %>
+      <p class="govuk-body">We’ve sent a link to confirm your email address to <strong><%= @email %></strong>. This will complete your registration onto the CORE service.</p>
+    <% else %>
+      <p class="govuk-body">We’ve sent a link to reset your password to <strong><%= @email %></strong>.</p>
+    <% end %>
     <p class="govuk-body">You’ll only receive this link if your email address already exists in our system.</p>
     <p class="govuk-body">If you don’t receive the email within 5 minutes, check your spam or junk folders. Try again if you still haven’t received the email.</p>
   </div>

docs/Gemfile.lock

@@ -210,11 +210,11 @@ GEM
       jekyll-feed (~> 0.9)
       jekyll-seo-tag (~> 2.1)
     minitest (5.19.0)
-    nokogiri (1.18.8-arm64-darwin)
+    nokogiri (1.18.9-arm64-darwin)
       racc (~> 1.4)
-    nokogiri (1.18.8-x86_64-darwin)
+    nokogiri (1.18.9-x86_64-darwin)
       racc (~> 1.4)
-    nokogiri (1.18.8-x86_64-linux-gnu)
+    nokogiri (1.18.9-x86_64-linux-gnu)
       racc (~> 1.4)
     octokit (4.25.1)
       faraday (>= 1, < 3)
@@ -226,7 +226,7 @@ GEM
     rb-fsevent (0.11.2)
     rb-inotify (0.10.1)
       ffi (~> 1.0)
-    rexml (3.3.9)
+    rexml (3.4.2)
     rouge (3.26.0)
     ruby2_keywords (0.0.5)
     rubyzip (2.3.2)

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,75 @@ 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
+    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,74 @@ 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
+    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

@@ -59,7 +59,7 @@ RSpec.describe Merge::MergeOrganisationsService do
         allow(Organisation).to receive(:find).with(absorbing_organisation.id).and_return(absorbing_organisation)
         allow(absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid)
         expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid")
-        merge_organisations_service.call
+        expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid)
 
         absorbing_organisation.reload
         merging_organisation.reload
@@ -103,7 +103,7 @@ RSpec.describe Merge::MergeOrganisationsService do
           allow(Organisation).to receive(:find).with(absorbing_organisation.id).and_return(absorbing_organisation)
           allow(absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid)
           expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid")
-          merge_organisations_service.call
+          expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid)
 
           absorbing_organisation.reload
           merging_organisation.reload
@@ -140,7 +140,7 @@ RSpec.describe Merge::MergeOrganisationsService do
           allow(Organisation).to receive(:find).with(absorbing_organisation.id).and_return(absorbing_organisation)
           allow(absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid)
           expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid")
-          merge_organisations_service.call
+          expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid)
 
           absorbing_organisation.reload
           expect(absorbing_organisation.child_organisations.count).to eq(3)
@@ -557,7 +557,7 @@ RSpec.describe Merge::MergeOrganisationsService do
             allow(Organisation).to receive(:find).with(absorbing_organisation.id).and_return(absorbing_organisation)
             allow(absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid)
             expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid")
-            merge_organisations_service.call
+            expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid)
 
             absorbing_organisation.reload
             merging_organisation.reload
@@ -590,7 +590,7 @@ RSpec.describe Merge::MergeOrganisationsService do
           allow(Organisation).to receive(:find).with(absorbing_organisation.id).and_return(absorbing_organisation)
           allow(absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid)
           expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid")
-          merge_organisations_service.call
+          expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid)
 
           absorbing_organisation.reload
           expect(absorbing_organisation.sales_logs.count).to eq(0)
@@ -652,7 +652,7 @@ RSpec.describe Merge::MergeOrganisationsService do
             allow(Organisation).to receive(:find).with(absorbing_organisation.id).and_return(absorbing_organisation)
             allow(absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid)
             expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid")
-            merge_organisations_service.call
+            expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid)
 
             absorbing_organisation.reload
             expect(absorbing_organisation.sales_logs.count).to eq(0)
@@ -688,7 +688,7 @@ RSpec.describe Merge::MergeOrganisationsService do
             allow(Organisation).to receive(:find).with(absorbing_organisation.id).and_return(absorbing_organisation)
             allow(absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid)
             expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid")
-            merge_organisations_service.call
+            expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid)
 
             absorbing_organisation.reload
             expect(absorbing_organisation.lettings_logs.count).to eq(0)
@@ -852,7 +852,7 @@ RSpec.describe Merge::MergeOrganisationsService do
             allow(Organisation).to receive(:find).with(absorbing_organisation.id).and_return(absorbing_organisation)
             allow(absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid)
             expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid")
-            merge_organisations_service.call
+            expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid)
 
             absorbing_organisation.reload
             merging_organisation.reload
@@ -971,7 +971,7 @@ RSpec.describe Merge::MergeOrganisationsService do
         allow(Organisation).to receive(:find).with(absorbing_organisation.id).and_return(absorbing_organisation)
         allow(absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid)
         expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid")
-        merge_organisations_service.call
+        expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid)
 
         absorbing_organisation.reload
         merging_organisation.reload
@@ -1096,7 +1096,7 @@ RSpec.describe Merge::MergeOrganisationsService do
           allow(Organisation).to receive(:find).with(absorbing_organisation.id).and_return(absorbing_organisation)
           allow(absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid)
           expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid")
-          merge_organisations_service.call
+          expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid)
 
           absorbing_organisation.reload
           merging_organisation.reload
@@ -1166,7 +1166,7 @@ RSpec.describe Merge::MergeOrganisationsService do
         allow(Organisation).to receive(:find).with(new_absorbing_organisation.id).and_return(new_absorbing_organisation)
         allow(new_absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid)
         expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid")
-        merge_organisations_service.call
+        expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid)
 
         new_absorbing_organisation.reload
         merging_organisation.reload
@@ -1210,7 +1210,7 @@ RSpec.describe Merge::MergeOrganisationsService do
           allow(Organisation).to receive(:find).with(new_absorbing_organisation.id).and_return(new_absorbing_organisation)
           allow(new_absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid)
           expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid")
-          merge_organisations_service.call
+          expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid)
 
           new_absorbing_organisation.reload
           merging_organisation.reload
@@ -1247,7 +1247,7 @@ RSpec.describe Merge::MergeOrganisationsService do
           allow(Organisation).to receive(:find).with(new_absorbing_organisation.id).and_return(new_absorbing_organisation)
           allow(new_absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid)
           expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid")
-          merge_organisations_service.call
+          expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid)
 
           new_absorbing_organisation.reload
           expect(new_absorbing_organisation.child_organisations.count).to eq(3)
@@ -1352,7 +1352,7 @@ RSpec.describe Merge::MergeOrganisationsService do
           allow(Organisation).to receive(:find).with(new_absorbing_organisation.id).and_return(new_absorbing_organisation)
           allow(new_absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid)
           expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid")
-          merge_organisations_service.call
+          expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid)
 
           new_absorbing_organisation.reload
           merging_organisation.reload
@@ -1384,7 +1384,7 @@ RSpec.describe Merge::MergeOrganisationsService do
           allow(Organisation).to receive(:find).with(new_absorbing_organisation.id).and_return(new_absorbing_organisation)
           allow(new_absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid)
           expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid")
-          merge_organisations_service.call
+          expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid)
 
           new_absorbing_organisation.reload
           expect(new_absorbing_organisation.sales_logs.count).to eq(0)
@@ -1423,7 +1423,7 @@ RSpec.describe Merge::MergeOrganisationsService do
             allow(Organisation).to receive(:find).with(new_absorbing_organisation.id).and_return(new_absorbing_organisation)
             allow(new_absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid)
             expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid")
-            merge_organisations_service.call
+            expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid)
 
             new_absorbing_organisation.reload
             expect(new_absorbing_organisation.sales_logs.count).to eq(0)
@@ -1459,7 +1459,7 @@ RSpec.describe Merge::MergeOrganisationsService do
             allow(Organisation).to receive(:find).with(new_absorbing_organisation.id).and_return(new_absorbing_organisation)
             allow(new_absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid)
             expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid")
-            merge_organisations_service.call
+            expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid)
 
             new_absorbing_organisation.reload
             expect(new_absorbing_organisation.lettings_logs.count).to eq(0)
@@ -1565,7 +1565,7 @@ RSpec.describe Merge::MergeOrganisationsService do
             allow(Organisation).to receive(:find).with(new_absorbing_organisation.id).and_return(new_absorbing_organisation)
             allow(new_absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid)
             expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid")
-            merge_organisations_service.call
+            expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid)
 
             new_absorbing_organisation.reload
             merging_organisation.reload
@@ -1655,7 +1655,7 @@ RSpec.describe Merge::MergeOrganisationsService do
         allow(Organisation).to receive(:find).with(new_absorbing_organisation.id).and_return(new_absorbing_organisation)
         allow(new_absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid)
         expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid")
-        merge_organisations_service.call
+        expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid)
 
         new_absorbing_organisation.reload
         merging_organisation.reload
@@ -1697,7 +1697,7 @@ RSpec.describe Merge::MergeOrganisationsService do
           allow(Organisation).to receive(:find).with(new_absorbing_organisation.id).and_return(new_absorbing_organisation)
           allow(new_absorbing_organisation).to receive(:save!).and_raise(ActiveRecord::RecordInvalid)
           expect(Rails.logger).to receive(:error).with("Organisation merge failed with: Record invalid")
-          merge_organisations_service.call
+          expect { merge_organisations_service.call }.to raise_error(ActiveRecord::RecordInvalid)
 
           new_absorbing_organisation.reload
           merging_organisation.reload