department-of-veterans-affairs / caseflow

Caseflow is a web application that enables the tracking and processing of appealed claims at the Board of Veterans' Appeals.
Other
54 stars 18 forks source link

Use JudgeTeam membership to determine judges and attorneys #8436

Closed lowellrex closed 1 year ago

lowellrex commented 5 years ago

We currently use the VACOLS.STAFF table to determine if a user is a judge or an attorney (by way of methods on the User model). This leads to problems when employees are misidentified in VACOLS as attorneys and complexity around when somebody is an acting judge. #8117 moved membership in judge teams into the JudgeTeam model which we should be able to use determine whether somebody is a judge or an attorney without relying on VACOLS.

Acceptance criteria

lowellrex commented 5 years ago

LARGE. Will need to confirm that the Caseflow-only way of identifying judges and attorneys (using JudgeTeams) matches what exists in VACOLS and deconflicting the differences with the Board when we discover them (cc @marvokdolor-gov ).

lowellrex commented 5 years ago

Did some pairing with Tomas and winnowed down the list of judges in VACOLS that differ from judges with JudgeTeams in Caseflow to 27. More work to do here, but this should give whoever works on this next a good head start.

AC

Worth noting that "acting judges" do not have judge teams, so wherever we check for the judge vacols role (which acting judges have), we will need to allow for them to pass that check as well (maybe by hitting VACOLS then as a backup?).

rails c> u = JudgeTeam.first.admins.first
> judges_and_acting_judges_in_vacols = VACOLS::Staff.where(sactive: 'A', svlj: ['J', 'A'])
> judges_and_acting_judges_in_vacols.count
# 282
> JudgeTeam.count
# 99

> judges_in_vacols = VACOLS::Staff.where(sactive: 'A', svlj: 'J')
> judges_in_vacols.count
# 132

# Why are there 33 extra judges in VACOLS?

# Are there any acting judges with judge teams?
> acting_judges = VACOLS::Staff.where(sactive: 'A', svlj: 'A')
> acting_judges.count
# 150
> acting_judges_css_ids = acting_judges.pluck(:sdomainid)
> judge_team_css_ids = JudgeTeam.all.pluck(:name)
> acting_judges_without_judge_teams = acting_judges_css_ids - judge_team_css_ids
> acting_judges_without_judge_teams.count
# 150
> acting_judges_without_judge_teams.uniq.count
# 150

> judges_in_vacols = VACOLS::Staff.where(sactive: 'A', svlj: 'J')
> judges_in_vacols_css_ids = judges_in_vacols.pluck(:sdomainid)
> judges_in_vacols_css_ids.count
# 132
> judges_in_vacols_css_ids.uniq.count
# 130

> judges_in_vacols_css_ids = judges_in_vacols_css_ids.uniq
> judge_team_css_ids = JudgeTeam.all.pluck(:name)
> judge_team_not_judges_in_vacols = judge_team_css_ids - judges_in_vacols_css_ids
# []

> judges_in_vacols_wo_judge_teams = judges_in_vacols_css_ids - judge_team_css_ids
> judges_in_vacols_wo_judge_teams.count
# 31

# Get rid of the nil in our list
> judges_in_vacols_wo_judge_teams = judges_in_vacols_wo_judge_teams.compact
> judges_in_vacols_wo_judge_teams.count
# 30

> caseflow_1 = VACOLS::Staff.find_by(sdomainid: 'CASEFLOW1')
# This is a Caseflow staff record. I think we can remove it from our 30.
> judges_in_vacols_wo_judge_teams = judges_in_vacols_wo_judge_teams - ['CASEFLOW1']
> judges_in_vacols_wo_judge_teams.count
# 29

# Who are these RO admins?
> roadmins_css_ids = VACOLS::Staff.where(susrtyp: "ROADMIN").pluck(:sdomainid).compact
> roadmins_css_ids.count
# 11
> (judges_in_vacols_wo_judge_teams - roadmins_css_ids).count
# 29
# No overlap, so disregard!

# Let's take a look at the remaining CSS Ids and see if anything jumps out at us!
> judges_in_vacols_wo_judge_teams.sort
# Looks like the chairman and Nicholas are both on this list. We can remove them.
> judges_in_vacols_wo_judge_teams = judges_in_vacols_wo_judge_teams - ["BVACMASON", "BVANHOLTZ"]
> judges_in_vacols_wo_judge_teams.count
# 27

# Let's look at each remaining judge's name to see if we can smell anything else FISHY.
> VACOLS::Staff.where(sdomainid: judges_in_vacols_wo_judge_teams).pluck(:sdomainid, :snamef, :snamel)

# Keep working from here.
lpciferri commented 5 years ago

FYI - with this change, Supervisory Senior Counsel will still need to request cases like judges do. They have this functionality today, because they look like judges in VACOLS. Let me know if I can provide more info when we pick this back up again.

lpciferri commented 5 years ago

Moved to Ready for dev pipeline because of the earlier estimate comment

lomky commented 5 years ago

Copying in a slack convo around Judge on Legacy Tasks: https://dsva.slack.com/archives/C2ZAMLK88/p1570711992055200

I'm reading up on the JudgeLegacyTasks, and I noticed a discrepancy in how we determine judgeness across the various task types.

JudgeLegacyDecisionReviewTask gives available actions to the judge, and decides if someone is a judge via the method current_user.judge_in_vacols? (see 1) JudgeLegacyAssignTask gives available actions to the judge, and decides if someone is a judge via checking for the role param with value judge (see 2) Relatedly, AttorneyLegacyTask uses the role param. (see 3)

It feels like they should be doing this the same way? Does anyone know which is our preferred way to check the role, or if they're used in different cases which one is best here?

  1. https://github.com/department-of-veterans-affairs/caseflow/blob/master/app/models/legacy_tasks/judge_legacy_decision_review_task.rb#L13
  2. https://github.com/department-of-veterans-affairs/caseflow/blob/master/app/models/legacy_tasks/judge_legacy_assign_task.rb#L5
  3. https://github.com/department-of-veterans-affairs/caseflow/blob/master/app/models/legacy_tasks/attorney_legacy_task.rb#L5