Merge pull request #5260 from nhsuk/fix-race-condition-mav-906

Fix race condition when committing patient imports

Author: misaka

app/controllers/class_imports_controller.rb

@@ -107,15 +107,9 @@ def approve
     @class_import.reviewed_at << Time.zone.now
     @class_import.committing!
 
-    @class_import
-      .changesets
-      .from_file
-      .ready_for_review
-      .in_batches(of: 100) do |batch|
-        CommitPatientChangesetsJob.perform_async(batch.ids)
-      end
-
-    @class_import.changesets.from_file.ready_for_review.each(&:committing!)
+    @class_import.commit_changesets(
+      @class_import.changesets.from_file.ready_for_review
+    )
 
     redirect_to imports_path, flash: { info: "Import started" }
   end

app/controllers/cohort_imports_controller.rb

@@ -106,15 +106,9 @@ def approve
     @cohort_import.reviewed_at << Time.zone.now
     @cohort_import.committing!
 
-    @cohort_import
-      .changesets
-      .from_file
-      .ready_for_review
-      .in_batches(of: 100) do |batch|
-        CommitPatientChangesetsJob.perform_async(batch.ids)
-      end
-
-    @cohort_import.changesets.from_file.ready_for_review.each(&:committing!)
+    @cohort_import.commit_changesets(
+      @cohort_import.changesets.from_file.ready_for_review
+    )
 
     redirect_to imports_path, flash: { info: "Import started" }
   end

app/jobs/commit_patient_changesets_job.rb

@@ -41,16 +41,10 @@ def perform(patient_changeset_ids)
       end
     end
 
-    if import.changesets.committing.none?
-      if import.changesets.needs_re_review.any?
-        trigger_re_review(import)
-      else
-        import.update_columns(processed_at: Time.zone.now, status: :processed)
-      end
-      import.postprocess_rows!
-      reset_counts(import)
-      import.update_columns(**counts)
+    if finished_committing_changesets?(import)
+      run_post_commit_tasks(import, counts)
     end
+
     SyncPatientTeamJob.perform_later(SchoolMove, imported_school_move_ids)
     import.post_commit!
   end
@@ -93,6 +87,22 @@ def review_consistent?(changeset)
     !inconsistent
   end
 
+  def finished_committing_changesets?(import)
+    import.changesets.committing.none?
+  end
+
+  # Tasks that get run after all the other batches have run
+  def run_post_commit_tasks(import, counts)
+    if import.changesets.needs_re_review.any?
+      trigger_re_review(import)
+    else
+      import.update_columns(processed_at: Time.zone.now, status: :processed)
+    end
+    import.postprocess_rows!
+    reset_counts(import)
+    import.update_columns(**counts)
+  end
+
   def trigger_re_review(import)
     import.calculating_re_review!
     import.changesets.needs_re_review.each do |changeset|

app/models/patient_import.rb

@@ -77,6 +77,15 @@ def validate_changeset_uniqueness!
     end
   end
 
+  def commit_changesets(changesets)
+    changesets_ids = changesets.ids
+
+    changesets.update_all(status: :committing)
+    changesets_ids.each_slice(100) do |batch_ids|
+      CommitPatientChangesetsJob.perform_async(batch_ids)
+    end
+  end
+
   private
 
   def check_rows_are_unique