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

Refactor RoundRobinTaskDistributor #9531

Merged
merged 9 commits into from
Feb 20, 2019
2 changes: 1 addition & 1 deletion app/models/bva_dispatch_task_distributor.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class BvaDispatchTaskDistributor < RoundRobinTaskDistributor
def initialize(list_of_assignees: BvaDispatch.singleton.users.order(:id).pluck(:css_id),
def initialize(assignee_pool: BvaDispatch.singleton.users.order(:id),
task_class: BvaDispatchTask)
super
end
Expand Down
4 changes: 2 additions & 2 deletions app/models/colocated_task_distributor.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class ColocatedTaskDistributor < RoundRobinTaskDistributor
def initialize(list_of_assignees: Colocated.singleton.non_admins.sort_by(&:id).pluck(:css_id),
def initialize(assignee_pool: Colocated.singleton.non_admins.sort_by(&:id),
task_class: Task)
super
end
Expand All @@ -8,7 +8,7 @@ def next_assignee(options = {})
open_assignee = options.dig(:appeal)
&.tasks
&.active
&.find_by(assigned_to: User.where(css_id: list_of_assignees))
&.find_by(assigned_to: assignee_pool)
&.assigned_to

open_assignee || super()
Expand Down
6 changes: 0 additions & 6 deletions app/models/judge_assign_task_distributor.rb

This file was deleted.

35 changes: 19 additions & 16 deletions app/models/round_robin_task_distributor.rb
Original file line number Diff line number Diff line change
@@ -1,40 +1,43 @@
class RoundRobinTaskDistributor
include ActiveModel::Model

attr_accessor :list_of_assignees, :task_class
validates :task_class, :assignee_pool, presence: true
validate :assignee_pool_must_contain_only_users

def assignee_users
User.where(css_id: list_of_assignees)
end
attr_accessor :assignee_pool, :task_class

def latest_task
task_class.where(assigned_to: assignee_users).max_by(&:created_at)
task_class.where(assigned_to: assignee_pool).max_by(&:created_at)
end

def last_assignee_css_id
latest_task&.assigned_to&.css_id
def last_assignee
latest_task&.assigned_to
end

def last_assignee_index
list_of_assignees.index(last_assignee_css_id)
assignee_pool.index(last_assignee)
end

def next_assignee_index
return 0 unless last_assignee_css_id
return 0 unless last_assignee
return 0 unless last_assignee_index

(last_assignee_index + 1) % list_of_assignees.length
(last_assignee_index + 1) % assignee_pool.length
end

def next_assignee_css_id
if list_of_assignees.blank?
fail Caseflow::Error::RoundRobinTaskDistributorError, message: "list_of_assignees cannot be empty"
def next_assignee(_options = {})
unless valid?
fail Caseflow::Error::RoundRobinTaskDistributorError, message: errors.full_messages.join(", ")
end

list_of_assignees[next_assignee_index]
assignee_pool[next_assignee_index]
end

def next_assignee(_options = {})
User.find_by_css_id_or_create_with_default_station_id(next_assignee_css_id)
private

def assignee_pool_must_contain_only_users
unless assignee_pool.all? { |a| a.is_a?(User) }
errors.add(:assignee_pool, COPY::TASK_DISTRIBUTOR_ASSIGNEE_POOL_USERS_ONLY_MESSAGE)
end
end
end
4 changes: 0 additions & 4 deletions app/models/tasks/bva_dispatch_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ def outcode(appeal, params, user)

private

def list_of_assignees
BvaDispatch.singleton.users.order(:id).pluck(:css_id)
end

def create_decision_document!(params)
DecisionDocument.create!(params).tap do |decision_document|
decision_document.submit_for_processing!
Expand Down
3 changes: 2 additions & 1 deletion client/COPY.json
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@
"WITHDRAW_HEARING_MODAL_TITLE": "Confirm withdraw hearing",
"WITHDRAW_HEARING_MODAL_BODY": "Withdrawing this hearing request will remove this veteran from the hearing pool.",
"WITHDRAW_HEARING_SUCCESS_MESSAGE_TITLE": "You have successfully withdrawn %s's hearing request",
"WITHDRAW_HEARING_SUCCESS_MESSAGE_BODY": "%s has been removed from the hearing schedule and has been sent to the next appropriate location"
"WITHDRAW_HEARING_SUCCESS_MESSAGE_BODY": "%s has been removed from the hearing schedule and has been sent to the next appropriate location",

"TASK_DISTRIBUTOR_ASSIGNEE_POOL_USERS_ONLY_MESSAGE": "must contain only Users"
}
2 changes: 1 addition & 1 deletion spec/models/colocated_task_distributor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
describe ".next_assignee" do
let(:iterations) { 6 }

context "when the list_of_assignees is a populated array" do
context "when the assignee_pool is a populated array" do
let(:total_distribution_count) { iterations * assignee_pool_size }

before do
Expand Down
4 changes: 2 additions & 2 deletions spec/models/organizations/bva_dispatch_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
it "should throw an error" do
expect { subject }.to raise_error do |error|
expect(error).to be_a(Caseflow::Error::RoundRobinTaskDistributorError)
expect(error.message).to eq("list_of_assignees cannot be empty")
expect(error.message).to eq("Assignee pool can't be blank")
end
end
end
Expand All @@ -45,7 +45,7 @@
it "should throw an error" do
expect { subject }.to raise_error do |error|
expect(error).to be_a(Caseflow::Error::RoundRobinTaskDistributorError)
expect(error.message).to eq("list_of_assignees cannot be empty")
expect(error.message).to eq("Assignee pool can't be blank")
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/models/organizations/colocated_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
it "should throw an error" do
expect { subject }.to raise_error do |error|
expect(error).to be_a(Caseflow::Error::RoundRobinTaskDistributorError)
expect(error.message).to eq("list_of_assignees cannot be empty")
expect(error.message).to eq("Assignee pool can't be blank")
end
end
end
Expand Down Expand Up @@ -53,7 +53,7 @@
it "should throw an error" do
expect { subject }.to raise_error do |error|
expect(error).to be_a(Caseflow::Error::RoundRobinTaskDistributorError)
expect(error.message).to eq("list_of_assignees cannot be empty")
expect(error.message).to eq("Assignee pool can't be blank")
end
end
end
Expand Down
40 changes: 32 additions & 8 deletions spec/models/round_robin_task_distributor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
let!(:assignee_pool) { FactoryBot.create_list(:user, assignee_pool_size) }
let(:task_class) { Task }
let(:round_robin_distributor) do
RoundRobinTaskDistributor.new(list_of_assignees: assignee_pool.pluck(:css_id), task_class: task_class)
RoundRobinTaskDistributor.new(assignee_pool: assignee_pool, task_class: task_class)
end

describe ".latest_task" do
Expand Down Expand Up @@ -40,18 +40,42 @@
end

describe ".next_assignee" do
context "when the list_of_assignees is an empty array" do
let(:round_robin_distributor) { RoundRobinTaskDistributor.new(list_of_assignees: [], task_class: task_class) }
context "the distributor is invalid" do
context "the assignee_pool is empty" do
let!(:assignee_pool) { [] }

it "should raise an error" do
expect { round_robin_distributor.next_assignee }.to(raise_error) do |error|
expect(error).to be_a(Caseflow::Error::RoundRobinTaskDistributorError)
expect(error.message).to eq("list_of_assignees cannot be empty")
it "raises an error" do
expect { round_robin_distributor.next_assignee }.to(raise_error) do |error|
expect(error).to be_a(Caseflow::Error::RoundRobinTaskDistributorError)
expect(error.message).to eq("Assignee pool can't be blank")
end
end
end

context "the task_class is blank" do
let(:task_class) { nil }

it "raises an error" do
expect { round_robin_distributor.next_assignee }.to(raise_error) do |error|
expect(error).to be_a(Caseflow::Error::RoundRobinTaskDistributorError)
expect(error.message).to eq("Task class can't be blank")
end
end
end

context "the assignee_pool contains items that aren't Users" do
let!(:assignee_pool) { FactoryBot.create_list(:user, assignee_pool_size).pluck(:css_id) }

it "raises an error" do
expect { round_robin_distributor.next_assignee }.to(raise_error) do |error|
expect(error).to be_a(Caseflow::Error::RoundRobinTaskDistributorError)
expect(error.message).to eq("Assignee pool #{COPY::TASK_DISTRIBUTOR_ASSIGNEE_POOL_USERS_ONLY_MESSAGE}")
end
end
end
end

context "when the list_of_assignees is a populated array" do
context "the assignee_pool is a populated array" do
let(:iterations) { 4 }
let(:total_distribution_count) { iterations * assignee_pool_size }

Expand Down