Merge pull request #5884 from nhsuk/background-processing-of-parent-removal

Process bulk removal of parents from an import as a background task

Author: murugapl

app/components/app_import_status_component.rb

@@ -20,6 +20,7 @@ def status_text
       "in_re_review" => "Review",
       "committing" => "Importing",
       "processed" => "Completed",
+      "removing_parent_relationships" => "Completed",
       "partially_processed" => "Partially completed",
       "low_pds_match_rate" => "Failed",
       "cancelled" => "Cancelled"
@@ -36,6 +37,7 @@ def status_color
       "in_re_review" => "blue",
       "committing" => "blue",
       "processed" => "green",
+      "removing_parent_relationships" => "green",
       "partially_processed" => "green",
       "low_pds_match_rate" => "red",
       "cancelled" => "grey"

app/components/app_import_summary_component.html.erb

@@ -55,4 +55,16 @@
         <% end %>
     <% end %>
   <% end %>
+
+  <% if Flipper.enabled?(:import_bulk_remove_parents) &&
+        import.is_a?(PatientImport) &&
+        import.parent_relationships.present? %>
+    <% if import.removing_parent_relationships? %>
+      <p class="nhsuk-u-secondary-text-colour">Parent-child relationships are currently being removed from this import</p>
+    <% elsif import.processed? && import.parent_relationships.any? %>
+      <p>
+        <%= link_to "Remove all parent-child relationships from import", imports_bulk_remove_parents_path(import.class.name.underscore, import.id) %>
+      </p>
+    <% end %>
+  <% end %>
 <% end %>

app/controllers/imports/bulk_remove_parents_controller.rb

@@ -51,14 +51,7 @@ def set_import
     end
 
     def set_consents
-      @consents =
-        Consent
-          .includes(patient: { parent_relationships: :parent })
-          .joins(patient: :parent_relationships)
-          .merge(@import.patients)
-          .merge(@import.parent_relationships)
-          .where("consents.parent_id = parent_relationships.parent_id")
-          .not_invalidated
+      @consents = @import.parent_relationship_consents
     end
 
     def success_flash_text

app/forms/bulk_remove_parents_form.rb

@@ -4,6 +4,8 @@ class BulkRemoveParentsForm
   include ActiveModel::Model
   include ActiveModel::Attributes
 
+  BATCH_SIZE = 100
+
   attr_accessor :current_user
   attr_reader :import, :consents
 
@@ -24,15 +26,18 @@ def initialize(import:, consents:, current_user:, **attributes)
 
   def save!
     return false unless valid?
-
-    if remove_option == "unconsented_only"
-      import.destroy_parent_relationships_without_consent!(consents)
-    else
-      import.destroy_parent_relationships_and_invalidate_consents!(
-        current_user,
-        consents
-      )
-    end
+    import.update!(status: :removing_parent_relationships)
+
+    import
+      .parent_relationship_ids
+      .each_slice(BATCH_SIZE) do |batch_ids|
+        BulkRemoveParentRelationshipsJob.perform_later(
+          import.to_global_id.to_s,
+          batch_ids,
+          current_user.id,
+          remove_option
+        )
+      end
 
     true
   end

app/jobs/bulk_remove_parent_relationships_job.rb

@@ -0,0 +1,73 @@
+# frozen_string_literal: true
+
+class BulkRemoveParentRelationshipsJob < ApplicationJob
+  queue_as :imports
+
+  def perform(
+    import_global_id,
+    parent_relationship_ids_batch,
+    user_id,
+    remove_option
+  )
+    import = GlobalID::Locator.locate(import_global_id)
+    user = User.find(user_id)
+
+    ActiveRecord::Base.transaction do
+      parent_relationships =
+        import
+          .parent_relationships
+          .where(id: parent_relationship_ids_batch)
+          .includes(:parent, :patient)
+
+      consents =
+        import.parent_relationship_consents(scope: parent_relationships)
+
+      parent_relationships_to_remove =
+        if remove_option == "unconsented_only"
+          consented_pairs = consents.pluck(:patient_id, :parent_id).to_set
+
+          parent_relationships.reject do |pr|
+            consented_pairs.include?([pr.patient_id, pr.parent_id])
+          end
+        else
+          parent_relationships
+        end
+
+      invalidate_consents!(user, consents) if remove_option == "all"
+
+      parents_to_check = parent_relationships_to_remove.map(&:parent)
+      patient_ids = parent_relationships_to_remove.map(&:patient_id)
+
+      parent_relationships_to_remove.each(&:destroy!)
+
+      parents_to_check.each do |parent|
+        next if parent.destroyed?
+        next if parent.parent_relationships.exists?
+        next if parent.consents.exists?
+
+        parent.destroy!
+      end
+
+      StatusUpdaterJob.perform_bulk(patient_ids.zip)
+    end
+
+    mark_complete_if_finished(import, remove_option)
+  end
+
+  private
+
+  def invalidate_consents!(user, consents)
+    timestamp = Time.current.to_fs(:long)
+    invalidation_note =
+      "Consent invalidated on #{timestamp} " \
+        "because #{user.full_name} removed all parent-child relationships from an import."
+
+    consents.update_all(notes: invalidation_note, invalidated_at: Time.current)
+  end
+
+  def mark_complete_if_finished(import, remove_option)
+    return unless import.remaining_parent_relationships(remove_option:).empty?
+
+    import.update!(status: :processed)
+  end
+end

app/models/concerns/csv_importable.rb

@@ -38,7 +38,15 @@ module CSVImportable
             )
           end
     scope :status_for_imported_records,
-          -> { where(status: %i[processed partially_processed]) }
+          -> do
+            where(
+              status: %i[
+                processed
+                partially_processed
+                removing_parent_relationships
+              ]
+            )
+          end
 
     enum :status,
          {
@@ -52,7 +60,8 @@ module CSVImportable
            in_re_review: 7,
            committing: 8,
            partially_processed: 9,
-           cancelled: 10
+           cancelled: 10,
+           removing_parent_relationships: 11
          },
          default: :pending_import,
          validate: true

app/models/patient_import.rb

@@ -128,61 +128,23 @@ def commit_changesets(changesets)
     end
   end
 
-  def destroy_parent_relationships_and_invalidate_consents!(
-    current_user,
-    consents
-  )
-    ActiveRecord::Base.transaction do
-      parent_relationships =
-        self.parent_relationships.includes(:parent, :patient)
-
-      timestamp = Time.current.to_fs(:long)
-      user_name = current_user&.full_name || "system"
-      invalidation_note =
-        "Consent invalidated on #{timestamp} " \
-          "because #{user_name} removed all parent-child relationships from an import."
-
-      # Invalidate consents first before destroying parent relationships, which
-      # will make consents return an empty set.
-      consents.update_all(
-        notes: invalidation_note,
-        invalidated_at: Time.current
-      )
-
-      patient_ids_to_update = consents.map(&:patient_id).uniq
-
-      parents_to_check = parent_relationships.map(&:parent)
-
-      parent_relationships.destroy_all
-
-      parents_to_check.each do |parent|
-        if !parent.destroyed? && parent.parent_relationships.empty? &&
-             parent.consents.empty?
-          parent.destroy!
-        end
-      end
-
-      StatusUpdaterJob.perform_bulk(patient_ids_to_update.zip)
+  def remaining_parent_relationships(remove_option:)
+    if remove_option == "unconsented_only"
+      parent_relationships -
+        parent_relationship_consents.map(&:parent_relationship)
+    else
+      parent_relationships
     end
   end
 
-  def destroy_parent_relationships_without_consent!(consents)
-    ActiveRecord::Base.transaction do
-      parent_relationships_without_consents =
-        parent_relationships.includes(:parent, :patient) -
-          consents.map(&:parent_relationship).uniq
-
-      parents_to_check = parent_relationships_without_consents.map(&:parent)
-
-      parent_relationships_without_consents.each(&:destroy!)
-
-      parents_to_check.each do |parent|
-        if !parent.destroyed? && parent.parent_relationships.empty? &&
-             parent.consents.empty?
-          parent.destroy!
-        end
-      end
-    end
+  def parent_relationship_consents(scope: parent_relationships)
+    Consent
+      .includes(patient: { parent_relationships: :parent })
+      .joins(patient: :parent_relationships)
+      .merge(patients)
+      .merge(scope)
+      .where("consents.parent_id = parent_relationships.parent_id")
+      .not_invalidated
   end
 
   private

app/views/imports/bulk_remove_parents/new.html.erb

@@ -57,7 +57,7 @@
 
     <%= render AppPaginationComponent.new(pagy: @pagy) %>
 
-    <%= f.govuk_collection_radio_buttons :remove_option, %w[unconsented_only all], :itself %>
+    <%= f.govuk_collection_radio_buttons :remove_option, %w[unconsented_only all], :itself, legend: { text: "Which relationships do you want to remove?" } %>
   <% end %>
 
   <div class="nhsuk-button-group">

app/views/imports/show.html.erb

@@ -68,7 +68,7 @@
       ) %>
 <% end %>
 
-<% if import.processed? || import.partially_processed? %>
+<% if import.processed? || import.partially_processed? || import.removing_parent_relationships? %>
 
   <% if @cancelled.present? %>
     <h3 class="nhsuk-u-reading-width">
@@ -132,12 +132,4 @@
     </h3>
     <%= render AppVaccinationRecordTableComponent.new(@vaccination_records, current_user:, pagy: @pagy) %>
   <% end %>
-
-  <% if Flipper.enabled?(:import_bulk_remove_parents) &&
-        import.is_a?(PatientImport) && import.processed? &&
-        import.parent_relationships.present? %>
-    <p>
-      <%= link_to "Remove all parent-child relationships from import", imports_bulk_remove_parents_path(import.class.name.underscore, import.id) %>
-    </p>
-  <% end %>
 <% end %>

config/locales/en.yml

@@ -691,8 +691,8 @@ en:
           cancel: Cancel
       create:
         success_flash:
-          unconsented_only: Parent–child relationships without a submitted consent response have been removed.
-          all: All parent-child relationships included in this import have been removed.
+          unconsented_only: Parent–child relationships without a submitted consent response are being removed.
+          all: All parent-child relationships included in this import are being removed.
   mailers:
     consent_form_mailer:
       reasons_for_refusal: