From 204534eeb7aa2c5f5a15a72b893f7a1dbff390f4 Mon Sep 17 00:00:00 2001 From: Tomas D'Stefano Date: Mon, 23 Dec 2024 17:56:49 +0000 Subject: [PATCH 1/9] Move features to system spec Added the test for the secondary filters --- .../results/search_results_disabled_spec.rb | 8 +- .../v2/results/search_results_enabled_spec.rb | 112 ++++++++++++------ 2 files changed, 78 insertions(+), 42 deletions(-) rename spec/{features => system}/find/v2/results/search_results_disabled_spec.rb (68%) rename spec/{features => system}/find/v2/results/search_results_enabled_spec.rb (85%) diff --git a/spec/features/find/v2/results/search_results_disabled_spec.rb b/spec/system/find/v2/results/search_results_disabled_spec.rb similarity index 68% rename from spec/features/find/v2/results/search_results_disabled_spec.rb rename to spec/system/find/v2/results/search_results_disabled_spec.rb index 3a09a70657..9b126ba264 100644 --- a/spec/features/find/v2/results/search_results_disabled_spec.rb +++ b/spec/system/find/v2/results/search_results_disabled_spec.rb @@ -2,21 +2,21 @@ require 'rails_helper' -feature 'V2 results - disabled' do +RSpec.describe 'V2 results - disabled', service: :find do before do allow(Settings.features).to receive_messages(v2_results: false) end scenario 'when I visit the results page' do when_i_visit_the_find_results_page - then_i_see_not_authorised + then_i_see_not_found end def when_i_visit_the_find_results_page visit find_v2_results_path end - def then_i_see_not_authorised - expect(page.status_code).to eq(401) + def then_i_see_not_found + expect(page.status_code).to eq(404) end end diff --git a/spec/features/find/v2/results/search_results_enabled_spec.rb b/spec/system/find/v2/results/search_results_enabled_spec.rb similarity index 85% rename from spec/features/find/v2/results/search_results_enabled_spec.rb rename to spec/system/find/v2/results/search_results_enabled_spec.rb index 23740caf6d..9fca2dc871 100644 --- a/spec/features/find/v2/results/search_results_enabled_spec.rb +++ b/spec/system/find/v2/results/search_results_enabled_spec.rb @@ -2,11 +2,10 @@ require 'rails_helper' -feature 'V2 results - enabled' do +RSpec.describe 'V2 results - enabled', :js, service: :find do before do Timecop.travel(Find::CycleTimetable.mid_cycle) allow(Settings.features).to receive_messages(v2_results: true) - given_i_am_authenticated end scenario 'when I filter by visa sponsorship' do @@ -133,10 +132,20 @@ scenario 'filter by specific secondary subjects' do when_i_visit_the_find_results_page - and_i_filter_by_mathematics + and_i_search_for_the_mathematics_option + then_i_can_only_see_the_mathematics_option + when_i_clear_my_search_for_secondary_options + then_i_can_see_all_secondary_options + when_i_filter_by_mathematics then_i_see_only_mathematics_courses and_the_mathematics_secondary_option_is_checked and_i_see_that_there_is_one_course_found + + when_i_search_for_specific_secondary_options + then_i_can_only_see_options_that_i_searched + + when_i_clear_my_search_for_secondary_options + then_i_can_see_all_secondary_options end scenario 'filter by many secondary subjects' do @@ -162,10 +171,6 @@ then_i_see_no_courses_found end - def given_i_am_authenticated - page.driver.browser.authorize 'admin', 'password' - end - def given_there_are_courses_that_sponsor_visa create(:course, :with_full_time_sites, :can_sponsor_skilled_worker_visa, name: 'Biology', course_code: 'S872') create(:course, :with_full_time_sites, :can_sponsor_student_visa, name: 'Chemistry', course_code: 'K592') @@ -244,76 +249,101 @@ def when_i_visit_the_find_results_page_passing_mathematics_in_the_params visit(find_v2_results_path(subjects: ['G1'])) end + def and_i_search_for_the_mathematics_option + page.find('[data-filter-search-target="searchInput"]').set('Math') + end + + def then_i_can_only_see_the_mathematics_option + expect(secondary_options).to eq(['Mathematics']) + end + + def then_i_can_see_all_secondary_options + expect(secondary_options.size).to eq(37) + end + + def when_i_search_for_specific_secondary_options + page.find('[data-filter-search-target="searchInput"]').set('Com') + end + + def then_i_can_only_see_options_that_i_searched + expect(secondary_options).to eq(['Communication and media studies', 'Computing']) + end + + def when_i_clear_my_search_for_secondary_options + fill_in 'filter-search-0-input', with: '' + end + def and_i_filter_by_courses_that_sponsor_visa - check 'Only show courses with visa sponsorship' + check 'Only show courses with visa sponsorship', visible: :all and_i_apply_the_filters end - def and_i_filter_by_mathematics - check 'Mathematics' + def when_i_filter_by_mathematics + check 'Mathematics', visible: :all and_i_apply_the_filters end + alias_method :and_i_filter_by_mathematics, :when_i_filter_by_mathematics def and_i_filter_by_chemistry - check 'Chemistry' + check 'Chemistry', visible: :all and_i_apply_the_filters end def and_i_filter_only_by_part_time_courses - uncheck 'Full time (12 months)' - check 'Part time (18 to 24 months)' + uncheck 'Full time (12 months)', visible: :all + check 'Part time (18 to 24 months)', visible: :all and_i_apply_the_filters end def when_i_filter_only_by_full_time_courses - uncheck 'Part time (18 to 24 months)' - check 'Full time (12 months)' + uncheck 'Part time (18 to 24 months)', visible: :all + check 'Full time (12 months)', visible: :all and_i_apply_the_filters end def when_i_filter_by_part_time_and_full_time_courses - check 'Part time (18 to 24 months)' - check 'Full time (12 months)' + check 'Part time (18 to 24 months)', visible: :all + check 'Full time (12 months)', visible: :all and_i_apply_the_filters end def and_i_filter_by_qts_only_courses - check 'QTS only' + check 'QTS only', visible: :all and_i_apply_the_filters end def and_i_filter_by_qts_with_pgce_or_pgde_courses - check 'QTS with PGCE or PGDE' + check 'QTS with PGCE or PGDE', visible: :all and_i_apply_the_filters end def and_i_filter_by_courses_open_for_applications - check 'Only show courses open for applications' + check 'Only show courses open for applications', visible: :all and_i_apply_the_filters end def and_i_filter_by_further_education_courses - check 'Only show further education courses' + check 'Only show further education courses', visible: :all and_i_apply_the_filters end def and_i_filter_by_courses_with_special_education_needs - check 'Only show courses with a SEND specialism' + check 'Only show courses with a SEND specialism', visible: :all and_i_apply_the_filters end def and_i_filter_by_salaried_courses - check 'Salary' + check 'Salary', visible: :all, visible: :all and_i_apply_the_filters end def and_i_filter_by_fee_courses - check 'Fee - no salary' + check 'Fee - no salary', visible: :all and_i_apply_the_filters end def and_i_filter_by_apprenticeship_courses - check 'Teaching apprenticeship - with salary' + check 'Teaching apprenticeship - with salary', visible: :all and_i_apply_the_filters end @@ -331,7 +361,7 @@ def then_i_see_only_part_time_courses end def and_the_part_time_filter_is_checked - expect(page).to have_checked_field('Part time (18 to 24 months)') + expect(page).to have_checked_field('Part time (18 to 24 months)', visible: :all) end def then_i_see_only_full_time_courses @@ -341,7 +371,7 @@ def then_i_see_only_full_time_courses end def and_the_full_time_filter_is_checked - expect(page).to have_checked_field('Full time (12 months)') + expect(page).to have_checked_field('Full time (12 months)', visible: :all) end def then_i_see_all_courses_containing_all_study_types @@ -360,7 +390,7 @@ def then_i_see_only_qts_only_courses end def and_the_qts_only_filter_is_checked - expect(page).to have_checked_field('QTS only') + expect(page).to have_checked_field('QTS only', visible: :all) end def then_i_see_only_qts_with_pgce_or_pgde_courses @@ -373,7 +403,7 @@ def then_i_see_only_qts_with_pgce_or_pgde_courses end def and_the_qts_with_pgce_or_pgde_filter_is_checked - expect(page).to have_checked_field('QTS with PGCE or PGDE') + expect(page).to have_checked_field('QTS with PGCE or PGDE', visible: :all) end def then_i_see_only_further_education__courses @@ -383,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') + expect(page).to have_checked_field('Only show further education courses', visible: :all) end def then_i_see_only_courses_with_special_education_needs @@ -427,27 +457,27 @@ def then_i_see_only_courses_that_are_open_for_applications end def and_the_visa_sponsorship_filter_is_checked - expect(page).to have_checked_field('Only show courses with visa sponsorship') + expect(page).to have_checked_field('Only show courses with visa sponsorship', visible: :all) end def and_the_open_for_application_filter_is_checked - expect(page).to have_checked_field('Only show courses open for applications') + expect(page).to have_checked_field('Only show courses open for applications', visible: :all) end def and_the_special_education_needs_filter_is_checked - expect(page).to have_checked_field('Only show courses with a SEND specialism') + expect(page).to have_checked_field('Only show courses with a SEND specialism', visible: :all) end def and_the_fee_filter_is_checked - expect(page).to have_checked_field('Fee - no salary') + expect(page).to have_checked_field('Fee - no salary', visible: :all) end def and_the_salary_filter_is_checked - expect(page).to have_checked_field('Salary') + expect(page).to have_checked_field('Salary', visible: :all) end def and_the_apprenticeship_filter_is_checked - expect(page).to have_checked_field('Teaching apprenticeship - with salary') + expect(page).to have_checked_field('Teaching apprenticeship - with salary', visible: :all) end def and_i_apply_the_filters @@ -484,11 +514,11 @@ def then_i_see_mathematics_and_chemistry_courses end def and_the_mathematics_secondary_option_is_checked - expect(page).to have_checked_field('Mathematics') + expect(page).to have_checked_field('Mathematics', visible: :all) end def and_the_chemistry_secondary_option_is_checked - expect(page).to have_checked_field('Chemistry') + expect(page).to have_checked_field('Chemistry', visible: :all) end def then_i_see_no_courses_found @@ -498,6 +528,12 @@ def then_i_see_no_courses_found private + def secondary_options + page.all( + '[data-filter-search-target="optionsList"]', wait: 2 + ).map(&:text).join(' ').split("\n") + end + def results page.first('.app-search-results') end From 478ead093314e8c46490a54fe06ef8d437f4cab9 Mon Sep 17 00:00:00 2001 From: Tomas D'Stefano Date: Mon, 23 Dec 2024 17:57:40 +0000 Subject: [PATCH 2/9] Add secondary filter search options Within the secondary filters we will allow the option search so if someone searches within the filter for "Bio" they will see only "Biology" option. Add the js and the stimulus and a start to switch to stimulus bit by bit. --- .../components/find/_filter-search.scss | 10 ++++ app/assets/stylesheets/find_application.scss | 1 + app/controllers/find/v2/results_controller.rb | 8 --- app/javascript/find/application.js | 6 ++ .../controllers/filter_search_controller.js | 59 +++++++++++++++++++ app/views/find/v2/results/index.html.erb | 12 +++- package.json | 1 + yarn.lock | 5 ++ 8 files changed, 91 insertions(+), 11 deletions(-) create mode 100644 app/assets/stylesheets/components/find/_filter-search.scss create mode 100644 app/javascript/find/controllers/filter_search_controller.js diff --git a/app/assets/stylesheets/components/find/_filter-search.scss b/app/assets/stylesheets/components/find/_filter-search.scss new file mode 100644 index 0000000000..1f3f344d52 --- /dev/null +++ b/app/assets/stylesheets/components/find/_filter-search.scss @@ -0,0 +1,10 @@ +.filter-search { + .govuk-checkboxes { + position: relative; + max-height: 200px; + overflow-x: hidden; + overflow-y: auto; + margin-bottom: 20px; + padding-bottom: 10px; + } +} diff --git a/app/assets/stylesheets/find_application.scss b/app/assets/stylesheets/find_application.scss index 28ea4a4bab..a10c8beb04 100644 --- a/app/assets/stylesheets/find_application.scss +++ b/app/assets/stylesheets/find_application.scss @@ -17,6 +17,7 @@ @import "components/find/feedback"; @import "components/find/filter-layout"; @import "components/find/filter"; +@import "components/find/filter-search"; @import "components/find/filters"; @import "components/find/map"; diff --git a/app/controllers/find/v2/results_controller.rb b/app/controllers/find/v2/results_controller.rb index b8ea33d236..6857f08942 100644 --- a/app/controllers/find/v2/results_controller.rb +++ b/app/controllers/find/v2/results_controller.rb @@ -3,8 +3,6 @@ module Find module V2 class ResultsController < Find::ApplicationController - before_action :enforce_basic_auth - def index @search_courses_form = SearchCoursesForm.new(search_courses_params) @courses = CoursesQuery.call(params: @search_courses_form.search_params) @@ -28,12 +26,6 @@ def search_courses_params funding: [] ) end - - def enforce_basic_auth - authenticate_or_request_with_http_basic do |username, password| - BasicAuthenticable.authenticate(username, password) - end - end end end end diff --git a/app/javascript/find/application.js b/app/javascript/find/application.js index d048ae7abf..8e7cb46e14 100644 --- a/app/javascript/find/application.js +++ b/app/javascript/find/application.js @@ -8,6 +8,12 @@ import initAutocomplete from './autocomplete' import dfeAutocomplete from './dfe-autocomplete' import CookieBanner from '../cookie_banner' +import { Application } from "@hotwired/stimulus"; +import FilterSearchController from "./controllers/filter_search_controller"; + +window.Stimulus = Application.start(); +Stimulus.register("filter-search", FilterSearchController); + window.jQuery = jQuery window.$ = jQuery diff --git a/app/javascript/find/controllers/filter_search_controller.js b/app/javascript/find/controllers/filter_search_controller.js new file mode 100644 index 0000000000..2f73640453 --- /dev/null +++ b/app/javascript/find/controllers/filter_search_controller.js @@ -0,0 +1,59 @@ +import { Controller } from "@hotwired/stimulus"; + +// Connects to data-controller="filter-search" +export default class extends Controller { + static targets = ["optionsList", "searchInput", "legend"]; + + static instanceCounter = 0; + + connect() { + const list = this.element.querySelector(".govuk-checkboxes, .govuk-radios"); + list.dataset.filterSearchTarget = "optionsList"; + + const legend = this.element.querySelector("legend"); + legend.dataset.filterSearchTarget = "legend"; + + // This will be unique for each filter-search instance + this.instanceId = this.constructor.instanceCounter++; + + const searchInput = this.createSearchInput(); + list.before(searchInput); + } + + createSearchInput() { + const container = document.createElement("div"); + + const inputId = `${this.identifier}-${this.instanceId}-input`; + const labelText = `${this.legendTarget.innerText}`; + + container.innerHTML = ` + + + `; + + return container; + } + + search() { + const optionItems = this.optionsListTarget.children; + const searchValue = this.searchInputTarget.value.toLowerCase(); + + this.toggleItems(optionItems, searchValue); + } + + toggleItems(items, searchValue) { + Array.from(items).forEach(function (item) { + if (item.textContent.toLowerCase().indexOf(searchValue) > -1) { + item.style.display = ""; + } else { + item.style.display = "none"; + } + }); + } +} diff --git a/app/views/find/v2/results/index.html.erb b/app/views/find/v2/results/index.html.erb index 9caf5eb044..2e807da90f 100644 --- a/app/views/find/v2/results/index.html.erb +++ b/app/views/find/v2/results/index.html.erb @@ -18,9 +18,15 @@ <%= form.govuk_check_box :can_sponsor_visa, "true", multiple: false, label: { text: t("helpers.label.courses_query_filters.can_sponsor_visa"), size: "s" } %> <% end %> - <%= form.govuk_check_boxes_fieldset :subjects, legend: { text: t("helpers.legend.courses_query_filters.secondary_html"), size: "s" }, hint: { text: t("helpers.hint.courses_query_filters.secondary"), class: "govuk-!-font-size-16" }, class: "app-filter__group", hidden: false, multiple: false, form_group: { class: "app-filter__group" } do %> - <%= form.govuk_collection_check_boxes :subjects, form.object.secondary_subjects, :subject_code, :subject_name, legend: nil, include_hidden: false, small: true %> - <% end %> +
+ <%= form.govuk_check_boxes_fieldset :subjects, legend: { text: t("helpers.legend.courses_query_filters.secondary_html"), size: "s" }, hint: { text: t("helpers.hint.courses_query_filters.secondary"), class: "govuk-!-font-size-16" }, class: "app-filter__group govuk-checkboxes--small", hidden: false, multiple: false do %> + <% form.object.secondary_subjects.each do |subject| %> + <%= form.govuk_check_box :subjects, + subject.subject_code, + label: { text: subject.subject_name } %> + <% end %> + <% end %> +
<%= form.govuk_check_boxes_fieldset :study_types, legend: { text: t("helpers.legend.courses_query_filters.study_type_html"), size: "s" }, class: "app-filter__group govuk-checkboxes--small", multiple: false, form_group: { class: "app-filter__group" } do %> <%= form.govuk_check_box :study_types, "full_time", label: { text: t("helpers.label.courses_query_filters.study_type_options.full_time"), size: "s" }, include_hidden: false %> diff --git a/package.json b/package.json index 96c5e9650c..5d6ee26732 100644 --- a/package.json +++ b/package.json @@ -32,6 +32,7 @@ ] }, "dependencies": { + "@hotwired/stimulus": "^3.2.2", "@ministryofjustice/frontend": "2.0.0", "accessible-autocomplete": "^2.0.4", "core-js": "^3.9.1", diff --git a/yarn.lock b/yarn.lock index c8d006657f..54ac13cc25 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1003,6 +1003,11 @@ resolved "https://registry.yarnpkg.com/@eslint/js/-/js-8.56.0.tgz#ef20350fec605a7f7035a01764731b2de0f3782b" integrity sha512-gMsVel9D7f2HLkBma9VbtzZRehRogVRfbr++f06nL2vnCGCNlzOD+/MUov/F4p8myyAHspEhVobgjpX64q5m6A== +"@hotwired/stimulus@^3.2.2": + version "3.2.2" + resolved "https://registry.yarnpkg.com/@hotwired/stimulus/-/stimulus-3.2.2.tgz#071aab59c600fed95b97939e605ff261a4251608" + integrity sha512-eGeIqNOQpXoPAIP7tC1+1Yc1yl1xnwYqg+3mzqxyrbE5pg5YFBZcA6YoTiByJB6DKAEsiWtl6tjTJS4IYtbB7A== + "@humanwhocodes/config-array@^0.11.13": version "0.11.14" resolved "https://registry.yarnpkg.com/@humanwhocodes/config-array/-/config-array-0.11.14.tgz#d78e481a039f7566ecc9660b4ea7fe6b1fec442b" From 2ef9d9640ba5a87c257a9dc2124efb0bda2b9c71 Mon Sep 17 00:00:00 2001 From: Tomas D'Stefano Date: Mon, 23 Dec 2024 17:59:18 +0000 Subject: [PATCH 3/9] Enable the features flag again for the v2 results page Enabling in QA. The reason of the change is because Selenium doesn't play well with basic authentication Fix standard js errors --- app/javascript/find/application.js | 8 +-- .../controllers/filter_search_controller.js | 50 +++++++++---------- config/routes/find.rb | 2 +- config/settings/qa.yml | 1 + package.json | 3 +- .../v2/results/search_results_enabled_spec.rb | 2 +- 6 files changed, 34 insertions(+), 32 deletions(-) diff --git a/app/javascript/find/application.js b/app/javascript/find/application.js index 8e7cb46e14..f1dcfe2d23 100644 --- a/app/javascript/find/application.js +++ b/app/javascript/find/application.js @@ -8,11 +8,11 @@ import initAutocomplete from './autocomplete' import dfeAutocomplete from './dfe-autocomplete' import CookieBanner from '../cookie_banner' -import { Application } from "@hotwired/stimulus"; -import FilterSearchController from "./controllers/filter_search_controller"; +import { Application } from '@hotwired/stimulus' +import FilterSearchController from './controllers/filter_search_controller' -window.Stimulus = Application.start(); -Stimulus.register("filter-search", FilterSearchController); +window.Stimulus = Application.start() +Stimulus.register('filter-search', FilterSearchController) window.jQuery = jQuery window.$ = jQuery diff --git a/app/javascript/find/controllers/filter_search_controller.js b/app/javascript/find/controllers/filter_search_controller.js index 2f73640453..3a18521fef 100644 --- a/app/javascript/find/controllers/filter_search_controller.js +++ b/app/javascript/find/controllers/filter_search_controller.js @@ -1,30 +1,30 @@ -import { Controller } from "@hotwired/stimulus"; +import { Controller } from '@hotwired/stimulus' // Connects to data-controller="filter-search" export default class extends Controller { - static targets = ["optionsList", "searchInput", "legend"]; + static targets = ['optionsList', 'searchInput', 'legend'] - static instanceCounter = 0; + static instanceCounter = 0 - connect() { - const list = this.element.querySelector(".govuk-checkboxes, .govuk-radios"); - list.dataset.filterSearchTarget = "optionsList"; + connect () { + const list = this.element.querySelector('.govuk-checkboxes, .govuk-radios') + list.dataset.filterSearchTarget = 'optionsList' - const legend = this.element.querySelector("legend"); - legend.dataset.filterSearchTarget = "legend"; + const legend = this.element.querySelector('legend') + legend.dataset.filterSearchTarget = 'legend' // This will be unique for each filter-search instance - this.instanceId = this.constructor.instanceCounter++; + this.instanceId = this.constructor.instanceCounter++ - const searchInput = this.createSearchInput(); - list.before(searchInput); + const searchInput = this.createSearchInput() + list.before(searchInput) } - createSearchInput() { - const container = document.createElement("div"); + createSearchInput () { + const container = document.createElement('div') - const inputId = `${this.identifier}-${this.instanceId}-input`; - const labelText = `${this.legendTarget.innerText}`; + const inputId = `${this.identifier}-${this.instanceId}-input` + const labelText = `${this.legendTarget.innerText}` container.innerHTML = `