test: the patient_id is included in the logs

MartinVanIJcken · MartinVanIJcken · commit e2b42bf72013 · 2025-12-03T15:46:10.000Z
Jira-Issue: MAV-1069
diff --git a/app/lib/team_cached_counts.rb b/app/lib/team_cached_counts.rb
@@ -39,7 +39,8 @@ def school_moves
     Rails
       .cache
       .fetch(school_moves_key) do
-        SchoolMovePolicy::Scope.new(current_user, SchoolMove).resolve.count
+        x = SchoolMovePolicy::Scope.new(current_user, SchoolMove).resolve
+        x.count
       end
   end
 
diff --git a/spec/controllers/consent_forms_controller_spec.rb b/spec/controllers/consent_forms_controller_spec.rb
@@ -61,4 +61,27 @@
 
     it_behaves_like "a method that updates team cached counts"
   end
+
+  describe "PatientLoggingConcern" do
+    let(:consent_form) { create(:consent_form, :recorded) }
+    let(:patient) { create(:patient) }
+
+    it_behaves_like "a controller that logs the patient ID",
+                    -> do
+                      get :edit_match,
+                          params: {
+                            id: consent_form.id,
+                            patient_id: patient.id
+                          }
+                    end
+
+    it_behaves_like "a controller that logs the patient ID",
+                    -> do
+                      post :update_match,
+                           params: {
+                             id: consent_form.id,
+                             patient_id: patient.id
+                           }
+                    end
+  end
 end
diff --git a/spec/controllers/draft_consents_controller_spec.rb b/spec/controllers/draft_consents_controller_spec.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+describe DraftConsentsController do
+  let(:patient) { create(:patient) }
+  let(:programme) { create(:programme) }
+
+  describe "PatientLoggingConcern" do
+    before do
+      allow(controller).to receive_messages(
+        session: {
+          "consent" => {
+            "patient_id" => patient.id,
+            "programme_id" => programme.id
+          }
+        }
+      )
+
+      # Avoid hitting complex persistence paths irrelevant to logging in update action
+      allow(controller).to receive(:handle_confirm).and_return(nil)
+    end
+
+    it_behaves_like "a controller that logs the patient ID",
+                    -> { get :show, params: { id: "confirm" } }
+
+    it_behaves_like "a controller that logs the patient ID",
+                    -> do
+                      put :update,
+                          params: {
+                            id: "confirm",
+                            draft_consent: {
+                              wizard_step: :confirm,
+                              notes: "ok"
+                            }
+                          }
+                    end
+  end
+end
diff --git a/spec/controllers/draft_vaccination_records_controller_spec.rb b/spec/controllers/draft_vaccination_records_controller_spec.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+describe DraftVaccinationRecordsController do
+  let(:patient) { create(:patient) }
+
+  describe "PatientLoggingConcern" do
+    before do
+      allow(controller).to receive(:session).and_return(
+        { "vaccination_record" => { "patient_id" => patient.id } }
+      )
+    end
+
+    it_behaves_like "a controller that logs the patient ID",
+                    -> { get :show, params: { id: "confirm" } }
+
+    it_behaves_like "a controller that logs the patient ID",
+                    -> do
+                      put :update,
+                          params: {
+                            id: "confirm",
+                            draft_vaccination_record: {
+                              wizard_step: :confirm,
+                              notes: "ok"
+                            }
+                          }
+                    end
+  end
+end
diff --git a/spec/controllers/imports/issues_controller_spec.rb b/spec/controllers/imports/issues_controller_spec.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+describe Imports::IssuesController do
+  let(:patient) { create(:patient) }
+
+  describe "PatientLoggingConcern" do
+    before do
+      # Ensure the patient appears in import issues
+      patient.update!(pending_changes: { foo: "bar" })
+    end
+
+    it_behaves_like "a controller that logs the patient ID",
+                    -> do
+                      get :show, params: { id: patient.id, type: "patient" }
+                    end
+
+    it_behaves_like "a controller that logs the patient ID",
+                    -> do
+                      patch :update,
+                            params: {
+                              id: patient.id,
+                              type: "patient",
+                              import_duplicate_form: {
+                                apply_changes: true
+                              }
+                            }
+                    end
+  end
+end
diff --git a/spec/controllers/parent_relationships_controller_spec.rb b/spec/controllers/parent_relationships_controller_spec.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+describe ParentRelationshipsController do
+  let(:patient) { create(:patient) }
+  let(:relationship) { create(:parent_relationship, patient:) }
+
+  describe "PatientLoggingConcern" do
+    it_behaves_like "a controller that logs the patient ID",
+                    -> do
+                      get :edit,
+                          params: {
+                            patient_id: patient.id,
+                            id: relationship.parent_id
+                          }
+                    end
+
+    it_behaves_like "a controller that logs the patient ID",
+                    -> do
+                      patch :update,
+                            params: {
+                              patient_id: patient.id,
+                              id: relationship.parent_id,
+                              parent_relationship: {
+                                type: relationship.type
+                              }
+                            }
+                    end
+  end
+end
diff --git a/spec/controllers/patient_sessions/activities_controller_spec.rb b/spec/controllers/patient_sessions/activities_controller_spec.rb
@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+describe PatientSessions::ActivitiesController do
+  let(:session) { create(:session) }
+  let(:patient) { create(:patient) }
+
+  before do
+    create(
+      :patient_location,
+      patient: patient,
+      location: session.location,
+      academic_year: session.academic_year
+    )
+  end
+
+  describe "PatientLoggingConcern" do
+    it_behaves_like "a controller that logs the patient ID",
+                    -> do
+                      get :show,
+                          params: {
+                            session_slug: session.slug,
+                            patient_id: patient.id
+                          }
+                    end
+
+    it_behaves_like "a controller that logs the patient ID",
+                    -> do
+                      post :create,
+                           params: {
+                             session_slug: session.slug,
+                             patient_id: patient.id,
+                             note: {
+                               body: "Hello"
+                             }
+                           }
+                    end
+  end
+end
diff --git a/spec/controllers/patients/archive_controller_spec.rb b/spec/controllers/patients/archive_controller_spec.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+describe Patients::ArchiveController do
+  let(:patient) { create(:patient) }
+
+  describe "PatientLoggingConcern" do
+    it_behaves_like "a controller that logs the patient ID",
+                    -> { get :new, params: { patient_id: patient.id } }
+
+    it_behaves_like "a controller that logs the patient ID",
+                    -> do
+                      post :create,
+                           params: {
+                             patient_id: patient.id,
+                             patient_archive_form: {
+                               nhs_number: patient.nhs_number,
+                               type: "other",
+                               other_details: "duplicate"
+                             }
+                           }
+                    end
+  end
+end
diff --git a/spec/controllers/patients/edit_controller_spec.rb b/spec/controllers/patients/edit_controller_spec.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+describe Patients::EditController do
+  let(:patient) { create(:patient) }
+
+  describe "PatientLoggingConcern" do
+    it_behaves_like "a controller that logs the patient ID",
+                    -> { get :edit_nhs_number, params: { id: patient.id } }
+    it_behaves_like "a controller that logs the patient ID",
+                    -> do
+                      put :update_nhs_number,
+                          params: {
+                            id: patient.id,
+                            patient: {
+                              nhs_number: patient.nhs_number
+                            }
+                          }
+                    end
+  end
+end
diff --git a/spec/controllers/patients_controller_spec.rb b/spec/controllers/patients_controller_spec.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+describe PatientsController do
+  let(:team) { create(:team) }
+  let(:user) { create(:user, team:) }
+  let(:session) { create(:session, team:) }
+  let(:patient) { create(:patient, session:) }
+
+  before { allow(controller).to receive(:current_user).and_return(user) }
+
+  describe "PatientLoggingConcern" do
+    it_behaves_like "a controller that logs the patient ID",
+                    -> { get :show, params: { id: patient.id } }
+
+    it_behaves_like "a controller that logs the patient ID",
+                    -> { get :log, params: { id: patient.id } }
+
+    it_behaves_like "a controller that logs the patient ID",
+                    -> { get :edit, params: { id: patient.id } }
+  end
+end
diff --git a/spec/controllers/vaccination_records_controller_spec.rb b/spec/controllers/vaccination_records_controller_spec.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+describe VaccinationRecordsController do
+  let(:team) { create(:team) }
+  let(:user) { create(:user, team:) }
+  let(:vaccination_record) { create(:vaccination_record, team:) }
+  let(:patient) { vaccination_record.patient }
+
+  before { allow(controller).to receive(:current_user).and_return(user) }
+
+  describe "PatientLoggingConcern" do
+    it_behaves_like "a controller that logs the patient ID",
+                    -> { get :show, params: { id: vaccination_record.id } }
+
+    # it_behaves_like "a controller that logs the patient ID",
+    #                 -> { put :update, params: { id: vaccination_record.id } }
+  end
+end
diff --git a/spec/support/logging_helper.rb b/spec/support/logging_helper.rb
@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+module LoggingHelper
+  def capture_log_tags
+    captured = []
+
+    original = SemanticLogger.method(:tagged)
+
+    allow(SemanticLogger).to receive(:tagged) do |tags, &blk|
+      captured << tags
+      blk ? blk.call : nil
+    end
+
+    yield
+
+    captured
+  ensure
+    allow(SemanticLogger).to receive(:tagged) do |*args, &blk|
+      original.call(*args, &blk)
+    end
+  end
+end
+
+RSpec.configure { |config| config.include LoggingHelper }
diff --git a/spec/support/shared_examples/a_controller_that_logs_the_patient_id.rb b/spec/support/shared_examples/a_controller_that_logs_the_patient_id.rb
@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+shared_examples "a controller that logs the patient ID" do |action|
+  let(:user) { create(:user, :support) }
+
+  before do
+    # Skip authentication for this test
+    allow(controller).to receive_messages(
+      authenticate_user!: true,
+      ensure_team_is_selected: true,
+      set_user_cis2_info: true,
+      verify_policy_scoped: true,
+      current_user: user
+    )
+
+    # Ensure the current user has access to all resources
+    # so that patient id can be retrieved despite invalid authentication
+    allow(SchoolMovePolicy::Scope).to receive(:new).and_return(
+      instance_double(SchoolMovePolicy::Scope, resolve: SchoolMove)
+    )
+
+    allow(PatientPolicy::Scope).to receive(:new).and_return(
+      instance_double(PatientPolicy::Scope, resolve: Patient)
+    )
+
+    allow(ProgrammePolicy::Scope).to receive(:new).and_return(
+      instance_double(ProgrammePolicy::Scope, resolve: Programme)
+    )
+
+    allow(SessionPolicy::Scope).to receive(:new).and_return(
+      instance_double(SessionPolicy::Scope, resolve: Session)
+    )
+
+    allow(ConsentFormPolicy::Scope).to receive(:new).and_return(
+      instance_double(ConsentFormPolicy::Scope, resolve: ConsentForm)
+    )
+
+    allow(VaccinationRecordPolicy::Scope).to receive(:new).and_return(
+      instance_double(
+        VaccinationRecordPolicy::Scope,
+        resolve: VaccinationRecord
+      )
+    )
+  end
+
+  it "tags logs with patient_id" do
+    tags = capture_log_tags { instance_exec(&action) }
+    expect(tags.select { |tag| tag.is_a?(Hash) }).to include a_hash_including(patient_id: patient.id)
+  end
+end
