From d48bf656247ca8e5a5051add238ec414bb5f4e57 Mon Sep 17 00:00:00 2001
From: Thomas Leese <thomas@leese.io>
Date: Tue, 15 Oct 2024 09:23:29 +0100
Subject: [PATCH] Don't always remove patients from sessions

If the patient session has been interacted with in some way (either
consent received, or triaged, or even vaccinated) we can't safely remove
the patient from the session as it's unclear what state they're in.

https://good-machine.sentry.io/issues/5990842901
---
 app/models/class_import.rb        | 12 +++++++-----
 app/models/consent_form.rb        |  6 +++++-
 app/models/immunisation_import.rb | 11 +++++++----
 app/models/session.rb             |  2 ++
 spec/models/class_import_spec.rb  | 14 ++++++++++++++
 5 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/app/models/class_import.rb b/app/models/class_import.rb
index 4e59873b4..caf69065d 100644
--- a/app/models/class_import.rb
+++ b/app/models/class_import.rb
@@ -56,10 +56,12 @@ def postprocess_rows!
 
     unknown_patients = session.patients - patients
 
-    unknown_patients.each do |unknown_patient|
-      unknown_patient.update!(school: nil)
-    end
-
-    session.patients.delete(unknown_patients)
+    session
+      .patient_sessions
+      .where(patient: unknown_patients)
+      .find_each do |patient_session|
+        patient_session.patient.update!(school: nil)
+        patient_session.destroy! if patient_session.added_to_session?
+      end
   end
 end
diff --git a/app/models/consent_form.rb b/app/models/consent_form.rb
index f6f5c36d6..f1727d030 100644
--- a/app/models/consent_form.rb
+++ b/app/models/consent_form.rb
@@ -356,7 +356,11 @@ def match_with_patient!(patient)
       if school && school != patient.school
         patient.update!(school:)
 
-        patient.patient_sessions.find_by(session: scheduled_session)&.destroy!
+        existing_patient_session =
+          patient.patient_sessions.find_by(session: scheduled_session)
+        if existing_patient_session&.added_to_session?
+          existing_patient_session.destroy!
+        end
 
         upcoming_session =
           Session
diff --git a/app/models/immunisation_import.rb b/app/models/immunisation_import.rb
index e08be63b3..b9d1adf78 100644
--- a/app/models/immunisation_import.rb
+++ b/app/models/immunisation_import.rb
@@ -148,9 +148,12 @@ def postprocess_rows!
         .includes(:vaccination_records)
         .select { _1.vaccinated?(programme) }
 
-    PatientSession.where(
-      session: team.sessions.upcoming,
-      patient: already_vaccinated_patients
-    ).delete_all
+    PatientSession
+      .where(
+        session: team.sessions.upcoming,
+        patient: already_vaccinated_patients
+      )
+      .select(&:added_to_session?)
+      .each(&:destroy!)
   end
 end
diff --git a/app/models/session.rb b/app/models/session.rb
index 162651f74..1d860df1a 100644
--- a/app/models/session.rb
+++ b/app/models/session.rb
@@ -131,6 +131,7 @@ def create_patient_sessions!
         required_programmes.all? { |programme| patient.vaccinated?(programme) }
       end
 
+    # First we remove patients from any other upcoming sessions.
     unvaccinated_patients.each do |patient|
       sessions_other_than_self = patient.upcoming_sessions.reject { _1 == self }
       next if sessions_other_than_self.empty?
@@ -142,6 +143,7 @@ def create_patient_sessions!
         .each(&:destroy!)
     end
 
+    # Next we can add the unvaccinated patients to this session.
     PatientSession.import!(
       %i[patient_id session_id],
       unvaccinated_patients.map { [_1.id, id] },
diff --git a/spec/models/class_import_spec.rb b/spec/models/class_import_spec.rb
index dcf3f2cab..b23faf14d 100644
--- a/spec/models/class_import_spec.rb
+++ b/spec/models/class_import_spec.rb
@@ -357,6 +357,20 @@
         expect { record! }.to change { existing_patient.reload.school }.to(nil)
         expect(session.reload.patients).not_to include(existing_patient)
       end
+
+      context "when the patient has been seen" do
+        let(:existing_patient) do
+          create(:patient, :consent_given_triage_not_needed, session:)
+        end
+
+        it "doesn't remove the patient from the session" do
+          expect(session.patients).to include(existing_patient)
+          expect { record! }.to change { existing_patient.reload.school }.to(
+            nil
+          )
+          expect(session.reload.patients).to include(existing_patient)
+        end
+      end
     end
   end
 end