Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add action required by workflow and filter #1671

Merged
merged 8 commits into from
Sep 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions app/components/timeline_entry/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 5 additions & 4 deletions app/forms/assessor_interface/filter_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
53 changes: 42 additions & 11 deletions app/lib/application_form_status_updater.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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:,
Expand All @@ -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

Expand Down Expand Up @@ -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?
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
13 changes: 13 additions & 0 deletions app/lib/filters/action_required_by.rb
Original file line number Diff line number Diff line change
@@ -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
10 changes: 10 additions & 0 deletions app/models/application_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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",
Expand Down
36 changes: 20 additions & 16 deletions app/models/timeline_event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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? }
Expand All @@ -91,15 +98,13 @@ 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?
validates :assessment_section,
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?

Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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?

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand All @@ -43,6 +49,8 @@ def status_filter_options

private

ACTION_REQUIRED_BY_OPTIONS = %w[admin assessor external].freeze

STATUS_FILTER_OPTIONS = {
preliminary_check: [],
submitted: [],
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
16 changes: 12 additions & 4 deletions app/views/assessor_interface/application_forms/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@
<div class="app-checkbox-filter__container">
<div class="app-checkbox-filter__container-inner">
<%= 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 %>
</div>
</div>
</section>
Expand Down Expand Up @@ -48,6 +48,14 @@
<% end %>
</section>

<section id="app-applications-filters-action-required-by">
<%= 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 %>
</section>

<section id="app-applications-filters-status">
<%= f.govuk_check_boxes_fieldset :statuses, legend: { size: "s" }, small: true do %>
<% @view_object.status_filter_options.each do |option, suboptions| %>
Expand All @@ -67,7 +75,7 @@
<% end %>
<% end %>
</section>
<%- end -%>
<% end %>
</div>

<div class="govuk-grid-column-two-thirds">
Expand Down
Loading