From 2240a623693db29bc926f9e9912832ab4590ac5d Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Thu, 21 Mar 2024 17:04:50 +0000 Subject: [PATCH] Refactor show all filter This makes a number of small changes to the code to improve the readability. --- app/forms/assessor_interface/filter_form.rb | 4 ++-- .../{show_all_applications.rb => show_all.rb} | 4 +++- .../application_forms_index_view_object.rb | 3 ++- .../application_forms/index.html.erb | 6 +++--- config/feature_flags.yml | 11 ++++++----- ..._applications_spec.rb => show_all_spec.rb} | 4 +++- .../filtering_application_forms_spec.rb | 19 ++++++++++++++----- ...pplication_forms_index_view_object_spec.rb | 3 ++- 8 files changed, 35 insertions(+), 19 deletions(-) rename app/lib/filters/{show_all_applications.rb => show_all.rb} (90%) rename spec/lib/filters/{show_all_applications_spec.rb => show_all_spec.rb} (96%) diff --git a/app/forms/assessor_interface/filter_form.rb b/app/forms/assessor_interface/filter_form.rb index 543a233fb0..7af948fdf1 100644 --- a/app/forms/assessor_interface/filter_form.rb +++ b/app/forms/assessor_interface/filter_form.rb @@ -10,8 +10,8 @@ class AssessorInterface::FilterForm :location, :name, :reference, + :show_all, :stage, :submitted_at_after, - :submitted_at_before, - :show_all + :submitted_at_before end diff --git a/app/lib/filters/show_all_applications.rb b/app/lib/filters/show_all.rb similarity index 90% rename from app/lib/filters/show_all_applications.rb rename to app/lib/filters/show_all.rb index 8ea22fc03c..b927d3ab69 100644 --- a/app/lib/filters/show_all_applications.rb +++ b/app/lib/filters/show_all.rb @@ -1,5 +1,7 @@ +# frozen_string_literal: true + module Filters - class ShowAllApplications < Base + class ShowAll < Base def apply unless show_all? ninety_days_ago = 90.days.ago diff --git a/app/view_objects/assessor_interface/application_forms_index_view_object.rb b/app/view_objects/assessor_interface/application_forms_index_view_object.rb index 6d510c747a..ec8d783d99 100644 --- a/app/view_objects/assessor_interface/application_forms_index_view_object.rb +++ b/app/view_objects/assessor_interface/application_forms_index_view_object.rb @@ -23,6 +23,7 @@ def filter_form def assessor_filter_options ApplicationForm + .submitted .joins(:assessor) .pluck(Arel.sql("DISTINCT ON(assessor_id) assessor_id"), "staff.name") .sort_by { |_id, name| name } @@ -74,7 +75,7 @@ def application_forms_without_counted_filters ::Filters::Email, ::Filters::Reference, ::Filters::SubmittedAt, - ::Filters::ShowAllApplications, + ::Filters::ShowAll, ] filters.reduce( ApplicationForm.includes(region: :country).submitted, diff --git a/app/views/assessor_interface/application_forms/index.html.erb b/app/views/assessor_interface/application_forms/index.html.erb index c68f967e09..12deaa73b7 100644 --- a/app/views/assessor_interface/application_forms/index.html.erb +++ b/app/views/assessor_interface/application_forms/index.html.erb @@ -63,11 +63,11 @@ <% end %> <% end %> - - <% if FeatureFlags::FeatureFlag.active?(:show_all_applicants_filter) %> + + <% if FeatureFlags::FeatureFlag.active?(:show_all_applications_filter) %>
<%= f.govuk_check_boxes_fieldset :show_all, legend: nil, multiple: false, small: true do %> - <%= f.govuk_check_box :show_all, :true, multiple: false, label: { text: "Show applications completed older than 90 days ago" }, checked: @view_object.filter_form.show_all == "true" %> + <%= f.govuk_check_box :show_all, :true, multiple: false, label: { text: "Show applications completed over 90 days ago" }, checked: @view_object.filter_form.show_all == "true" %> <% end %>
<% end %> diff --git a/config/feature_flags.yml b/config/feature_flags.yml index 7aff586196..fe7aa43b65 100644 --- a/config/feature_flags.yml +++ b/config/feature_flags.yml @@ -28,15 +28,16 @@ feature_flags: When service_open is deactivated, and this flag is enabled, the user will have full access to the service. Should be inactive on production. + show_all_applications_filter: + author: Nick Diplos + description: > + Adds a filter that allows assessors to view applications that are older + than 90 days old. + teacher_applications: author: Thomas Leese description: > Allow starting an application on this service directly after completing an eligibility check. - show_all_applicants_filter: - author: Nick Diplos - description: > - Adds a filter that allows assessors to view applications that are older than 90 days old. - parent_controller: SupportInterface::FeatureFlagsController diff --git a/spec/lib/filters/show_all_applications_spec.rb b/spec/lib/filters/show_all_spec.rb similarity index 96% rename from spec/lib/filters/show_all_applications_spec.rb rename to spec/lib/filters/show_all_spec.rb index 3f5eba7bbe..c671148c29 100644 --- a/spec/lib/filters/show_all_applications_spec.rb +++ b/spec/lib/filters/show_all_spec.rb @@ -1,6 +1,8 @@ +# frozen_string_literal: true + require "rails_helper" -RSpec.describe Filters::ShowAllApplications do +RSpec.describe Filters::ShowAll do subject(:filtered_scope) { described_class.apply(scope:, params:) } let!(:application_awarded_recently) do diff --git a/spec/system/assessor_interface/filtering_application_forms_spec.rb b/spec/system/assessor_interface/filtering_application_forms_spec.rb index 36a30a898d..01d27113fe 100644 --- a/spec/system/assessor_interface/filtering_application_forms_spec.rb +++ b/spec/system/assessor_interface/filtering_application_forms_spec.rb @@ -4,12 +4,14 @@ RSpec.describe "Assessor filtering application forms", type: :system do before do - FeatureFlags::FeatureFlag.activate(:show_all_applicants_filter) given_the_service_is_open given_there_are_application_forms given_i_am_authorized_as_an_assessor_user + given_show_all_applications_is_activated end + after { given_show_all_applications_is_deactivated } + it "applies the filters" do when_i_visit_the(:assessor_applications_page) @@ -59,6 +61,14 @@ def given_there_are_application_forms application_forms end + def given_show_all_applications_is_activated + FeatureFlags::FeatureFlag.activate(:show_all_applications_filter) + end + + def given_show_all_applications_is_deactivated + FeatureFlags::FeatureFlag.deactivate(:show_all_applications_filter) + end + def when_i_visit_the_applications_page visit assessor_interface_application_forms_path end @@ -190,13 +200,13 @@ def then_i_see_a_list_of_applications_filtered_by_stage end def and_i_apply_the_show_all_filter - show_all = + show_all_item = assessor_applications_page.show_all_filter.items.find do |item| - item.label.text == "Show applications completed older than 90 days ago" + item.label.text == "Show applications completed over 90 days ago" rescue Capybara::ElementNotFound false end - show_all.checkbox.click + show_all_item.checkbox.click assessor_applications_page.apply_filters.click end @@ -258,7 +268,6 @@ def application_forms submitted_at: 7.months.ago, ), ] - @application_forms end def assessors diff --git a/spec/view_objects/assessor_interface/application_forms_index_view_object_spec.rb b/spec/view_objects/assessor_interface/application_forms_index_view_object_spec.rb index cfa47dbcb6..49886acb26 100644 --- a/spec/view_objects/assessor_interface/application_forms_index_view_object_spec.rb +++ b/spec/view_objects/assessor_interface/application_forms_index_view_object_spec.rb @@ -83,7 +83,7 @@ context "with a stage filter" do before do - expect_any_instance_of(Filters::ShowAllApplications).to receive( + expect_any_instance_of(Filters::ShowAll).to receive( :apply, ).and_return(ApplicationForm.none) end @@ -160,6 +160,7 @@ describe "#country_filter_options" do subject(:country_filter_options) { view_object.country_filter_options } + it do is_expected.to include( '',