From a37069db0091c9e98d552441c1a905f77052d816 Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Wed, 25 Sep 2024 10:24:11 +0100 Subject: [PATCH] Replace usage of single session date Sessions no longer have a single date, instead they can run over multiple dates. This change supports the underyling database change, while maintaining the current functionality. Further work will be required to update the UI to allow the user to pick multiple dates, updating consent to handle multiple session dates, and updating the reminder and other emails. --- ...pp_session_summary_card_component.html.erb | 4 +- .../app_session_summary_card_component.rb | 6 +-- .../vaccination_records_controller.rb | 4 +- app/lib/govuk_notify_personalisation.rb | 6 ++- app/models/immunisation_import_row.rb | 35 ++++++++---- app/models/session.rb | 53 +++++++++++++++---- .../_confirmation_agreed.html.erb | 5 +- app/views/programmes/sessions.html.erb | 6 +-- app/views/sessions/edit/timeline.html.erb | 2 +- app/views/sessions/show.html.erb | 10 +--- spec/factories/sessions.rb | 10 +++- spec/features/e2e_journey_spec.rb | 2 +- ...accination_records_with_duplicates_spec.rb | 2 +- .../self_consent_gillick_competent_spec.rb | 2 +- ...self_consent_not_gillick_competent_spec.rb | 2 +- spec/mailers/consent_mailer_spec.rb | 10 ++-- spec/models/immunisation_import_spec.rb | 4 +- 17 files changed, 110 insertions(+), 53 deletions(-) diff --git a/app/components/app_session_summary_card_component.html.erb b/app/components/app_session_summary_card_component.html.erb index 5de6239fd..a157c5113 100644 --- a/app/components/app_session_summary_card_component.html.erb +++ b/app/components/app_session_summary_card_component.html.erb @@ -12,8 +12,8 @@ end summary_list.with_row do |row| - row.with_key { "Date" } - row.with_value { date } + row.with_key { "Dates" } + row.with_value { dates } end summary_list.with_row do |row| diff --git a/app/components/app_session_summary_card_component.rb b/app/components/app_session_summary_card_component.rb index 682557214..c5e3b0b38 100644 --- a/app/components/app_session_summary_card_component.rb +++ b/app/components/app_session_summary_card_component.rb @@ -15,8 +15,8 @@ def vaccines @session.programmes.map(&:name) end - def date - @session.date.to_fs(:long_day_of_week) + def dates + safe_join(@session.dates.map { _1.value.to_fs(:long_day_of_week) }, tag.br) end def time @@ -42,7 +42,7 @@ def consent_reminders def deadline_for_responses return nil if @session.close_consent_at.blank? - if @session.date == @session.close_consent_at + if @session.dates.map(&:value).min == @session.close_consent_at "Allow responses until the day of the session" else close_consent_at = @session.close_consent_at.to_fs(:long_day_of_week) diff --git a/app/controllers/vaccination_records_controller.rb b/app/controllers/vaccination_records_controller.rb index 85cda9ec9..d9cf3c4a0 100644 --- a/app/controllers/vaccination_records_controller.rb +++ b/app/controllers/vaccination_records_controller.rb @@ -43,11 +43,11 @@ def vaccination_records :programme, :vaccine, patient: [:cohort, :school, { parent_relationships: :parent }], - session: :location + session: %i[dates location] ) .where(programme:) .order(:recorded_at) - .strict_loading + # .strict_loading # TODO: Once we move Session#set_timeline_attributes this can be uncommented. end def dps_export diff --git a/app/lib/govuk_notify_personalisation.rb b/app/lib/govuk_notify_personalisation.rb index e616b7126..72f05d34b 100644 --- a/app/lib/govuk_notify_personalisation.rb +++ b/app/lib/govuk_notify_personalisation.rb @@ -141,11 +141,13 @@ def reason_for_refusal end def session_date - session.date.to_fs(:short_day_of_week) + # TODO: Handle multiple dates. + session.dates.map(&:value).min.to_fs(:short_day_of_week) end def session_short_date - session.date.to_fs(:short) + # TODO: Handle multiple dates. + session.dates.map(&:value).min.to_fs(:short) end def short_patient_name diff --git a/app/models/immunisation_import_row.rb b/app/models/immunisation_import_row.rb index 8bd93c07e..960fed37f 100644 --- a/app/models/immunisation_import_row.rb +++ b/app/models/immunisation_import_row.rb @@ -118,17 +118,32 @@ def session return unless valid? @session ||= - Session - .create_with(active: false) - .find_or_create_by!( - team:, - date: session_date, - location:, - time_of_day: :all_day - ) - .tap do - _1.programmes << @programme unless _1.programmes.include?(@programme) + if ( + session = + Session.for_date(session_date).find_by( + team:, + location:, + time_of_day: :all_day + ) + ) + unless session.programmes.include?(@programme) + session.programmes << @programme + end + session + else + ActiveRecord::Base.transaction do + session = + Session.create!( + active: false, + team:, + location:, + time_of_day: :all_day + ) + session.dates.create!(value: session_date) + session.programmes << @programme + session end + end end def patient_session diff --git a/app/models/session.rb b/app/models/session.rb index 2ceb26b06..ba5a11f2a 100644 --- a/app/models/session.rb +++ b/app/models/session.rb @@ -31,7 +31,8 @@ class Session < ApplicationRecord DEFAULT_DAYS_FOR_REMINDER = 2 - attr_accessor :reminder_days_after, + attr_accessor :date, + :reminder_days_after, :reminder_days_after_custom, :close_consent_on @@ -51,10 +52,21 @@ class Session < ApplicationRecord enum :time_of_day, %w[morning afternoon all_day], validate: { if: :active? } - scope :past, -> { where(date: ..Time.zone.yesterday) } - scope :in_progress, -> { where(date: Time.zone.today) } - scope :future, -> { where(date: Time.zone.tomorrow..) } - scope :tomorrow, -> { where(date: Time.zone.tomorrow) } + scope :for_date, + ->(value) do + where( + SessionDate + .where("session_id = sessions.id") + .where(value:) + .arel + .exists + ) + end + + scope :past, -> { for_date(..Time.zone.yesterday) } + scope :in_progress, -> { for_date(Time.zone.today) } + scope :future, -> { for_date(Time.zone.tomorrow..) } + scope :tomorrow, -> { for_date(Time.zone.tomorrow) } scope :send_consent_requests_today, -> { active.where(send_consent_requests_at: Time.zone.today) } @@ -64,6 +76,8 @@ class Session < ApplicationRecord after_initialize :set_timeline_attributes after_validation :set_timeline_timestamps + after_save :ensure_session_date_exists + validate :programmes_part_of_team on_wizard_step :location, exact: true do @@ -85,7 +99,9 @@ class Session < ApplicationRecord presence: true, comparison: { greater_than_or_equal_to: -> { Time.zone.today }, - less_than_or_equal_to: ->(object) { object.date } + less_than_or_equal_to: ->(object) do + object.dates.map(&:value).min + end } validates :reminder_days_after, inclusion: { in: %w[default custom] } @@ -104,7 +120,7 @@ class Session < ApplicationRecord end def in_progress? - date.to_date == Time.zone.today + dates.map(&:value).include?(Date.current) end def wizard_steps @@ -112,7 +128,7 @@ def wizard_steps end def days_between_consent_and_session - (date - send_consent_requests_at).to_i + (dates.map(&:value).min - send_consent_requests_at).to_i end def days_between_consent_and_reminder @@ -147,7 +163,8 @@ def set_timeline_attributes end unless close_consent_at.nil? - self.close_consent_on = close_consent_at == date ? "default" : "custom" + self.close_consent_on = + close_consent_at == dates.map(&:value).min ? "default" : "custom" end end @@ -163,10 +180,26 @@ def set_timeline_timestamps end ) close_consent_on = - self.close_consent_on == "default" ? date : close_consent_at + ( + if self.close_consent_on == "default" + dates.map(&:value).min + else + close_consent_at + end + ) self.send_consent_reminders_at = send_consent_requests_at + reminder_days_after.days self.close_consent_at = close_consent_on end + + def ensure_session_date_exists + return if date.nil? + + # TODO: Replace with UI to add/remove dates. + ActiveRecord::Base.transaction do + dates.delete_all + dates.create!(value: date) + end + end end diff --git a/app/views/parent_interface/consent_forms/_confirmation_agreed.html.erb b/app/views/parent_interface/consent_forms/_confirmation_agreed.html.erb index 8156bf54c..8591e5238 100644 --- a/app/views/parent_interface/consent_forms/_confirmation_agreed.html.erb +++ b/app/views/parent_interface/consent_forms/_confirmation_agreed.html.erb @@ -1,6 +1,9 @@ <% title = t("consent_forms.confirmation_agreed.title.#{@consent_form.programme.type}", full_name: @consent_form.full_name, - date: @session.date.to_fs(:long)) %> + date: @session.dates.map(&:value).min.to_fs(:long)) %> + +<% # TODO: Handle multiple dates. %> + <% content_for :page_title, title %> <%= govuk_panel(title_text: title, classes: "app-panel nhsuk-u-margin-bottom-6") %> diff --git a/app/views/programmes/sessions.html.erb b/app/views/programmes/sessions.html.erb index 5eb870a56..694747f92 100644 --- a/app/views/programmes/sessions.html.erb +++ b/app/views/programmes/sessions.html.erb @@ -20,7 +20,7 @@ <%= govuk_table(html_attributes: { class: "nhsuk-table-responsive" }) do |table| %> <% table.with_head do |head| %> <% head.with_row do |row| %> - <% row.with_cell(text: "Date") %> + <% row.with_cell(text: "Dates") %> <% row.with_cell(text: "Time") %> <% row.with_cell(text: "Location") %> <% row.with_cell(text: "Consent period") %> @@ -32,8 +32,8 @@ <% sessions.each do |session| %> <% body.with_row do |row| %> <% row.with_cell do %> - Date - <%= session.date.to_fs(:long) %> + Dates + <%= safe_join(session.dates.map { _1.value.to_fs(:long) }, tag.br) %> <% end %> <% row.with_cell do %> Time diff --git a/app/views/sessions/edit/timeline.html.erb b/app/views/sessions/edit/timeline.html.erb index b9eb7e324..122d563af 100644 --- a/app/views/sessions/edit/timeline.html.erb +++ b/app/views/sessions/edit/timeline.html.erb @@ -14,7 +14,7 @@ Information:

- Session scheduled for <%= @session.date.to_fs(:long_day_of_week) %> (<%= @session.human_enum_name(:time_of_day) %>) + First session scheduled for <%= @session.dates.map(&:value).min.to_fs(:long_day_of_week) %> (<%= @session.human_enum_name(:time_of_day) %>)

<% end %> diff --git a/app/views/sessions/show.html.erb b/app/views/sessions/show.html.erb index 34909b767..6be5d3a44 100644 --- a/app/views/sessions/show.html.erb +++ b/app/views/sessions/show.html.erb @@ -1,5 +1,3 @@ -<% page_title = session_location(@session) %> - <% content_for :before_main do %> <%= render AppBreadcrumbComponent.new(items: [ { text: "Home", href: dashboard_path }, @@ -8,13 +6,7 @@ ]) %> <% end %> -<%= h1 page_title: do %> - <%= page_title %> - - <%= @session.date.to_fs(:long) %> (<%= @session.human_enum_name(:time_of_day) %>) ยท - <%= t("children", count: @patient_sessions.size) %> in cohort - -<% end %> +<%= h1 session_location(@session) %>

<%= govuk_tag(text: "In progress", colour: "blue", classes: "nhsuk-u-margin-right-3") %> diff --git a/spec/factories/sessions.rb b/spec/factories/sessions.rb index 3b61c6bbd..a999d7481 100644 --- a/spec/factories/sessions.rb +++ b/spec/factories/sessions.rb @@ -25,7 +25,10 @@ # FactoryBot.define do factory :session do - transient { programme { association :programme } } + transient do + date { Date.current } + programme { association :programme } + end programmes { [programme] } team { programmes.first&.team || association(:team) } @@ -39,6 +42,11 @@ active { true } + after(:create) do |session, evaluator| + next if (date = evaluator.date).nil? + create(:session_date, session:, value: date) + end + trait :draft do active { false } end diff --git a/spec/features/e2e_journey_spec.rb b/spec/features/e2e_journey_spec.rb index 1afd3c8da..a15cd36cf 100644 --- a/spec/features/e2e_journey_spec.rb +++ b/spec/features/e2e_journey_spec.rb @@ -148,7 +148,7 @@ def and_enter_timelines_and_confirm_the_session_details expect(page).to have_content( "Deadline for responsesAllow responses until the day of the session" ) - expect(page).to have_content("DateFriday 1 March 2024") + expect(page).to have_content("DatesFriday 1 March 2024") click_on "Confirm" end diff --git a/spec/features/import_vaccination_records_with_duplicates_spec.rb b/spec/features/import_vaccination_records_with_duplicates_spec.rb index 3817fcd27..a125446f4 100644 --- a/spec/features/import_vaccination_records_with_duplicates_spec.rb +++ b/spec/features/import_vaccination_records_with_duplicates_spec.rb @@ -119,7 +119,7 @@ def and_an_existing_patient_record_exists create( :vaccination_record, programme: @programme, - administered_at: @session.date.in_time_zone + 12.hours, + administered_at: @session.dates.first.value.in_time_zone + 12.hours, notes: "Foo", recorded_at: Time.zone.yesterday, batch: @batch, diff --git a/spec/features/self_consent_gillick_competent_spec.rb b/spec/features/self_consent_gillick_competent_spec.rb index 7a0489f72..ef5e94def 100644 --- a/spec/features/self_consent_gillick_competent_spec.rb +++ b/spec/features/self_consent_gillick_competent_spec.rb @@ -26,7 +26,7 @@ def given_an_hpv_programme_is_underway end def and_it_is_the_day_of_a_vaccination_session - travel_to(@session.date) + travel_to(@session.dates.first.value) end def and_there_is_a_child_without_parental_consent diff --git a/spec/features/self_consent_not_gillick_competent_spec.rb b/spec/features/self_consent_not_gillick_competent_spec.rb index 7601fbb1a..609096144 100644 --- a/spec/features/self_consent_not_gillick_competent_spec.rb +++ b/spec/features/self_consent_not_gillick_competent_spec.rb @@ -21,7 +21,7 @@ def given_an_hpv_programme_is_underway end def and_it_is_the_day_of_a_vaccination_session - travel_to(@session.date) + travel_to(@session.dates.first.value) end def and_there_is_a_child_without_parental_consent diff --git a/spec/mailers/consent_mailer_spec.rb b/spec/mailers/consent_mailer_spec.rb index 7852a5ed0..32848881a 100644 --- a/spec/mailers/consent_mailer_spec.rb +++ b/spec/mailers/consent_mailer_spec.rb @@ -67,7 +67,8 @@ let(:patient) { create(:patient) } let(:parent) { patient.parents.first } - let(:session) { create(:session, patients: [patient]) } + let(:date) { Date.current } + let(:session) { create(:session, date:, patients: [patient]) } it { should have_attributes(to: [parent.email]) } @@ -76,8 +77,8 @@ mail.message.header["personalisation"].unparsed_value end - it { should include(session_date: session.date.strftime("%A %-d %B")) } - it { should include(session_short_date: session.date.strftime("%-d %B")) } + it { should include(session_date: date.strftime("%A %-d %B")) } + it { should include(session_short_date: date.strftime("%-d %B")) } it do expect(personalisation).to include( @@ -105,6 +106,7 @@ let(:patient) { create(:patient) } let(:parent) { patient.parents.first } + let(:date) { Date.current } let(:session) { create(:session, patients: [patient]) } it { should have_attributes(to: [parent.email]) } @@ -114,7 +116,7 @@ mail.message.header["personalisation"].unparsed_value end - it { should include(session_date: session.date.strftime("%A %-d %B")) } + it { should include(session_date: date.strftime("%A %-d %B")) } it do expect(personalisation).to include( diff --git a/spec/models/immunisation_import_spec.rb b/spec/models/immunisation_import_spec.rb index 40c2184bb..6bb41e9ad 100644 --- a/spec/models/immunisation_import_spec.rb +++ b/spec/models/immunisation_import_spec.rb @@ -213,7 +213,9 @@ expect(immunisation_import.sessions.count).to eq(1) session = immunisation_import.sessions.first - expect(session.date).to eq(Date.new(2024, 5, 14)) + expect(session.dates.map(&:value)).to contain_exactly( + Date.new(2024, 5, 14) + ) expect(session.time_of_day).to eq("all_day") end end