Merge pull request #5564 from nhsuk/restore-add-programmes

Allow editing programmes at any point

Author: thomasleese

app/controllers/draft_sessions_controller.rb

@@ -10,7 +10,7 @@ class DraftSessionsController < ApplicationController
   before_action :set_programmes, if: -> { current_step == :programmes }
   before_action :set_year_group_options, if: -> { current_step == :year_groups }
 
-  with_options only: :show, if: -> { current_step == :dates_check } do
+  with_options only: :show, if: :is_check_step? do
     before_action :set_catch_up_year_groups
     before_action :set_catch_up_patients_vaccinated_percentage
     before_action :set_catch_up_patients_receiving_consent_requests_count
@@ -24,7 +24,7 @@ class DraftSessionsController < ApplicationController
   def show
     authorize @session, @session.new_record? ? :new? : :edit?
 
-    skip_step if current_step == :dates_check && should_skip_dates_check?
+    skip_step if is_check_step? && should_skip_check?
 
     render_wizard
   end
@@ -50,6 +50,8 @@ def update
 
   private
 
+  def is_check_step? = step.end_with?("-check")
+
   def is_confirm_step? = step == "confirm"
 
   def set_draft_session
@@ -191,11 +193,11 @@ def set_back_link_path
         wizard_path("dates")
       elsif current_step == @draft_session.wizard_steps.first
         @draft_session.return_to == "school" ? schools_path : sessions_path
-      elsif previous_step == "dates-check"
-        # The `dates-check` page is special in that it skips forward if it
-        # doesn't need to be shown, however this leads to users getting stuck
-        # in a loop.
-        wizard_path("dates")
+      elsif is_check_step?
+        # The checks page are special in that they skip forward if they don't
+        # need to be shown, however this leads to users getting stuck in a
+        # loop.
+        wizard_path(previous_step.split("-").first)
       else
         previous_wizard_path
       end
@@ -284,6 +286,7 @@ def update_params
       programmes: {
         programme_types: []
       },
+      programmes_check: [],
       register_attendance: %i[requires_registration],
       school: %i[location_id],
       year_groups: {
@@ -333,7 +336,12 @@ def go_to_confirm_after_submission?
     # means we can't always skip straight to the confirmation page.
 
     if current_step == :dates && steps.include?("dates-check") &&
-         !should_skip_dates_check?
+         !should_skip_check?
+      return false
+    end
+
+    if current_step == :programmes && steps.include?("programmes-check") &&
+         !should_skip_check?
       return false
     end
 
@@ -348,19 +356,29 @@ def go_to_confirm_after_submission?
       return !is_confirm_step?
     end
 
+    # When first creating a session we go straight to the confirmation page
+    # after setting the dates.
+
     current_step == :dates_check ||
       (
         current_step == :dates && steps.include?("dates-check") &&
-          should_skip_dates_check?
+          should_skip_check?
       ) || (current_step == :dates && !steps.include?("dates-check"))
   end
 
-  def should_skip_dates_check?
+  def should_skip_check?
     if @draft_session.editing? &&
+         @draft_session.session.programme_types ==
+           @draft_session.programme_types &&
          @draft_session.session.dates == @draft_session.dates
       return true
     end
 
+    if @draft_session.programme_types.empty? ||
+         @draft_session.year_groups.empty? || @draft_session.dates.empty?
+      return true
+    end
+
     any_programme_has_high_unvaccinated_count =
       @draft_session.programmes.any? do |programme|
         programme_has_high_unvaccinated_count?(programme)

app/models/draft_session.rb

@@ -40,6 +40,7 @@ def wizard_steps
     steps << :school if school? && !editing?
 
     steps << :programmes
+    steps << :programmes_check
     steps << :year_groups if school? && !editing?
 
     steps << :dates

app/views/draft_sessions/_check.html.erb

@@ -0,0 +1,38 @@
+<% content_for :before_main do %>
+  <%= govuk_back_link(href: @back_link_path) %>
+<% end %>
+
+<% title = "Have you imported historical vaccination records for #{@draft_session.programmes.map(&:name).to_sentence}?" %>
+<% content_for :page_title, title %>
+
+<%= govuk_panel(title_text: title, classes: %w[app-panel--interruption]) do %>
+  <p>
+    <%= @catch_up_patients_vaccinated_percentage == 0 ? "" : "Only " %>
+    <%= @catch_up_patients_vaccinated_percentage %>% of children in
+    <%= format_year_groups(@catch_up_year_groups) %> in
+    this session have vaccination records. This is unusually low coverage for
+    catch-up year groups.
+  </p>
+
+  <p>
+    Check and confirm that vaccination records have been imported for all
+    children in this school before you continue.
+  </p>
+
+  <% if @draft_session.open_for_consent? && @catch_up_patients_receiving_consent_requests_count > 0 %>
+    <p>
+      Scheduling this session now will send consent requests to
+      <%= pluralize(@catch_up_patients_receiving_consent_requests_count, "parent") %>
+      of children in <%= format_year_groups(@catch_up_year_groups) %> on
+      <%= @draft_session.send_consent_requests_at.to_fs(:long) %>. Many of them
+      may be parents of already vaccinated children.
+    </p>
+  <% end %>
+
+  <%= form_with model: @draft_session, url: wizard_path, method: :put do |f| %>
+    <div class="nhsuk-button-group">
+      <%= f.govuk_submit submit_text, class: "nhsuk-button--reverse" %>
+      <%= link_to cancel_text, cancel_href %>
+    </div>
+  <% end %>
+<% end %>

app/views/draft_sessions/confirm.html.erb

@@ -24,10 +24,7 @@
         summary_list.with_row do |row|
           row.with_key { "Programmes" }
           row.with_value { render AppProgrammeTagsComponent.new(@draft_session.programmes) }
-      
-          unless @draft_session.editing?
-            row.with_action(text: "Change", href: draft_session_path("programmes"), visually_hidden_text: "programmes")
-          end
+          row.with_action(text: "Change", href: draft_session_path("programmes"), visually_hidden_text: "programmes")
         end
 
         summary_list.with_row do |row|

app/views/draft_sessions/dates_check.html.erb

@@ -1,38 +1,4 @@
-<% content_for :before_main do %>
-  <%= govuk_back_link(href: @back_link_path) %>
-<% end %>
-
-<% title = t(".title", programmes: @draft_session.programmes.map(&:name).to_sentence) %>
-<% content_for :page_title, title %>
-
-<%= govuk_panel(title_text: title, classes: %w[app-panel--interruption]) do %>
-  <p>
-    <%= @catch_up_patients_vaccinated_percentage == 0 ? "" : "Only " %>
-    <%= @catch_up_patients_vaccinated_percentage %>% of children in
-    <%= format_year_groups(@catch_up_year_groups) %> in
-    this session have vaccination records. This is unusually low coverage for
-    catch-up year groups.
-  </p>
-
-  <p>
-    Check and confirm that vaccination records have been imported for all
-    children in this school before you continue.
-  </p>
-
-  <% if @draft_session.open_for_consent? && @catch_up_patients_receiving_consent_requests_count > 0 %>
-    <p>
-      Scheduling this session now will send consent requests to
-      <%= pluralize(@catch_up_patients_receiving_consent_requests_count, "parent") %>
-      of children in <%= format_year_groups(@catch_up_year_groups) %> on
-      <%= @draft_session.send_consent_requests_at.to_fs(:long) %>. Many of them
-      may be parents of already vaccinated children.
-    </p>
-  <% end %>
-
-  <%= form_with model: @draft_session, url: wizard_path, method: :put do |f| %>
-    <div class="nhsuk-button-group">
-      <%= f.govuk_submit "Keep session dates", class: "nhsuk-button--reverse" %>
-      <%= link_to "Remove session dates", draft_session_path("dates") %>
-    </div>
-  <% end %>
-<% end %>
+<%= render "check",
+          submit_text: "Keep session dates",
+          cancel_text: "Remove session dates",
+          cancel_href: draft_session_path("dates") %>

app/views/draft_sessions/programmes_check.html.erb

@@ -0,0 +1,4 @@
+<%= render "check",
+          submit_text: "Continue",
+          cancel_text: "Cancel",
+          cancel_href: draft_session_path("programmes") %>

config/locales/en.yml

@@ -623,9 +623,6 @@ en:
         other: Why are they refusing to give consent?
         personal_choice: Why are they refusing to give consent?
         will_be_vaccinated_elsewhere: Where will their child get their vaccination?
-  draft_sessions:
-    dates_check:
-      title: Have you imported historical vaccination records for %{programmes}?
   import_duplicate_form:
     options:
       label:

config/locales/wicked.en.yml

@@ -34,6 +34,7 @@ en:
     parent: parent
     parent_details: parent-details
     programmes: programmes
+    programmes_check: programmes-check
     questions: questions
     reason_for_refusal: reason-for-refusal
     reason_for_refusal_notes: reason-for-refusal-notes

spec/features/edit_session_programmes_spec.rb

@@ -3,6 +3,21 @@
 describe "Edit session programmes" do
   around { |example| travel_to(Time.zone.local(2024, 2, 1)) { example.run } }
 
+  scenario "add new programme to existing session" do
+    given_a_school_exists
+    and_the_school_has_unvaccinated_catch_up_patients
+    and_an_hpv_session_already_exists
+
+    when_i_visit_the_session_page
+    and_i_click_on_edit_session
+    and_i_add_the_mmr_programme
+    then_i_see_the_warning_panel_about_unvaccinated_patients
+
+    when_i_click_continue
+    then_i_should_see_the_mmr_programme
+    screenshot_and_save_page
+  end
+
   scenario "choosing programmes with a high number of unvaccinated catch up patients" do
     given_a_school_exists
     and_the_school_has_unvaccinated_catch_up_patients
@@ -14,16 +29,27 @@
     and_i_choose_the_date
     then_i_see_the_warning_panel_about_unvaccinated_patients
 
-    when_i_click_continue
+    when_i_click_keep_session_dates
     then_i_should_see_the_mmr_programme
   end
 
   def given_a_school_exists
-    @team =
-      create(:team, :with_one_nurse, programmes: [Programme.hpv, Programme.mmr])
+    @programmes = [Programme.hpv, Programme.mmr]
+
+    @team = create(:team, :with_one_nurse, programmes: @programmes)
+
+    @location = create(:school, team: @team, programmes: @programmes)
+  end
 
-    @location =
-      create(:school, team: @team, programmes: [Programme.hpv, Programme.mmr])
+  def and_an_hpv_session_already_exists
+    @session =
+      create(
+        :session,
+        :scheduled,
+        team: @team,
+        location: @location,
+        programmes: [Programme.hpv]
+      )
   end
 
   def and_the_school_has_unvaccinated_catch_up_patients
@@ -33,15 +59,30 @@ def and_the_school_has_unvaccinated_catch_up_patients
       :eligible_for_vaccination,
       location: @location,
       year_group: 9,
-      programmes: [Programme.hpv, Programme.mmr]
+      programmes: @programmes
     )
   end
 
+  def when_i_visit_the_session_page
+    sign_in @team.users.first
+    visit session_path(@session)
+  end
+
   def when_i_visit_the_school_page
     sign_in @team.users.first
     visit school_sessions_path(@location)
   end
 
+  def and_i_click_on_edit_session
+    click_on "Edit session"
+  end
+
+  def and_i_add_the_mmr_programme
+    click_on "Change programmes"
+    check "MMR"
+    click_on "Continue"
+  end
+
   def and_i_create_a_new_session
     click_on "Add a new session"
   end
@@ -83,6 +124,10 @@ def then_i_see_the_warning_panel_about_unvaccinated_patients
   end
 
   def when_i_click_continue
+    click_on "Continue"
+  end
+
+  def when_i_click_keep_session_dates
     click_on "Keep session dates"
   end