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

[PART 1] Make cached appeal attribute table joins a scope on tasks #14263

Merged
merged 8 commits into from
May 18, 2020
58 changes: 2 additions & 56 deletions app/jobs/update_cached_appeals_attributes_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ def cache_ama_appeals
appeals = Appeal.includes(:available_hearing_locations).where(id: open_appeals_from_tasks(Appeal.name))
request_issues_to_cache = request_issue_counts_for_appeal_ids(appeals.pluck(:id))
veteran_names_to_cache = veteran_names_for_file_numbers(appeals.pluck(:veteran_file_number))
appeal_assignees_to_cache = assignees_for_caseflow_appeal_ids(appeals.pluck(:id), Appeal.name)

appeal_aod_status = aod_status_for_appeals(appeals)
representative_names = representative_names_for_appeals(appeals)
Expand All @@ -43,7 +42,6 @@ def cache_ama_appeals
{
appeal_id: appeal.id,
appeal_type: Appeal.name,
assignee_label: appeal_assignees_to_cache[appeal.id],
case_type: appeal.type,
closest_regional_office_city: regional_office ? regional_office[:city] : COPY::UNKNOWN_REGIONAL_OFFICE,
closest_regional_office_key: regional_office ? appeal.closest_regional_office : COPY::UNKNOWN_REGIONAL_OFFICE,
Expand All @@ -57,8 +55,7 @@ def cache_ama_appeals
}
end

update_columns = [:assignee_label,
:case_type,
update_columns = [:case_type,
:closest_regional_office_city,
:closest_regional_office_key,
:docket_type,
Expand Down Expand Up @@ -155,7 +152,6 @@ def cache_legacy_appeal_vacols_data(all_vacols_ids)

issue_counts_to_cache = issues_counts_for_vacols_folders(batch_vacols_ids)
aod_status_to_cache = VACOLS::Case.aod(batch_vacols_ids)
appeal_assignees_to_cache = assignees_for_vacols_id(vacols_cases)

correspondent_ids = vacols_folders.map { |folder| folder[:correspondent_id] }
veteran_names_to_cache = veteran_names_for_correspondent_ids(correspondent_ids)
Expand All @@ -165,7 +161,6 @@ def cache_legacy_appeal_vacols_data(all_vacols_ids)

{
vacols_id: folder[:vacols_id],
assignee_label: appeal_assignees_to_cache[folder[:vacols_id]],
case_type: vacols_case[:status],
docket_number: folder[:docket_number],
issue_count: issue_counts_to_cache[folder[:vacols_id]] || 0,
Expand All @@ -174,7 +169,7 @@ def cache_legacy_appeal_vacols_data(all_vacols_ids)
}
end

update_columns = [:assignee_label, :docket_number, :issue_count, :veteran_name, :case_type, :is_aod]
update_columns = [:docket_number, :issue_count, :veteran_name, :case_type, :is_aod]
CachedAppeal.import values_to_cache, on_duplicate_key_update: { conflict_target: [:vacols_id],
columns: update_columns }

Expand Down Expand Up @@ -242,36 +237,6 @@ def representative_names_for_appeals(appeals)
).pluck("claimants.decision_review_id, bgs_power_of_attorneys.representative_name").to_h
end

def assignees_for_caseflow_appeal_ids(appeal_ids, appeal_type)
active_appeals = caseflow_appeals_assignees(appeal_ids, appeal_type, Task.active)
on_hold_appeals = caseflow_appeals_assignees(appeal_ids, appeal_type, Task.on_hold)

on_hold_appeals.merge(active_appeals)
end

def assignees_for_vacols_id(vacols_cases)
# Grab statuses from input hash of VACOLS cases.
vacols_statuses = vacols_cases.keys.map do |vacols_id|
[vacols_id, vacols_cases[vacols_id][:location]]
end.to_h

# Grab the appeal_ids for the VACOLS cases in CASEFLOW status
caseflow_vacols_ids = vacols_statuses.select { |_key, value| value == "CASEFLOW" }.keys
caseflow_vacols_to_appeal_id = LegacyAppeal.where(vacols_id: caseflow_vacols_ids).pluck(:vacols_id, :id).to_h

# Lookup more detailed Caseflow location for CASEFLOW vacols status
caseflow_statuses_by_appeal_id = assignees_for_caseflow_appeal_ids(caseflow_vacols_to_appeal_id.values,
LegacyAppeal.name)
# Map back to VACOLS id
caseflow_statuses_by_vacol_id = {}
caseflow_vacols_to_appeal_id.each do |vacols_id, appeal_id|
caseflow_statuses_by_vacol_id[vacols_id] = caseflow_statuses_by_appeal_id[appeal_id]
end

# Overwrite VACOLS Caseflow location with Caseflow detailed location
vacols_statuses.merge(caseflow_statuses_by_vacol_id)
end

def case_fields_for_vacols_ids(vacols_ids)
# array of arrays will become hash with bfkey as key.
# [
Expand All @@ -296,25 +261,6 @@ def case_fields_for_vacols_ids(vacols_ids)
end.to_h
end

def caseflow_appeals_assignees(appeal_ids, appeal_type, tasks)
ordered_tasks = tasks
.visible_in_queue_table_view
.where(appeal_type: appeal_type, appeal_id: appeal_ids)
.order(:appeal_id, created_at: :desc)

first_task_assignees_per_caseflow_appeal(ordered_tasks)
end

def first_task_assignees_per_caseflow_appeal(tasks)
org_tasks = tasks.joins("left join organizations on tasks.assigned_to_id = organizations.id")
.where("tasks.assigned_to_type = 'Organization'").pluck(:created_at, :appeal_id, "organizations.name")
user_tasks = tasks.joins("left join users on tasks.assigned_to_id = users.id")
.where("tasks.assigned_to_type = 'User'").pluck(:created_at, :appeal_id, "users.css_id")

# Combine the user & org tasks, preferring the most recently created (last) task
(org_tasks + user_tasks).sort_by { |task| task[0] }.map { |task| task.drop(1) }.to_h
end

def issues_counts_for_vacols_folders(vacols_ids)
VACOLS::CaseIssue.where(isskey: vacols_ids).group(:isskey).count
end
Expand Down
6 changes: 0 additions & 6 deletions app/models/cached_appeal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,4 @@

class CachedAppeal < CaseflowRecord
self.table_name = "cached_appeal_attributes"

def self.left_join_from_tasks_clause
"left join #{CachedAppeal.table_name} "\
"on #{CachedAppeal.table_name}.appeal_id = #{Task.table_name}.appeal_id "\
"and #{CachedAppeal.table_name}.appeal_type = #{Task.table_name}.appeal_type"
end
end
13 changes: 5 additions & 8 deletions app/models/queue_column.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,13 @@ def self.filter_option_hash(value, label)
private

def case_type_options(tasks)
options = tasks.joins(CachedAppeal.left_join_from_tasks_clause)
.group(:case_type).count.each_pair.map do |option, count|
options = tasks.with_cached_appeals.group(:case_type).count.each_pair.map do |option, count|
label = self.class.format_option_label(option, count)
self.class.filter_option_hash(option, label)
end

# Add the AOD option as the first option in the list.
aod_counts = tasks.joins(CachedAppeal.left_join_from_tasks_clause).group(:is_aod).count[true]
aod_counts = tasks.with_cached_appeals.group(:is_aod).count[true]
if aod_counts
aod_option_key = Constants.QUEUE_CONFIG.FILTER_OPTIONS.IS_AOD.key
aod_option_label = self.class.format_option_label("AOD", aod_counts)
Expand All @@ -86,15 +85,14 @@ def case_type_options(tasks)
end

def docket_type_options(tasks)
tasks.joins(CachedAppeal.left_join_from_tasks_clause).group(:docket_type).count.each_pair.map do |option, count|
tasks.with_cached_appeals.group(:docket_type).count.each_pair.map do |option, count|
label = self.class.format_option_label(Constants::DOCKET_NAME_FILTERS[option], count)
self.class.filter_option_hash(option, label)
end
end

def regional_office_options(tasks)
tasks.joins(CachedAppeal.left_join_from_tasks_clause)
.group(:closest_regional_office_city).count.each_pair.map do |option, count|
tasks.with_cached_appeals.group(:closest_regional_office_city).count.each_pair.map do |option, count|
label = self.class.format_option_label(option, count)
self.class.filter_option_hash(option, label)
end
Expand All @@ -108,8 +106,7 @@ def task_type_options(tasks)
end

def assignee_options(tasks)
tasks.joins(CachedAppeal.left_join_from_tasks_clause)
.group(:assignee_label).count.each_pair.map do |option, count|
tasks.with_assignees.group("assignees.display_name").count(:all).each_pair.map do |option, count|
label = self.class.format_option_label(option, count)
self.class.filter_option_hash(option, label)
end
Expand Down
8 changes: 3 additions & 5 deletions app/models/queue_tabs/assign_hearing_tab.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ def tasks
ScheduleHearingTask
.includes(*task_includes)
.active
.with_cached_appeals
.where(appeal_type: appeal_type)
.joins(CachedAppeal.left_join_from_tasks_clause)

@tasks ||=
if appeal_type == "LegacyAppeal"
Expand Down Expand Up @@ -85,16 +85,14 @@ def self.serialize_columns
end

def power_of_attorney_name_options
tasks.joins(CachedAppeal.left_join_from_tasks_clause)
.group(:power_of_attorney_name).count.each_pair.map do |option, count|
tasks.with_cached_appeals.group(:power_of_attorney_name).count.each_pair.map do |option, count|
label = QueueColumn.format_option_label(option, count)
QueueColumn.filter_option_hash(option, label)
end
end

def suggested_location_options
tasks.joins(CachedAppeal.left_join_from_tasks_clause)
.group(:suggested_hearing_location).count.each_pair.map do |option, count|
tasks.with_cached_appeals.group(:suggested_hearing_location).count.each_pair.map do |option, count|
label = QueueColumn.format_option_label(option, count)
QueueColumn.filter_option_hash(option, label)
end
Expand Down
11 changes: 6 additions & 5 deletions app/models/serializers/work_queue/task_column_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -144,14 +144,15 @@ def self.serialize_attribute?(params, columns)

attribute :assigned_to do |object, params|
columns = [Constants.QUEUE_CONFIG.COLUMNS.TASK_ASSIGNEE.name]
assignee = object.assigned_to

if serialize_attribute?(params, columns)
{
css_id: object.assigned_to.try(:css_id),
is_organization: object.assigned_to.is_a?(Organization),
name: object.appeal.assigned_to_location,
type: object.assigned_to.class.name,
id: object.assigned_to.id
css_id: assignee.try(:css_id),
is_organization: assignee.is_a?(Organization),
name: assignee.is_a?(Organization) ? assignee.name : assignee.css_id,
type: assignee.class.name,
id: assignee.id
}
else
{
Expand Down
14 changes: 8 additions & 6 deletions app/models/serializers/work_queue/task_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,15 @@ class WorkQueue::TaskSerializer
end

attribute :assigned_to do |object|
assignee = object.assigned_to

{
css_id: object.assigned_to.try(:css_id),
is_organization: object.assigned_to.is_a?(Organization),
name: object.appeal.assigned_to_location,
full_name: object.assigned_to.try(:full_name),
type: object.assigned_to.class.name,
id: object.assigned_to.id
css_id: assignee.try(:css_id),
full_name: assignee.try(:full_name),
is_organization: assignee.is_a?(Organization),
name: assignee.is_a?(Organization) ? assignee.name : assignee.css_id,
type: assignee.class.name,
id: assignee.id
}
end

Expand Down
32 changes: 32 additions & 0 deletions app/models/task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,12 @@ class << self; undef_method :open; end
)
}

scope :with_assignees, -> { joins(Task.joins_with_assignees_clause) }

scope :with_assigners, -> { joins(Task.joins_with_assigners_clause) }

scope :with_cached_appeals, -> { joins(Task.joins_with_cached_appeals_clause) }

############################################################################################
## class methods
class << self
Expand Down Expand Up @@ -172,6 +178,32 @@ def create_child_task(parent, current_user, params)
instructions: params[:instructions]
)
end

def assigners_table_clause
"(SELECT id, full_name AS display_name FROM users) AS assigners"
end

def joins_with_assigners_clause
"LEFT JOIN #{Task.assigners_table_clause} ON assigners.id = tasks.assigned_by_id"
end

def assignees_table_clause
"(SELECT id, 'Organization' AS type, name AS display_name FROM organizations " \
"UNION " \
"SELECT id, 'User' AS type, css_id AS display_name FROM users)" \
"AS assignees"
end

def joins_with_assignees_clause
"INNER JOIN #{Task.assignees_table_clause} ON " \
"assignees.id = tasks.assigned_to_id AND assignees.type = tasks.assigned_to_type"
end

def joins_with_cached_appeals_clause
"left join #{CachedAppeal.table_name} "\
"on #{CachedAppeal.table_name}.appeal_id = #{Task.table_name}.appeal_id "\
"and #{CachedAppeal.table_name}.appeal_type = #{Task.table_name}.appeal_type"
end
end

########################################################################################
Expand Down
6 changes: 4 additions & 2 deletions app/models/task_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def table_column_from_name(column_name)
when Constants.QUEUE_CONFIG.COLUMNS.APPEAL_TYPE.name
"cached_appeal_attributes.case_type"
when Constants.QUEUE_CONFIG.COLUMNS.TASK_ASSIGNEE.name
"cached_appeal_attributes.assignee_label"
"assignees.display_name"
when Constants.QUEUE_CONFIG.POWER_OF_ATTORNEY_COLUMN_NAME
"cached_appeal_attributes.power_of_attorney_name"
when Constants.QUEUE_CONFIG.SUGGESTED_HEARING_LOCATION_COLUMN_NAME
Expand All @@ -67,7 +67,9 @@ def initialize(args)
end

def filtered_tasks
where_clause.empty? ? tasks : tasks.joins(CachedAppeal.left_join_from_tasks_clause).where(*where_clause)
return tasks if where_clause.empty?

tasks.with_assignees.with_cached_appeals.where(*where_clause)
end

# filter_params = ["col=docketNumberColumn&val=legacy|evidence_submission", "col=taskColumn&val=TranslationTask"]
Expand Down
10 changes: 3 additions & 7 deletions app/models/task_sorter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def sorted_tasks

# Always join to the CachedAppeal and users tables because we sometimes need it, joining does not slow down the
# application, and conditional logic to only join sometimes adds unnecessary complexity.
tasks.joins(CachedAppeal.left_join_from_tasks_clause).joins(left_join_from_users_clause).order(order_clause)
tasks.with_assignees.with_assigners.with_cached_appeals.order(order_clause)
end

private
Expand Down Expand Up @@ -67,16 +67,12 @@ def default_order_clause
# postgres to use as a reference for sorting as a task's label is not stored in the database.
def task_type_order_clause
task_types_sorted_by_label = Task.descendants.sort_by(&:label).map(&:name)
task_type_sort_position = "type in '#{task_types_sorted_by_label.join(',')}'"
task_type_sort_position = "tasks.type in '#{task_types_sorted_by_label.join(',')}'"
"position(#{task_type_sort_position}) #{sort_order}"
end

def assigner_order_clause
"substring(users.full_name,\'([a-zA-Z]+)$\') #{sort_order}"
end

def left_join_from_users_clause
"left join users on users.id = tasks.assigned_by_id"
"substring(assigners.display_name,\'([a-zA-Z]+)$\') #{sort_order}"
end

def column_is_valid
Expand Down
4 changes: 2 additions & 2 deletions client/constants/QUEUE_CONFIG.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@
"TASK_ASSIGNEE": {
"filterable": true,
"name": "assignedToColumn",
"sorting_table": "cached_appeal_attributes",
"sorting_columns": ["assignee_label"]
"sorting_table": "assignees",
"sorting_columns": ["display_name"]
},
"TASK_ASSIGNER": {
"name": "completedToNameColumn"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class RemoveAssigneeLabelFromCachedAppeal < ActiveRecord::Migration[5.2]
def change
safety_assured do
remove_column :cached_appeal_attributes, :assignee_label, :string, comment: "Queues will now use the actual task assignee rather than the appeal's assigned to location"
end
end
end
1 change: 0 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,6 @@
create_table "cached_appeal_attributes", id: false, force: :cascade do |t|
t.integer "appeal_id"
t.string "appeal_type"
t.string "assignee_label", comment: "Who is currently most responsible for the appeal"
t.string "case_type", comment: "The case type, i.e. original, post remand, CAVC remand, etc"
t.string "closest_regional_office_city", comment: "Closest regional office to the veteran"
t.string "closest_regional_office_key", comment: "Closest regional office to the veteran in 4 character key"
Expand Down
1 change: 0 additions & 1 deletion docs/schema/caseflow.csv
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ board_grant_effectuations,updated_at,datetime,,,,,x,
cached_appeal_attributes,,,,,,,,
cached_appeal_attributes,appeal_id,integer,,,,,x,
cached_appeal_attributes,appeal_type,string,,,,,x,
cached_appeal_attributes,assignee_label,string,,,,,,Who is currently most responsible for the appeal
cached_appeal_attributes,case_type,string,,,,,x,"The case type, i.e. original, post remand, CAVC remand, etc"
cached_appeal_attributes,closest_regional_office_city,string,,,,,x,Closest regional office to the veteran
cached_appeal_attributes,closest_regional_office_key,string,,,,,x,Closest regional office to the veteran in 4 character key
Expand Down
1 change: 0 additions & 1 deletion spec/factories/cached_appeals.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
case_type { "Original" }
is_aod { false }
appeal_id { rand(1..9_999) }
assignee_label { "BVAAABSHIRE" }
vacols_id { nil }
end
end
Loading