Merge pull request #4673 from nhsuk/fix-original-session

Improve how session is chosen for consent form

Author: thomasleese

app/components/app_consent_confirmation_component.rb

@@ -37,6 +37,8 @@ def full_name
   end
 
   def panel_text
+    location = (@consent_form.session.school? ? " at school" : "")
+
     if response_given?
       if @consent_form.health_answers_require_triage?
         <<-END_OF_TEXT
@@ -45,12 +47,12 @@ def panel_text
           your answers and get in touch again soon.
         END_OF_TEXT
       else
-        "#{full_name} is due to get the #{given_vaccinations} at school" +
+        "#{full_name} is due to get the #{given_vaccinations}#{location}" +
           (session_dates.present? ? " on #{session_dates}" : "")
       end
     else
       "You’ve told us that you do not want #{full_name} to get the" \
-        " #{refused_vaccinations} at school"
+        " #{refused_vaccinations}#{location}"
     end
   end
 
@@ -86,7 +88,7 @@ def given_vaccinations_are
 
   def session_dates
     @consent_form
-      .actual_session
+      .session
       .today_or_future_dates
       .map { it.to_fs(:short_day_of_week) }
       .to_sentence(two_words_connector: " or ", last_word_connector: " or ")

app/controllers/concerns/consent_form_mailer_concern.rb

@@ -14,8 +14,7 @@ def send_consent_form_confirmation(consent_form)
               consent_form:,
               programmes:
             )
-          elsif consent_form.actual_session.clinic? ||
-                consent_form.actual_session.completed?
+          elsif consent_form.session.clinic? || consent_form.session.completed?
             EmailDeliveryJob.perform_later(
               :consent_confirmation_clinic,
               consent_form:,

app/controllers/consent_forms_controller.rb

@@ -40,8 +40,7 @@ def update_match
         .sessions
         .includes(:location_programme_year_groups, :programmes)
         .has_programmes(@consent_form.programmes)
-        .find_by(academic_year: AcademicYear.pending) ||
-        @consent_form.original_session
+        .find_by(academic_year: AcademicYear.pending) || @consent_form.session
 
     programme = session.programmes_for(patient: @patient).first
 

app/controllers/parent_interface/consent_forms/base_controller.rb

@@ -11,6 +11,7 @@ class ConsentForms::BaseController < ApplicationController
     prepend_before_action :set_session
     prepend_before_action :set_consent_form
     before_action :authenticate_consent_form_user!
+    before_action :check_if_past_deadline!
     before_action :set_privacy_policy_url
 
     private
@@ -30,7 +31,7 @@ def set_session
       if params[:session_slug]
         @session = Session.find_by!(slug: params[:session_slug])
       elsif @consent_form.present?
-        @session = @consent_form.original_session
+        @session = @consent_form.session
       end
     end
 
@@ -61,12 +62,6 @@ def set_subteam
         end
     end
 
-    def authenticate_consent_form_user!
-      unless session[:consent_form_id] == @consent_form.id
-        redirect_to @header_path
-      end
-    end
-
     def set_header_path
       @header_path =
         start_parent_interface_consent_forms_path(
@@ -99,5 +94,20 @@ def set_service_guide_url
     def set_privacy_policy_url
       @privacy_policy_url = @team.privacy_policy_url
     end
+
+    def authenticate_consent_form_user!
+      unless session[:consent_form_id] == @consent_form.id
+        redirect_to @header_path
+      end
+    end
+
+    def check_if_past_deadline!
+      return if @session.open_for_consent?
+
+      redirect_to deadline_passed_parent_interface_consent_forms_path(
+                    @session.slug,
+                    @programmes.map(&:type).join("-")
+                  )
+    end
   end
 end

app/controllers/parent_interface/consent_forms_controller.rb

@@ -7,9 +7,9 @@ class ConsentFormsController < ConsentForms::BaseController
     skip_before_action :set_consent_form, only: %i[start create deadline_passed]
     skip_before_action :authenticate_consent_form_user!,
                        only: %i[start create deadline_passed]
+    skip_before_action :check_if_past_deadline!, only: :deadline_passed
 
-    before_action :clear_session_edit_variables, only: %i[confirm]
-    before_action :check_if_past_deadline, except: %i[deadline_passed]
+    before_action :clear_session_edit_variables, only: :confirm
 
     def start
     end
@@ -65,12 +65,5 @@ def record
     def clear_session_edit_variables
       session.delete(:follow_up_changes_start_page)
     end
-
-    def check_if_past_deadline
-      return if @session.open_for_consent?
-      redirect_to action: :deadline_passed,
-                  programme_types: @programmes.map(&:type).join("-"),
-                  session_slug: @session.slug
-    end
   end
 end

app/jobs/consent_form_matching_job.rb

@@ -14,10 +14,10 @@ def perform(consent_form)
     # Match if we find a patient with the PDS NHS number
     return if match_with_exact_nhs_number
 
-    # Look for patients in the original session with no NHS number
-    if session_patients.count == 1
+    # Look for patients in the original location with no NHS number
+    if location_patients.count == 1
       # If we found exactly one, match the consent form to this patient
-      match_patient(session_patients.first)
+      match_patient(location_patients.first)
     end
 
     # Look for patients in any session with matching details
@@ -67,11 +67,16 @@ def match_with_exact_nhs_number
     @consent_form.match_with_patient!(patient, current_user: nil)
   end
 
-  def session_patients
-    @session_patients ||=
-      @consent_form
-        .original_session
-        .patients
+  def location_patients
+    @location_patients ||=
+      Patient
+        .joins(:patient_locations)
+        .where(
+          patient_locations: {
+            location: @consent_form.location,
+            academic_year: @consent_form.academic_year
+          }
+        )
         .includes(:school, :parents)
         .match_existing(nhs_number: nil, **query)
   end

app/lib/govuk_notify_personalisation.rb

@@ -26,9 +26,7 @@ def initialize(
     @programmes =
       programmes.presence || consent_form&.programmes.presence ||
         [consent&.programme || vaccination_record&.programme].compact
-    @session =
-      session || consent_form&.actual_session ||
-        consent_form&.original_session || vaccination_record&.session
+    @session = session || consent_form&.session || vaccination_record&.session
     @team =
       session&.team || consent_form&.team || consent&.team ||
         vaccination_record&.team

app/models/consent_form.rb

@@ -353,28 +353,32 @@ def reason_notes_must_be_provided?
     reason.in?(Consent::REASON_FOR_REFUSAL_REQUIRES_NOTES)
   end
 
-  def original_session
-    # The session that the consent form was filled out for.
-    @original_session ||=
-      Session
-        .joins(:programmes)
-        .where(programmes:)
-        .preload(:programmes)
-        .find_by(academic_year:, location:, team:)
-  end
-
-  def actual_session
-    # The session that the patient is expected to be seen in.
-    @actual_session ||=
-      (location_is_clinic? && original_session) ||
-        (
-          school &&
-            school
-              .sessions
-              .has_programmes(programmes)
-              .includes(:session_dates)
-              .find_by(academic_year:)
-        ) || team.generic_clinic_session(academic_year:)
+  def session
+    # This tries to find the most approriate session for this consent form.
+    # It's used when generating links to patients in a session, or when
+    # deciding which dates to show in an email. Under the hood, patients
+    # belong to locations, not sessions.
+    #
+    # Although unlikely to happen in production, there can be a scenario
+    # where multiple sessions at the same location offer the same programmes.
+    # In this case, we have to make a guess about which is the most relevant
+    # session.
+
+    @session ||=
+      begin
+        session_location = school || location
+
+        sessions_to_search =
+          Session.has_programmes(programmes).where(
+            academic_year:,
+            location: session_location,
+            team:
+          )
+
+        sessions_to_search.find { !it.completed? } ||
+          sessions_to_search.first ||
+          team.generic_clinic_session(academic_year:)
+      end
   end
 
   def find_or_create_parent_with_relationship_to!(patient:)

spec/features/parental_consent_clinic_spec.rb

@@ -27,7 +27,7 @@
     then_i_can_check_my_answers
 
     when_i_submit_the_consent_form
-    then_i_see_a_confirmation_page
+    then_i_see_a_confirmation_page_in_school
 
     when_the_nurse_checks_the_school_moves
     then_the_nurse_should_see_one_move
@@ -57,7 +57,7 @@
     then_i_can_check_my_answers
 
     when_i_submit_the_consent_form
-    then_i_see_a_confirmation_page
+    then_i_see_a_confirmation_page_in_clinic
 
     when_the_nurse_checks_the_school_moves
     then_the_nurse_should_see_one_move
@@ -87,7 +87,7 @@
     then_i_can_check_my_answers
 
     when_i_submit_the_consent_form
-    then_i_see_a_confirmation_page
+    then_i_see_a_confirmation_page_in_clinic
 
     when_the_nurse_checks_the_school_moves
     then_the_nurse_should_see_no_moves
@@ -229,9 +229,15 @@ def when_i_submit_the_consent_form
     click_on "Confirm"
   end
 
-  def then_i_see_a_confirmation_page
-    # TODO: "will get their HPV vaccination at the clinic"
+  def then_i_see_a_confirmation_page_in_school
+    expect(page).to have_content("is due to get the HPV vaccination at school")
+
+    perform_enqueued_jobs # match consent form with patient
+  end
+
+  def then_i_see_a_confirmation_page_in_clinic
     expect(page).to have_content("is due to get the HPV vaccination")
+    expect(page).not_to have_content("at school")
 
     perform_enqueued_jobs # match consent form with patient
   end