Merge pull request #5150 from nhsuk/remove_patient_teams_sync_from_transactions

thomasleese · web-flow · commit 2c30d2bacd64 · 2025-11-10T11:54:41.000Z
Move all perform_later calls outside transaction
diff --git a/app/jobs/commit_import_job.rb b/app/jobs/commit_import_job.rb
@@ -27,6 +27,7 @@ def perform(import_global_id)
     end
 
     counts = import.class.const_get(:COUNT_COLUMNS).index_with(0)
+    imported_school_move_ids = []
 
     ActiveRecord::Base.transaction do
       import
@@ -37,11 +38,10 @@ def perform(import_global_id)
 
           import_patients_and_parents(changesets, import)
 
-          import_school_moves(changesets, import)
+          imported_school_move_ids |= import_school_moves(changesets, import)
 
           import_pds_search_results(changesets, import)
         end
-
       import.postprocess_rows!
 
       reset_counts(import)
@@ -52,7 +52,7 @@ def perform(import_global_id)
         **counts
       )
     end
-
+    SyncPatientTeamJob.perform_later(SchoolMove, imported_school_move_ids)
     import.post_commit!
   end
 
@@ -124,12 +124,11 @@ def import_school_moves(changesets, import)
       school_move.confirm! if school_move.patient.persisted?
     end
     school_move_import_records = importable_school_moves.to_a
-    imported_ids =
-      SchoolMove.import!(
-        school_move_import_records,
-        on_duplicate_key_update: :all
-      ).ids
-    SyncPatientTeamJob.perform_later(SchoolMove, imported_ids)
+
+    SchoolMove.import!(
+      school_move_import_records,
+      on_duplicate_key_update: :all
+    ).ids
   end
 
   def import_pds_search_results(changesets, import)
diff --git a/app/jobs/commit_patient_changesets_job.rb b/app/jobs/commit_patient_changesets_job.rb
@@ -27,6 +27,7 @@ def perform(import_global_id)
     end
 
     counts = import.class.const_get(:COUNT_COLUMNS).index_with(0)
+    imported_school_move_ids = []
 
     ActiveRecord::Base.transaction do
       import
@@ -37,11 +38,10 @@ def perform(import_global_id)
 
           import_patients_and_parents(changesets, import)
 
-          import_school_moves(changesets, import)
+          imported_school_move_ids |= import_school_moves(changesets, import)
 
           import_pds_search_results(changesets, import)
         end
-
       import.postprocess_rows!
 
       reset_counts(import)
@@ -52,7 +52,7 @@ def perform(import_global_id)
         **counts
       )
     end
-
+    SyncPatientTeamJob.perform_later(SchoolMove, imported_school_move_ids)
     import.post_commit!
   end
 
@@ -124,12 +124,11 @@ def import_school_moves(changesets, import)
       school_move.confirm! if school_move.patient.persisted?
     end
     school_move_import_records = importable_school_moves.to_a
-    imported_ids =
-      SchoolMove.import!(
-        school_move_import_records,
-        on_duplicate_key_update: :all
-      ).ids
-    SyncPatientTeamJob.perform_later(SchoolMove, imported_ids)
+
+    SchoolMove.import!(
+      school_move_import_records,
+      on_duplicate_key_update: :all
+    ).ids
   end
 
   def import_pds_search_results(changesets, import)
diff --git a/app/lib/location_sessions_factory.rb b/app/lib/location_sessions_factory.rb
@@ -8,19 +8,31 @@ def initialize(location, academic_year:, sync_patient_teams_now: false)
   end
 
   def call
-    ActiveRecord::Base.transaction do
-      if location.generic_clinic?
-        find_or_create_session!(programmes: location.programmes)
-      else
-        ProgrammeGrouper
-          .call(location.programmes)
-          .values
-          .reject { |programmes| catch_up_only?(programmes:) }
-          .reject { |programmes| already_exists?(programmes:) }
-          .map { |programmes| create_session!(programmes:) }
-      end
+    imported_patient_location_ids =
+      ActiveRecord::Base.transaction do
+        if location.generic_clinic?
+          find_or_create_session!(programmes: location.programmes)
+        else
+          ProgrammeGrouper
+            .call(location.programmes)
+            .values
+            .reject { |programmes| catch_up_only?(programmes:) }
+            .reject { |programmes| already_exists?(programmes:) }
+            .map { |programmes| create_session!(programmes:) }
+        end
 
-      add_patients!
+        add_patients!
+      end
+    if sync_patient_teams_now
+      SyncPatientTeamJob.perform_now(
+        PatientLocation,
+        imported_patient_location_ids
+      )
+    else
+      SyncPatientTeamJob.perform_later(
+        PatientLocation,
+        imported_patient_location_ids
+      )
     end
   end
 
@@ -61,17 +73,11 @@ def find_or_create_session!(programmes:)
   end
 
   def add_patients!
-    imported_ids =
-      PatientLocation.import!(
-        %i[patient_id location_id academic_year],
-        patient_ids.map { [it, location.id, academic_year] },
-        on_duplicate_key_ignore: true
-      ).ids
-    if sync_patient_teams_now
-      SyncPatientTeamJob.perform_now(PatientLocation, imported_ids)
-    else
-      SyncPatientTeamJob.perform_later(PatientLocation, imported_ids)
-    end
+    PatientLocation.import!(
+      %i[patient_id location_id academic_year],
+      patient_ids.map { [it, location.id, academic_year] },
+      on_duplicate_key_ignore: true
+    ).ids
   end
 
   def patient_ids
diff --git a/app/models/class_import.rb b/app/models/class_import.rb
@@ -50,7 +50,6 @@ class ClassImport < PatientImport
 
   def postprocess_rows!
     # Remove patients already in the sessions but not in the class list.
-
     birth_academic_years =
       year_groups.map { it.to_birth_academic_year(academic_year:) }
 
@@ -77,14 +76,15 @@ def postprocess_rows!
         )
       end
 
-    imported_ids =
+    @imported_school_move_ids ||= []
+    @imported_school_move_ids |=
       SchoolMove.import!(school_moves, on_duplicate_key_ignore: true).ids
-    SyncPatientTeamJob.perform_later(SchoolMove, imported_ids)
 
     PatientsAgedOutOfSchoolJob.perform_async(location_id)
   end
 
   def post_commit!
+    SyncPatientTeamJob.perform_later(SchoolMove, @imported_school_move_ids)
   end
 
   private
diff --git a/app/models/concerns/contributes_to_patient_teams.rb b/app/models/concerns/contributes_to_patient_teams.rb
@@ -229,7 +229,7 @@ def sync_patient_teams_table_on_patient_ids(pk_ids)
           sterile_key = connection.quote(PatientTeam.sources.fetch(key.to_s))
           patient_relationships_to_remove =
             select("#{patient_id_source} as patient_id")
-              .where("#{table_name}.id = ANY(ARRAY[?])", pk_ids)
+              .where("#{table_name}.id = ANY(ARRAY[?]::bigint[])", pk_ids)
               .distinct
               .to_sql
           connection.execute <<-SQL
@@ -241,7 +241,7 @@ def sync_patient_teams_table_on_patient_ids(pk_ids)
         end
 
         where(
-          "#{table_name}.id = ANY(ARRAY[?])",
+          "#{table_name}.id = ANY(ARRAY[?]::bigint[])",
           pk_ids
         ).distinct.add_patient_team_relationships
 
diff --git a/app/models/patient.rb b/app/models/patient.rb
@@ -661,12 +661,13 @@ def update_from_pds!(pds_patient)
       raise NHSNumberMismatch
     end
 
+    archived_ids = []
     ActiveRecord::Base.transaction do
       self.date_of_death = pds_patient.date_of_death
 
       if date_of_death_changed?
         if date_of_death.present?
-          archive_due_to_deceased!
+          archived_ids = archive_due_to_deceased!
           clear_pending_sessions!
         end
 
@@ -697,6 +698,8 @@ def update_from_pds!(pds_patient)
 
       save!
     end
+
+    SyncPatientTeamJob.perform_later(ArchiveReason, archived_ids)
   end
 
   def invalidate!
@@ -810,15 +813,13 @@ def archive_due_to_deceased!
         ArchiveReason.new(team:, patient: self, type: :deceased)
       end
 
-    imported_ids =
-      ArchiveReason.import!(
-        archive_reasons,
-        on_duplicate_key_update: {
-          conflict_target: %i[team_id patient_id],
-          columns: %i[type]
-        }
-      ).ids
-    SyncPatientTeamJob.perform_later(ArchiveReason, imported_ids)
+    ArchiveReason.import!(
+      archive_reasons,
+      on_duplicate_key_update: {
+        conflict_target: %i[team_id patient_id],
+        columns: %i[type]
+      }
+    ).ids
   end
 
   def fhir_mapper = @fhir_mapper ||= FHIRMapper::Patient.new(self)
diff --git a/app/models/school_move.rb b/app/models/school_move.rb
@@ -63,13 +63,15 @@ def assign_from(school:, home_educated:, team:)
   end
 
   def confirm!(user: nil)
+    imported_archive_reason_ids = []
     ActiveRecord::Base.transaction do
       update_patient!
-      update_archive_reasons!(user:)
+      imported_archive_reason_ids = update_archive_reasons!(user:)
       update_sessions!
       create_log_entry!(user:)
       destroy! if persisted?
     end
+    SyncPatientTeamJob.perform_later(ArchiveReason, imported_archive_reason_ids)
   end
 
   def ignore!
@@ -99,9 +101,7 @@ def update_archive_reasons!(user:)
         )
       end
 
-    imported_ids =
-      ArchiveReason.import!(archive_reasons, on_duplicate_key_ignore: true).ids
-    SyncPatientTeamJob.perform_later(ArchiveReason, imported_ids)
+    ArchiveReason.import!(archive_reasons, on_duplicate_key_ignore: true).ids
   end
 
   def update_sessions!
