Merge pull request #6096 from NHSDigital/alistair/vaccs-import-deduplicate-rows

Deduplicate rows in `ImmunisationImport`s

Author: alistair-white-horne-tng

app/models/immunisation_import.rb

@@ -67,7 +67,24 @@ def records_count
   private
 
   def check_rows_are_unique
-    # there is no uniqueness check for immunisations
+    rows
+      .map(&:full_row_deduplication_attributes)
+      .tally
+      .each do |full_row_deduplication_attributes, count|
+        next if count <= 1
+
+        matching_rows =
+          rows.select do
+            it.full_row_deduplication_attributes ==
+              full_row_deduplication_attributes
+          end
+        matching_rows.each do |row|
+          row.errors.add(
+            :base,
+            "This record appears more than once in the file. Remove any duplicates."
+          )
+        end
+      end
   end
 
   def parse_row(data)

app/models/immunisation_import_row.rb

@@ -103,7 +103,6 @@ def initialize(data:, team:, type:)
     @type = type&.to_sym
   end
 
-  # Convenience predicate helpers mirroring the enum on ImmunisationImport
   def point_of_care? = type == :point_of_care
 
   def national_reporting_flu? = national_reporting? && programme&.flu?
@@ -119,80 +118,13 @@ def national_reporting_not_administered?
   def to_vaccination_record
     return if invalid? || national_reporting_not_administered? || patient.nil?
 
-    outcome = (administered ? "administered" : reason_not_administered_value)
-    source =
-      if imms_api_record?
-        "nhs_immunisations_api"
-      elsif offline_recording?
-        "service"
-      elsif point_of_care?
-        "historical_upload"
-      else
-        "national_reporting"
-      end
-
-    attributes = {
-      disease_types:,
-      dose_sequence: dose_sequence_value,
-      full_dose: true,
-      outcome:,
-      patient:,
-      performed_at_date:,
-      performed_by_user:,
-      performed_ods_code: performed_ods_code&.to_s,
-      programme_type: programme&.type,
-      protocol:,
-      session:,
-      supplied_by:
-    }
-
-    attributes.merge!(location:, location_name:) unless imms_api_record?
-    attributes.merge!(notify_parents: true) if session
-
-    if performed_by_user.nil? &&
-         (performed_by_family_name.present? || performed_by_given_name.present?)
-      attributes.merge!(
-        performed_by_family_name: performed_by_family_name&.to_s,
-        performed_by_given_name: performed_by_given_name&.to_s
-      )
-    end
-
-    if national_reporting?
-      attributes.merge!(
-        local_patient_id: local_patient_id&.to_s,
-        local_patient_id_uri: local_patient_id_uri&.to_s
-      )
-    end
-
-    attributes_to_stage_if_already_exists = {
-      batch_number: batch_name&.to_s,
-      batch_expiry: batch_expiry&.to_date,
-      discarded_at: nil,
-      notes: notes&.to_s,
-      performed_at_time:,
-      source:,
-      vaccine_id: vaccine&.id
-    }
-
-    delivery_attributes = {
-      delivery_method: delivery_method_value,
-      delivery_site: delivery_site_value
-    }
-
     vaccination_record =
-      if national_reporting?
-        VaccinationRecord.find_or_initialize_by(
-          attributes.merge(
-            attributes_to_stage_if_already_exists,
-            delivery_attributes
-          )
-        )
-      elsif uuid.present?
+      if uuid.present?
         VaccinationRecord
           .find_by!(uuid: uuid.to_s)
           .tap { it.stage_changes(attributes) }
       else
-        VaccinationRecord.find_or_initialize_by(attributes)
+        VaccinationRecord.find_or_initialize_by(deduplication_attributes)
       end
 
     if vaccination_record.persisted?
@@ -240,11 +172,94 @@ def to_archive_reason
   end
 
   def set_patient(candidates: nil)
+    # Invalidate the memoised `@attributes`, which might reference the old value of `@patient` (likely to be `nil`)
+    @attributes = nil
+
     @patient =
       existing_patients(candidates:)&.first ||
         Patient.new(new_patient_attributes)
   end
 
+  def attributes
+    @attributes ||=
+      begin
+        outcome =
+          (administered ? "administered" : reason_not_administered_value)
+
+        attributes = {
+          disease_types:,
+          dose_sequence: dose_sequence_value,
+          full_dose: true,
+          outcome:,
+          patient:,
+          performed_at_date:,
+          performed_by_user:,
+          performed_ods_code: performed_ods_code&.to_s,
+          programme_type: programme&.type,
+          protocol:,
+          session:,
+          supplied_by:
+        }
+
+        attributes.merge!(location:, location_name:) unless imms_api_record?
+        attributes.merge!(notify_parents: true) if session
+
+        if performed_by_user.nil? &&
+             (
+               performed_by_family_name.present? ||
+                 performed_by_given_name.present?
+             )
+          attributes.merge!(
+            performed_by_family_name: performed_by_family_name&.to_s,
+            performed_by_given_name: performed_by_given_name&.to_s
+          )
+        end
+
+        if national_reporting?
+          attributes.merge!(
+            local_patient_id: local_patient_id&.to_s,
+            local_patient_id_uri: local_patient_id_uri&.to_s
+          )
+        end
+
+        attributes
+      end
+  end
+
+  def attributes_to_stage_if_already_exists
+    @attributes_to_stage_if_already_exists ||= {
+      batch_number: batch_name&.to_s,
+      batch_expiry: batch_expiry&.to_date,
+      discarded_at: nil,
+      notes: notes&.to_s,
+      performed_at_time:,
+      source:,
+      vaccine_id: vaccine&.id
+    }
+  end
+
+  def delivery_attributes
+    @delivery_attributes ||= {
+      delivery_method: delivery_method_value,
+      delivery_site: delivery_site_value
+    }
+  end
+
+  def deduplication_attributes
+    if national_reporting?
+      attributes.merge(
+        attributes_to_stage_if_already_exists,
+        delivery_attributes
+      )
+    else
+      attributes
+    end
+  end
+
+  def full_row_deduplication_attributes
+    deduplication_attributes.merge(new_patient_attributes)
+  end
+
   def batch_expiry = @data[:batch_expiry_date]
 
   def batch_name = @data[:batch_number] || @data[:vaccination_batch_number]
@@ -340,7 +355,7 @@ def location_name
     end
   end
 
-  def performed_at_date = date_of_vaccination.to_date
+  def performed_at_date = date_of_vaccination&.to_date
 
   def performed_at_time = time_of_vaccination&.to_time
 
@@ -399,6 +414,18 @@ def session
       end
   end
 
+  def source
+    if imms_api_record?
+      "nhs_immunisations_api"
+    elsif offline_recording?
+      "service"
+    elsif point_of_care?
+      "historical_upload"
+    else
+      "national_reporting"
+    end
+  end
+
   def protocol
     if imms_api_record?
       nil
@@ -450,7 +477,10 @@ def programmes_by_name
         end
   end
 
-  delegate :default_dose_sequence, :maximum_dose_sequence, to: :programme
+  delegate :default_dose_sequence,
+           :maximum_dose_sequence,
+           to: :programme,
+           allow_nil: true
 
   def offline_recording? = session_id.present?
 
@@ -546,7 +576,7 @@ def delivery_method_value
     end
   end
 
-  def disease_types = vaccine&.disease_types || programme.disease_types
+  def disease_types = vaccine&.disease_types || programme&.disease_types
 
   def dose_sequence_value
     value =

spec/factories/immunisation_imports.rb

@@ -41,7 +41,7 @@
     csv_filename { Faker::File.file_name(ext: "csv") }
     rows_count { rand(100..1000) }
 
-    type { "point_of_care" }
+    type { team.type }
 
     trait :csv_removed do
       csv_data { nil }

spec/factories/teams.rb

@@ -60,10 +60,13 @@
 
     trait :national_reporting do
       type { :national_reporting }
+
       email { nil }
       phone { nil }
       privacy_notice_url { nil }
       privacy_policy_url { nil }
+
+      programmes { [Programme.flu, Programme.hpv] }
     end
 
     trait :with_one_nurse do

spec/features/import_vaccination_records_national_reporting_spec.rb

@@ -52,15 +52,7 @@
   end
 
   def given_mavis_logins_are_configured
-    programmes = [Programme.flu, Programme.hpv]
-    @team =
-      create(
-        :team,
-        :national_reporting,
-        :with_one_nurse,
-        ods_code: "R1L",
-        programmes: programmes
-      )
+    @team = create(:team, :national_reporting, :with_one_nurse, ods_code: "R1L")
     create(:school, team: @team, urn: 100_000)
   end
 

spec/fixtures/files/immunisation_import/national_reporting/duplicate_row.csv

@@ -0,0 +1,3 @@
+ORGANISATION_CODE,SCHOOL_URN,SCHOOL_NAME,NHS_NUMBER,PERSON_FORENAME,PERSON_SURNAME,PERSON_DOB,PERSON_GENDER_CODE,PERSON_POSTCODE,VACCINATED,DATE_OF_VACCINATION,VACCINE_GIVEN,BATCH_NUMBER,BATCH_EXPIRY_DATE,ANATOMICAL_SITE,PERFORMING_PROFESSIONAL_FORENAME,PERFORMING_PROFESSIONAL_SURNAME,REASON_NOT_VACCINATED,DOSE_SEQUENCE,CONSENT_TYPE,LOCAL_PATIENT_ID,LOCAL_PATIENT_ID_URI,CARE_SETTING
+RYG,100000,Hogwarts,9449308357,Harry,Potter,20010101,male,AA11 1AA,Y,20251109,AstraZeneca Fluenz LAIV,ABC123,20251030,nasal,Albus,Dumbledore,,,Parental Consent,1234,ABCD,
+RYG,100000,Hogwarts,9449308357,Harry,Potter,20010101,male,AA11 1AA,Y,20251109,AstraZeneca Fluenz LAIV,ABC123,20251030,nasal,Albus,Dumbledore,,,Parental Consent,1234,ABCD,

spec/fixtures/files/immunisation_import/point_of_care/duplicate_row.csv

@@ -0,0 +1,3 @@
+ORGANISATION_CODE,SCHOOL_URN,SCHOOL_NAME,NHS_NUMBER,PERSON_FORENAME,PERSON_SURNAME,PERSON_DOB,PERSON_GENDER_CODE,PERSON_POSTCODE,VACCINATED,DATE_OF_VACCINATION,PROGRAMME,VACCINE_GIVEN,BATCH_NUMBER,BATCH_EXPIRY_DATE,ANATOMICAL_SITE,PERFORMING_PROFESSIONAL_FORENAME,PERFORMING_PROFESSIONAL_SURNAME,REASON_NOT_VACCINATED,CONSENT_TYPE,LOCAL_PATIENT_ID,LOCAL_PATIENT_ID_URI
+R1L,120026,shaftesbury junior school ,7420180008,Chyna,Pickle,20120912,Not Specified,LE3 2DA,Yes,20250514,Flu,,123013325,20220730,Left Buttock,Vaccinator1,Name1,,Parental Consent,LocalPatient1,www.LocalPatient1
+R1L,120026,shaftesbury junior school ,7420180008,Chyna,Pickle,20120912,Not Specified,LE3 2DA,Yes,20250514,Flu,,ABC123,20231001,Left Buttock,Vaccinator1,Name1,,Parental Consent,LocalPatient1,www.LocalPatient1