Merge pull request #3359 from nhsuk/remove-patient-organisation

Simplify patient-organisation relationship

Author: thomasleese

app/components/app_patient_cohort_table_component.html.erb

@@ -18,7 +18,7 @@
         <% row.with_cell do %>
           <span class="nhsuk-table-responsive__heading">Actions</span>
           <%= form_with model: @patient, builder: GOVUKDesignSystemFormBuilder::FormBuilder do |f| %>
-            <%= f.hidden_field :organisation_id, value: "" %>
+            <%= f.hidden_field :organisation_id, value: organisation.id %>
             <%= f.govuk_submit "Remove from cohort", class: "app-button--secondary-warning app-button--small" %>
           <% end %>
         <% end %>

app/components/app_patient_cohort_table_component.rb

@@ -1,15 +1,23 @@
 # frozen_string_literal: true
 
 class AppPatientCohortTableComponent < ViewComponent::Base
-  def initialize(patient)
+  def initialize(patient, current_user:)
     super
 
     @patient = patient
+    @current_user = current_user
   end
 
   private
 
-  attr_reader :patient
+  attr_reader :patient, :current_user
 
-  delegate :organisation, :year_group, to: :patient
+  delegate :year_group, to: :patient
+
+  def organisation
+    @organisation ||=
+      if current_user.selected_organisation.patients.include?(patient)
+        current_user.selected_organisation
+      end
+  end
 end

app/controllers/api/organisations_controller.rb

@@ -24,6 +24,8 @@ def destroy
         log_destroy(SessionNotification.where(session: sessions))
         log_destroy(VaccinationRecord.where(session: sessions))
 
+        patient_ids = organisation.patients.pluck(:id)
+
         patient_sessions = PatientSession.where(session: sessions)
         log_destroy(GillickAssessment.where(patient_session: patient_sessions))
         log_destroy(PreScreening.where(patient_session: patient_sessions))
@@ -32,22 +34,25 @@ def destroy
         log_destroy(SessionDate.where(session: sessions))
         log_destroy(sessions)
 
-        patients = organisation.patients
+        organisation.patients
 
-        log_destroy(SchoolMove.where(patient: patients))
+        log_destroy(SchoolMove.where(patient_id: patient_ids))
         log_destroy(SchoolMove.where(organisation:))
-        log_destroy(SchoolMoveLogEntry.where(patient: patients))
-        log_destroy(AccessLogEntry.where(patient: patients))
-        log_destroy(NotifyLogEntry.where(patient: patients))
+        log_destroy(SchoolMoveLogEntry.where(patient_id: patient_ids))
+        log_destroy(AccessLogEntry.where(patient_id: patient_ids))
+        log_destroy(NotifyLogEntry.where(patient_id: patient_ids))
         # In local dev we can end up with NotifyLogEntries without a patient
         log_destroy(NotifyLogEntry.where(patient_id: nil))
-        log_destroy(VaccinationRecord.where(patient: patients))
+        log_destroy(VaccinationRecord.where(patient_id: patient_ids))
 
         log_destroy(ConsentForm.where(organisation:))
         log_destroy(Consent.where(organisation:))
         log_destroy(Triage.where(organisation:))
 
-        patients.includes(:parents).in_batches { log_destroy(it) }
+        Patient
+          .where(id: patient_ids)
+          .includes(:parents)
+          .in_batches { log_destroy(it) }
 
         batches = Batch.where(organisation:)
         log_destroy(VaccinationRecord.where(batch: batches))

app/controllers/cohorts_controller.rb

@@ -11,7 +11,7 @@ def index
     birth_academic_years = @programme.birth_academic_years
 
     @patient_count_by_birth_academic_year =
-      patients_in_cohort
+      patients_in_organisation
         .where(birth_academic_year: birth_academic_years)
         .group(:birth_academic_year)
         .count
@@ -24,7 +24,7 @@ def show
     @birth_academic_year = Integer(params[:id])
 
     patients =
-      patients_in_cohort
+      patients_in_organisation
         .where(birth_academic_year: @birth_academic_year)
         .not_deceased
         .includes(:school)
@@ -39,7 +39,7 @@ def set_programme
     @programme = policy_scope(Programme).find_by!(type: params[:programme_type])
   end
 
-  def patients_in_cohort
-    Patient.where(organisation: current_user.selected_organisation)
+  def patients_in_organisation
+    current_user.selected_organisation.patients
   end
 end

app/controllers/patients/edit_controller.rb

@@ -43,10 +43,14 @@ def set_patient
   def existing_patient
     @existing_patient ||=
       if nhs_number.present?
-        policy_scope(Patient)
-          .or(Patient.where(organisation: nil))
-          .includes(parent_relationships: :parent)
-          .find_by(nhs_number:)
+        policy_scope(Patient).includes(parent_relationships: :parent).find_by(
+          nhs_number:
+        ) ||
+          Patient
+            .where
+            .missing(:patient_sessions)
+            .includes(parent_relationships: :parent)
+            .find_by(nhs_number:)
       end
   end
 

app/controllers/patients_controller.rb

@@ -32,19 +32,14 @@ def edit
   end
 
   def update
-    old_organisation = @patient.organisation
-
     organisation_id = params.dig(:patient, :organisation_id).presence
 
     ActiveRecord::Base.transaction do
-      @patient.update!(organisation_id:)
-
-      if organisation_id.nil?
-        @patient
-          .patient_sessions
-          .where(session: old_organisation.sessions)
-          .destroy_all_if_safe
-      end
+      @patient
+        .patient_sessions
+        .joins(:session)
+        .where(session: { organisation_id: })
+        .destroy_all_if_safe
     end
 
     path =
@@ -68,7 +63,6 @@ def set_patient
     @patient =
       policy_scope(Patient).includes(
         :gp_practice,
-        :organisation,
         :school,
         consents: %i[parent patient],
         parent_relationships: :parent,

app/lib/patient_merger.rb

@@ -95,13 +95,6 @@ def call
       patient_to_destroy.cohort_imports.clear
       patient_to_destroy.immunisation_imports.clear
 
-      # Add patient back to the cohort if the patient to destroy was in the cohort.
-      if patient_to_keep.organisation_id.nil?
-        patient_to_keep.update!(
-          organisation_id: patient_to_destroy.organisation_id
-        )
-      end
-
       patient_to_destroy.reload.destroy!
 
       StatusUpdater.call(patient: patient_to_keep)

app/lib/reports/school_moves_exporter.rb

@@ -50,7 +50,7 @@ def school_move_log_entries
       begin
         historical_patients =
           Patient
-            .where.not(organisation:)
+            .where.not(id: organisation.patients.select(:id))
             .where(
               SchoolMoveLogEntry
                 .where("patient_id = patients.id")

app/models/organisation.rb

@@ -39,17 +39,17 @@ class Organisation < ApplicationRecord
   has_many :locations
   has_many :organisation_programmes,
            -> { joins(:programme).order(:"programmes.type") }
-  has_many :patients
   has_many :sessions
   has_many :teams
 
   has_many :community_clinics, through: :teams
   has_many :locations, through: :teams
   has_many :patient_sessions, through: :sessions
+  has_many :patients, through: :patient_sessions
   has_many :programmes, through: :organisation_programmes
   has_many :schools, through: :teams
-  has_many :vaccines, through: :programmes
   has_many :vaccination_records, through: :sessions
+  has_many :vaccines, through: :programmes
 
   has_and_belongs_to_many :users
 

app/models/patient.rb

@@ -28,7 +28,6 @@
 #  created_at                :datetime         not null
 #  updated_at                :datetime         not null
 #  gp_practice_id            :bigint
-#  organisation_id           :bigint
 #  school_id                 :bigint
 #
 # Indexes
@@ -39,13 +38,11 @@
 #  index_patients_on_names_family_first   (family_name,given_name)
 #  index_patients_on_names_given_first    (given_name,family_name)
 #  index_patients_on_nhs_number           (nhs_number) UNIQUE
-#  index_patients_on_organisation_id      (organisation_id)
 #  index_patients_on_school_id            (school_id)
 #
 # Foreign Keys
 #
 #  fk_rails_...  (gp_practice_id => locations.id)
-#  fk_rails_...  (organisation_id => organisations.id)
 #  fk_rails_...  (school_id => locations.id)
 #
 class Patient < ApplicationRecord
@@ -56,11 +53,10 @@ class Patient < ApplicationRecord
   include PendingChangesConcern
   include Schoolable
 
-  audited associated_with: :organisation
+  audited
   has_associated_audits
 
   belongs_to :gp_practice, class_name: "Location", optional: true
-  belongs_to :organisation, optional: true
 
   has_many :access_log_entries
   has_many :consent_notifications
@@ -77,11 +73,12 @@ class Patient < ApplicationRecord
   has_many :vaccination_records, -> { kept }
   has_many :vaccination_statuses
 
-  has_many :parents, through: :parent_relationships
   has_many :gillick_assessments, through: :patient_sessions
+  has_many :parents, through: :parent_relationships
   has_many :pre_screenings, through: :patient_sessions
   has_many :session_attendances, through: :patient_sessions
   has_many :sessions, through: :patient_sessions
+  has_many :organisations, through: :sessions
 
   has_many :sessions_for_current_academic_year,
            -> { for_current_academic_year },
@@ -354,6 +351,8 @@ def invalidate!
     update_column(:invalidated_at, Time.current)
   end
 
+  def not_in_organisation? = patient_sessions.empty?
+
   def dup_for_pending_changes
     dup.tap do |new_patient|
       new_patient.nhs_number = nil
@@ -385,7 +384,6 @@ def self.from_consent_form(consent_form)
       given_name: consent_form.given_name,
       home_educated: consent_form.home_educated,
       nhs_number: consent_form.nhs_number,
-      organisation: consent_form.organisation,
       preferred_family_name: consent_form.preferred_family_name,
       preferred_given_name: consent_form.preferred_given_name,
       school: consent_form.school