Skip to content

Commit

Permalink
Handle legacy parameters for further education filters in v2/results
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tomas-stefano committed Dec 31, 2024
1 parent 14119c0 commit d258e3e
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 14 deletions.
4 changes: 3 additions & 1 deletion app/controllers/find/v2/results_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
33 changes: 31 additions & 2 deletions app/forms/search_courses_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
4 changes: 2 additions & 2 deletions app/services/courses_query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/views/find/v2/results/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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 %>
Expand Down
26 changes: 26 additions & 0 deletions spec/forms/search_courses_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]) }

Expand Down
2 changes: 1 addition & 1 deletion spec/services/courses_query_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
41 changes: 34 additions & 7 deletions spec/system/find/v2/results/search_results_enabled_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit d258e3e

Please sign in to comment.