Ensure nhs_number_first_added is always set

Jira-Issue: MAV-7091

Author: jf-x-dev

app/models/concerns/patient_import_concern.rb

@@ -12,6 +12,7 @@ def import_patients_and_parents(changesets, import)
         .uniq { [_1.parent, _1.patient] }
 
     deduplicate_patients!(patients, relationships)
+    patients.each(&:ensure_nhs_number_first_added_at)
 
     patients_with_nhs_number_changes =
       patients.select(&:nhs_number_previously_changed?)

app/models/patient.rb

@@ -497,8 +497,7 @@ class Patient < ApplicationRecord
                it.blank? ? nil : it.normalise_whitespace.gsub(/\s/, "")
              end
 
-  before_validation :set_nhs_number_first_added_at,
-                    if: :will_save_change_to_nhs_number?
+  before_validation :ensure_nhs_number_first_added_at
   after_update :sync_vaccinations_to_nhs_immunisations_api
   after_commit :generate_important_notice_if_needed, on: :update
   after_commit :search_vaccinations_from_nhs_immunisations_api, on: :update
@@ -814,6 +813,15 @@ def pds_lookup_match?
 
   def notifier = Notifier::Patient.new(self)
 
+  def ensure_nhs_number_first_added_at
+    return unless will_save_change_to_nhs_number?
+
+    old_nhs_number, new_nhs_number = nhs_number_change_to_be_saved
+    return unless old_nhs_number.blank? && new_nhs_number.present?
+
+    self.nhs_number_first_added_at ||= Time.current
+  end
+
   private
 
   def locations_are_correct_type
@@ -837,14 +845,6 @@ def destroy_childless_parents
     end
   end
 
-  def set_nhs_number_first_added_at
-    old_nhs_number, new_nhs_number = nhs_number_change_to_be_saved
-
-    return unless old_nhs_number.blank? && new_nhs_number.present?
-
-    self.nhs_number_first_added_at ||= Time.current
-  end
-
   def archive_due_to_deceased!
     archive_reasons =
       teams.map do |team|

app/models/patient_changeset.rb

@@ -383,7 +383,10 @@ def set_child_attribute_if_valid(
     end
 
     if in_pending_changes && !in_existing_patient
-      existing_patient[attribute] = child_attributes[attribute.to_s]
+      existing_patient.public_send(
+        "#{attribute}=",
+        child_attributes[attribute.to_s]
+      )
     end
   end
 

spec/jobs/commit_patient_changesets_job_spec.rb

@@ -130,6 +130,19 @@
       expect(gae.parents).not_to be_empty
     end
 
+    it "sets nhs_number_first_added_at for newly imported patients with NHS numbers" do
+      freeze_time do
+        perform_job
+
+        timestamps =
+          Patient.where(nhs_number: %w[9990000018 9990000026 9990000034]).pluck(
+            :nhs_number_first_added_at
+          )
+
+        expect(timestamps).to all(eq(Time.current))
+      end
+    end
+
     it "stores statistics on the import" do
       # stree-ignore
       expect {
@@ -199,6 +212,38 @@
       end
     end
 
+    context "with an existing patient matching the import except for the NHS number" do
+      let!(:patient) do
+        create(
+          :patient,
+          given_name: "Jimmy",
+          preferred_given_name: "Jim",
+          family_name: "Smith",
+          date_of_birth: Date.new(2010, 1, 2),
+          nhs_number: nil,
+          nhs_number_first_added_at: nil,
+          address_line_1: "10 Downing Street",
+          address_town: "London",
+          address_postcode: "SW1A 1AA",
+          registration: "ABC",
+          school: location,
+          parents: []
+        )
+      end
+
+      before { PatientChangeset.all.map(&:calculate_review_data!) }
+
+      it "sets nhs_number_first_added_at when an NHS number is auto-accepted onto an existing patient" do
+        freeze_time do
+          expect { perform_job }.to change {
+            patient.reload.nhs_number_first_added_at
+          }.from(nil).to(Time.current)
+        end
+
+        expect(patient.reload.nhs_number).to eq("9990000026")
+      end
+    end
+
     context "with an existing patient matching the name but a different case" do
       before do
         create(

spec/models/immunisation_import_spec.rb

@@ -276,6 +276,19 @@
           .and change(immunisation_import, :new_record_count).to(11)
       end
 
+      it "sets nhs_number_first_added_at for imported patients with NHS numbers" do
+        immunisation_import.process!
+
+        timestamps =
+          immunisation_import
+            .patients
+            .where.not(nhs_number: nil)
+            .pluck(:nhs_number_first_added_at)
+
+        expect(timestamps).not_to be_empty
+        expect(timestamps).to all(eq(Time.current))
+      end
+
       it "ignores and counts duplicate records" do
         duplicate_import.parse_rows!
         duplicate_import.process!

spec/models/patient_spec.rb

@@ -1103,6 +1103,20 @@
       end
     end
 
+    it "sets nhs_number_first_added_at when the NHS number was assigned before save" do
+      patient =
+        create(:patient, nhs_number: nil, nhs_number_first_added_at: nil)
+      patient.nhs_number = "9449310475"
+      pds_patient = PDS::Patient.new(nhs_number: "9449310475")
+
+      freeze_time do
+        expect { patient.update_from_pds!(pds_patient) }.to change(
+          patient,
+          :nhs_number_first_added_at
+        ).from(nil).to(Time.current)
+      end
+    end
+
     context "when the NHS number doesn't match" do
       let(:pds_patient) { PDS::Patient.new(nhs_number: "abc") }
 
@@ -1414,6 +1428,24 @@
     end
   end
 
+  describe "#apply_pending_changes!" do
+    let(:patient) do
+      create(:patient, nhs_number: nil, nhs_number_first_added_at: nil)
+    end
+
+    before do
+      patient.update!(pending_changes: { "nhs_number" => "9449310475" })
+    end
+
+    it "sets nhs_number_first_added_at when a pending NHS number is applied" do
+      freeze_time do
+        expect { patient.apply_pending_changes! }.to change {
+          patient.reload.nhs_number_first_added_at
+        }.from(nil).to(Time.current)
+      end
+    end
+  end
+
   describe "#dup_for_pending_changes" do
     subject(:new_patient) { old_patient.dup_for_pending_changes }