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

Prioritize AOD and CAVC cases during Bulk Assignment #15125

Merged
merged 10 commits into from
Sep 2, 2020
4 changes: 4 additions & 0 deletions app/models/appeal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,10 @@ def veteran_middle_initial
veteran_middle_name&.first
end

def cavc?
false if cavc == "not implemented for AMA"
end

yoomlam marked this conversation as resolved.
Show resolved Hide resolved
def cavc
"not implemented for AMA"
end
Expand Down
11 changes: 4 additions & 7 deletions app/models/legacy_appeal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,8 @@ class UnknownLocationError < StandardError; end
end

# To match Appeals AOD behavior
def aod?
aod
end

def advanced_on_docket?
aod
end
alias aod? aod
alias advanced_on_docket? aod

cache_attribute :dic do
issues.map(&:dic).include?(true)
Expand Down Expand Up @@ -785,6 +780,8 @@ def cavc
type == "Court Remand"
end

alias cavc? cavc

# Adding anything to this to_hash can trigger a lazy load which slows down
# welcome gate dramatically. Don't add anything to it without also adding it to
# the query in VACOLS::CaseAssignment.
Expand Down
8 changes: 8 additions & 0 deletions app/models/task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,14 @@ def joins_with_cached_appeals_clause
"on #{CachedAppeal.table_name}.appeal_id = #{Task.table_name}.appeal_id "\
"and #{CachedAppeal.table_name}.appeal_type = #{Task.table_name}.appeal_type"
end

def order_by_appeal_priority_clause
Arel.sql(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arel.sql is needed to address DEPRECATION WARNING: Dangerous query method (method whose arguments are used as raw SQL) called with non-attribute argument(s) warning.

Non-attribute arguments will be disallowed in Rails 6.0. This method should not be called with user-provided values, such as request parameters or model attributes. Known-safe values can be passed by wrapping them in Arel.sql()

"CASE WHEN cached_appeal_attributes.is_aod = TRUE THEN 0 ELSE 1 END, "\
"CASE WHEN cached_appeal_attributes.case_type = 'Court Remand' THEN 0 ELSE 1 END, "\
"tasks.created_at"
)
yoomlam marked this conversation as resolved.
Show resolved Hide resolved
end
end

########################################################################################
Expand Down
34 changes: 23 additions & 11 deletions app/workflows/bulk_task_assignment.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# frozen_string_literal: true

# Assign multiple cases, which are selected based on parameters.
# Cases are prioritized in the following order:
# - CAVC AOD Cases
# - AOD Cases
# - CAVC Cases
# - remaining cases
class BulkTaskAssignment
include ActiveModel::Model

Expand Down Expand Up @@ -37,18 +43,24 @@ def process

def tasks_to_be_assigned
@tasks_to_be_assigned ||= begin
tasks = task_type.constantize
.active.where(assigned_to_id: organization.id)
.limit(task_count).order(:created_at)
yoomlam marked this conversation as resolved.
Show resolved Hide resolved
if regional_office
tasks = tasks.joins(
"INNER JOIN appeals ON appeals.id = appeal_id AND appeal_type = '#{Appeal.name}'"
).where("closest_regional_office = ?", regional_office) +
tasks.joins("INNER JOIN legacy_appeals ON legacy_appeals.id = appeal_id \
AND appeal_type = '#{LegacyAppeal.name}'").where("closest_regional_office = ?", regional_office)
end
tasks
prioritized_tasks(tasks_satisfying_params).first(task_count.to_i)
yoomlam marked this conversation as resolved.
Show resolved Hide resolved
end
end

def tasks_satisfying_params
tasks = task_type.constantize.active.where(assigned_to_id: organization.id)
if regional_office
tasks = tasks.joins(
"INNER JOIN appeals ON appeals.id = appeal_id AND appeal_type = '#{Appeal.name}'"
).where("closest_regional_office = ?", regional_office) +
tasks.joins("INNER JOIN legacy_appeals ON legacy_appeals.id = appeal_id \
AND appeal_type = '#{LegacyAppeal.name}'").where("closest_regional_office = ?", regional_office)
end
yoomlam marked this conversation as resolved.
Show resolved Hide resolved
tasks
end
yoomlam marked this conversation as resolved.
Show resolved Hide resolved

def prioritized_tasks(tasks)
tasks.with_cached_appeals.order(Task.order_by_appeal_priority_clause)
end

def assigned_to
Expand Down
63 changes: 62 additions & 1 deletion spec/workflows/bulk_task_assignment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
created_at: 1.day.ago)
end

# Even it is the oldest task, it should skip it becasue it is on hold
# Even it is the oldest task, it should skip it because it is on hold
let!(:no_show_hearing_task4) do
create(:no_show_hearing_task,
:on_hold,
Expand Down Expand Up @@ -134,6 +134,67 @@
expect(task_with_legacy_appeal.assigned_by).to eq assigned_by
end
end

def create_no_show_hearing_task_for_appeal(appeal, creation_time = 1.day.ago)
create(:no_show_hearing_task, appeal: appeal, assigned_to: organization, created_at: creation_time)
end

context "when there are priority appeals" do
let(:regional_office) { nil }
let(:task_count) { 20 }

let(:aod_appeal) { create(:appeal, :advanced_on_docket_due_to_motion) }
# Since cavc is not currently supported, ignore CAVC AMA appeals for now; use CAVC legacy appeals for now
let(:aod_legacy_appeal) { create(:legacy_appeal, vacols_case: create(:case, :aod)) }
let(:cavc_legacy_appeal) { create(:legacy_appeal, vacols_case: create(:case, :type_cavc_remand)) }
let(:cavc_aod_legacy_appeal) { create(:legacy_appeal, vacols_case: create(:case, :aod, :type_cavc_remand)) }
yoomlam marked this conversation as resolved.
Show resolved Hide resolved

before do
create_no_show_hearing_task_for_appeal(aod_appeal, 3.days.ago)
create_no_show_hearing_task_for_appeal(aod_legacy_appeal, 2.days.ago)
create_no_show_hearing_task_for_appeal(cavc_legacy_appeal)
create_no_show_hearing_task_for_appeal(cavc_aod_legacy_appeal)

UpdateCachedAppealsAttributesJob.perform_now
end

let(:expected_appeal_ordering) { [cavc_aod_legacy_appeal, aod_appeal, aod_legacy_appeal, cavc_legacy_appeal] }

subject { BulkTaskAssignment.new(params).process }

it "sorts priority appeals first" do
prioritized_assigned_tasks = subject
expect(prioritized_assigned_tasks.first(4).map(&:appeal)).to eq(expected_appeal_ordering)

appeals_of_returned_tasks = prioritized_assigned_tasks.map(&:appeal)
expect(appeals_of_returned_tasks).to include(no_show_hearing_task1.appeal)
expect(appeals_of_returned_tasks).to include(no_show_hearing_task2.appeal)
expect(appeals_of_returned_tasks).to include(no_show_hearing_task3.appeal)
end

context "when task_count param is less than the number of AOD and CAVC appeals" do
let(:task_count) { 2 }
it "sorts priority appeals first" do
expect(subject.map(&:appeal)).to eq(expected_appeal_ordering.first(task_count))
end
end

context "comparing with semantically correct but slower implementation" do
def slower_prioritized_tasks(tasks)
tasks.sort_by { |task| [task.appeal.aod? ? 0 : 1, task.appeal.cavc? ? 0 : 1, task.created_at] }
end

it "sorts correctly" do
bulk_task_assignment = BulkTaskAssignment.new(params)
tasks = bulk_task_assignment.send(:tasks_satisfying_params)
expect(tasks.count).to eq(7)
prioritized_tasks = bulk_task_assignment.send(:prioritized_tasks, tasks)
expect(prioritized_tasks).to eq(slower_prioritized_tasks(tasks))

expect(prioritized_tasks.first(4).map(&:appeal)).to eq(expected_appeal_ordering)
end
end
yoomlam marked this conversation as resolved.
Show resolved Hide resolved
end
end
end
end