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

Allow SCM team members to move AMA cases #13786

Merged
merged 17 commits into from
Mar 30, 2020
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .reek.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ detectors:
- TaskSorter#sort_requires_case_norm?
- VBMSCaseflowLogger#log
- Veteran
- JudgeDecisionReviewTask#additional_available_actions
UncommunicativeVariableName:
exclude:
- Address
Expand Down
17 changes: 13 additions & 4 deletions app/models/tasks/attorney_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ class AttorneyTask < Task
after_update :send_back_to_judge_assign, if: :attorney_task_just_cancelled?

def available_actions(user)
# Both the judge who assigned this task and the judge who is assigned the parent review task get these actions
if parent.is_a?(JudgeTask) && (parent.assigned_to == user || assigned_by == user)
if can_be_moved_by_user?(user)
return [
Constants.TASK_ACTIONS.ASSIGN_TO_ATTORNEY.to_h,
Constants.TASK_ACTIONS.CANCEL_TASK.to_h
Expand Down Expand Up @@ -54,6 +53,14 @@ def stays_with_reassigned_parent?

private

def can_be_moved_by_user?(user)
return false unless parent.is_a?(JudgeTask)

# The judge who is assigned the parent review task, the assigning judge, and SpecialCaseMovementTeam members can
# cancel or reassign this task
parent.assigned_to == user || assigned_by == user || user&.can_act_on_behalf_of_judges?
end

def child_attorney_tasks_are_completed
if parent&.children_attorney_tasks&.open&.any?
errors.add(:parent, "has open child tasks")
Expand All @@ -65,7 +72,9 @@ def assigned_to_role_is_valid
end

def assigned_by_role_is_valid
errors.add(:assigned_by, "has to be a judge") if assigned_by && !assigned_by.judge_in_vacols?
if assigned_by && (!assigned_by.judge_in_vacols? && !assigned_by.can_act_on_behalf_of_judges?)
errors.add(:assigned_by, "has to be a judge or special case movement team member")
end
end

def attorney_task_just_cancelled?
Expand All @@ -84,6 +93,6 @@ def cancel_parent_judge_review
end

def open_judge_assign_task
JudgeAssignTask.create!(appeal: appeal, parent: appeal.root_task, assigned_to: assigned_by)
JudgeAssignTask.create!(appeal: appeal, parent: appeal.root_task, assigned_to: parent.assigned_to)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ensures the JudgeAssignTask does not get assigned to the assigning SCM user if an attorney task is cancelled.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure the Judge on the JDRTask is who should always be the assigning judge in the case of cancelling? Pondering this.

Copy link
Contributor

Choose a reason for hiding this comment

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

How does this affect AVLJ cases?

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'm trying to think of a normal scenario that this would affect, but cannot come up with one.
Judge assigns a case to Attorney. They are both the assigner of the attorney task and the assignee of the attorney tasks parent. The parent cannot be reassigned until the attorney task is completed. If cancelled, it will go back to the Judge either way.
I don't know of a scenario that would affect AVLJ cases either.

end
end
4 changes: 3 additions & 1 deletion app/models/tasks/judge_decision_review_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
class JudgeDecisionReviewTask < JudgeTask
before_create :verify_user_task_unique

def additional_available_actions(_user)
def additional_available_actions(user)
return [] unless assigned_to == user

lomky marked this conversation as resolved.
Show resolved Hide resolved
judge_checkout_label = if ama?
Constants.TASK_ACTIONS.JUDGE_AMA_CHECKOUT.to_h
else
Expand Down
5 changes: 5 additions & 0 deletions app/models/tasks/judge_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ def available_actions(user)
Constants.TASK_ACTIONS.REASSIGN_TO_JUDGE.to_h,
additional_available_actions(user)
].flatten
elsif user&.can_act_on_behalf_of_judges?
[
Constants.TASK_ACTIONS.REASSIGN_TO_JUDGE.to_h,
additional_available_actions(user)
].flatten
lomky marked this conversation as resolved.
Show resolved Hide resolved
else
[]
end
Expand Down
8 changes: 4 additions & 4 deletions app/repositories/task_action_repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ def mail_assign_to_organization_data(task, _user = nil)
end

def cancel_task_data(task, _user = nil)
assigner_name = task.assigned_by&.full_name || "the assigner"
return_to_name = task.is_a?(AttorneyTask) ? task.parent.assigned_to.full_name : task.assigned_by&.full_name
{
modal_title: COPY::CANCEL_TASK_MODAL_TITLE,
modal_body: format(COPY::CANCEL_TASK_MODAL_DETAIL, assigner_name),
modal_body: format(COPY::CANCEL_TASK_MODAL_DETAIL, return_to_name || "the assigner"),
message_title: format(COPY::CANCEL_TASK_CONFIRMATION, task.appeal.veteran_full_name),
message_detail: format(COPY::MARK_TASK_COMPLETE_CONFIRMATION_DETAIL, assigner_name)
}
Expand Down Expand Up @@ -118,10 +118,10 @@ def judge_dispatch_return_to_attorney_data(task, _user = nil)
}
end

def assign_to_attorney_data(task, _user = nil)
def assign_to_attorney_data(task, user)
{
selected: nil,
options: nil,
options: user.can_act_on_behalf_of_judges? ? users_to_options(Attorney.list_all) : nil,
lomky marked this conversation as resolved.
Show resolved Hide resolved
type: task.is_a?(LegacyTask) ? AttorneyLegacyTask.name : AttorneyTask.name
}
end
Expand Down
4 changes: 3 additions & 1 deletion app/workflows/attorney_task_creator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ def call
attr_reader :judge_assign_task, :task_params

def tasks
judge_review_task = JudgeDecisionReviewTask.create!(judge_assign_task.slice(:appeal, :assigned_to, :parent))
judge_review_task = JudgeDecisionReviewTask.create!(
judge_assign_task.slice(:appeal, :assigned_to, :parent).merge(assigned_by: task_params[:assigned_by])
)
lomky marked this conversation as resolved.
Show resolved Hide resolved
judge_assign_task.update!(status: Constants.TASK_STATUSES.completed)
attorney_task = AttorneyTask.create!(task_params.merge(parent: judge_review_task))
[attorney_task, judge_review_task, judge_assign_task]
Expand Down
20 changes: 17 additions & 3 deletions client/app/queue/AssignToAttorneyModalView.jsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import * as React from 'react';
import { connect } from 'react-redux';
import { bindActionCreators } from 'redux';
import PropTypes from 'prop-types';

import { AssignWidgetModal } from './components/AssignWidget';

import COPY from '../../COPY.json';
import COPY from '../../COPY';

import {
taskById
Expand Down Expand Up @@ -37,7 +38,7 @@ class AssignToAttorneyModalView extends React.PureComponent {
}

render = () => {
const { task, userId } = this.props;
const { task, userId, match } = this.props;
const previousAssigneeId = task ? task.assignedTo.id.toString() : null;

if (!previousAssigneeId) {
Expand All @@ -46,16 +47,29 @@ class AssignToAttorneyModalView extends React.PureComponent {

return <AssignWidgetModal
isModal
match={match}
lomky marked this conversation as resolved.
Show resolved Hide resolved
userId={userId}
onTaskAssignment={this.handleAssignment}
previousAssigneeId={previousAssigneeId}
selectedTasks={[task]} />;
}
}

AssignToAttorneyModalView.propTypes = {
task: PropTypes.shape({
assignedTo: PropTypes.shape({
id: PropTypes.number
})
}),
userId: PropTypes.string,
match: PropTypes.object,
initialAssignTasksToUser: PropTypes.func,
reassignTasksToUser: PropTypes.func
};

const mapStateToProps = (state, ownProps) => {
return {
task: taskById(state, { taskId: ownProps.taskId })
task: taskById(state, { taskId: ownProps.match.params.taskId })
};
};

Expand Down
6 changes: 3 additions & 3 deletions client/app/queue/CaseDetailsLoadingScreen.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import PropTypes from 'prop-types';

import { onReceiveAppealDetails, onReceiveTasks, setAttorneysOfJudge, fetchAllAttorneys } from './QueueActions';

class CaseDetailLoadingScreen extends React.PureComponent {
class CaseDetailsLoadingScreen extends React.PureComponent {
lomky marked this conversation as resolved.
Show resolved Hide resolved
loadActiveAppealAndTask = () => {
const {
appealId,
Expand Down Expand Up @@ -113,7 +113,7 @@ const mapDispatchToProps = (dispatch) => bindActionCreators({
fetchAllAttorneys
}, dispatch);

CaseDetailLoadingScreen.propTypes = {
CaseDetailsLoadingScreen.propTypes = {
children: PropTypes.array,
appealId: PropTypes.string,
userId: PropTypes.number,
Expand All @@ -125,4 +125,4 @@ CaseDetailLoadingScreen.propTypes = {
fetchAllAttorneys: PropTypes.func
};

export default (connect(mapStateToProps, mapDispatchToProps)(CaseDetailLoadingScreen));
export default (connect(mapStateToProps, mapDispatchToProps)(CaseDetailsLoadingScreen));
2 changes: 1 addition & 1 deletion client/app/queue/QueueApp.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ class QueueApp extends React.PureComponent {

routedAdvancedOnDocketMotion = (props) => <AdvancedOnDocketMotionView {...props.match.params} />;

routedAssignToAttorney = (props) => <AssignToAttorneyModalView userId={this.props.userId} {...props.match.params} />;
routedAssignToAttorney = (props) => <AssignToAttorneyModalView userId={this.props.userId} match={props.match} />;
lomky marked this conversation as resolved.
Show resolved Hide resolved

routedAssignToSingleTeam = (props) => <AssignToView isTeamAssign assigneeAlreadySelected {...props.match.params} />;

Expand Down
18 changes: 14 additions & 4 deletions client/app/queue/components/AssignWidget.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import pluralize from 'pluralize';
import COPY from '../../../COPY';
import { sprintf } from 'sprintf-js';
import { fullWidth } from '../constants';
import { taskActionData } from '../utils';

import QueueFlowModal from './QueueFlowModal';

const OTHER = 'OTHER';
Expand Down Expand Up @@ -131,14 +133,22 @@ class AssignWidget extends React.PureComponent {
let placeholderOther = COPY.ASSIGN_WIDGET_LOADING;
let selectedOptionOther = null;

if (attorneys.error) {
placeholderOther = COPY.ASSIGN_WIDGET_ERROR_LOADING_ATTORNEYS;
}

if (attorneys.data) {
optionsOther = attorneys.data.map(optionFromAttorney);
placeholderOther = COPY.ASSIGN_WIDGET_DROPDOWN_PLACEHOLDER;
selectedOptionOther = _.find(optionsOther, (option) => option.value === selectedAssigneeSecondary);
} else if (this.props.isModal) {
optionsOther = taskActionData({
...this.props,
task: selectedTasks[0]
})?.options;
lomky marked this conversation as resolved.
Show resolved Hide resolved
}

if (attorneys.error) {
placeholderOther = COPY.ASSIGN_WIDGET_ERROR_LOADING_ATTORNEYS;
if (optionsOther?.length) {
placeholderOther = COPY.ASSIGN_WIDGET_DROPDOWN_PLACEHOLDER;
selectedOptionOther = _.find(optionsOther, (option) => option.value === selectedAssigneeSecondary);
}

const Widget = <React.Fragment>
Expand Down
68 changes: 43 additions & 25 deletions spec/controllers/judge_assign_tasks_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
describe "POST /judge_assign_tasks" do
let!(:attorney) { create(:user) }
let!(:judge) { create(:user) }
let!(:user) { judge }
let!(:second_judge) { create(:user) }
let!(:attorney_staff) { create(:staff, :attorney_role, sdomainid: attorney.css_id) }
let!(:judge_staff) { create(:staff, :judge_role, sdomainid: judge.css_id) }
Expand All @@ -21,41 +22,58 @@
end
end

before { User.authenticate!(user: judge) }
before { User.authenticate!(user: user) }

subject { post :create, params: { tasks: params } }

context "when cases will be assigned to an attorney" do
it "returns the newly created tasks along with their new parents and old parents" do
subject
shared_examples "attorney task assignment" do
it "returns the newly created tasks along with their new parents and old parents" do
subject

expect(response.status).to eq 200
response_body = JSON.parse(response.body)["tasks"]["data"]
expect(response_body.count).to eq 9
expect(response.status).to eq 200
response_body = JSON.parse(response.body)["tasks"]["data"]
expect(response_body.count).to eq 9

attorney_tasks = response_body.select { |task| task["attributes"]["type"].eql? AttorneyTask.name }
expect(attorney_tasks.count).to eq 3
attorney_tasks.each do |attorney_task|
expect(attorney_task["attributes"]["assigned_to"]["id"]).to eq attorney.id
expect(attorney_task["attributes"]["assigned_by"]["pg_id"]).to eq judge.id
expect(attorney_task["attributes"]["status"]).to eq Constants.TASK_STATUSES.assigned
expect(AttorneyTask.find(attorney_task["id"]).parent.type).to eq JudgeDecisionReviewTask.name
attorney_tasks = response_body.select { |task| task["attributes"]["type"].eql? AttorneyTask.name }
expect(attorney_tasks.count).to eq 3
attorney_tasks.each do |attorney_task|
expect(attorney_task["attributes"]["assigned_to"]["id"]).to eq attorney.id
expect(attorney_task["attributes"]["assigned_by"]["pg_id"]).to eq user.id
expect(attorney_task["attributes"]["status"]).to eq Constants.TASK_STATUSES.assigned
expect(AttorneyTask.find(attorney_task["id"]).parent.type).to eq JudgeDecisionReviewTask.name
end

review_tasks = response_body.select { |task| task["attributes"]["type"].eql? JudgeDecisionReviewTask.name }
expect(review_tasks.count).to eq 3
review_tasks.each do |review_task|
judge_review_task = JudgeDecisionReviewTask.find(review_task["id"])
expect(review_task["attributes"]["assigned_to"]["id"]).to eq judge.id
expect(review_task["attributes"]["assigned_by"]["pg_id"]).to eq user.id
expect(judge_review_task.children.first.type).to eq AttorneyTask.name
expect(judge_review_task.reload.status).to eq Constants.TASK_STATUSES.on_hold
end

assign_tasks = response_body.select { |task| task["attributes"]["type"].eql? JudgeAssignTask.name }
expect(assign_tasks.count).to eq 3
expect(assign_tasks.all? do |assign_task|
assign_task["attributes"]["status"].eql? Constants.TASK_STATUSES.completed
end).to eq true
end
end

it_behaves_like "attorney task assignment"

context "when the assigner is a member of the scm team" do
let(:user) { create(:user) }

review_tasks = response_body.select { |task| task["attributes"]["type"].eql? JudgeDecisionReviewTask.name }
expect(review_tasks.count).to eq 3
review_tasks.each do |review_task|
judge_review_task = JudgeDecisionReviewTask.find(review_task["id"])
expect(review_task["attributes"]["assigned_to"]["id"]).to eq judge.id
expect(judge_review_task.children.first.type).to eq AttorneyTask.name
expect(judge_review_task.reload.status).to eq Constants.TASK_STATUSES.on_hold
before do
SpecialCaseMovementTeam.singleton.add_user(user)
FeatureToggle.enable!(:scm_view_judge_assign_queue)
end
after { FeatureToggle.disable!(:scm_view_judge_assign_queue) }

assign_tasks = response_body.select { |task| task["attributes"]["type"].eql? JudgeAssignTask.name }
expect(assign_tasks.count).to eq 3
expect(assign_tasks.all? do |assign_task|
assign_task["attributes"]["status"].eql? Constants.TASK_STATUSES.completed
end).to eq true
it_behaves_like "attorney task assignment"
lomky marked this conversation as resolved.
Show resolved Hide resolved
end
end

Expand Down
5 changes: 4 additions & 1 deletion spec/controllers/post_decision_motions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,10 @@

body = JSON.parse(response.body)
expect(body["errors"]).to match_array(
[{ "detail" => "Assigned by has to be a judge, Assigned to has to be an attorney" }]
[{
"detail" =>
"Assigned by has to be a judge or special case movement team member, Assigned to has to be an attorney"
}]
)
end
end
Expand Down
35 changes: 0 additions & 35 deletions spec/controllers/tasks_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -274,41 +274,6 @@

subject { post :create, params: { tasks: params } }

context "Attorney task" do
context "when current user is a judge" do
let(:ama_appeal) { create(:appeal) }
let(:ama_judge_task) { create(:ama_judge_task, assigned_to: user, appeal: ama_appeal) }
let(:role) { :judge_role }

let(:params) do
[{
"external_id": ama_appeal.uuid,
"type": AttorneyTask.name,
"assigned_to_id": attorney.id,
"parent_id": ama_judge_task.id
}]
end

it "should be successful" do
subject

expect(response.status).to eq 200

response_body = JSON.parse(response.body)["tasks"]["data"]
expect(response_body.second["attributes"]["type"]).to eq AttorneyTask.name
expect(response_body.second["attributes"]["appeal_id"]).to eq ama_appeal.id
expect(response_body.second["attributes"]["docket_number"]).to eq ama_appeal.docket_number
expect(response_body.second["attributes"]["appeal_type"]).to eq Appeal.name

attorney_task = AttorneyTask.find_by(appeal: ama_appeal)
expect(attorney_task.status).to eq Constants.TASK_STATUSES.assigned
expect(attorney_task.assigned_to).to eq attorney
expect(attorney_task.parent_id).to eq ama_judge_task.id
expect(ama_judge_task.reload.status).to eq Constants.TASK_STATUSES.on_hold
end
end
end

lomky marked this conversation as resolved.
Show resolved Hide resolved
context "VSO user" do
let(:user) { create(:default_user, roles: ["VSO"]) }
let(:vso) { create(:vso) }
Expand Down
12 changes: 12 additions & 0 deletions spec/factories/appeal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,18 @@
end
end

## Appeal with a realistic task tree
## The appeal is assigned to a judge at decision review
## Leaves incorrectly open & incomplete Hearing / Evidence Window task branches
## for those dockets. Strongly suggest you provide a judge and attorney.
trait :at_judge_review do
at_attorney_drafting
after(:create) do |appeal|
create(:decision_document, appeal: appeal)
appeal.tasks.where(type: AttorneyTask.name).first.completed!
end
end
hschallhorn marked this conversation as resolved.
Show resolved Hide resolved

trait :with_straight_vacate_stream do
after(:create) do |appeal, evaluator|
mail_task = create(
Expand Down
Loading