From 1aaac595fd9661113db3b2133a1154896a5c3f82 Mon Sep 17 00:00:00 2001 From: Lori Bailey <44073106+elceebee@users.noreply.github.com> Date: Thu, 13 Jun 2024 14:18:02 +0100 Subject: [PATCH 1/4] Remove unused methodsfrom specs --- .../editing_course_about_this_course_copy_content_spec.rb | 5 ----- .../editing_course_school_placements_copy_content_spec.rb | 5 ----- 2 files changed, 10 deletions(-) diff --git a/spec/features/publish/courses/editing_course_about_this_course_copy_content_spec.rb b/spec/features/publish/courses/editing_course_about_this_course_copy_content_spec.rb index 0eee9ad3b0..5f15e72451 100644 --- a/spec/features/publish/courses/editing_course_about_this_course_copy_content_spec.rb +++ b/spec/features/publish/courses/editing_course_about_this_course_copy_content_spec.rb @@ -94,11 +94,6 @@ def when_i_click_on_the_link_in_the_warning_box click_on 'About this course' end - def then_the_focus_is_on_the_input - about = find_field 'About this course' - expect(about.focus?).to be true - end - def when_i_visit_the_about_this_course_edit_page visit about_this_course_publish_provider_recruitment_cycle_course_path( provider.provider_code, diff --git a/spec/features/publish/courses/editing_course_school_placements_copy_content_spec.rb b/spec/features/publish/courses/editing_course_school_placements_copy_content_spec.rb index d3cb25cab5..8ccd9e22b6 100644 --- a/spec/features/publish/courses/editing_course_school_placements_copy_content_spec.rb +++ b/spec/features/publish/courses/editing_course_school_placements_copy_content_spec.rb @@ -88,11 +88,6 @@ def then_i_do_not_see_copied_course_data expect(find_field('How placements work').value).to eq @course.enrichments.first.how_school_placements_work end - def then_the_focus_is_on_the_input - about = find_field 'Interview process' - expect(about.focus?).to be true - end - def when_i_visit_the_school_placements_edit_page visit school_placements_publish_provider_recruitment_cycle_course_path( provider.provider_code, From 722d3d5962f21443e9b31918d3da70441ae96fe2 Mon Sep 17 00:00:00 2001 From: Lori Bailey <44073106+elceebee@users.noreply.github.com> Date: Thu, 13 Jun 2024 13:24:33 +0100 Subject: [PATCH 2/4] Fetch course list for copy content option --- .../courses/about_this_course_controller.rb | 1 + .../courses/gcse_requirements_controller.rb | 1 + .../courses/interview_process_controller.rb | 1 + .../courses/school_placements_controller.rb | 1 + ...rse_about_this_course_copy_content_spec.rb | 27 ++++- ...rse_gcse_requirements_copy_content_spec.rb | 106 ++++++++++++++++++ ...rse_interview_process_copy_content_spec.rb | 23 ++++ ...rse_school_placements_copy_content_spec.rb | 23 ++++ 8 files changed, 181 insertions(+), 2 deletions(-) create mode 100644 spec/features/publish/courses/editing_course_gcse_requirements_copy_content_spec.rb diff --git a/app/controllers/publish/courses/about_this_course_controller.rb b/app/controllers/publish/courses/about_this_course_controller.rb index 722a3080eb..df3ff9d451 100644 --- a/app/controllers/publish/courses/about_this_course_controller.rb +++ b/app/controllers/publish/courses/about_this_course_controller.rb @@ -24,6 +24,7 @@ def update redirect_to redirect_path else + fetch_course_list_to_copy_from render :edit end end diff --git a/app/controllers/publish/courses/gcse_requirements_controller.rb b/app/controllers/publish/courses/gcse_requirements_controller.rb index d828386b5a..d0b605019d 100644 --- a/app/controllers/publish/courses/gcse_requirements_controller.rb +++ b/app/controllers/publish/courses/gcse_requirements_controller.rb @@ -29,6 +29,7 @@ def update @gcse_requirements_form.save(course) redirect_to publish_provider_recruitment_cycle_course_path else + fetch_course_list_to_copy_from @errors = @gcse_requirements_form.errors.messages render :edit end diff --git a/app/controllers/publish/courses/interview_process_controller.rb b/app/controllers/publish/courses/interview_process_controller.rb index a3726d99a6..8a1030b0b7 100644 --- a/app/controllers/publish/courses/interview_process_controller.rb +++ b/app/controllers/publish/courses/interview_process_controller.rb @@ -23,6 +23,7 @@ def update redirect_to redirect_path else + fetch_course_list_to_copy_from render :edit end end diff --git a/app/controllers/publish/courses/school_placements_controller.rb b/app/controllers/publish/courses/school_placements_controller.rb index 3be72dd242..dc1e291ba4 100644 --- a/app/controllers/publish/courses/school_placements_controller.rb +++ b/app/controllers/publish/courses/school_placements_controller.rb @@ -29,6 +29,7 @@ def update redirect_to preview_or_course_description else + fetch_course_list_to_copy_from render :edit end end diff --git a/spec/features/publish/courses/editing_course_about_this_course_copy_content_spec.rb b/spec/features/publish/courses/editing_course_about_this_course_copy_content_spec.rb index 5f15e72451..6bfb24d528 100644 --- a/spec/features/publish/courses/editing_course_about_this_course_copy_content_spec.rb +++ b/spec/features/publish/courses/editing_course_about_this_course_copy_content_spec.rb @@ -13,7 +13,7 @@ and_i_select_the_other_course_from_the_copy_content_dropdown then_i_see_the_copied_course_data - then_i_see_the_warning_that_changes_are_not_saved + and_i_see_the_warning_that_changes_are_not_saved and_the_warning_has_a_link_to_the_about_course_input_field end end @@ -29,6 +29,16 @@ and_i_do_not_see_the_warning_that_changes_are_not_saved end + scenario 'copy course content options are available after validation' do + given_i_am_authenticated_as_a_provider_user + and_there_is_a_course_i_want_to_edit + and_there_is_a_course_with_data_i_want_to_copy + + when_i_visit_the_about_this_course_edit_page + when_i_submit_without_data + then_i_can_still_copy_content_from_another_course + end + private def given_i_am_authenticated_as_a_provider_user @@ -43,6 +53,18 @@ def and_there_is_a_course_with_data_i_want_to_copy course_to_copy('About this other course') end + def when_i_submit_without_data + fill_in 'About this course', with: '' + click_on 'Update about this course' + end + + def then_i_can_still_copy_content_from_another_course + when_i_select_the_other_course_from_the_copy_content_dropdown + + then_i_see_the_copied_course_data + and_i_see_the_warning_that_changes_are_not_saved + end + def and_there_is_a_course_without_data_i_try_to_copy course_to_copy(nil) end @@ -64,8 +86,9 @@ def and_i_select_the_other_course_from_the_copy_content_dropdown click_on 'Copy content' end + alias_method :when_i_select_the_other_course_from_the_copy_content_dropdown, :and_i_select_the_other_course_from_the_copy_content_dropdown - def then_i_see_the_warning_that_changes_are_not_saved + def and_i_see_the_warning_that_changes_are_not_saved expect(page).to have_content 'Your changes are not yet saved' expect(page).to have_content "We have copied this field from #{copied_course_name_and_code}." expect(page).to have_link 'About this course' diff --git a/spec/features/publish/courses/editing_course_gcse_requirements_copy_content_spec.rb b/spec/features/publish/courses/editing_course_gcse_requirements_copy_content_spec.rb new file mode 100644 index 0000000000..053067f53a --- /dev/null +++ b/spec/features/publish/courses/editing_course_gcse_requirements_copy_content_spec.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +require 'rails_helper' + +feature 'Editing GCSE requirements section, copying content from another course' do + scenario 'source course has gces requirements data to copy' do + given_i_am_authenticated_as_a_provider_user + and_there_is_a_course_i_want_to_edit + and_there_is_a_course_with_data_i_want_to_copy + + when_i_visit_the_gcse_requirements_edit_page + and_i_select_the_other_course_from_the_copy_content_dropdown + + then_i_see_the_copied_course_data + and_i_see_the_warning_that_changes_are_not_saved + end + + scenario 'copy course content options are available after validation' do + given_i_am_authenticated_as_a_provider_user + and_there_is_a_course_i_want_to_edit + and_there_is_a_course_with_data_i_want_to_copy + + when_i_visit_the_gcse_requirements_edit_page + when_i_submit_without_data + then_i_can_still_copy_content_from_another_course + end + + private + + def given_i_am_authenticated_as_a_provider_user + given_i_am_authenticated(user: create(:user, :with_provider)) + end + + def and_there_is_a_course_i_want_to_edit + given_a_course_exists + end + + def when_i_submit_without_data + click_on 'Update GCSEs and equivalency tests' + end + + def then_i_can_still_copy_content_from_another_course + when_i_select_the_other_course_from_the_copy_content_dropdown + + then_i_see_the_copied_course_data + and_i_see_the_warning_that_changes_are_not_saved + end + + def and_there_is_a_course_with_data_i_want_to_copy + @copied_course ||= create( + :course, + provider: current_user.providers.first, + accept_pending_gcse: true, + accept_gcse_equivalency: true, + accept_english_gcse_equivalency: false, + accept_maths_gcse_equivalency: true, + accept_science_gcse_equivalency: false, + additional_gcse_equivalencies: 'Some text about gcse equivalences' + ) + end + + def copied_course_name_and_code + "#{@copied_course.name} (#{@copied_course.course_code})" + end + + def and_i_select_the_other_course_from_the_copy_content_dropdown + select copied_course_name_and_code, from: 'Copy from' + + click_on 'Copy content' + end + alias_method :when_i_select_the_other_course_from_the_copy_content_dropdown, :and_i_select_the_other_course_from_the_copy_content_dropdown + + def and_i_see_the_warning_that_changes_are_not_saved + expect(page).to have_content 'Your changes are not yet saved' + expect(page).to have_content "We’ve copied these fields from #{copied_course_name_and_code}:" + expect(page).to have_link 'Accept pending GCSE' + expect(page).to have_link 'Accept GCSE equivalency' + expect(page).to have_link 'Accept Maths GCSE equivalency' + expect(page).to have_link 'Additional GCSE equivalencies' + expect(page).to have_content 'Please check them and make your changes before saving.' + end + + def then_i_see_the_copied_course_data + expect(page.find('[data-qa="gcse_requirements__pending_gcse_yes_radio"]')).to be_checked + expect(page.find('[data-qa="gcse_requirements__gcse_equivalency_yes_radio"]')).to be_checked + expect( + find_field( + 'Details about equivalency tests you offer or accept' + ).value + ).to eq 'Some text about gcse equivalences' + expect(find_field('English')).not_to be_checked + expect(find_field('Maths')).to be_checked + end + + def when_i_visit_the_gcse_requirements_edit_page + visit gcses_pending_or_equivalency_tests_publish_provider_recruitment_cycle_course_path( + provider.provider_code, + course.recruitment_cycle_year, + course.course_code + ) + end + + def provider + @provider ||= @current_user.providers.first + end +end diff --git a/spec/features/publish/courses/editing_course_interview_process_copy_content_spec.rb b/spec/features/publish/courses/editing_course_interview_process_copy_content_spec.rb index 377b606c57..ca47531f63 100644 --- a/spec/features/publish/courses/editing_course_interview_process_copy_content_spec.rb +++ b/spec/features/publish/courses/editing_course_interview_process_copy_content_spec.rb @@ -29,6 +29,16 @@ and_i_do_not_see_the_warning_that_changes_are_not_saved end + scenario 'copy course content options are available after validation' do + given_i_am_authenticated_as_a_provider_user + and_there_is_a_course_i_want_to_edit + and_there_is_a_course_with_data_i_want_to_copy + + when_i_visit_the_interview_process_edit_page + when_i_submit_with_too_many_words + then_i_can_still_copy_content_from_another_course + end + private def given_i_am_authenticated_as_a_provider_user @@ -47,6 +57,18 @@ def and_there_is_a_course_without_data_i_try_to_copy course_to_copy(nil) end + def when_i_submit_with_too_many_words + fill_in 'Interview process', with: Faker::Lorem.sentence(word_count: 251) + click_on 'Update interview process' + end + + def then_i_can_still_copy_content_from_another_course + when_i_select_the_other_course_from_the_copy_content_dropdown + + then_i_see_the_copied_course_data + and_i_see_the_warning_that_changes_are_not_saved + end + def course_to_copy(interview_process) @copied_course ||= create( :course, @@ -64,6 +86,7 @@ def and_i_select_the_other_course_from_the_copy_content_dropdown click_on 'Copy content' end + alias_method :when_i_select_the_other_course_from_the_copy_content_dropdown, :and_i_select_the_other_course_from_the_copy_content_dropdown def and_i_see_the_warning_that_changes_are_not_saved expect(page).to have_content 'Your changes are not yet saved' diff --git a/spec/features/publish/courses/editing_course_school_placements_copy_content_spec.rb b/spec/features/publish/courses/editing_course_school_placements_copy_content_spec.rb index 8ccd9e22b6..0bae16a17c 100644 --- a/spec/features/publish/courses/editing_course_school_placements_copy_content_spec.rb +++ b/spec/features/publish/courses/editing_course_school_placements_copy_content_spec.rb @@ -27,6 +27,16 @@ and_i_do_not_see_the_warning_that_changes_are_not_saved end + scenario 'copy course content options are available after validation' do + given_i_am_authenticated_as_a_provider_user + and_there_is_a_course_i_want_to_edit + and_there_is_a_course_with_data_i_want_to_copy + + when_i_visit_the_school_placements_edit_page + when_i_submit_without_data + then_i_can_still_copy_content_from_another_course + end + private def given_i_am_authenticated_as_a_provider_user @@ -37,6 +47,18 @@ def and_there_is_a_course_i_want_to_edit given_a_course_exists(enrichments: [build(:course_enrichment, :published)]) end + def when_i_submit_without_data + fill_in 'How placements work', with: '' + click_on 'Update how placements work' + end + + def then_i_can_still_copy_content_from_another_course + when_i_select_the_other_course_from_the_copy_content_dropdown + + then_i_see_the_copied_course_data + and_i_see_the_warning_that_changes_are_not_saved + end + def and_there_is_a_course_with_data_i_want_to_copy course_to_copy('About this other course') end @@ -62,6 +84,7 @@ def and_i_select_the_other_course_from_the_copy_content_dropdown click_on 'Copy content' end + alias_method :when_i_select_the_other_course_from_the_copy_content_dropdown, :and_i_select_the_other_course_from_the_copy_content_dropdown def and_i_see_the_warning_that_changes_are_not_saved expect(page).to have_content 'Your changes are not yet saved' From 9e9f6b1dfb9925dd6cc7ce601ff268857787fcc5 Mon Sep 17 00:00:00 2001 From: Lori Bailey <44073106+elceebee@users.noreply.github.com> Date: Thu, 13 Jun 2024 16:06:48 +0100 Subject: [PATCH 3/4] Refactor copy content partials to shared component --- ..._course_content_warning_component.html.erb | 23 +++++ .../copy_course_content_warning_component.rb | 34 +++++++ .../subject_requirements_controller.rb | 1 + app/services/courses/copy.rb | 10 +- ...copy_about_course_content_warning.html.erb | 30 ------ .../courses/about_this_course/edit.html.erb | 6 +- .../subject_requirements/edit.html.erb | 6 +- app/views/publish/courses/fees/edit.html.erb | 6 +- .../courses/gcse_requirements/edit.html.erb | 6 +- ...interview_process_content_warning.html.erb | 30 ------ .../courses/interview_process/edit.html.erb | 8 +- ...school_placements_content_warning.html.erb | 30 ------ .../courses/school_placements/edit.html.erb | 6 +- config/locales/en.yml | 18 ++-- ...ourse_content_warning_component_preview.rb | 32 ++++++ ...y_course_content_warning_component_spec.rb | 45 +++++++++ ...rse_about_this_course_copy_content_spec.rb | 2 +- ...rse_gcse_requirements_copy_content_spec.rb | 24 ++++- ...rse_interview_process_copy_content_spec.rb | 2 +- ...rse_school_placements_copy_content_spec.rb | 2 +- ..._subject_requirements_copy_content_spec.rb | 99 +++++++++++++++++++ 21 files changed, 302 insertions(+), 118 deletions(-) create mode 100644 app/components/providers/copy_course_content_warning_component.html.erb create mode 100644 app/components/providers/copy_course_content_warning_component.rb delete mode 100644 app/views/publish/courses/about_this_course/_copy_about_course_content_warning.html.erb delete mode 100644 app/views/publish/courses/interview_process/_copy_interview_process_content_warning.html.erb delete mode 100644 app/views/publish/courses/school_placements/_copy_school_placements_content_warning.html.erb create mode 100644 spec/components/providers/copy_course_content_warning_component_preview.rb create mode 100644 spec/components/providers/copy_course_content_warning_component_spec.rb create mode 100644 spec/features/publish/courses/editing_course_subject_requirements_copy_content_spec.rb diff --git a/app/components/providers/copy_course_content_warning_component.html.erb b/app/components/providers/copy_course_content_warning_component.html.erb new file mode 100644 index 0000000000..0b73931979 --- /dev/null +++ b/app/components/providers/copy_course_content_warning_component.html.erb @@ -0,0 +1,23 @@ +<% if field_links.present? %> + <%= govuk_notification_banner( + title_text: t("notification_banner.warning"), + classes: "govuk-notification-banner--warning", + html_attributes: { + data: { qa: "copy-course-warning" }, + role: "alert" + } + ) do |notification_banner| %> + <% notification_banner.with_heading( + text: t("components.providers.copy_course_content_warning_component.changes_not_saved") + ) %> +

+ <%= copied_fields_from %> +

+ +

<%= please_check_changes %>

+ <% end %> +<% end %> diff --git a/app/components/providers/copy_course_content_warning_component.rb b/app/components/providers/copy_course_content_warning_component.rb new file mode 100644 index 0000000000..4de1c11e35 --- /dev/null +++ b/app/components/providers/copy_course_content_warning_component.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module Providers + class CopyCourseContentWarningComponent < ApplicationComponent + include I18n + def initialize(copied_fields, form_identifier, source_course, **) + super(**) + @copied_fields = copied_fields + @form_identifier = form_identifier + @source_course = source_course + end + + def field_links + @copied_fields.map do |name, field| + [name, "##{@form_identifier}-#{field.gsub('_', '-')}-field"] + end + end + + def please_check_changes + translation_base = 'components.providers.copy_course_content_warning_component.please_check_changes' + t("#{translation_base}.#{plural? ? 'plural' : 'singular'}") + end + + def copied_fields_from + translation_base = 'components.providers.copy_course_content_warning_component.copied_fields_from' + t("#{translation_base}.#{plural? ? 'plural' : 'singular'}", + name_and_code: "#{@source_course.name} (#{@source_course.course_code})") + end + + def plural? + @copied_fields.length > 1 + end + end +end diff --git a/app/controllers/publish/courses/degrees/subject_requirements_controller.rb b/app/controllers/publish/courses/degrees/subject_requirements_controller.rb index 11b8c9f31b..cfdd122671 100644 --- a/app/controllers/publish/courses/degrees/subject_requirements_controller.rb +++ b/app/controllers/publish/courses/degrees/subject_requirements_controller.rb @@ -32,6 +32,7 @@ def update redirect_to preview_publish_provider_recruitment_cycle_course_path(provider.provider_code, course.recruitment_cycle_year, course.course_code) else set_backlink + fetch_course_list_to_copy_from @errors = @subject_requirements_form.errors.messages render :edit end diff --git a/app/services/courses/copy.rb b/app/services/courses/copy.rb index 48d6e43d66..a844325e0b 100644 --- a/app/services/courses/copy.rb +++ b/app/services/courses/copy.rb @@ -3,11 +3,11 @@ module Courses class Copy GCSE_FIELDS = [ - ['Accept pending GCSE', 'accept_pending_gcse'], - ['Accept GCSE equivalency', 'accept_gcse_equivalency'], - ['Accept English GCSE equivalency', 'accept_english_gcse_equivalency'], - ['Accept Maths GCSE equivalency', 'accept_maths_gcse_equivalency'], - ['Additional GCSE equivalencies', 'additional_gcse_equivalencies'] + ['Accept pending GCSE', 'accept_pending_gcse', 'accept-pending-gcse-true'], + ['Accept GCSE equivalency', 'accept_gcse_equivalency', 'accept-gcse-equivalency-true'], + ['Accept English GCSE equivalency', 'accept_english_gcse_equivalency', 'accept-english-gcse-equivalency-english'], + ['Accept Maths GCSE equivalency', 'accept_maths_gcse_equivalency', 'accept-maths-gcse-equivalency-maths'], + ['Additional GCSE equivalencies', 'additional_gcse_equivalencies', 'additional-gcse-equivalencies'] ].freeze SUBJECT_REQUIREMENTS_FIELDS = [ diff --git a/app/views/publish/courses/about_this_course/_copy_about_course_content_warning.html.erb b/app/views/publish/courses/about_this_course/_copy_about_course_content_warning.html.erb deleted file mode 100644 index b7a49ecbca..0000000000 --- a/app/views/publish/courses/about_this_course/_copy_about_course_content_warning.html.erb +++ /dev/null @@ -1,30 +0,0 @@ -<%= govuk_notification_banner( - title_text: t("notification_banner.warning"), - classes: "govuk-notification-banner--warning", - html_attributes: { - data: { qa: "copy-course-warning" }, - role: "alert" - } - ) do |notification_banner| %> - <% notification_banner.with_heading(text: t("publish.providers.about_course.edit.changes_not_saved")) %> -

- <%= t( - "publish.providers.about_course.edit.copied_fields_from", - name_and_code: "#{@source_course.name} (#{@source_course.course_code})" - ) %> -

- -

- <%= t("publish.providers.about_course.edit.please_check_changes") %> -

-<% end %> diff --git a/app/views/publish/courses/about_this_course/edit.html.erb b/app/views/publish/courses/about_this_course/edit.html.erb index 884841d64b..7a091ab8b7 100644 --- a/app/views/publish/courses/about_this_course/edit.html.erb +++ b/app/views/publish/courses/about_this_course/edit.html.erb @@ -4,7 +4,11 @@ ) %> <% if params[:copy_from].present? && @copied_fields.any? %> - <%= render partial: "copy_about_course_content_warning" %> + <%= render Providers::CopyCourseContentWarningComponent.new( + @copied_fields, + "publish-course-about-this-course-form", + @source_course + ) %> <% end %>
diff --git a/app/views/publish/courses/degrees/subject_requirements/edit.html.erb b/app/views/publish/courses/degrees/subject_requirements/edit.html.erb index 1d39fb05d4..b7d6de29c1 100644 --- a/app/views/publish/courses/degrees/subject_requirements/edit.html.erb +++ b/app/views/publish/courses/degrees/subject_requirements/edit.html.erb @@ -6,7 +6,11 @@ <% end %> <% if params[:copy_from].present? %> - <%= render partial: "publish/courses/copy_content_warning", locals: { copied_fields: @copied_fields } %> + <%= render Providers::CopyCourseContentWarningComponent.new( + @copied_fields, + "publish-subject-requirement-form", + @source_course + ) %> <% end %>
diff --git a/app/views/publish/courses/fees/edit.html.erb b/app/views/publish/courses/fees/edit.html.erb index 3b4931fb4d..d4baa590b1 100644 --- a/app/views/publish/courses/fees/edit.html.erb +++ b/app/views/publish/courses/fees/edit.html.erb @@ -2,7 +2,11 @@ <% content_for :page_title, title_with_error_prefix("#{page_title} – #{@course.name_and_code}", @course_fee_form.errors.any?) %> <% if params[:copy_from].present? %> - <%= render partial: "publish/courses/copy_content_warning", locals: { copied_fields: @copied_fields } %> + <%= render Providers::CopyCourseContentWarningComponent.new( + @copied_fields, + "publish-course-information-form", + @source_course + ) %> <% end %>
diff --git a/app/views/publish/courses/gcse_requirements/edit.html.erb b/app/views/publish/courses/gcse_requirements/edit.html.erb index 2d535992a3..97fd985324 100644 --- a/app/views/publish/courses/gcse_requirements/edit.html.erb +++ b/app/views/publish/courses/gcse_requirements/edit.html.erb @@ -3,7 +3,11 @@ <% content_for :page_title, title_with_error_prefix(page_title, @errors && @errors.any?) %> <% if params[:copy_from].present? %> - <%= render partial: "publish/courses/copy_content_warning", locals: { copied_fields: @copied_fields } %> + <%= render Providers::CopyCourseContentWarningComponent.new( + @copied_fields.map { |name, _field, target| [name, target] }, + "publish-gcse-requirements-form", + @source_course + ) %> <% end %>
diff --git a/app/views/publish/courses/interview_process/_copy_interview_process_content_warning.html.erb b/app/views/publish/courses/interview_process/_copy_interview_process_content_warning.html.erb deleted file mode 100644 index 89d63d15e5..0000000000 --- a/app/views/publish/courses/interview_process/_copy_interview_process_content_warning.html.erb +++ /dev/null @@ -1,30 +0,0 @@ -<%= govuk_notification_banner( - title_text: t("notification_banner.warning"), - classes: "govuk-notification-banner--warning", - html_attributes: { - data: { qa: "copy-course-warning" }, - role: "alert" - } - ) do |notification_banner| %> - <% notification_banner.with_heading(text: t("publish.providers.interview_process.edit.changes_not_saved")) %> -

- <%= t( - "publish.providers.interview_process.edit.copied_fields_from", - name_and_code: "#{@source_course.name} (#{@source_course.course_code})" - ) %> -

-
    - <% @copied_fields.each do |name, field| %> -
  • - <%= govuk_link_to( - name, - "#publish-course-interview-process-form-#{field.gsub('_', '-')}-field", - class: "govuk-notification-banner__link" - ) %> -
    • - <% end %> -
-

- <%= t("publish.providers.interview_process.edit.please_check_changes") %> -

-<% end %> diff --git a/app/views/publish/courses/interview_process/edit.html.erb b/app/views/publish/courses/interview_process/edit.html.erb index cfae64f2f1..b1971958bf 100644 --- a/app/views/publish/courses/interview_process/edit.html.erb +++ b/app/views/publish/courses/interview_process/edit.html.erb @@ -3,8 +3,12 @@ @interview_process_form.errors.any? ) %> -<% if params[:copy_from].present? && @copied_fields.any? %> - <%= render partial: "copy_interview_process_content_warning" %> +<% if params[:copy_from].present? %> + <%= render Providers::CopyCourseContentWarningComponent.new( + @copied_fields, + "publish-course-interview-process-form", + @source_course + ) %> <% end %>
diff --git a/app/views/publish/courses/school_placements/_copy_school_placements_content_warning.html.erb b/app/views/publish/courses/school_placements/_copy_school_placements_content_warning.html.erb deleted file mode 100644 index c1fe404b06..0000000000 --- a/app/views/publish/courses/school_placements/_copy_school_placements_content_warning.html.erb +++ /dev/null @@ -1,30 +0,0 @@ -<%= govuk_notification_banner( - title_text: t("notification_banner.warning"), - classes: "govuk-notification-banner--warning", - html_attributes: { - data: { qa: "copy-course-warning" }, - role: "alert" - } - ) do |notification_banner| %> - <% notification_banner.with_heading(text: t("publish.providers.school_placements.edit.changes_not_saved")) %> -

- <%= t( - "publish.providers.school_placements.edit.copied_fields_from", - name_and_code: "#{@source_course.name} (#{@source_course.course_code})" - ) %> -

-
    - <% @copied_fields.each do |name, field| %> -
  • - <%= govuk_link_to( - name, - "#publish-course-school-placements-form-#{field.gsub('_', '-')}-field", - class: "govuk-notification-banner__link" - ) %> -
    • - <% end %> -
-

- <%= t("publish.providers.school_placements.edit.please_check_changes") %> -

-<% end %> diff --git a/app/views/publish/courses/school_placements/edit.html.erb b/app/views/publish/courses/school_placements/edit.html.erb index 00072c6123..db20125b88 100644 --- a/app/views/publish/courses/school_placements/edit.html.erb +++ b/app/views/publish/courses/school_placements/edit.html.erb @@ -4,7 +4,11 @@ ) %> <% if params[:copy_from].present? && @copied_fields.any? %> - <%= render partial: "copy_school_placements_content_warning", locals: { copied_fields: @copied_fields } %> + <%= render Providers::CopyCourseContentWarningComponent.new( + @copied_fields, + "publish-course-school-placements-form", + @source_course + ) %> <% end %>
diff --git a/config/locales/en.yml b/config/locales/en.yml index de9f687348..0d595f4dff 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -168,6 +168,15 @@ en: school_direct_salaried_training_programme: label: "Salaried" components: + providers: + copy_course_content_warning_component: + copied_fields_from: + plural: "We have copied these fields from %{name_and_code}:" + singular: "We have copied this field from %{name_and_code}:" + please_check_changes: + plural: Please check them and make your changes before saving + singular: Please check it and make your changes before saving + changes_not_saved: Your changes are not yet saved page_titles: sign_in: index: "Sign in" @@ -384,9 +393,6 @@ en: interview_process_success: Interview process page_title: Interview process (optional) - %{course_name_and_code} submit_button: Update interview process - changes_not_saved: Your changes are not yet saved - copied_fields_from: We have copied this field from %{name_and_code}. - please_check_changes: Please check it and make your changes before saving. include_information_about_html:

Include information about:

    @@ -427,9 +433,6 @@ en: You will be placed in different schools during your training. You cannott pick which schools you want to be in, but your training provider will place you in schools you can travel to.

    - changes_not_saved: Your changes are not yet saved - copied_fields_from: We have copied this field from %{name_and_code}. - please_check_changes: Please check it and make your changes before saving. about_course: edit: about_this_course: About this course @@ -455,9 +458,6 @@ en:
  • link to your organisation's website for people who want more detail about who you are and what you do
view_examples: View examples of great course summaries - changes_not_saved: Your changes are not yet saved - copied_fields_from: We have copied this field from %{name_and_code}. - please_check_changes: Please check it and make your changes before saving. study_mode: form: select_all_that_apply: Select all that apply diff --git a/spec/components/providers/copy_course_content_warning_component_preview.rb b/spec/components/providers/copy_course_content_warning_component_preview.rb new file mode 100644 index 0000000000..464f48ef27 --- /dev/null +++ b/spec/components/providers/copy_course_content_warning_component_preview.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Providers + class CopyCourseContentWarningComponentPreview < ViewComponent::Preview + def with_many_fields + source_course = Course.new(name: 'Course name', course_code: 'AFGT') + copied_fields = [ + ['About this course', 'about_this_course'], + ['How placements work', 'how_placements_work'] + ] + render( + CopyCourseContentWarningComponent.new( + copied_fields, + 'form-identifier', + source_course + ) + ) + end + + def with_one_field + source_course = Course.new(name: 'Course name', course_code: 'AFGT') + copied_fields = [['How placements work', 'how_placements_work']] + render( + CopyCourseContentWarningComponent.new( + copied_fields, + 'form-identifier', + source_course + ) + ) + end + end +end diff --git a/spec/components/providers/copy_course_content_warning_component_spec.rb b/spec/components/providers/copy_course_content_warning_component_spec.rb new file mode 100644 index 0000000000..37bea8333d --- /dev/null +++ b/spec/components/providers/copy_course_content_warning_component_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Providers::CopyCourseContentWarningComponent do + alias_method :component, :page + + context 'without copied fields' do + it 'does not render anything' do + render_inline(described_class.new([], 'form-identifier', build(:course))) + + expect(component.text).to eq '' + end + end + + context 'with just one copied field' do + it 'renders singular text' do + copied_fields = [['How placements work', 'how_placements_work']] + source_course = build(:course) + render_inline(described_class.new(copied_fields, 'form-identifier', source_course)) + + expect(component).to have_content 'Your changes are not yet saved' + expect(component).to have_content 'Please check it and make your changes before saving' + expect(component).to have_content "We have copied this field from #{source_course.name} (#{source_course.course_code})" + expect(component.find_link('How placements work')[:href]).to eq '#form-identifier-how-placements-work-field' + end + end + + context 'with more than one copied field' do + it 'renders plural text' do + copied_fields = [ + ['How placements work', 'how_placements_work'], + ['About this course', 'about_this_course'] + ] + source_course = build(:course) + render_inline(described_class.new(copied_fields, 'form-identifier', source_course)) + + expect(component).to have_content 'Your changes are not yet saved' + expect(component).to have_content 'Please check them and make your changes before saving' + expect(component).to have_content "We have copied these fields from #{source_course.name} (#{source_course.course_code})" + expect(component.find_link('How placements work')[:href]).to eq '#form-identifier-how-placements-work-field' + expect(component.find_link('About this course')[:href]).to eq '#form-identifier-about-this-course-field' + end + end +end diff --git a/spec/features/publish/courses/editing_course_about_this_course_copy_content_spec.rb b/spec/features/publish/courses/editing_course_about_this_course_copy_content_spec.rb index 6bfb24d528..68585437c7 100644 --- a/spec/features/publish/courses/editing_course_about_this_course_copy_content_spec.rb +++ b/spec/features/publish/courses/editing_course_about_this_course_copy_content_spec.rb @@ -90,7 +90,7 @@ def and_i_select_the_other_course_from_the_copy_content_dropdown def and_i_see_the_warning_that_changes_are_not_saved expect(page).to have_content 'Your changes are not yet saved' - expect(page).to have_content "We have copied this field from #{copied_course_name_and_code}." + expect(page).to have_content "We have copied this field from #{copied_course_name_and_code}:" expect(page).to have_link 'About this course' expect(page).to have_content 'Please check it and make your changes before saving' end diff --git a/spec/features/publish/courses/editing_course_gcse_requirements_copy_content_spec.rb b/spec/features/publish/courses/editing_course_gcse_requirements_copy_content_spec.rb index 053067f53a..ff2de8edfd 100644 --- a/spec/features/publish/courses/editing_course_gcse_requirements_copy_content_spec.rb +++ b/spec/features/publish/courses/editing_course_gcse_requirements_copy_content_spec.rb @@ -13,6 +13,7 @@ then_i_see_the_copied_course_data and_i_see_the_warning_that_changes_are_not_saved + and_links_in_warning_match_input_ids end scenario 'copy course content options are available after validation' do @@ -52,7 +53,7 @@ def and_there_is_a_course_with_data_i_want_to_copy provider: current_user.providers.first, accept_pending_gcse: true, accept_gcse_equivalency: true, - accept_english_gcse_equivalency: false, + accept_english_gcse_equivalency: true, accept_maths_gcse_equivalency: true, accept_science_gcse_equivalency: false, additional_gcse_equivalencies: 'Some text about gcse equivalences' @@ -72,12 +73,27 @@ def and_i_select_the_other_course_from_the_copy_content_dropdown def and_i_see_the_warning_that_changes_are_not_saved expect(page).to have_content 'Your changes are not yet saved' - expect(page).to have_content "We’ve copied these fields from #{copied_course_name_and_code}:" + expect(page).to have_content "We have copied these fields from #{copied_course_name_and_code}:" expect(page).to have_link 'Accept pending GCSE' expect(page).to have_link 'Accept GCSE equivalency' expect(page).to have_link 'Accept Maths GCSE equivalency' + expect(page).to have_link 'Accept English GCSE equivalency' expect(page).to have_link 'Additional GCSE equivalencies' - expect(page).to have_content 'Please check them and make your changes before saving.' + expect(page).to have_content 'Please check them and make your changes before saving' + end + + def and_links_in_warning_match_input_ids + expect( + (find_link 'Accept pending GCSE')[:href].remove('#') + ).to eq page.find('[data-qa="gcse_requirements__pending_gcse_yes_radio"]')[:id] + expect( + (find_link 'Accept GCSE equivalency')[:href].remove('#') + ).to eq page.find('[data-qa="gcse_requirements__gcse_equivalency_yes_radio"]')[:id] + expect( + (find_link 'Accept Maths GCSE equivalency')[:href].remove('#') + ).to eq(find_field('Maths')[:id]) + expect((find_link 'Accept English GCSE equivalency')[:href].remove('#')).to eq(find_field('English')[:id]) + expect((find_link 'Additional GCSE equivalencies')[:href].remove('#')).to eq(find_field('Details about equivalency tests you offer or accept')[:id]) end def then_i_see_the_copied_course_data @@ -88,7 +104,7 @@ def then_i_see_the_copied_course_data 'Details about equivalency tests you offer or accept' ).value ).to eq 'Some text about gcse equivalences' - expect(find_field('English')).not_to be_checked + expect(find_field('English')).to be_checked expect(find_field('Maths')).to be_checked end diff --git a/spec/features/publish/courses/editing_course_interview_process_copy_content_spec.rb b/spec/features/publish/courses/editing_course_interview_process_copy_content_spec.rb index ca47531f63..fd110456e7 100644 --- a/spec/features/publish/courses/editing_course_interview_process_copy_content_spec.rb +++ b/spec/features/publish/courses/editing_course_interview_process_copy_content_spec.rb @@ -90,7 +90,7 @@ def and_i_select_the_other_course_from_the_copy_content_dropdown def and_i_see_the_warning_that_changes_are_not_saved expect(page).to have_content 'Your changes are not yet saved' - expect(page).to have_content "We have copied this field from #{copied_course_name_and_code}." + expect(page).to have_content "We have copied this field from #{copied_course_name_and_code}:" expect(page).to have_link 'Interview process' expect(page).to have_content 'Please check it and make your changes before saving' end diff --git a/spec/features/publish/courses/editing_course_school_placements_copy_content_spec.rb b/spec/features/publish/courses/editing_course_school_placements_copy_content_spec.rb index 0bae16a17c..0296eddf7c 100644 --- a/spec/features/publish/courses/editing_course_school_placements_copy_content_spec.rb +++ b/spec/features/publish/courses/editing_course_school_placements_copy_content_spec.rb @@ -88,7 +88,7 @@ def and_i_select_the_other_course_from_the_copy_content_dropdown def and_i_see_the_warning_that_changes_are_not_saved expect(page).to have_content 'Your changes are not yet saved' - expect(page).to have_content "We have copied this field from #{copied_course_name_and_code}." + expect(page).to have_content "We have copied this field from #{copied_course_name_and_code}:" expect(page).to have_link 'How placements work' expect(page).to have_content 'Please check it and make your changes before saving' end diff --git a/spec/features/publish/courses/editing_course_subject_requirements_copy_content_spec.rb b/spec/features/publish/courses/editing_course_subject_requirements_copy_content_spec.rb new file mode 100644 index 0000000000..3ce0a2f414 --- /dev/null +++ b/spec/features/publish/courses/editing_course_subject_requirements_copy_content_spec.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +require 'rails_helper' + +feature 'Editing subject requirements section, copying content from another course' do + scenario 'source course has gces requirements data to copy' do + given_i_am_authenticated_as_a_provider_user + and_there_is_a_course_i_want_to_edit + and_there_is_a_course_with_data_i_want_to_copy + + when_i_visit_the_subject_requirements_edit_page + and_i_select_the_other_course_from_the_copy_content_dropdown + + then_i_see_the_copied_course_data + and_i_see_the_warning_that_changes_are_not_saved + and_links_in_warning_match_input_ids + end + + scenario 'copy course content options are available after validation' do + given_i_am_authenticated_as_a_provider_user + and_there_is_a_course_i_want_to_edit + and_there_is_a_course_with_data_i_want_to_copy + + when_i_visit_the_subject_requirements_edit_page + when_i_submit_without_data + then_i_can_still_copy_content_from_another_course + end + + private + + def given_i_am_authenticated_as_a_provider_user + given_i_am_authenticated(user: create(:user, :with_provider)) + end + + def and_there_is_a_course_i_want_to_edit + given_a_course_exists(:secondary, additional_degree_subject_requirements: nil, degree_subject_requirements: nil) + end + + def when_i_submit_without_data + click_on 'Update degree requirements' + end + + def then_i_can_still_copy_content_from_another_course + when_i_select_the_other_course_from_the_copy_content_dropdown + + then_i_see_the_copied_course_data + and_i_see_the_warning_that_changes_are_not_saved + end + + def and_there_is_a_course_with_data_i_want_to_copy + @copied_course ||= create( + :course, :secondary, + provider: current_user.providers.first, + additional_degree_subject_requirements: true, + degree_subject_requirements: 'Some degree requirements' + ) + end + + def copied_course_name_and_code + "#{@copied_course.name} (#{@copied_course.course_code})" + end + + def and_i_select_the_other_course_from_the_copy_content_dropdown + select copied_course_name_and_code, from: 'Copy from' + + click_on 'Copy content' + end + alias_method :when_i_select_the_other_course_from_the_copy_content_dropdown, :and_i_select_the_other_course_from_the_copy_content_dropdown + + def and_i_see_the_warning_that_changes_are_not_saved + expect(page).to have_content 'Your changes are not yet saved' + expect(page).to have_content "We have copied these fields from #{copied_course_name_and_code}:" + expect(page).to have_link 'Additional degree subject requirements' + expect(page).to have_link 'Degree subject requirements' + expect(page).to have_content 'Please check them and make your changes before saving' + end + + def and_links_in_warning_match_input_ids + expect((find_link 'Additional degree subject requirements')[:href].remove('#')).to eq(find_field('No')[:id]) + expect((find_link 'Degree subject requirements')[:href].remove('#')).to eq(find_field('Degree subject requirements')[:id]) + end + + def then_i_see_the_copied_course_data + expect(find_field('Yes')).to be_checked + expect(find_field('Degree subject requirements').value).to eq 'Some degree requirements' + end + + def when_i_visit_the_subject_requirements_edit_page + visit degrees_subject_requirements_publish_provider_recruitment_cycle_course_path( + provider.provider_code, + course.recruitment_cycle_year, + course.course_code + ) + end + + def provider + @provider ||= @current_user.providers.first + end +end From c09b556dcd85370bd5ee19650602a63e4920c07b Mon Sep 17 00:00:00 2001 From: Lori Bailey <44073106+elceebee@users.noreply.github.com> Date: Fri, 14 Jun 2024 13:23:07 +0100 Subject: [PATCH 4/4] Formatting, review comments --- .../providers/copy_course_content_warning_component.rb | 9 +++++---- ...iting_course_about_this_course_copy_content_spec.rb | 2 +- ...iting_course_gcse_requirements_copy_content_spec.rb | 10 +++++----- ...iting_course_interview_process_copy_content_spec.rb | 2 +- ...ng_course_subject_requirements_copy_content_spec.rb | 4 ++-- 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/app/components/providers/copy_course_content_warning_component.rb b/app/components/providers/copy_course_content_warning_component.rb index 4de1c11e35..59eff5ed79 100644 --- a/app/components/providers/copy_course_content_warning_component.rb +++ b/app/components/providers/copy_course_content_warning_component.rb @@ -2,7 +2,6 @@ module Providers class CopyCourseContentWarningComponent < ApplicationComponent - include I18n def initialize(copied_fields, form_identifier, source_course, **) super(**) @copied_fields = copied_fields @@ -18,13 +17,15 @@ def field_links def please_check_changes translation_base = 'components.providers.copy_course_content_warning_component.please_check_changes' - t("#{translation_base}.#{plural? ? 'plural' : 'singular'}") + I18n.t("#{translation_base}.#{plural? ? 'plural' : 'singular'}") end def copied_fields_from translation_base = 'components.providers.copy_course_content_warning_component.copied_fields_from' - t("#{translation_base}.#{plural? ? 'plural' : 'singular'}", - name_and_code: "#{@source_course.name} (#{@source_course.course_code})") + I18n.t( + "#{translation_base}.#{plural? ? 'plural' : 'singular'}", + name_and_code: "#{@source_course.name} (#{@source_course.course_code})" + ) end def plural? diff --git a/spec/features/publish/courses/editing_course_about_this_course_copy_content_spec.rb b/spec/features/publish/courses/editing_course_about_this_course_copy_content_spec.rb index 68585437c7..678f9ae223 100644 --- a/spec/features/publish/courses/editing_course_about_this_course_copy_content_spec.rb +++ b/spec/features/publish/courses/editing_course_about_this_course_copy_content_spec.rb @@ -100,7 +100,7 @@ def and_i_do_not_see_the_warning_that_changes_are_not_saved end def and_the_warning_has_a_link_to_the_about_course_input_field - href = (find_link 'About this course')[:href] + href = find_link('About this course')[:href] about_this_course_id = (find_field 'About this course')[:id] expect(about_this_course_id).to eq(href.remove('#')) end diff --git a/spec/features/publish/courses/editing_course_gcse_requirements_copy_content_spec.rb b/spec/features/publish/courses/editing_course_gcse_requirements_copy_content_spec.rb index ff2de8edfd..20b58fcf58 100644 --- a/spec/features/publish/courses/editing_course_gcse_requirements_copy_content_spec.rb +++ b/spec/features/publish/courses/editing_course_gcse_requirements_copy_content_spec.rb @@ -84,16 +84,16 @@ def and_i_see_the_warning_that_changes_are_not_saved def and_links_in_warning_match_input_ids expect( - (find_link 'Accept pending GCSE')[:href].remove('#') + find_link('Accept pending GCSE')[:href].remove('#') ).to eq page.find('[data-qa="gcse_requirements__pending_gcse_yes_radio"]')[:id] expect( - (find_link 'Accept GCSE equivalency')[:href].remove('#') + find_link('Accept GCSE equivalency')[:href].remove('#') ).to eq page.find('[data-qa="gcse_requirements__gcse_equivalency_yes_radio"]')[:id] expect( - (find_link 'Accept Maths GCSE equivalency')[:href].remove('#') + find_link('Accept Maths GCSE equivalency')[:href].remove('#') ).to eq(find_field('Maths')[:id]) - expect((find_link 'Accept English GCSE equivalency')[:href].remove('#')).to eq(find_field('English')[:id]) - expect((find_link 'Additional GCSE equivalencies')[:href].remove('#')).to eq(find_field('Details about equivalency tests you offer or accept')[:id]) + expect(find_link('Accept English GCSE equivalency')[:href].remove('#')).to eq(find_field('English')[:id]) + expect(find_link('Additional GCSE equivalencies')[:href].remove('#')).to eq(find_field('Details about equivalency tests you offer or accept')[:id]) end def then_i_see_the_copied_course_data diff --git a/spec/features/publish/courses/editing_course_interview_process_copy_content_spec.rb b/spec/features/publish/courses/editing_course_interview_process_copy_content_spec.rb index fd110456e7..fb67e72041 100644 --- a/spec/features/publish/courses/editing_course_interview_process_copy_content_spec.rb +++ b/spec/features/publish/courses/editing_course_interview_process_copy_content_spec.rb @@ -100,7 +100,7 @@ def and_i_do_not_see_the_warning_that_changes_are_not_saved end def and_the_warning_has_a_link_to_the_interview_process_input_field - href = (find_link 'Interview process')[:href] + href = find_link('Interview process')[:href] interview_process_id = (find_field 'Interview process')[:id] expect(interview_process_id).to eq(href.remove('#')) end diff --git a/spec/features/publish/courses/editing_course_subject_requirements_copy_content_spec.rb b/spec/features/publish/courses/editing_course_subject_requirements_copy_content_spec.rb index 3ce0a2f414..c7b52c77e4 100644 --- a/spec/features/publish/courses/editing_course_subject_requirements_copy_content_spec.rb +++ b/spec/features/publish/courses/editing_course_subject_requirements_copy_content_spec.rb @@ -76,8 +76,8 @@ def and_i_see_the_warning_that_changes_are_not_saved end def and_links_in_warning_match_input_ids - expect((find_link 'Additional degree subject requirements')[:href].remove('#')).to eq(find_field('No')[:id]) - expect((find_link 'Degree subject requirements')[:href].remove('#')).to eq(find_field('Degree subject requirements')[:id]) + expect(find_link('Additional degree subject requirements')[:href].remove('#')).to eq(find_field('No')[:id]) + expect(find_link('Degree subject requirements')[:href].remove('#')).to eq(find_field('Degree subject requirements')[:id]) end def then_i_see_the_copied_course_data