From 14119c0655cc4eb6dcb0fa01f3ad09905ec231a8 Mon Sep 17 00:00:00 2001 From: Tomas D'Stefano Date: Fri, 27 Dec 2024 08:19:31 +0000 Subject: [PATCH 1/2] Change label on further education --- config/locales/find.yml | 2 +- spec/system/find/v2/results/search_results_enabled_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config/locales/find.yml b/config/locales/find.yml index cff4bde39f..d7f20db0a6 100644 --- a/config/locales/find.yml +++ b/config/locales/find.yml @@ -335,7 +335,7 @@ en: qualification_options: qts: QTS only qts_with_pgce_or_pgde: QTS with PGCE or PGDE - further_education: Only show further education courses + further_education: Further education courses funding_options: fee: Fee - no salary salary: Salary diff --git a/spec/system/find/v2/results/search_results_enabled_spec.rb b/spec/system/find/v2/results/search_results_enabled_spec.rb index c955821f28..64f70f73a5 100644 --- a/spec/system/find/v2/results/search_results_enabled_spec.rb +++ b/spec/system/find/v2/results/search_results_enabled_spec.rb @@ -323,7 +323,7 @@ def and_i_filter_by_courses_open_for_applications end def and_i_filter_by_further_education_courses - check 'Only show further education courses', visible: :all + check 'Further education courses', visible: :all and_i_apply_the_filters end @@ -413,7 +413,7 @@ def then_i_see_only_further_education__courses end def and_the_further_education_filter_is_checked - expect(page).to have_checked_field('Only show further education courses', visible: :all) + expect(page).to have_checked_field('Further education courses', visible: :all) end def then_i_see_only_courses_with_special_education_needs From d258e3e8a579146b57520823a99bd6046e015d03 Mon Sep 17 00:00:00 2001 From: Tomas D'Stefano Date: Mon, 30 Dec 2024 14:33:17 +0000 Subject: [PATCH 2/2] Handle legacy parameters for further education filters in v2/results This commit updates the handling of legacy parameters (`age_group` and `qualification`) to ensure compatibility when filtering by "further education" in the old v1 `/results` endpoint. - **SearchCoursesForm**: - Introduced `age_group` and `qualification` attributes. - Added methods to map old parameters (`age_group: 'further_education'` or `qualification: ['pgce pgde']`) to the `level: 'further_education'` parameter. - Ensures `search_params` excludes legacy parameters while still applying their equivalent filtering logic. - **CoursesQuery**: - Simplified logic for `further_education_scope` to focus on `level: 'further_education'`. If a user selects "further education" in the updated v2 filters, legacy v1 requests using 1. `age_group = further_education` (v1 second question) 2. `qualification = ['pgce pgde']` will continue to work. (v1 qualification filter when user chooses further education). This ensures backward compatibility while moving towards the new parameter conventions. --- app/controllers/find/v2/results_controller.rb | 4 +- app/forms/search_courses_form.rb | 33 ++++++++++++++- app/services/courses_query.rb | 4 +- app/views/find/v2/results/index.html.erb | 2 +- spec/forms/search_courses_form_spec.rb | 26 ++++++++++++ spec/services/courses_query_spec.rb | 2 +- .../v2/results/search_results_enabled_spec.rb | 41 +++++++++++++++---- 7 files changed, 98 insertions(+), 14 deletions(-) diff --git a/app/controllers/find/v2/results_controller.rb b/app/controllers/find/v2/results_controller.rb index 6857f08942..d2fb74f09f 100644 --- a/app/controllers/find/v2/results_controller.rb +++ b/app/controllers/find/v2/results_controller.rb @@ -18,11 +18,13 @@ def search_courses_params :can_sponsor_visa, :send_courses, :applications_open, - :further_education, + :level, :funding, + :age_group, subjects: [], study_types: [], qualifications: [], + qualification: [], funding: [] ) end diff --git a/app/forms/search_courses_form.rb b/app/forms/search_courses_form.rb index 5e4dd40cf6..e638f8366e 100644 --- a/app/forms/search_courses_form.rb +++ b/app/forms/search_courses_form.rb @@ -9,11 +9,24 @@ class SearchCoursesForm < ApplicationForm attribute :applications_open, :boolean attribute :study_types attribute :qualifications - attribute :further_education, :boolean + attribute :level attribute :funding + attribute :age_group + attribute :qualification + def search_params - attributes.symbolize_keys.compact + attributes + .symbolize_keys + .then { |params| params.except(*old_parameters) } + .then { |params| transform_old_parameters(params) } + .compact + end + + def level + return 'further_education' if old_further_education_parameters? + + super end def secondary_subjects @@ -22,4 +35,20 @@ def secondary_subjects .where.not(subject_name: ['Modern Languages']) .order(:subject_name) end + + private + + def transform_old_parameters(params) + params.tap do + params[:level] = level + end + end + + def old_further_education_parameters? + age_group == 'further_education' || qualification == ['pgce pgde'] + end + + def old_parameters + %i[age_group qualification] + end end diff --git a/app/services/courses_query.rb b/app/services/courses_query.rb index eae361a0f1..24b4a7472c 100644 --- a/app/services/courses_query.rb +++ b/app/services/courses_query.rb @@ -93,9 +93,9 @@ def qualifications_scope end def further_education_scope - return @scope if params[:further_education].blank? + return @scope if params[:level] != 'further_education' - @applied_scopes[:further_education] = params[:further_education] + @applied_scopes[:level] = params[:level] @scope.where(level: Course.levels[:further_education]) end diff --git a/app/views/find/v2/results/index.html.erb b/app/views/find/v2/results/index.html.erb index 650e221ee2..5be945025b 100644 --- a/app/views/find/v2/results/index.html.erb +++ b/app/views/find/v2/results/index.html.erb @@ -39,7 +39,7 @@ <% end %> <%= form.govuk_check_boxes_fieldset :further_education, legend: { text: t("helpers.legend.courses_query_filters.further_education_html"), size: "s" }, hint: { text: t("helpers.hint.courses_query_filters.further_education"), class: "govuk-!-font-size-16" }, class: "app-filter__group govuk-checkboxes--small", multiple: false, form_group: { class: "app-filter__group" } do %> - <%= form.govuk_check_box :further_education, "true", label: { text: t("helpers.label.courses_query_filters.further_education"), size: "s" }, multiple: false %> + <%= form.govuk_check_box :level, "further_education", label: { text: t("helpers.label.courses_query_filters.further_education"), size: "s" }, multiple: false %> <% end %> <%= form.govuk_check_boxes_fieldset :send_courses, multiple: false, legend: { text: t("helpers.legend.courses_query_filters.special_education_needs_html"), size: "s" }, class: "app-filter__group govuk-checkboxes--small", hidden: false, form_group: { class: "app-filter__group" } do %> diff --git a/spec/forms/search_courses_form_spec.rb b/spec/forms/search_courses_form_spec.rb index 0ee6a230aa..05526cc2ab 100644 --- a/spec/forms/search_courses_form_spec.rb +++ b/spec/forms/search_courses_form_spec.rb @@ -60,6 +60,32 @@ end end + context 'when further education is provided' do + context 'when new level params' do + let(:form) { described_class.new(level: 'further_education') } + + it 'returns level search params' do + expect(form.search_params).to eq({ level: 'further_education' }) + end + end + + context 'when old age group params is used' do + let(:form) { described_class.new(age_group: 'further_education') } + + it 'returns level search params' do + expect(form.search_params).to eq({ level: 'further_education' }) + end + end + + context 'when old qualification params is used as string' do + let(:form) { described_class.new(qualification: ['pgce pgde']) } + + it 'returns level search params' do + expect(form.search_params).to eq({ level: 'further_education' }) + end + end + end + context 'when funding is provided' do let(:form) { described_class.new(funding: %w[fee salary]) } diff --git a/spec/services/courses_query_spec.rb b/spec/services/courses_query_spec.rb index 74f142ac7c..d547509c0a 100644 --- a/spec/services/courses_query_spec.rb +++ b/spec/services/courses_query_spec.rb @@ -174,7 +174,7 @@ let!(:regular_course) do create(:course, :with_full_time_sites, level: 'secondary') end - let(:params) { { further_education: 'true' } } + let(:params) { { level: 'further_education' } } it 'returns courses for further education only' do expect(results).to match_collection( diff --git a/spec/system/find/v2/results/search_results_enabled_spec.rb b/spec/system/find/v2/results/search_results_enabled_spec.rb index 64f70f73a5..4d0f2c6dcf 100644 --- a/spec/system/find/v2/results/search_results_enabled_spec.rb +++ b/spec/system/find/v2/results/search_results_enabled_spec.rb @@ -51,13 +51,32 @@ and_i_see_that_there_are_two_courses_found end - scenario 'when I filter by further education only courses' do - given_there_are_courses_containing_all_levels - when_i_visit_the_find_results_page - and_i_filter_by_further_education_courses - then_i_see_only_further_education__courses - and_the_further_education_filter_is_checked - and_i_see_that_there_is_one_course_found + context 'when I filter by further education only courses' do + before do + given_there_are_courses_containing_all_levels + end + + scenario 'when I filter by further education only courses' do + when_i_visit_the_find_results_page + and_i_filter_by_further_education_courses + then_i_see_only_further_education__courses + and_the_further_education_filter_is_checked + and_i_see_that_there_is_one_course_found + end + + scenario 'when I filter by the old age group further education parameter' do + when_i_visit_the_find_results_page_using_the_old_age_group_parameter + then_i_see_only_further_education__courses + and_the_further_education_filter_is_checked + and_i_see_that_there_is_one_course_found + end + + scenario 'when I filter by the old pgce pgde further education parameter' do + when_i_visit_the_find_results_page_using_the_old_pgce_pgde_parameter + then_i_see_only_further_education__courses + and_the_further_education_filter_is_checked + and_i_see_that_there_is_one_course_found + end end scenario 'when I filter by applications open' do @@ -249,6 +268,14 @@ def when_i_visit_the_find_results_page_passing_mathematics_in_the_params visit(find_v2_results_path(subjects: ['G1'])) end + def when_i_visit_the_find_results_page_using_the_old_age_group_parameter + visit(find_v2_results_path(age_group: 'further_education')) + end + + def when_i_visit_the_find_results_page_using_the_old_pgce_pgde_parameter + visit(find_v2_results_path(qualification: ['pgce pgde'])) + end + def and_i_search_for_the_mathematics_option page.find('[data-filter-search-target="searchInput"]').set('Math') end