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

Fix priority scope during case distribution #15376

Closed
wants to merge 13 commits into from

Conversation

yoomlam
Copy link
Contributor

@yoomlam yoomlam commented Oct 5, 2020

Resolves #15352

Description

Adjust priority scope used by distribution algorithm to use AOD motions attached to specific appeals (due to #14085).

Acceptance Criteria

  • Priority ama appeals are pulled for distribution ONLY if one of the following are true
    • aod_based_on_age is true on the appeal
    • The claimant's dob is over 75 years ago
    • The claimant has a granted age base aod motion
    • The appeal has a granted aod motion associated with the appeal
    • The appeal is a cavc remand (not yet implemented)
  • Scope is created for appeal model that captures all this with no N+1 queries (should only produce one db call if possible)

The first 3 AC have a lot of overlap. What should we consider as our source of truth as I assume we don't want to check all 3?

Testing Plan

Ensure spec/models/docket_spec.rb covers all scenarios by using FactoryBot like in the new tests to create different type of appeals for manual testing.

Also refer to testing plans on:

@yoomlam yoomlam added Product: caseflow-queue Type: Bug Feature: auto-case-distribution Stakeholder: BVA Functionality associated with the Board of Veterans' Appeals workflows/feature requests Team: Echo 🐬 Priority: High Escalations from Support, blocking issue/NO workaround, or "first in" priority for new work. labels Oct 5, 2020
@yoomlam yoomlam self-assigned this Oct 5, 2020
@codeclimate
Copy link

codeclimate bot commented Oct 5, 2020

Code Climate has analyzed commit 1c61539 and detected 0 issues on this pull request.

View more on Code Climate.

Comment on lines +138 to +139
join_appeal_specific_aod_motions
.joins(claimants: :person)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not as simple as adding another OR to the where clause in priority method above due to ArgumentError: Relation passed to #or must be structurally compatible error -- solved with a tip from SO.

@@ -192,10 +192,8 @@
end

trait :inapplicable_aod_motion do
established_at { Time.zone.tomorrow }
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 no longer use established_at to determine AOD applicability. Rely on the young claimant age and ungranted AOD motion below to make appeal ineligible for AOD.

Comment on lines -120 to +122
.having("count(case when advance_on_docket_motions.granted and advance_on_docket_motions.created_at > appeals.established_at then 1 end) = ?", 0)
.having("count(case when appeal_aod_motions.granted then 1 end) = ?", 0) # AOD motions associated with appeal
Copy link
Contributor Author

Choose a reason for hiding this comment

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

appeal_aod_motions checks AOD motions associated directly with appeal (#14085).
Removed checking established_at.

Comment on lines +107 to +114
join_aod_motions.where(aod_based_on_age: true)
.or(join_aod_motions.where("people.date_of_birth <= ?", 75.years.ago))
.or( # check AOD motions associated through the claimant
join_aod_motions
.where("advance_on_docket_motions.granted = ?", true)
.where("advance_on_docket_motions.reason = ?", :age)
)
.or(join_aod_motions.where("appeal_aod_motions.granted = ?", true)) # AOD motions associated with appeal
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Compare against ACs. Did not include CAVC remand check since it's not yet implemented.

Comment on lines +52 to +56
let!(:aod_based_on_age_field_appeal) do
create(:appeal,
:with_post_intake_tasks,
docket_type: Constants.AMA_DOCKETS.direct_review).tap { |a| a.update(aod_based_on_age: true) }
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.

Claimant's age may not satisfy AOD requirements, but aod_based_on_age is set on the appeal.

trait :advanced_on_docket_due_to_motion do
# the appeal has to be established before the motion is created to apply to it.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

established_at is no longer relevant for AOD motions.

Comment on lines +132 to +136
joins(Arel.sql(<<~JOIN_AOD_MOTION_ON_APPEAL_ID))
LEFT OUTER JOIN advance_on_docket_motions AS appeal_aod_motions
ON appeal_aod_motions.appeal_id = appeals.id
AND appeal_aod_motions.appeal_type = 'Appeal'
JOIN_AOD_MOTION_ON_APPEAL_ID
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yoomlam yoomlam mentioned this pull request Oct 6, 2020
8 tasks
Copy link
Contributor

@ajspotts ajspotts left a comment

Choose a reason for hiding this comment

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

nice work, testing coverage looks 👍 lgtm 🚀

@yoomlam
Copy link
Contributor Author

yoomlam commented Oct 13, 2020

Waiting for Board confirmation. Merge this PR only if non-age-related AOD motions should be attached to specific appeals (due to #14085).

@yoomlam
Copy link
Contributor Author

yoomlam commented Oct 29, 2020

Do not merge -- see #13175 (comment)

@yoomlam
Copy link
Contributor Author

yoomlam commented Mar 26, 2021

ACD has been revisited several times since this PR was created, so this PR probably no longer has relevance.

@yoomlam yoomlam closed this Mar 26, 2021
@yoomlam yoomlam deleted the yoom/15352-fix-priority-scope branch March 26, 2021 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: auto-case-distribution Priority: High Escalations from Support, blocking issue/NO workaround, or "first in" priority for new work. Product: caseflow-queue Stakeholder: BVA Functionality associated with the Board of Veterans' Appeals workflows/feature requests Team: Echo 🐬 Type: Bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix priority logic for appeals with aod motions for distributions
2 participants