From 549e1f130f526f1fe3175c4051bb04386691f757 Mon Sep 17 00:00:00 2001 From: Vasili Kachalko Date: Wed, 25 Sep 2024 12:07:00 +0200 Subject: [PATCH] fix spes for bulk assignment and filtering --- .../admin/form_answers_controller.rb | 3 +- app/controllers/application_controller.rb | 2 +- .../assessor/form_answers_controller.rb | 1 + app/controllers/concerns/form_answer_mixin.rb | 1 + .../lieutenant/form_answers_controller.rb | 1 + .../eligibility_assignment_collection.rb | 2 +- .../bulk_assign_eligibility.html.slim | 2 +- .../_vertical_admin_award_year.html.slim | 2 +- config/webpack/base.js | 5 - .../bulk_assessors_assignment_spec.rb | 2 +- .../bulk_eligibility_assignment_spec.rb | 40 +++++ .../bulk_lieutenants_assignment_spec.rb | 7 +- .../nominations_bulk_action_form_spec.rb | 142 +++++++++++++++++- 13 files changed, 193 insertions(+), 17 deletions(-) create mode 100644 spec/features/admin/form_answers/bulk_eligibility_assignment_spec.rb diff --git a/app/controllers/admin/form_answers_controller.rb b/app/controllers/admin/form_answers_controller.rb index 60269cce8..5579ad44b 100644 --- a/app/controllers/admin/form_answers_controller.rb +++ b/app/controllers/admin/form_answers_controller.rb @@ -27,7 +27,7 @@ class Admin::FormAnswersController < Admin::BaseController expose(:target_scope) do if params[:year].to_s == "all_years" FormAnswer.all - elsif params[:year] + elsif params[:year].present? (year = AwardYear.find_by(year: params[:year])) ? year.form_answers : FormAnswer.none else @award_year.form_answers @@ -59,6 +59,7 @@ class Admin::FormAnswersController < Admin::BaseController def index authorize :form_answer, :index? search_params = save_or_load_search! + return if search_params.nil? # Redirected from save_or_load_search! @search = FormAnswerSearch.new(target_scope, current_admin).search(search_params) @search.ordered_by = "company_or_nominee_name" unless @search.ordered_by diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 78b5688c9..a8ad019e5 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -210,7 +210,7 @@ def require_to_be_account_admin! end def load_award_year_and_settings - if params[:year] && AwardYear::AVAILABLE_YEARS.include?(params[:year].to_i) + if params[:year].present? && AwardYear::AVAILABLE_YEARS.include?(params[:year].to_i) @award_year = AwardYear.for_year(params[:year].to_i).first_or_create else @award_year = AwardYear.current diff --git a/app/controllers/assessor/form_answers_controller.rb b/app/controllers/assessor/form_answers_controller.rb index 0639bd0da..03bff9233 100644 --- a/app/controllers/assessor/form_answers_controller.rb +++ b/app/controllers/assessor/form_answers_controller.rb @@ -33,6 +33,7 @@ class Assessor::FormAnswersController < Assessor::BaseController def index authorize :form_answer, :index? search_params = save_or_load_search! + return if search_params.nil? # Redirected from save_or_load_search! scope = current_assessor.applications_scope( params[:year].to_s == "all_years" ? nil : @award_year diff --git a/app/controllers/concerns/form_answer_mixin.rb b/app/controllers/concerns/form_answer_mixin.rb index 1be3bc11a..4ee0546ef 100644 --- a/app/controllers/concerns/form_answer_mixin.rb +++ b/app/controllers/concerns/form_answer_mixin.rb @@ -94,6 +94,7 @@ def financial_data_ops def save_or_load_search! search_params = params[:search] || default_filters + if params[:bulk_assign_lieutenants] || params[:bulk_assign_assessors] || params[:bulk_assign_eligibility] bulk_params = params.permit( diff --git a/app/controllers/lieutenant/form_answers_controller.rb b/app/controllers/lieutenant/form_answers_controller.rb index d2441cb5d..51c6068de 100644 --- a/app/controllers/lieutenant/form_answers_controller.rb +++ b/app/controllers/lieutenant/form_answers_controller.rb @@ -55,6 +55,7 @@ def index authorize :form_answer, :index? search_params = save_or_load_search! + return if search_params.nil? # Redirected from save_or_load_search! scope = current_lieutenant.nominations_scope( params[:year].to_s == "all_years" ? nil : @award_year diff --git a/app/models/eligibility_assignment_collection.rb b/app/models/eligibility_assignment_collection.rb index 7c781d821..9cc476494 100644 --- a/app/models/eligibility_assignment_collection.rb +++ b/app/models/eligibility_assignment_collection.rb @@ -18,7 +18,7 @@ def notice_message "Groups have" else "Group has" - end.concat " been assigned the #{state} status." + end.concat " been assigned the #{I18n.t(state, scope: "form_answers.state")} status." end private diff --git a/app/views/admin/form_answers/bulk_assign_eligibility.html.slim b/app/views/admin/form_answers/bulk_assign_eligibility.html.slim index e1bf5fcc4..c36cdea0d 100644 --- a/app/views/admin/form_answers/bulk_assign_eligibility.html.slim +++ b/app/views/admin/form_answers/bulk_assign_eligibility.html.slim @@ -20,5 +20,5 @@ h3.govuk-heading-m = f.hidden_field :form_answer_ids, value: @form.form_answer_ids.join(",") .govuk-button-group - = f.submit "Bulk assign eligibility status", class: "govuk-button" + = f.submit "Save", class: "govuk-button" = link_to "Cancel", [namespace_name, :form_answers, year: params[:year], search_id: params[:search_id]], class: "govuk-link" diff --git a/app/views/layouts/_vertical_admin_award_year.html.slim b/app/views/layouts/_vertical_admin_award_year.html.slim index 187052af0..d61af6269 100644 --- a/app/views/layouts/_vertical_admin_award_year.html.slim +++ b/app/views/layouts/_vertical_admin_award_year.html.slim @@ -6,7 +6,7 @@ .govuk-select.dropdown-checkboxes__selection.dropdown-toggle#select-award-year aria-expanded="false" aria-label='Select award year' span class="govuk-visually-hidden" | Award year:  - - current_year = params[:year] ? params[:year].to_i : AwardYear.current.year + - current_year = params[:year].present? ? params[:year].to_i : AwardYear.current.year - current_year_text = "#{current_year - 1} - #{current_year}" = params[:year].to_s == "all_years" ? "All Years" : current_year_text diff --git a/config/webpack/base.js b/config/webpack/base.js index 7cad4f7a0..532f44b64 100644 --- a/config/webpack/base.js +++ b/config/webpack/base.js @@ -2,11 +2,6 @@ const { webpackConfig, merge } = require('@rails/webpacker') const customConfig = { resolve: { extensions: ['.css', '.scss'] - }, - devServer: { - client: { - overlay: false, - }, } } diff --git a/spec/features/admin/form_answers/bulk_assessors_assignment_spec.rb b/spec/features/admin/form_answers/bulk_assessors_assignment_spec.rb index 3700c3740..1443c468b 100644 --- a/spec/features/admin/form_answers/bulk_assessors_assignment_spec.rb +++ b/spec/features/admin/form_answers/bulk_assessors_assignment_spec.rb @@ -25,11 +25,11 @@ first("#check_all").set(true) click_button("Bulk assign to national assessor sub-group", match: :first) - custom_select "Sub-group 9", from: "Select national assessor sub-group" click_button "Bulk assign groups to national assessor sub-group" + expect(page).to have_content("Groups have been assigned to the national assessor sub-group") expect(page).to have_content("Sub-group 9") expect(form_answer_1.reload.sub_group.text).to eq("Sub-group 9") diff --git a/spec/features/admin/form_answers/bulk_eligibility_assignment_spec.rb b/spec/features/admin/form_answers/bulk_eligibility_assignment_spec.rb new file mode 100644 index 000000000..121e9a1ae --- /dev/null +++ b/spec/features/admin/form_answers/bulk_eligibility_assignment_spec.rb @@ -0,0 +1,40 @@ +require "rails_helper" +include Warden::Test::Helpers + +Warden.test_mode! + +describe "Admin assigns assessors", %( + As Admin + I want to be able to assign national assessor sub-gruops +) do + + let(:subject) { create(:admin) } + let!(:form_answer_1) { create(:form_answer, :submitted) } + let!(:form_answer_2) { create(:form_answer, :submitted) } + let!(:award_year) { AwardYear.current } + let!(:settings) { create(:settings, :expired_submission_deadlines) } + + + describe "Form submission" do + before do + login_as(subject, scope: :admin) + visit admin_form_answers_path + end + + it "assigns eligibility" do + all("#bulk_action_ids_").each { |i| i.set(true) } + + click_button("Bulk assign eligibility status", match: :first) + + save_and_open_page + choose("eligibility_assignment_collection_state_admin_eligible") + + click_button "Save" + + expect(page).to have_content("Groups have been assigned the Eligible by admin status") + + expect(form_answer_1.reload.state).to eq("admin_eligible") + expect(form_answer_2.reload.state).to eq("admin_eligible") + end + end +end diff --git a/spec/features/admin/form_answers/bulk_lieutenants_assignment_spec.rb b/spec/features/admin/form_answers/bulk_lieutenants_assignment_spec.rb index 72b4c9b39..39956412b 100644 --- a/spec/features/admin/form_answers/bulk_lieutenants_assignment_spec.rb +++ b/spec/features/admin/form_answers/bulk_lieutenants_assignment_spec.rb @@ -26,13 +26,12 @@ it "assigns the lieutenant" do first("#check_all").set(true) click_button("Bulk assign to Lord Lieutenancy office", match: :first) - find(:css, "#modal-bulk-assign-lieutenants").should be_visible custom_select ceremonial_county_2.name, from: "Select Lord Lieutenancy office" - within "#modal-bulk-assign-lieutenants" do - click_button "Bulk assign groups to Lord Lieutenancy office" - end + click_button "Bulk assign groups to Lord Lieutenancy office" + + expect(page).to have_content("Groups have been assigned to the Lord Lieutenancy office") expect(form_answer_1.reload.ceremonial_county.name).to eq("B") expect(form_answer_2.reload.ceremonial_county.name).to eq("B") diff --git a/spec/forms/nominations_bulk_action_form_spec.rb b/spec/forms/nominations_bulk_action_form_spec.rb index e8a07208f..76bb8ea1c 100644 --- a/spec/forms/nominations_bulk_action_form_spec.rb +++ b/spec/forms/nominations_bulk_action_form_spec.rb @@ -1,9 +1,147 @@ require "rails_helper" describe NominationsBulkActionForm do + include Rails.application.routes.url_helpers + + let(:params) { {} } + subject { described_class.new(params) } + describe "#valid?" do - it "fails" do - expect(true).to be_falsey + context "when bulk_action is not present" do + it "adds a bulk_base error and returns false" do + expect(subject.valid?).to be_falsey + expect(subject.errors[:bulk_base]).to include("You must select at least one group from the list below before clicking a bulk action button.") + end + end + + context "when kind is lieutenants" do + let(:params) { { bulk_assign_lieutenants: true, bulk_action: { ids: [1, 2, 3] } } } + + it "returns true if bulk_action is present" do + expect(subject.valid?).to be_truthy + end + end + + context "when kind is assessors" do + let(:params) { { bulk_assign_assessors: true, bulk_action: { ids: [1, 2, 3] } } } + + it "returns true if bulk_action is present" do + expect(subject.valid?).to be_truthy + end + end + + context "when kind is eligibility" do + let(:params) { { bulk_assign_eligibility: true, bulk_action: { ids: [1, 2, 3] } } } + let(:valid_form_answer) { double(:form_answer, state: "submitted") } + + before do + allow(FormAnswer).to receive(:where).with(id: params[:bulk_action][:ids]).and_return([valid_form_answer]) + end + + it "returns true if form_answers are eligible for state change" do + expect(subject.valid?).to be_truthy + end + + it "returns false if form_answers are not eligible for state change" do + invalid_form_answer = double(:form_answer, state: "Invalid state") + allow(FormAnswer).to receive(:where).with(id: params[:bulk_action][:ids]).and_return([invalid_form_answer]) + + expect(subject.valid?).to be_falsey + expect(subject.errors[:bulk_base]).to include("To assign the eligibility status, you must only select groups that currently have ‘Nomination submitted’ or ‘Eligibility pending’ status.") + end + end + end + + describe "#base_error_messages" do + it "returns joined base error messages" do + subject.errors.add(:bulk_base, "Some error message") + expect(subject.base_error_messages).to eq("Some error message") + end + end + + describe "#redirect_url" do + context "when kind is lieutenants" do + let(:params) { { bulk_assign_lieutenants: true, bulk_action: { ids: [1, 2, 3] } } } + + it "returns the lieutenants redirect path" do + expect(subject.redirect_url).to eq(bulk_assign_lieutenants_admin_form_answers_path(params: params)) + end + end + + context "when kind is assessors" do + let(:params) { { bulk_assign_assessors: true, bulk_action: { ids: [1, 2, 3] } } } + + it "returns the assessors redirect path" do + expect(subject.redirect_url).to eq(bulk_assign_assessors_admin_form_answers_path(params: params)) + end + end + + context "when kind is eligibility" do + let(:params) { { bulk_assign_eligibility: true, bulk_action: { ids: [1, 2, 3] } } } + + it "returns the eligibility redirect path" do + expect(subject.redirect_url).to eq(bulk_assign_eligibility_admin_form_answers_path(params: params)) + end + end + + context "when kind is invalid" do + let(:params) { { bulk_action: { ids: [1, 2, 3] } } } + + it "raises an error" do + expect { subject.redirect_url }.to raise_error("Invalid bulk action type.") + end + end + end + + describe "#determine_kind" do + context "when bulk_assign_lieutenants is present" do + let(:params) { { bulk_assign_lieutenants: true } } + + it "returns 'lieutenants'" do + expect(subject.send(:determine_kind)).to eq("lieutenants") + end + end + + context "when bulk_assign_assessors is present" do + let(:params) { { bulk_assign_assessors: true } } + + it "returns 'assessors'" do + expect(subject.send(:determine_kind)).to eq("assessors") + end + end + + context "when bulk_assign_eligibility is present" do + let(:params) { { bulk_assign_eligibility: true } } + + it "returns 'eligibility'" do + expect(subject.send(:determine_kind)).to eq("eligibility") + end + end + end + + describe "#save_search_and_clean_params" do + context "when search params are present and need to be saved" do + let(:params) { { search: { search_filter: "custom_filter" } } } + let(:nomination_search) { double(:nomination_search, id: 1) } + + before do + allow(NominationSearch).to receive(:create).and_return(nomination_search) + end + + it "saves the search and removes unnecessary params" do + cleaned_params = subject.send(:save_search_and_clean_params, params) + expect(cleaned_params[:search_id]).to eq(1) + expect(cleaned_params[:search]).to be_nil + end + end + + context "when search params are the default search filter" do + let(:params) { { search: { search_filter: FormAnswerSearch.default_search[:search_filter] } } } + + it "does not save search and returns params unchanged" do + cleaned_params = subject.send(:save_search_and_clean_params, params) + expect(cleaned_params).to eq(params) + end end end end