From 7731046a8695909607e521c81615b7c8859697f9 Mon Sep 17 00:00:00 2001 From: Lori Bailey <44073106+elceebee@users.noreply.github.com> Date: Fri, 17 May 2024 17:37:03 +0100 Subject: [PATCH] Review content and bug fix --- .../publish/courses/study_mode_controller.rb | 2 +- app/forms/publish/course_study_mode_form.rb | 6 +++++ .../courses/_basic_details_tab.html.erb | 2 +- .../publish/courses/confirmation.html.erb | 2 +- .../publish/courses/study_mode/edit.html.erb | 2 +- config/locales/en.yml | 6 +---- .../courses/editing_course_study_mode_spec.rb | 13 ++++++++--- .../publish/courses/new_study_mode_spec.rb | 2 +- .../publish/course_study_mode_form_spec.rb | 22 ++++++++++++++++--- 9 files changed, 41 insertions(+), 16 deletions(-) diff --git a/app/controllers/publish/courses/study_mode_controller.rb b/app/controllers/publish/courses/study_mode_controller.rb index 259d8855d8..3d12d07ee2 100644 --- a/app/controllers/publish/courses/study_mode_controller.rb +++ b/app/controllers/publish/courses/study_mode_controller.rb @@ -16,7 +16,7 @@ def update @course_study_mode_form = CourseStudyModeForm.new(@course, params: study_mode_params) if @course_study_mode_form.save! - course_updated_message('Full time or part time') + course_updated_message I18n.t('publish.providers.study_mode.form.study_pattern') redirect_to details_publish_provider_recruitment_cycle_course_path( provider.provider_code, diff --git a/app/forms/publish/course_study_mode_form.rb b/app/forms/publish/course_study_mode_form.rb index f7ea897544..fcf8bf50ee 100644 --- a/app/forms/publish/course_study_mode_form.rb +++ b/app/forms/publish/course_study_mode_form.rb @@ -12,6 +12,12 @@ class CourseStudyModeForm < BaseModelForm presence: true, inclusion: { in: Course.study_modes.keys } + def study_mode_checked?(value) + mode = study_mode.nil? ? model.study_mode : study_mode + + mode == value || mode == 'full_time_or_part_time' + end + private def valid_before_save diff --git a/app/views/publish/courses/_basic_details_tab.html.erb b/app/views/publish/courses/_basic_details_tab.html.erb index dfac37cbcd..805563390b 100644 --- a/app/views/publish/courses/_basic_details_tab.html.erb +++ b/app/views/publish/courses/_basic_details_tab.html.erb @@ -87,7 +87,7 @@ end summary_list.with_row(html_attributes: { data: { qa: "course__study_mode" } }) do |row| - row.with_key { "Full time or part time" } + row.with_key { t("publish.providers.study_mode.form.study_pattern") } row.with_value { course.study_mode&.humanize } if course.cannot_change_study_mode? diff --git a/app/views/publish/courses/confirmation.html.erb b/app/views/publish/courses/confirmation.html.erb index 588c794ea2..bd735bba3b 100644 --- a/app/views/publish/courses/confirmation.html.erb +++ b/app/views/publish/courses/confirmation.html.erb @@ -91,7 +91,7 @@ <% end %> <% summary_list.with_row(html_attributes: { data: { qa: "course__study_mode" } }) do |row| %> - <% row.with_key { "Full time or part time" } %> + <% row.with_key { t("publish.providers.study_mode.form.study_pattern") } %> <% row.with_value { course.study_mode&.humanize } %> <% unless course.teacher_degree_apprenticeship? row.with_action( diff --git a/app/views/publish/courses/study_mode/edit.html.erb b/app/views/publish/courses/study_mode/edit.html.erb index 1f2079e621..700c69c616 100644 --- a/app/views/publish/courses/study_mode/edit.html.erb +++ b/app/views/publish/courses/study_mode/edit.html.erb @@ -35,7 +35,7 @@ <%= f.govuk_check_box :study_mode, value, label: { text: t("edit_options.study_modes.#{value}.label") }, - checked: course.study_mode.in?([value, "full_time_or_part_time"]), + checked: @course_study_mode_form.study_mode_checked?(value), link_errors: index.zero? %> <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 912c0d484f..fe194d4d54 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -130,8 +130,6 @@ en: label: "Part time" full_time: label: "Full time" - full_time_or_part_time: - label: "Full time or part time" apprenticeship: pg_teaching_apprenticeship: label: "Yes" @@ -839,7 +837,7 @@ en: publish/course_study_mode_form: attributes: study_mode: - blank: You must choose a study pattern. Select all that apply. + blank: Select a study pattern publish/course_withdrawal_form: attributes: confirm_course_code: @@ -982,8 +980,6 @@ en: label: "Part time" full_time: label: "Full time" - full_time_or_part_time: - label: "Full time or part time" can_sponsor_skilled_worker_visas: true: label: "Yes" diff --git a/spec/features/publish/courses/editing_course_study_mode_spec.rb b/spec/features/publish/courses/editing_course_study_mode_spec.rb index 2a10105608..31b0bb2702 100644 --- a/spec/features/publish/courses/editing_course_study_mode_spec.rb +++ b/spec/features/publish/courses/editing_course_study_mode_spec.rb @@ -35,10 +35,12 @@ end scenario 'updating with invalid data' do - and_there_is_a_course_with_no_study_mode + and_there_is_a_part_time_course_i_want_to_edit when_i_visit_the_course_study_mode_page + and_i_deselect_part_time_study_mode and_i_submit then_i_should_see_an_error_message + and_nothing_is_selected end def given_i_am_authenticated_as_a_provider_user @@ -73,7 +75,7 @@ def and_i_submit end def then_i_should_see_a_success_message - expect(page).to have_content(I18n.t('success.saved', value: 'Full time or part time')) + expect(page).to have_content(I18n.t('success.saved', value: 'Study pattern')) end def then_i_see_part_time_selected @@ -99,9 +101,14 @@ def then_i_see_full_time_selected expect(publish_course_study_mode_edit_page.part_time.checked?).to be false end + def and_nothing_is_selected + expect(publish_course_study_mode_edit_page.part_time.checked?).to be false + expect(publish_course_study_mode_edit_page.full_time.checked?).to be false + end + def then_i_should_see_an_error_message expect(publish_course_study_mode_edit_page.error_messages) - .to include('You must choose a study pattern. Select all that apply.') + .to include('Select a study pattern') end def provider diff --git a/spec/features/publish/courses/new_study_mode_spec.rb b/spec/features/publish/courses/new_study_mode_spec.rb index 4c74823937..44e6a2ecc2 100644 --- a/spec/features/publish/courses/new_study_mode_spec.rb +++ b/spec/features/publish/courses/new_study_mode_spec.rb @@ -72,7 +72,7 @@ def then_i_am_met_with_the_schools_page(study_mode) def then_i_am_met_with_errors expect(page).to have_content('There is a problem') - expect(page).to have_content('You must choose a study pattern. Select all that apply.').twice + expect(page).to have_content('Select a study pattern').twice end def selected_params(study_mode) diff --git a/spec/forms/publish/course_study_mode_form_spec.rb b/spec/forms/publish/course_study_mode_form_spec.rb index 9b522a648a..932cf1c9c5 100644 --- a/spec/forms/publish/course_study_mode_form_spec.rb +++ b/spec/forms/publish/course_study_mode_form_spec.rb @@ -12,7 +12,7 @@ module Publish let(:params) { {} } describe '#save!' do - it 'does not call the course.ensure_site_statuses_match_study_mode' do + it 'does not save any changes' do expect(subject.save!).to be false end end @@ -23,8 +23,12 @@ module Publish describe '#save!' do it 'does calls the course.ensure_site_statuses_match_study_mode' do - expect(course).to receive(:changed?) - expect(course).not_to receive(:ensure_site_statuses_match_study_mode) + allow(course).to receive(:changed?).and_return true + expect(course).to receive(:ensure_site_statuses_match_study_mode) + subject.save! + end + + it 'saves the study mode as part time' do subject.save! expect(course.study_mode).to eq 'part_time' @@ -36,6 +40,12 @@ module Publish let(:params) { { study_mode: %w[full_time part_time] } } it 'does calls the course.ensure_site_statuses_match_study_mode' do + allow(course).to receive(:changed?).and_return true + expect(course).to receive(:ensure_site_statuses_match_study_mode) + subject.save! + end + + it 'saves the study mode as full_time_or_part_time' do subject.save! expect(course.study_mode).to eq 'full_time_or_part_time' @@ -46,6 +56,12 @@ module Publish let(:params) { { study_mode: ['full_time'] } } it 'does calls the course.ensure_site_statuses_match_study_mode' do + allow(course).to receive(:changed?).and_return true + expect(course).to receive(:ensure_site_statuses_match_study_mode) + subject.save! + end + + it 'saves the study mode as full time' do subject.save! expect(course.study_mode).to eq 'full_time'