diff --git a/app/components/timeline_entry/component.rb b/app/components/timeline_entry/component.rb index 93af8c2310..d1870d1cd1 100644 --- a/app/components/timeline_entry/component.rb +++ b/app/components/timeline_entry/component.rb @@ -159,5 +159,16 @@ def information_changed_vars new_value: timeline_event.new_value, } end + + def action_required_by_changed_vars + { + action: + if timeline_event.new_value == "none" + "no" + else + timeline_event.new_value + end, + } + end end end diff --git a/app/forms/assessor_interface/filter_form.rb b/app/forms/assessor_interface/filter_form.rb index 9be8172860..dedd850a85 100644 --- a/app/forms/assessor_interface/filter_form.rb +++ b/app/forms/assessor_interface/filter_form.rb @@ -4,12 +4,13 @@ class AssessorInterface::FilterForm include ActiveModel::Model include ActiveRecord::AttributeAssignment - attr_accessor :assessor_ids, + attr_accessor :action_required_by, + :assessor_ids, + :email, :location, :name, :reference, - :email, :statuses, - :submitted_at_before, - :submitted_at_after + :submitted_at_after, + :submitted_at_before end diff --git a/app/lib/application_form_status_updater.rb b/app/lib/application_form_status_updater.rb index 0a5299be8e..30b061175f 100644 --- a/app/lib/application_form_status_updater.rb +++ b/app/lib/application_form_status_updater.rb @@ -9,8 +9,6 @@ def initialize(application_form:, user:) end def call - old_status = application_form.status - ActiveRecord::Base.transaction do application_form.update!( overdue_further_information:, @@ -27,10 +25,24 @@ def call waiting_on_reference:, ) - next if old_status == new_status + if (old_status = application_form.status) != status + application_form.update!(status:) + create_timeline_event( + event_type: "state_changed", + old_state: old_status, + new_state: status, + ) + end - application_form.update!(status: new_status) - create_timeline_event(old_state: old_status, new_state: new_status) + if (old_action_required_by = application_form.action_required_by) != + action_required_by + application_form.update!(action_required_by:) + create_timeline_event( + event_type: "action_required_by_changed", + old_value: old_action_required_by, + new_value: action_required_by, + ) + end end end @@ -117,8 +129,8 @@ def waiting_on_reference waiting_on?(requestables: reference_requests) end - def new_status - @new_status ||= + def status + @status ||= if dqt_trn_request&.potential_duplicate? "potential_duplicate_in_dqt" elsif application_form.withdrawn_at.present? @@ -150,6 +162,26 @@ def new_status end end + def action_required_by + @action_required_by ||= + if status == "preliminary_check" + "admin" + elsif %w[ + potential_duplicate_in_dqt + awarded_pending_checks + overdue + received + assessment_in_progress + submitted + ].include?(status) + "assessor" + elsif status == "waiting_on" + "external" + else + "none" + end + end + delegate :assessment, :dqt_trn_request, :region, @@ -198,17 +230,16 @@ def received?(requestables:) requestables.reject(&:reviewed?).any?(&:received?) end - def create_timeline_event(old_state:, new_state:) + def create_timeline_event(event_type:, **kwargs) creator = user.is_a?(String) ? nil : user creator_name = user.is_a?(String) ? user : "" TimelineEvent.create!( application_form:, - event_type: "state_changed", + event_type:, creator:, creator_name:, - new_state:, - old_state:, + **kwargs, ) end end diff --git a/app/lib/filters/action_required_by.rb b/app/lib/filters/action_required_by.rb new file mode 100644 index 0000000000..c0a8163c6b --- /dev/null +++ b/app/lib/filters/action_required_by.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class Filters::ActionRequiredBy < Filters::Base + def apply + return scope if action_required_by.empty? + + scope.where(action_required_by:) + end + + def action_required_by + Array(params[:action_required_by]).compact_blank + end +end diff --git a/app/models/application_form.rb b/app/models/application_form.rb index ed77f0c8a3..f454daf7d1 100644 --- a/app/models/application_form.rb +++ b/app/models/application_form.rb @@ -3,6 +3,7 @@ # Table name: application_forms # # id :bigint not null, primary key +# action_required_by :string default("none"), not null # age_range_max :integer # age_range_min :integer # age_range_status :string default("not_started"), not null @@ -67,6 +68,7 @@ # # Indexes # +# index_application_forms_on_action_required_by (action_required_by) # index_application_forms_on_assessor_id (assessor_id) # index_application_forms_on_english_language_provider_id (english_language_provider_id) # index_application_forms_on_family_name (family_name) @@ -122,6 +124,14 @@ class ApplicationForm < ApplicationRecord absence: true, if: :english_language_provider_other + enum action_required_by: { + admin: "admin", + assessor: "assessor", + external: "external", + none: "none", + }, + _prefix: true + enum status: { draft: "draft", submitted: "submitted", diff --git a/app/models/timeline_event.rb b/app/models/timeline_event.rb index 1eb1fa7e67..c41481fde2 100644 --- a/app/models/timeline_event.rb +++ b/app/models/timeline_event.rb @@ -54,14 +54,16 @@ # class TimelineEvent < ApplicationRecord belongs_to :application_form + belongs_to :assessment, optional: true + belongs_to :assessment_section, optional: true + belongs_to :assignee, class_name: "Staff", optional: true belongs_to :creator, polymorphic: true, optional: true - - validates :creator, presence: true, unless: -> { creator_name.present? } - validates :creator_name, - presence: true, - unless: -> { creator_id.present? && creator_type.present? } + belongs_to :note, optional: true + belongs_to :requestable, polymorphic: true, optional: true + belongs_to :work_history, optional: true enum event_type: { + action_required_by_changed: "action_required_by_changed", age_range_subjects_verified: "age_range_subjects_verified", assessment_section_recorded: "assessment_section_recorded", assessor_assigned: "assessor_assigned", @@ -75,9 +77,14 @@ class TimelineEvent < ApplicationRecord reviewer_assigned: "reviewer_assigned", state_changed: "state_changed", } + + validates :creator, presence: true, unless: -> { creator_name.present? } + validates :creator_name, + presence: true, + unless: -> { creator_id.present? && creator_type.present? } + validates :event_type, inclusion: { in: event_types.values } - belongs_to :assignee, class_name: "Staff", optional: true validates :assignee, absence: true, unless: -> { assessor_assigned? || reviewer_assigned? } @@ -91,7 +98,6 @@ class TimelineEvent < ApplicationRecord absence: true, unless: -> { state_changed? || assessment_section_recorded? } - belongs_to :assessment_section, optional: true validates :assessment_section, presence: true, if: :assessment_section_recorded? @@ -99,7 +105,6 @@ class TimelineEvent < ApplicationRecord absence: true, unless: :assessment_section_recorded? - belongs_to :note, optional: true validates :note, presence: true, if: :note_created? validates :note, absence: true, unless: :note_created? @@ -114,7 +119,6 @@ class TimelineEvent < ApplicationRecord absence: true, unless: :email_sent? - belongs_to :assessment, optional: true validates :assessment, :age_range_min, :age_range_max, @@ -130,7 +134,6 @@ class TimelineEvent < ApplicationRecord absence: true, unless: :age_range_subjects_verified? - belongs_to :requestable, polymorphic: true, optional: true validates :requestable_id, presence: true, if: :requestable_event_type? validates :requestable_type, presence: true, @@ -146,16 +149,17 @@ class TimelineEvent < ApplicationRecord absence: true, unless: :requestable_event_type? - belongs_to :work_history, optional: true - validates :column_name, - :old_value, + validates :old_value, :new_value, presence: true, - if: :information_changed? + if: -> { action_required_by_changed? || information_changed? } + validates :old_value, + :new_value, + absence: true, + unless: -> { action_required_by_changed? || information_changed? } + validates :column_name, presence: true, if: :information_changed? validates :work_history_id, :column_name, - :old_value, - :new_value, absence: true, unless: :information_changed? 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 f2fba4092f..595f6e861b 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 @@ -33,6 +33,12 @@ def country_filter_options ) end + def action_required_by_filter_options + ACTION_REQUIRED_BY_OPTIONS.map do |name| + action_required_by_filter_entry(name) + 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| @@ -43,6 +49,8 @@ def status_filter_options private + ACTION_REQUIRED_BY_OPTIONS = %w[admin assessor external].freeze + STATUS_FILTER_OPTIONS = { preliminary_check: [], submitted: [], @@ -71,18 +79,8 @@ def filter_params (session[:filter_params] || {}).with_indifferent_access end - def application_forms_with_pagy - @application_forms_with_pagy ||= - pagy( - ::Filters::Status.apply( - scope: application_forms_without_status_filter, - params: filter_params, - ).order(submitted_at: :asc), - ) - end - - def application_forms_without_status_filter - @application_forms_without_status_filter ||= + def application_forms_without_action_required_by_filter + @application_forms_without_action_required_by_filter ||= begin filters = [ ::Filters::Assessor, @@ -98,6 +96,39 @@ def application_forms_without_status_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, + params: filter_params, + ).order(submitted_at: :asc), + ) + end + + def action_required_by_filter_counts + @action_required_by_filter_counts ||= + application_forms_without_action_required_by_filter.group( + :action_required_by, + ).count + end + + def action_required_by_filter_entry(name) + OpenStruct.new( + id: name, + label: + "#{name.humanize} (#{action_required_by_filter_counts.fetch(name, 0)})", + ) + end + def status_filter_counts @status_filter_counts ||= begin diff --git a/app/views/assessor_interface/application_forms/index.html.erb b/app/views/assessor_interface/application_forms/index.html.erb index 10b918ed3c..8034d3d1da 100644 --- a/app/views/assessor_interface/application_forms/index.html.erb +++ b/app/views/assessor_interface/application_forms/index.html.erb @@ -17,10 +17,10 @@
<%= f.govuk_check_boxes_fieldset :assessor_ids, legend: { size: "s" }, small: true do %> - <%- @view_object.assessor_filter_options.each do |option| -%> + <% @view_object.assessor_filter_options.each do |option| %> <%= f.govuk_check_box :assessor_ids, option.id, label: { text: option.name }, checked: @view_object.filter_form.assessor_ids&.include?(option.id.to_s) %> - <%- end -%> - <%- end -%> + <% end %> + <% end %>
@@ -48,6 +48,14 @@ <% end %> +
+ <%= f.govuk_check_boxes_fieldset :action_required_by, legend: { size: "s" }, small: true do %> + <% @view_object.action_required_by_filter_options.each do |option| %> + <%= f.govuk_check_box :action_required_by, option.id, label: { text: option.label }, checked: @view_object.filter_form.action_required_by&.include?(option.id) %> + <% end %> + <% end %> +
+
<%= f.govuk_check_boxes_fieldset :statuses, legend: { size: "s" }, small: true do %> <% @view_object.status_filter_options.each do |option, suboptions| %> @@ -67,7 +75,7 @@ <% end %> <% end %>
- <%- end -%> + <% end %>
diff --git a/config/analytics.yml b/config/analytics.yml index b93764bc7a..f0c015e04d 100644 --- a/config/analytics.yml +++ b/config/analytics.yml @@ -22,68 +22,69 @@ - blob_id - created_at :application_forms: + - action_required_by + - age_range_max + - age_range_min + - age_range_status + - alternative_family_name + - alternative_given_names + - assessor_id + - awarded_at + - confirmed_no_sanctions + - created_at + - date_of_birth + - declined_at + - dqt_match + - english_language_citizenship_exempt + - english_language_proof_method + - english_language_provider_id + - english_language_provider_other + - english_language_provider_reference + - english_language_qualification_exempt + - english_language_status + - family_name + - given_names + - has_alternative_name + - has_work_history - id - - reference - - status + - identification_document_status + - needs_registration_number + - needs_work_history + - needs_written_statement - overdue_further_information - overdue_professional_standing - overdue_qualification - overdue_reference + - personal_information_status + - qualifications_status - received_further_information - received_professional_standing - received_qualification - received_reference - - waiting_on_further_information - - waiting_on_professional_standing - - waiting_on_qualification - - waiting_on_reference - - personal_information_status - - identification_document_status - - qualifications_status - - age_range_status - - subjects_status - - work_history_status - - registration_number_status - - written_statement_status - - teacher_id - - created_at - - updated_at - - given_names - - family_name - - date_of_birth - - age_range_min - - age_range_max - - has_alternative_name - - alternative_given_names - - alternative_family_name + - reduced_evidence_accepted + - reference - region_id - registration_number - - has_work_history - - subjects - - assessor_id + - registration_number_status + - requires_preliminary_check - reviewer_id + - status + - subjects + - subjects_status - submitted_at - - awarded_at - - declined_at + - teacher_id + - teaching_authority_provides_written_statement + - updated_at + - waiting_on_further_information + - waiting_on_professional_standing + - waiting_on_qualification + - waiting_on_reference - withdrawn_at + - work_history_status - working_days_since_submission - - needs_work_history - - needs_written_statement - - needs_registration_number - - teaching_authority_provides_written_statement - written_statement_confirmation - written_statement_optional - - confirmed_no_sanctions - - english_language_status - - english_language_citizenship_exempt - - english_language_qualification_exempt - - english_language_proof_method - - english_language_provider_id - - english_language_provider_other - - english_language_provider_reference - - reduced_evidence_accepted - - requires_preliminary_check - - dqt_match + - written_statement_status :assessment_sections: - id - assessment_id diff --git a/config/locales/components.en.yml b/config/locales/components.en.yml index 7edb25dcf5..399f9249f9 100644 --- a/config/locales/components.en.yml +++ b/config/locales/components.en.yml @@ -38,6 +38,7 @@ en: timeline_entry: title: + action_required_by_changed: Action required by changed assessor_assigned: Assessor assigned reviewer_assigned: Reviewer assigned state_changed: Status changed @@ -68,9 +69,10 @@ en: ReferenceRequest: Reference assessed information_changed: Information changed after submission description: + action_required_by_changed: Application requires %{action} action. assessor_assigned: "%{assignee_name} is assigned as the assessor." reviewer_assigned: "%{assignee_name} is assigned as the reviewer." - state_changed: Status changed from %{old_state} to %{new_state} + state_changed: Status changed from %{old_state} to %{new_state}. note_created: "%{text}" email_sent: "%{subject}" requestable_requested: diff --git a/config/locales/helpers.en.yml b/config/locales/helpers.en.yml index 16f13346c2..d39d438eb2 100644 --- a/config/locales/helpers.en.yml +++ b/config/locales/helpers.en.yml @@ -101,10 +101,10 @@ en: assessor_interface_create_note_form: text: Add a note to this application assessor_interface_filter_form: + email: Applicant email location: Country trained in name: Applicant name reference: Application reference number - email: Applicant email assessor_interface_professional_standing_request_location_form: received_options: true: "Yes" @@ -277,6 +277,7 @@ en: scotland_full_registration: Does the applicant have or are they eligible for full registration? school_details_cannot_be_verified_work_history_checked: Choose the schools that need reference details to be amended assessor_interface_filter_form: + action_required_by: Action required by assessor_ids: Assessor name states: Status of application submitted_at: Created date diff --git a/db/migrate/20230906085009_add_action_required_by_to_application_forms.rb b/db/migrate/20230906085009_add_action_required_by_to_application_forms.rb new file mode 100644 index 0000000000..529769b1fa --- /dev/null +++ b/db/migrate/20230906085009_add_action_required_by_to_application_forms.rb @@ -0,0 +1,10 @@ +class AddActionRequiredByToApplicationForms < ActiveRecord::Migration[7.0] + def change + add_column :application_forms, + :action_required_by, + :string, + default: "none", + null: false + add_index :application_forms, :action_required_by + end +end diff --git a/db/schema.rb b/db/schema.rb index 378fdc410d..356d3d0673 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -105,6 +105,8 @@ t.boolean "overdue_reference", default: false, null: false t.jsonb "dqt_match", default: {} t.datetime "withdrawn_at" + t.string "action_required_by", default: "none", null: false + t.index ["action_required_by"], name: "index_application_forms_on_action_required_by" t.index ["assessor_id"], name: "index_application_forms_on_assessor_id" t.index ["english_language_provider_id"], name: "index_application_forms_on_english_language_provider_id" t.index ["family_name"], name: "index_application_forms_on_family_name" diff --git a/lib/tasks/application_forms.rake b/lib/tasks/application_forms.rake index e2142f060a..26448c1aab 100644 --- a/lib/tasks/application_forms.rake +++ b/lib/tasks/application_forms.rake @@ -23,4 +23,15 @@ namespace :application_forms do count = BackfillPreliminaryChecks.call(user:) puts "Updated #{count} applications." end + + desc "Update the statuses of all application forms." + task :update_statuses, %i[staff_email] => :environment do |_task, args| + user = Staff.find_by!(email: args[:staff_email]) + ApplicationForm + .order(:id) + .find_each do |application_form| + ApplicationFormStatusUpdater.call(application_form:, user:) + puts "#{application_form.reference}: #{application_form.action_required_by} - #{application_form.status}" + end + end end diff --git a/spec/components/timeline_entry_spec.rb b/spec/components/timeline_entry_spec.rb index 093c9344ab..5b8d9692da 100644 --- a/spec/components/timeline_entry_spec.rb +++ b/spec/components/timeline_entry_spec.rb @@ -504,4 +504,18 @@ expect(component.text).to include(creator.name) end end + + context "action required by changed" do + let(:timeline_event) do + create(:timeline_event, :action_required_by_changed, new_value: "none") + end + + it "describes the event" do + expect(component.text.squish).to include("Application requires no action") + end + + it "attributes to the creator" do + expect(component.text).to include(creator.name) + end + end end diff --git a/spec/factories/application_forms.rb b/spec/factories/application_forms.rb index 81660d7f1d..31f30f0b19 100644 --- a/spec/factories/application_forms.rb +++ b/spec/factories/application_forms.rb @@ -3,6 +3,7 @@ # Table name: application_forms # # id :bigint not null, primary key +# action_required_by :string default("none"), not null # age_range_max :integer # age_range_min :integer # age_range_status :string default("not_started"), not null @@ -67,6 +68,7 @@ # # Indexes # +# index_application_forms_on_action_required_by (action_required_by) # index_application_forms_on_assessor_id (assessor_id) # index_application_forms_on_english_language_provider_id (english_language_provider_id) # index_application_forms_on_family_name (family_name) @@ -122,7 +124,24 @@ written_statement_status { "completed" } end + trait :action_required_by_admin do + action_required_by { "admin" } + end + + trait :action_required_by_assessor do + action_required_by { "assessor" } + end + + trait :action_required_by_external do + action_required_by { "external" } + end + + trait :action_required_by_none do + action_required_by { "none" } + end + trait :submitted do + action_required_by_assessor status { "submitted" } submitted_at { Time.zone.now } working_days_since_submission { 0 } @@ -141,6 +160,7 @@ trait :preliminary_check do submitted + action_required_by_admin requires_preliminary_check { true } status { "preliminary_check" } end @@ -152,6 +172,7 @@ trait :waiting_on do submitted + action_required_by_external status { "waiting_on" } end @@ -170,25 +191,28 @@ status { "awarded_pending_checks" } end + trait :potential_duplicate_in_dqt do + submitted + status { "potential_duplicate_in_dqt" } + end + trait :awarded do submitted + action_required_by_none status { "awarded" } awarded_at { Time.zone.now } end trait :declined do submitted + action_required_by_none status { "declined" } declined_at { Time.zone.now } end - trait :potential_duplicate_in_dqt do - submitted - status { "potential_duplicate_in_dqt" } - end - trait :withdrawn do submitted + action_required_by_none status { "withdrawn" } withdrawn_at { Time.zone.now } end diff --git a/spec/factories/timeline_events.rb b/spec/factories/timeline_events.rb index b201c87725..d9a2af715f 100644 --- a/spec/factories/timeline_events.rb +++ b/spec/factories/timeline_events.rb @@ -105,5 +105,11 @@ old_value { Faker::Internet.email } new_value { Faker::Internet.email } end + + trait :action_required_by_changed do + event_type { "action_required_by_changed" } + old_value { %w[admin assessor external none].sample } + new_value { %w[admin assessor external none].sample } + end end end diff --git a/spec/lib/application_form_status_updater_spec.rb b/spec/lib/application_form_status_updater_spec.rb index cd9aba538d..45e05b8a41 100644 --- a/spec/lib/application_form_status_updater_spec.rb +++ b/spec/lib/application_form_status_updater_spec.rb @@ -6,8 +6,42 @@ let(:application_form) { create(:application_form) } let(:user) { create(:staff) } + shared_examples "changes action required by" do |new_action_required_by| + it "changes action required by to #{new_action_required_by}" do + expect { call }.to change(application_form, :action_required_by).to( + new_action_required_by, + ) + end + + it "records a timeline event" do + expect { call }.to have_recorded_timeline_event( + :action_required_by_changed, + creator: user, + application_form:, + old_value: "none", + new_value: new_action_required_by, + ) + end + end + + shared_examples "doesn't change action required by" do + it "doesn't change action required by from none" do + expect { call }.to_not change(application_form, :action_required_by).from( + "none", + ) + end + + it "doesn't record a timeline event" do + expect { call }.to_not have_recorded_timeline_event( + :action_required_by_changed, + creator: user, + application_form:, + ) + end + end + shared_examples "changes status" do |new_status| - it "changes the status to #{new_status}" do + it "changes status to #{new_status}" do expect { call }.to change(application_form, :status).to(new_status) end @@ -31,6 +65,7 @@ create(:dqt_trn_request, :potential_duplicate, application_form:) end + include_examples "changes action required by", "assessor" include_examples "changes status", "potential_duplicate_in_dqt" end @@ -42,6 +77,7 @@ ) end + include_examples "doesn't change action required by" include_examples "changes status", "withdrawn" end @@ -53,6 +89,7 @@ ) end + include_examples "doesn't change action required by" include_examples "changes status", "declined" end @@ -64,6 +101,7 @@ ) end + include_examples "doesn't change action required by" include_examples "changes status", "awarded" end @@ -73,6 +111,7 @@ create(:dqt_trn_request, application_form:) end + include_examples "changes action required by", "assessor" include_examples "changes status", "awarded_pending_checks" end @@ -84,6 +123,7 @@ create(:further_information_request, :received, assessment:) end + include_examples "changes action required by", "assessor" include_examples "changes status", "received" it "changes received_further_information" do @@ -102,6 +142,7 @@ create(:further_information_request, :requested, assessment:) end + include_examples "changes action required by", "external" include_examples "changes status", "waiting_on" it "changes waiting_on_further_information" do @@ -120,6 +161,7 @@ create(:professional_standing_request, :requested, assessment:) end + include_examples "changes action required by", "external" include_examples "changes status", "waiting_on" it "changes waiting_on_professional_standing" do @@ -142,6 +184,7 @@ create(:professional_standing_request, :received, assessment:) end + include_examples "changes action required by", "assessor" include_examples "changes status", "submitted" it "doesn't change received_professional_standing" do @@ -158,6 +201,7 @@ create(:professional_standing_request, :received, assessment:) end + include_examples "changes action required by", "assessor" include_examples "changes status", "received" it "changes received_professional_standing" do @@ -177,6 +221,7 @@ create(:qualification_request, :received, assessment:) end + include_examples "changes action required by", "assessor" include_examples "changes status", "received" it "changes received_further_information" do @@ -195,6 +240,7 @@ create(:qualification_request, :requested, assessment:) end + include_examples "changes action required by", "external" include_examples "changes status", "waiting_on" it "changes waiting_on_qualification" do @@ -228,6 +274,7 @@ ) end + include_examples "changes action required by", "external" include_examples "changes status", "waiting_on" it "doesn't change received_reference" do @@ -256,6 +303,7 @@ end context "and it's the only reference request" do + include_examples "changes action required by", "assessor" include_examples "changes status", "received" it "changes received_reference" do @@ -269,6 +317,7 @@ context "and there are other reference requests" do before { create(:reference_request, :requested, assessment:) } + include_examples "changes action required by", "external" include_examples "changes status", "waiting_on" it "doesn't change received_reference" do @@ -297,6 +346,7 @@ ) end + include_examples "changes action required by", "assessor" include_examples "changes status", "received" it "changes received_reference" do @@ -315,6 +365,7 @@ create(:reference_request, :requested, assessment:) end + include_examples "changes action required by", "external" include_examples "changes status", "waiting_on" it "changes waiting_on_reference" do @@ -330,12 +381,14 @@ create(:assessment, :started, application_form:) end + include_examples "changes action required by", "assessor" include_examples "changes status", "assessment_in_progress" end context "with a submitted_at date" do before { application_form.update!(submitted_at: Time.zone.now) } + include_examples "changes action required by", "assessor" include_examples "changes status", "submitted" end @@ -347,6 +400,8 @@ it "doesn't record a timeline event" do expect { call }.to_not have_recorded_timeline_event(:state_changed) end + + include_examples "doesn't change action required by" end context "when preliminary check is required" do @@ -363,6 +418,7 @@ create(:assessment_section, :preliminary, assessment:) end + include_examples "changes action required by", "admin" include_examples "changes status", "preliminary_check" context "when teaching authority provides written statement" do @@ -373,11 +429,13 @@ create(:professional_standing_request, assessment:) end + include_examples "changes action required by", "admin" include_examples "changes status", "preliminary_check" context "when the preliminary check has passed" do before { preliminary_assessment_section.update!(passed: true) } + include_examples "changes action required by", "external" include_examples "changes status", "waiting_on" end @@ -390,11 +448,13 @@ preliminary_assessment_section.reload.update!(passed: false) end + include_examples "changes action required by", "admin" include_examples "changes status", "preliminary_check" context "and the application form is declined" do before { application_form.update!(declined_at: Time.zone.now) } + include_examples "doesn't change action required by" include_examples "changes status", "declined" end end diff --git a/spec/lib/filters/action_required_by_spec.rb b/spec/lib/filters/action_required_by_spec.rb new file mode 100644 index 0000000000..0cd2f274e3 --- /dev/null +++ b/spec/lib/filters/action_required_by_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe Filters::ActionRequiredBy do + subject(:filtered_scope) { described_class.apply(scope:, params:) } + + context "the params includes a action_required_by" do + let(:params) { { action_required_by: "assessor" } } + let(:scope) { ApplicationForm.all } + + let!(:included) { create(:application_form, :action_required_by_assessor) } + let!(:excluded) { create(:application_form, :action_required_by_admin) } + + it { is_expected.to contain_exactly(included) } + end + + context "the params include multiple action_required_by" do + let(:params) { { action_required_by: %w[assessor external] } } + let(:scope) { ApplicationForm.all } + + let!(:included) { create(:application_form, :action_required_by_assessor) } + let!(:excluded) { create(:application_form, :action_required_by_admin) } + + it { is_expected.to contain_exactly(included) } + end + + context "the params don't include action_required_by" do + let(:params) { {} } + let(:scope) { double } + + it { is_expected.to eq(scope) } + end +end diff --git a/spec/models/application_form_spec.rb b/spec/models/application_form_spec.rb index 6f4a626fd5..d1b6c51b36 100644 --- a/spec/models/application_form_spec.rb +++ b/spec/models/application_form_spec.rb @@ -3,6 +3,7 @@ # Table name: application_forms # # id :bigint not null, primary key +# action_required_by :string default("none"), not null # age_range_max :integer # age_range_min :integer # age_range_status :string default("not_started"), not null @@ -67,6 +68,7 @@ # # Indexes # +# index_application_forms_on_action_required_by (action_required_by) # index_application_forms_on_assessor_id (assessor_id) # index_application_forms_on_english_language_provider_id (english_language_provider_id) # index_application_forms_on_family_name (family_name) @@ -101,6 +103,16 @@ describe "columns" do it do + is_expected.to define_enum_for(:action_required_by) + .with_values( + admin: "admin", + assessor: "assessor", + external: "external", + none: "none", + ) + .with_prefix + .backed_by_column_of_type(:string) + is_expected.to define_enum_for(:status).with_values( draft: "draft", submitted: "submitted", diff --git a/spec/models/timeline_event_spec.rb b/spec/models/timeline_event_spec.rb index b6a199fd1e..2f4bea97c9 100644 --- a/spec/models/timeline_event_spec.rb +++ b/spec/models/timeline_event_spec.rb @@ -79,6 +79,7 @@ it do is_expected.to define_enum_for(:event_type).with_values( + action_required_by_changed: "action_required_by_changed", age_range_subjects_verified: "age_range_subjects_verified", assessment_section_recorded: "assessment_section_recorded", assessor_assigned: "assessor_assigned", @@ -386,5 +387,28 @@ it { is_expected.to validate_absence_of(:old_value) } it { is_expected.to validate_absence_of(:new_value) } end + + context "with an action required by changed event type" do + before { timeline_event.event_type = :action_required_by_changed } + + it { is_expected.to validate_absence_of(:assignee) } + it { is_expected.to validate_absence_of(:old_state) } + it { is_expected.to validate_absence_of(:new_state) } + it { is_expected.to validate_absence_of(:assessment_section) } + it { is_expected.to validate_absence_of(:note) } + it { is_expected.to validate_absence_of(:mailer_class_name) } + it { is_expected.to validate_absence_of(:mailer_action_name) } + it { is_expected.to validate_absence_of(:message_subject) } + it { is_expected.to validate_absence_of(:assessment) } + it { is_expected.to validate_absence_of(:age_range_min) } + it { is_expected.to validate_absence_of(:age_range_max) } + it { is_expected.to validate_absence_of(:subjects) } + it { is_expected.to validate_absence_of(:requestable_id) } + it { is_expected.to validate_absence_of(:requestable_type) } + it { is_expected.to validate_absence_of(:work_history_id) } + it { is_expected.to validate_absence_of(:column_name) } + it { is_expected.to validate_presence_of(:old_value) } + it { is_expected.to validate_presence_of(:new_value) } + end 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 5677589240..a45c7b8491 100644 --- a/spec/support/autoload/page_objects/assessor_interface/applications.rb +++ b/spec/support/autoload/page_objects/assessor_interface/applications.rb @@ -40,6 +40,11 @@ class Applications < SitePrism::Page "#assessor_interface_filter_form_submitted_at_before_1i" end + section :action_required_by_filter, + "#app-applications-filters-action-required-by" do + sections :items, GovukCheckboxItem, ".govuk-checkboxes__item" + end + section :status_filter, "#app-applications-filters-status" do sections :statuses, GovukCheckboxItem, ".govuk-checkboxes__item" end diff --git a/spec/system/assessor_interface/filtering_application_forms_spec.rb b/spec/system/assessor_interface/filtering_application_forms_spec.rb index 05f70e14d9..2710466191 100644 --- a/spec/system/assessor_interface/filtering_application_forms_spec.rb +++ b/spec/system/assessor_interface/filtering_application_forms_spec.rb @@ -36,6 +36,10 @@ and_i_apply_the_submitted_at_filter then_i_see_a_list_of_applications_filtered_by_submitted_at + when_i_clear_the_filters + and_i_apply_the_action_required_by_filter + 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 @@ -124,6 +128,24 @@ def then_i_see_a_list_of_applications_filtered_by_submitted_at expect(applications_page.search_results.first.name.text).to eq("John Smith") end + def and_i_apply_the_action_required_by_filter + admin_action_item = + applications_page.action_required_by_filter.items.find do |item| + item.label.text == "Admin (1)" + rescue Capybara::ElementNotFound + false + end + admin_action_item.checkbox.click + applications_page.apply_filters.click + end + + def then_i_see_a_list_of_applications_filtered_by_action_required_by + expect(applications_page.search_results.count).to eq(1) + expect(applications_page.search_results.first.name.text).to eq( + "Emma Dubois", + ) + end + def and_i_apply_the_status_filter awarded_state = applications_page.status_filter.statuses.find do |status| @@ -155,6 +177,7 @@ def application_forms create( :application_form, :submitted, + :action_required_by_admin, region: create(:region, country: create(:country, code: "FR")), given_names: "Emma", family_name: "Dubois", @@ -165,6 +188,7 @@ def application_forms create( :application_form, :submitted, + :action_required_by_assessor, region: create(:region, country: create(:country, code: "ES")), given_names: "Arnold", family_name: "Drummond",