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 30, 2024
1 parent fa188b5 commit d85c791
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 11 deletions.
2 changes: 2 additions & 0 deletions app/controllers/find/v2/results_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ def search_courses_params
:applications_open,
:level,
:funding,
:age_group,
subjects: [],
study_types: [],
qualifications: [],
qualification: [],
funding: []
)
end
Expand Down
31 changes: 30 additions & 1 deletion app/forms/search_courses_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,21 @@ class SearchCoursesForm < ApplicationForm
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
2 changes: 1 addition & 1 deletion app/services/courses_query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def qualifications_scope
end

def further_education_scope
return @scope if params[:level].blank? || params[:level] != 'further_education'
return @scope if params[:level] != 'further_education'

@applied_scopes[:level] = params[:level]

Expand Down
4 changes: 2 additions & 2 deletions spec/forms/search_courses_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@
end
end

context 'when old qualification params is used' do
let(:form) { described_class.new(qualification: ['']) }
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' })
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 d85c791

Please sign in to comment.