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] Create a "priority only" distribution #14691

Closed

Conversation

hschallhorn
Copy link
Contributor

Bumps #14604

Description

Please explain the changes you made here.

Acceptance Criteria

  • Priority distributions
    • Distribute priority appeals tied to a judge with no limit if none is provided
    • Distribute the oldest priority appeals regardless of docket if a limit is provided
  • Passing a limit with genpop 'any' to the hearing request docket only distributes the number of cases provided by the limit.

@hschallhorn hschallhorn self-assigned this Jul 14, 2020
@va-bot
Copy link
Collaborator

va-bot commented Jul 14, 2020

1 Warning
⚠️ This is a Big PR. Try to break this down if possible. Stacked pull requests encourage more detailed and thorough code reviews

Generated by 🚫 Danger

@codeclimate
Copy link

codeclimate bot commented Jul 14, 2020

Code Climate has analyzed commit 463d33c and detected 13 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 13

View more on Code Climate.

@hschallhorn hschallhorn changed the title Create a "priority only" distribution [PART 1] Create a "priority only" distribution Jul 14, 2020
@@ -59,7 +75,7 @@ def ama_distribution

def ama_statistics
{
batch_size: batch_size,
batch_size: priority_push? ? @appeals.count : batch_size,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

priority push does not work off of batch size, it either distributes without limit or distributes the provided limit.

 to priority push
if genpop == "any"
appeals_to_reject = appeals.flatten.sort_by(&:ready_for_distribution_at).drop(limit)
appeals = [appeals.first - appeals_to_reject, appeals.last - appeals_to_reject]
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Found a bug! Detailed here

expect(distribution.distributed_cases.length).to eq(1)
expect(distribution_judge.reload.tasks.map(&:appeal)).to match_array([not_tied])
end
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test for this bug fix

@@ -324,7 +324,8 @@ def self.distribute_priority_appeals(judge, genpop, limit, dry_run = false)
judge.vacols_attorney_id,
(genpop == "any" || genpop == "not_genpop") ? 1 : 0,
(genpop == "any" || genpop == "only_genpop") ? 1 : 0,
limit
limit,
limit.nil? ? 1 : 0
Copy link
Contributor Author

@hschallhorn hschallhorn Jul 14, 2020

Choose a reason for hiding this comment

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

Allows nil limit to be passed to distribute_priority_appeals, indicating that any and all cases should be distributed

end
end

def distribute!
def distribute!(limit = nil)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Allows us to stipulate a limit for priority push cases, when we get to the "aim to hit priority target" portion of the job

@@ -30,77 +30,6 @@
end

context "#distribute!" do
subject { Distribution.create!(judge: judge) }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

WHITESPACE!

bfac: "1",
bfmpro: "ACT",
bfcurloc: "81",
bfdloout: i.months.ago,
Copy link
Contributor Author

@hschallhorn hschallhorn Jul 15, 2020

Choose a reason for hiding this comment

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

bfdloout represents how long the case case been ready to distribute. The last one created will be the oldest case.

:advanced_on_docket_due_to_age,
:ready_for_distribution,
docket_type: Constants.AMA_DOCKETS.hearing)
appeal.tasks.find_by(type: DistributionTask.name).update(assigned_at: i.months.ago)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

DistributionTask.assigned at represents how long the case case been ready to distribute. The last one created will be the oldest case.

context "when there is no limit specified" do
it "distributes all priority cases associated with the judge" do
distributed_case_ids = priority_legacy_hearings_tied_to_judge.map(&:folder_nr)
.concat(priority_ama_hearings_tied_to_judge.map(&:uuid))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should only distribute priority cases tied to the judge

other_priority_ama_hearings.last.uuid,
priority_direct_review_cases.last.uuid,
evidence_submission_cases.last.uuid
]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should distribute the 4 oldest cases, regardless of what docket they are from.

@@ -1,6 +1,6 @@
# frozen_string_literal: true

module AmaCaseDistribution
module AutomaticCaseDistribution
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sneaky rename to more accurately describe the module

end
end

def requested_distribution
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sneaky rename to more accurately describe the method

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants