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

Check if the user is the judge or attorney on a case review to determine what actions they see in LegacyTasks #15591

Merged
merged 15 commits into from
Nov 16, 2020

Conversation

hschallhorn
Copy link
Contributor

@hschallhorn hschallhorn commented Nov 9, 2020

Resolves issue is some cases where a legacy appeal is sent back to the decision drafting attorney but they only see judge actions as they are an avlj.

Description

Another hacky fix in the ever evolving saga of "how the heck do we determine if a legacy case is assigned to an acting judge to write a decision to to sign a decision". Some context is provided in #14886.

Initially, we tried to use the logic that if a decision had already been drafted for an appeal, this appeal is probably assigned to the user to sign the decision as a judge.

To implement this, we initially checked if an attorney case review existed for the case in caseflow. While this was an alright start, sometimes cases are sent from an attorney to a judge outside of caseflow and no attorney case review will exist.

To resolve this, we moved to checking to see if there was a decision document listed on the vacols case assignment. The thought process here is that this will always be populated whether sent to the judge in caseflow or vacols.

The problem with this is outlined in #14886. If the case returns to the acting judge, say for rewriting the decision, our code assumes they are being assigned the case to sign the decision they wrote.

For some of these cases that were worked in caseflow, we can reach a happier middleground. We should check to see if they are the attorney that created the case review. If they are, the task will be an attorney task. It they were the judge on the case review, the task will be a judge task. If neither, we can fall back to old logic of checking to see if there is a decision already written.

While this will not resolve all cases, it will cut down on the number of batteam requests we get about this.

For instance, this batteam request could have been avoided if we had implemented this check.

user = User.find_by(css_id: "bvamsopko".upcase)
user.acting_judge_in_vacols?
=> true

appeal = LegacyAppeal.find_by(vacols_id: "3848262")
appeal.vacols_case_review.valid_document_id?
=> true
# based on our current logic, we should show this user judge actions as a decision exists already. SAD

appeal.attorney_case_review.attorney == user
=> true
# !! information in caseflow exists that definitively stipulates this user was the attorney on the case, we should show them attorney actions 

Acceptance Criteria

  • If an acting judge was the drafting attorney on a legacy case in caseflow, they should see attorney actions for the case assigned to them
  • If an acting judge was the reviewing judge on a legacy case in caseflow, they should see judge actions for the case assigned to them
  • If there is no attorney case review or if the user was neither the judge or attorney, fall back to checking if a decision has already been written for the case.

Testing plan

  1. Ensure das deprecation is turned off
FeatureToggle.enabled?(:legacy_das_deprication)
=> false
  1. Log in as bvaaaaaaabshire and go to your assign queue
  2. Assign a case to an acting judge
  3. If you get a "Case already assigned" error, you may need to fix some vacols data
judge = User.find_by(css_id: "BVAAABSHIRE")
judge.vacols_uniq_id
=> "ID6"
# Should be "AABSHIRE"

VACOLS::Staff.where(sdomainid: "BVAAABSHIRE").pluck(:slogid)
=> ["ID6", "ID7", "AABSHIRE"]
VACOLS::Staff.where.not(slogid: "AABSHIRE").where(sdomainid: "BVAAABSHIRE").destroy_all
VACOLS::Staff.where(sdomainid: "BVAAABSHIRE").pluck(:slogid)
=> ["AABSHIRE"]

# Clear our cache and try again 
Rails.cache.delete("#{Rails.env}_staff_record_#{judge.css_id}")
User.find_by(css_id: "BVAAABSHIRE").vacols_uniq_id
=> "AABSHIRE"
  1. If you get a "Deteam cannot be blank" error
acting_judge = User.find_by(css_id: "BVAACTING")
acting_judge.vacols_group_id
=> ""

VACOLS::Staff.find_by(sdomainid: "BVAACTING").update!(stitle: "D#{Random.rand(1..5)}")
Rails.cache.delete("#{Rails.env}_staff_record_#{acting_judge.css_id}")
User.find_by(css_id: "BVAACTING").vacols_group_id
=> "D5"
  1. Log in as the acting judge and go through checkout to send the case back to the judge
  2. Cases are being sent back to these avljs through vacols, so let's do that manually
VACOLS::Case.find(3662856).update_vacols_location!(acting_judge.vacols_uniq_id)
  1. Return to the case and ensure the user has "Decision ready for review" action, not "Ready for dispatch"
  2. Checkout master to see what the user used to see

UI changes

Before (user sees judge actions) After (user sees attorney actions)
Screen Shot 2020-11-09 at 5 47 10 PM Screen Shot 2020-11-09 at 5 46 50 PM

@hschallhorn hschallhorn added Type: Enhancement Enhancement to an existing feature Product: caseflow-queue Feature: caseflow-decisions Team: Echo 🐬 Priority: Medium Blocking issue w/workaround, or "second in" priority for new work. User: AVLJ Acting Veterans Law Judge labels Nov 9, 2020
@hschallhorn hschallhorn self-assigned this Nov 9, 2020
@hschallhorn hschallhorn changed the title Check if the user is the judge or attorney on a case review to determine what actions they see Check if the user is the judge or attorney on a case review to determine what actions they see in LegacyTasks Nov 9, 2020
@codeclimate
Copy link

codeclimate bot commented Nov 9, 2020

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

View more on Code Climate.

app/models/legacy_appeal.rb Show resolved Hide resolved
app/models/legacy_appeal.rb Show resolved Hide resolved
app/models/legacy_appeal.rb Show resolved Hide resolved
app/models/legacy_appeal.rb Outdated Show resolved Hide resolved
app/models/legacy_appeal.rb Outdated Show resolved Hide resolved
spec/models/legacy_appeal_spec.rb Outdated Show resolved Hide resolved
subject { appeal.assigned_to_acting_judge_as_judge?(acting_judge) }

context "when the attorney review process has happened outside of caseflow" do
context "a decision has not been written for the case" do
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: I think this belongs at the sibling level to when the attorney review process has happened outside of caseflow, rather than under 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.

I set up these examples as "when there is an attorney_case_review" and "when there is not an attorney_case_review", but gave them more "real world" descriptions. So I agree that if the attorney has not reviewed the case, the attorney review process hasn't actually happened, inside or outside of caseflow. But thought this grouped a bit nicer

spec/models/legacy_appeal_spec.rb Outdated Show resolved Hide resolved
spec/models/legacy_appeal_spec.rb Outdated Show resolved Hide resolved
Comment on lines 3028 to 3033
it_behaves_like "assumes the case assigned to the user to draft the decision"

it "falls back to check the presence of a decision document" do
expect_any_instance_of(VACOLS::CaseAssignment).to receive(:valid_document_id?).once
subject
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion/Question: aren't these redundant? I don't think we need the shared example here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is indeed! Removing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

juuuust kidding
the first set of tests checks when no attorney case review is present
this test checks when an attorney case review is present but not associated with the user.
Gotta hit all those conditional branches!

Copy link
Contributor

@lomky lomky left a comment

Choose a reason for hiding this comment

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

Looks great! Just in time for my 🦇 shift too 😹

@hschallhorn hschallhorn added the Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master label Nov 16, 2020
@va-bot va-bot merged commit e59d27d into master Nov 16, 2020
@va-bot va-bot deleted the hschallhorn/better-avlj-task-type-logic branch November 16, 2020 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: caseflow-decisions Priority: Medium Blocking issue w/workaround, or "second in" priority for new work. Product: caseflow-queue Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master Team: Echo 🐬 Type: Enhancement Enhancement to an existing feature User: AVLJ Acting Veterans Law Judge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants