From 77af443be1c428fbd879b0a8c702621bd41d2b62 Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Thu, 14 Sep 2023 21:10:13 +0100 Subject: [PATCH] Use Filters::Stage This changes the case management index page to use the new stage filter, and stop using the statuses filter. --- app/forms/assessor_interface/filter_form.rb | 2 +- .../application_forms_index_view_object.rb | 119 ++++-------- .../application_forms/index.html.erb | 20 +-- .../assessor_interface/applications.rb | 4 +- .../filtering_application_forms_spec.rb | 16 +- ...pplication_forms_index_view_object_spec.rb | 170 ++++-------------- 6 files changed, 89 insertions(+), 242 deletions(-) diff --git a/app/forms/assessor_interface/filter_form.rb b/app/forms/assessor_interface/filter_form.rb index dedd850a85..c4c5e5b39a 100644 --- a/app/forms/assessor_interface/filter_form.rb +++ b/app/forms/assessor_interface/filter_form.rb @@ -10,7 +10,7 @@ class AssessorInterface::FilterForm :location, :name, :reference, - :statuses, + :stage, :submitted_at_after, :submitted_at_before end 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 595f6e861b..4aa7ad551a 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 @@ -39,48 +39,29 @@ def action_required_by_filter_options end end - def status_filter_options - STATUS_FILTER_OPTIONS.each_with_object({}) do |(status, substatuses), memo| - memo[status_filter_entry(status)] = substatuses.map do |sub_status| - status_filter_entry(status, sub_status) - end - end + def stage_filter_options + STAGE_FILTER_OPTIONS.map { |name| stage_filter_entry(name) } end private ACTION_REQUIRED_BY_OPTIONS = %w[admin assessor external].freeze - STATUS_FILTER_OPTIONS = { - preliminary_check: [], - submitted: [], - assessment_in_progress: [], - waiting_on: %i[ - further_information - professional_standing - qualification - reference - ], - received: %i[ - further_information - professional_standing - qualification - reference - ], - overdue: [], - awarded_pending_checks: [], - awarded: [], - declined: [], - potential_duplicate_in_dqt: [], - withdrawn: [], - }.freeze + STAGE_FILTER_OPTIONS = %w[ + pre_assessment + not_started + assessment + verification + review + completed + ].freeze def filter_params (session[:filter_params] || {}).with_indifferent_access end - def application_forms_without_action_required_by_filter - @application_forms_without_action_required_by_filter ||= + def application_forms_without_counted_filters + @application_forms_without_counted_filters ||= begin filters = [ ::Filters::Assessor, @@ -96,19 +77,15 @@ def application_forms_without_action_required_by_filter end end - def application_forms_without_status_filter - @application_forms_without_status_filter ||= - ::Filters::ActionRequiredBy.apply( - scope: application_forms_without_action_required_by_filter, - params: filter_params, - ) - end - def application_forms_with_pagy @application_forms_with_pagy ||= pagy( - ::Filters::Status.apply( - scope: application_forms_without_status_filter, + ::Filters::Stage.apply( + scope: + ::Filters::ActionRequiredBy.apply( + scope: application_forms_without_counted_filters, + params: filter_params, + ), params: filter_params, ).order(submitted_at: :asc), ) @@ -116,9 +93,13 @@ def application_forms_with_pagy def action_required_by_filter_counts @action_required_by_filter_counts ||= - application_forms_without_action_required_by_filter.group( - :action_required_by, - ).count + ::Filters::Stage + .apply( + scope: application_forms_without_counted_filters, + params: filter_params, + ) + .group(:action_required_by) + .count end def action_required_by_filter_entry(name) @@ -129,52 +110,28 @@ def action_required_by_filter_entry(name) ) end - def status_filter_counts - @status_filter_counts ||= - begin - all_options = - STATUS_FILTER_OPTIONS.each_with_object( - {}, - ) do |(status, substatuses), memo| - memo[status.to_s] = nil - - substatuses.each do |substatus| - memo["#{status}_#{substatus}"] = status.to_s - end - end - - table = ApplicationForm.arel_table - - counts = - all_options.filter_map do |status, parent_status| - if parent_status.present? - Arel.star.count.filter( - table[:status].eq(parent_status).and(table[status].eq(true)), - ) - else - Arel.star.count.filter(table[:status].eq(status)) - end - end - - results = application_forms_without_status_filter.pick(*counts) - - Hash[all_options.keys.zip(results)] - end + def stage_filter_counts + @stage_filter_counts ||= + ::Filters::ActionRequiredBy + .apply( + scope: application_forms_without_counted_filters, + params: filter_params, + ) + .group(:stage) + .count end - def status_filter_entry(*status_path) - name = status_path.join("_") - + def stage_filter_entry(name) readable_name = I18n.t( - status_path.last, + name, scope: %i[components status_tag], - default: status_path.last.to_s.humanize, + default: name.to_s.humanize, ) OpenStruct.new( id: name, - label: "#{readable_name} (#{status_filter_counts.fetch(name, 0)})", + label: "#{readable_name} (#{stage_filter_counts.fetch(name, 0)})", ) end diff --git a/app/views/assessor_interface/application_forms/index.html.erb b/app/views/assessor_interface/application_forms/index.html.erb index 8034d3d1da..52a5b2ad48 100644 --- a/app/views/assessor_interface/application_forms/index.html.erb +++ b/app/views/assessor_interface/application_forms/index.html.erb @@ -56,22 +56,10 @@ <% end %> -
- <%= f.govuk_check_boxes_fieldset :statuses, legend: { size: "s" }, small: true do %> - <% @view_object.status_filter_options.each do |option, suboptions| %> - <% if suboptions.present? %> -
- <%= option.label %> -
- -
- <% suboptions.each do |option| %> - <%= f.govuk_check_box :statuses, option.id, label: { text: option.label }, checked: @view_object.filter_form.statuses&.include?(option.id) %> - <% end %> -
- <% else %> - <%= f.govuk_check_box :statuses, option.id, label: { text: option.label }, checked: @view_object.filter_form.statuses&.include?(option.id) %> - <% end %> +
+ <%= f.govuk_check_boxes_fieldset :stage, legend: { size: "s" }, small: true do %> + <% @view_object.stage_filter_options.each do |option| %> + <%= f.govuk_check_box :stage, option.id, label: { text: option.label }, checked: @view_object.filter_form.stage&.include?(option.id) %> <% end %> <% end %>
diff --git a/spec/support/autoload/page_objects/assessor_interface/applications.rb b/spec/support/autoload/page_objects/assessor_interface/applications.rb index a45c7b8491..c10461e36f 100644 --- a/spec/support/autoload/page_objects/assessor_interface/applications.rb +++ b/spec/support/autoload/page_objects/assessor_interface/applications.rb @@ -45,8 +45,8 @@ class Applications < SitePrism::Page sections :items, GovukCheckboxItem, ".govuk-checkboxes__item" end - section :status_filter, "#app-applications-filters-status" do - sections :statuses, GovukCheckboxItem, ".govuk-checkboxes__item" + section :stage_filter, "#app-applications-filters-stage" do + sections :items, GovukCheckboxItem, ".govuk-checkboxes__item" end sections :search_results, ".app-search-results__item" do diff --git a/spec/system/assessor_interface/filtering_application_forms_spec.rb b/spec/system/assessor_interface/filtering_application_forms_spec.rb index 2710466191..ae39a8af5f 100644 --- a/spec/system/assessor_interface/filtering_application_forms_spec.rb +++ b/spec/system/assessor_interface/filtering_application_forms_spec.rb @@ -41,8 +41,8 @@ then_i_see_a_list_of_applications_filtered_by_action_required_by when_i_clear_the_filters - and_i_apply_the_status_filter - then_i_see_a_list_of_applications_filtered_by_state + and_i_apply_the_stage_filter + then_i_see_a_list_of_applications_filtered_by_stage end private @@ -146,18 +146,18 @@ def then_i_see_a_list_of_applications_filtered_by_action_required_by ) end - def and_i_apply_the_status_filter - awarded_state = - applications_page.status_filter.statuses.find do |status| - status.label.text == "Awarded (1)" + def and_i_apply_the_stage_filter + completed_item = + applications_page.stage_filter.items.find do |item| + item.label.text == "Completed (1)" rescue Capybara::ElementNotFound false end - awarded_state.checkbox.click + completed_item.checkbox.click applications_page.apply_filters.click end - def then_i_see_a_list_of_applications_filtered_by_state + def then_i_see_a_list_of_applications_filtered_by_stage expect(applications_page.search_results.count).to eq(1) expect(applications_page.search_results.first.name.text).to eq("John Smith") end 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 d5e54faa3e..c1c8846646 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 @@ -71,9 +71,19 @@ it { is_expected.to be_empty } end - context "with a status filter" do + context "with an action required by filter" do before do - expect_any_instance_of(Filters::Status).to receive(:apply).and_return( + expect_any_instance_of(Filters::ActionRequiredBy).to receive( + :apply, + ).and_return(ApplicationForm.none) + end + + it { is_expected.to be_empty } + end + + context "with a stage filter" do + before do + expect_any_instance_of(Filters::Stage).to receive(:apply).and_return( ApplicationForm.none, ) end @@ -150,150 +160,42 @@ end end - describe "#status_filter_options" do - subject(:status_filter_options) { view_object.status_filter_options } + describe "#stage_filter_options" do + subject(:stage_filter_options) { view_object.stage_filter_options } it do is_expected.to eq( - { - OpenStruct.new( - id: "preliminary_check", - label: "Preliminary check (0)", - ) => [], - OpenStruct.new(id: "submitted", label: "Not started (0)") => [], - OpenStruct.new( - id: "assessment_in_progress", - label: "Assessment in progress (0)", - ) => [], - OpenStruct.new(id: "waiting_on", label: "Waiting on (0)") => [ - OpenStruct.new( - id: "waiting_on_further_information", - label: "Further information (0)", - ), - OpenStruct.new( - id: "waiting_on_professional_standing", - label: "Professional standing (0)", - ), - OpenStruct.new( - id: "waiting_on_qualification", - label: "Qualification (0)", - ), - OpenStruct.new(id: "waiting_on_reference", label: "Reference (0)"), - ], - OpenStruct.new(id: "received", label: "Received (0)") => [ - OpenStruct.new( - id: "received_further_information", - label: "Further information (0)", - ), - OpenStruct.new( - id: "received_professional_standing", - label: "Professional standing (0)", - ), - OpenStruct.new( - id: "received_qualification", - label: "Qualification (0)", - ), - OpenStruct.new(id: "received_reference", label: "Reference (0)"), - ], - OpenStruct.new(id: "overdue", label: "Overdue (0)") => [], - OpenStruct.new( - id: "awarded_pending_checks", - label: "Award pending (0)", - ) => [], - OpenStruct.new(id: "awarded", label: "Awarded (0)") => [], - OpenStruct.new(id: "declined", label: "Declined (0)") => [], - OpenStruct.new( - id: "potential_duplicate_in_dqt", - label: "Potential duplication in DQT (0)", - ) => [], - OpenStruct.new(id: "withdrawn", label: "Withdrawn (0)") => [], - }, + [ + OpenStruct.new(id: "pre_assessment", label: "Pre-assessment (0)"), + OpenStruct.new(id: "not_started", label: "Not started (0)"), + OpenStruct.new(id: "assessment", label: "Assessment (0)"), + OpenStruct.new(id: "verification", label: "Verification (0)"), + OpenStruct.new(id: "review", label: "Review (0)"), + OpenStruct.new(id: "completed", label: "Completed (0)"), + ], ) end context "with application forms" do before do - create_list(:application_form, 1, :preliminary_check) - create_list(:application_form, 2, :submitted) - create_list(:application_form, 3, :assessment_in_progress) - create_list( - :application_form, - 4, - :waiting_on, - waiting_on_further_information: true, - ) - create_list( - :application_form, - 5, - :received, - received_professional_standing: true, - ) - create_list(:application_form, 6, :overdue, overdue_qualification: true) - create_list(:application_form, 7, :awarded_pending_checks) - create_list(:application_form, 8, :awarded) - create_list(:application_form, 9, :declined) - create_list(:application_form, 10, :potential_duplicate_in_dqt) - create_list(:application_form, 11, :withdrawn) + create_list(:application_form, 1, :submitted, :pre_assessment_stage) + create_list(:application_form, 2, :submitted, :not_started_stage) + create_list(:application_form, 3, :submitted, :assessment_stage) + create_list(:application_form, 4, :submitted, :verification_stage) + create_list(:application_form, 5, :submitted, :review_stage) + create_list(:application_form, 6, :submitted, :completed_stage) end it do is_expected.to eq( - { - OpenStruct.new( - id: "preliminary_check", - label: "Preliminary check (1)", - ) => [], - OpenStruct.new(id: "submitted", label: "Not started (2)") => [], - OpenStruct.new( - id: "assessment_in_progress", - label: "Assessment in progress (3)", - ) => [], - OpenStruct.new(id: "waiting_on", label: "Waiting on (4)") => [ - OpenStruct.new( - id: "waiting_on_further_information", - label: "Further information (4)", - ), - OpenStruct.new( - id: "waiting_on_professional_standing", - label: "Professional standing (0)", - ), - OpenStruct.new( - id: "waiting_on_qualification", - label: "Qualification (0)", - ), - OpenStruct.new( - id: "waiting_on_reference", - label: "Reference (0)", - ), - ], - OpenStruct.new(id: "received", label: "Received (5)") => [ - OpenStruct.new( - id: "received_further_information", - label: "Further information (0)", - ), - OpenStruct.new( - id: "received_professional_standing", - label: "Professional standing (5)", - ), - OpenStruct.new( - id: "received_qualification", - label: "Qualification (0)", - ), - OpenStruct.new(id: "received_reference", label: "Reference (0)"), - ], - OpenStruct.new(id: "overdue", label: "Overdue (6)") => [], - OpenStruct.new( - id: "awarded_pending_checks", - label: "Award pending (7)", - ) => [], - OpenStruct.new(id: "awarded", label: "Awarded (8)") => [], - OpenStruct.new(id: "declined", label: "Declined (9)") => [], - OpenStruct.new( - id: "potential_duplicate_in_dqt", - label: "Potential duplication in DQT (10)", - ) => [], - OpenStruct.new(id: "withdrawn", label: "Withdrawn (11)") => [], - }, + [ + OpenStruct.new(id: "pre_assessment", label: "Pre-assessment (1)"), + OpenStruct.new(id: "not_started", label: "Not started (2)"), + OpenStruct.new(id: "assessment", label: "Assessment (3)"), + OpenStruct.new(id: "verification", label: "Verification (4)"), + OpenStruct.new(id: "review", label: "Review (5)"), + OpenStruct.new(id: "completed", label: "Completed (6)"), + ], ) end end