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

Create a Vacate appeal stream for a Motion to Vacate #13325

Merged
merged 11 commits into from
Feb 7, 2020
23 changes: 18 additions & 5 deletions app/models/appeal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ class Appeal < DecisionReview
has_many :record_synced_by_job, as: :record

enum stream_type: {
"Original": "Original",
"Vacate": "Vacate",
"De Novo": "De Novo"
"original": "original",
"vacate": "vacate",
"de_novo": "de_novo"
}

before_save :set_stream_docket_number_and_stream_type
Expand Down Expand Up @@ -81,7 +81,20 @@ def ui_hash
end

def type
stream_type || "Original"
stream_type&.titlecase || "Original"
end

def create_stream(stream_type)
ActiveRecord::Base.transaction do
Appeal.create!(slice(
:receipt_date,
:veteran_file_number,
:legacy_opt_in_approved,
:veteran_is_not_claimant
).merge(stream_type: stream_type, stream_docket_number: docket_number)).tap do |stream|
stream.create_claimant!(participant_id: claimant.participant_id, payee_code: claimant.payee_code)
end
end
end

# Returns the most directly responsible party for an appeal when it is at the Board,
Expand Down Expand Up @@ -434,7 +447,7 @@ def set_stream_docket_number_and_stream_type
if receipt_date && persisted?
self.stream_docket_number ||= docket_number
end
self.stream_type ||= type
self.stream_type ||= type.parameterize.underscore.to_sym
end

def maybe_create_translation_task
Expand Down
2 changes: 1 addition & 1 deletion app/models/decision_issue.rb
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ def create_remand_supplemental_claim!
)
fail AppealDTAPayeeCodeError, decision_review.id unless dta_payee_code

sc.create_claimants!(
sc.create_claimant!(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Small rename to be more accurate

participant_id: decision_review.claimant_participant_id,
payee_code: dta_payee_code
)
Expand Down
2 changes: 1 addition & 1 deletion app/models/decision_review.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ def start_review!
end

# Creates claimants for automatically generated decision reviews
def create_claimants!(participant_id:, payee_code:)
def create_claimant!(participant_id:, payee_code:)
remove_claimants!
claimants.create_without_intake!(participant_id: participant_id, payee_code: payee_code)
end
Expand Down
21 changes: 0 additions & 21 deletions app/models/tasks/judge_sign_motion_to_vacate_task.rb

This file was deleted.

1 change: 0 additions & 1 deletion app/models/tasks/judge_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
# - JudgeDispatchReturnTasks
# - JudgeAssignTasks
# - JudgeAddressMotionToVacateTasks
# - JudgeSignMotionToVacateTasks

class JudgeTask < Task
def available_actions(user)
Expand Down
17 changes: 0 additions & 17 deletions app/models/tasks/straight_vacate_task.rb

This file was deleted.

17 changes: 0 additions & 17 deletions app/models/tasks/vacate_and_de_novo_task.rb

This file was deleted.

17 changes: 0 additions & 17 deletions app/models/tasks/vacate_and_readjudication_task.rb

This file was deleted.

2 changes: 1 addition & 1 deletion app/workflows/initial_tasks_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def initialize(appeal)
def create_root_and_sub_tasks!
create_vso_tracking_tasks
ActiveRecord::Base.transaction do
create_subtasks!
create_subtasks! if @appeal.original?
end
end

Expand Down
101 changes: 59 additions & 42 deletions app/workflows/post_decision_motion_updater.rb
Original file line number Diff line number Diff line change
@@ -1,26 +1,42 @@
# frozen_string_literal: true

##
# The PostDecisionMotionUpdater validates and creates a PostDecisionMotion from the JudgeAddressMotionToVacateTask,
# and creates the subsequent tasks or appeal streams.

class PostDecisionMotionUpdater
include ActiveModel::Model

attr_reader :task, :params
attr_reader :task, :params, :disposition, :instructions
Copy link
Contributor

Choose a reason for hiding this comment

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

My turn for a newbie Rails question...

What does it mean to add attr_reader for :instructions here? That's not something that's saved in the DB for PostDecisionMotion, and instead is just being added to the created task.

Copy link
Contributor

Choose a reason for hiding this comment

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

It just means Ruby (not Rails, iiuc) will automatically make pdmu.instructions and pdmu.instructions= methods for you, to decouple it from the @instructions internal attribute.

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 think since we're using it in two places below, I thought it was nicer to just use "instructions" instead of params[:instructions]. But it's really not a meaningful or necessary change. I don't even think it's less code. It's nicer for disposition though, which is used several times.

attr_reader (and related: attr_writer, and attr_accessor combines read/write) are useful if want something that's not saved in DB columns to be an attribute. Some models don't even have DB tables, so all of their attributes are defined this way (see EndProduct.rb)


def initialize(task, params)
@task = task
@params = params
@disposition = @params[:disposition]
@instructions = @params[:instructions]
end

delegate :appeal, to: :task
Copy link
Contributor

Choose a reason for hiding this comment

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

Neat. I can see how delegate can be quite helpful. :)


def process
ActiveRecord::Base.transaction do
motion = create_motion
return unless motion
return unless post_decision_motion

handle_denial_or_dismissal
handle_grant

create_new_tasks
return if errors.messages.any?

task.update(status: Constants.TASK_STATUSES.completed)
end
end

private

def post_decision_motion
@post_decision_motion ||= create_motion
end

def create_motion
motion = PostDecisionMotion.new(
task: task,
Expand All @@ -32,7 +48,7 @@ def create_motion
motion.vacated_decision_issue_ids = params[:vacated_decision_issue_ids]
elsif disposition == "granted"
# For full grant, auto populate all decision issue IDs
motion.vacated_decision_issue_ids = task.appeal.decision_issues.map(&:id)
motion.vacated_decision_issue_ids = appeal.decision_issues.map(&:id)
end

unless motion.valid?
Expand All @@ -43,96 +59,97 @@ def create_motion
motion.save
end

def create_new_tasks
# We create an AbstractMotionToVacateTask as sibling to the judge task
# to serve as parent for all successive tasks. It is created when associated with
# the new task in order to pass responsibility for validation to child task
# We create an AbstractMotionToVacateTask as sibling to the JudgeAddressMotionToVacateTask
# to serve as parent for successive Denied or Dismissed tasks. It is created when associated with
# the new task in order to pass responsibility for validation to child task
def handle_denial_or_dismissal
return unless denied_or_dismissed?

abstract_task = create_abstract_task
unless abstract_task.valid?
errors.messages.merge!(abstract_task.errors.messages)
return
end

if grant_type?
judge_sign_task = create_judge_sign_task(abstract_task)
end

new_task = create_new_task((judge_sign_task || abstract_task))
new_task = create_new_task(abstract_task)

unless new_task.valid?
errors.messages.merge!(new_task.errors.messages)
return
end
new_task.save
end

def handle_grant
return unless grant_type?

task.update(status: Constants.TASK_STATUSES.completed)
vacate_stream = appeal.create_stream(:vacate)
create_new_stream_tasks(vacate_stream)
end

def create_abstract_task
AbstractMotionToVacateTask.new(
appeal: task.appeal,
appeal: appeal,
parent: task.parent,
assigned_to: task.assigned_to
assigned_to: judge_user
)
end

def create_new_task(parent)
task_class.new(
appeal: task.appeal,
appeal: appeal,
parent: parent,
assigned_by: task.assigned_to,
assigned_to: assigned_to,
instructions: [params[:instructions]]
assigned_by: judge_user,
assigned_to: attorney_user,
instructions: [instructions]
)
end

def create_judge_sign_task(parent)
JudgeSignMotionToVacateTask.new(
appeal: task.appeal,
parent: parent,
assigned_to: task.assigned_to
def create_new_stream_tasks(stream)
InitialTasksFactory.new(stream).create_root_and_sub_tasks!

jdrt = JudgeDecisionReviewTask.create!(appeal: stream, parent: stream.root_task, assigned_to: judge_user)
attorney_task = AttorneyTask.new(
appeal: stream, parent: jdrt, assigned_by: judge_user, assigned_to: attorney_user, instructions: [instructions]
)
end

def disposition
case params[:disposition]
when "partial"
"partially_granted"
else
params[:disposition]
unless attorney_task.valid?
errors.messages.merge!(attorney_task.errors.messages)
return
end

attorney_task.save
end

def task_class
@task_class ||= (task_type + "_task").classify.constantize
end

def task_type
grant_type? ? params[:vacate_type] : "#{params[:disposition]}_motion_to_vacate"
"#{disposition}_motion_to_vacate"
end

def grant_type?
%w[granted partial].include? params[:disposition]
%w[granted partially_granted].include? disposition
Copy link
Contributor Author

@leikkisa leikkisa Feb 3, 2020

Choose a reason for hiding this comment

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

I decided to make the front-end version of the disposition consistent with the backend, partial -> partially_granted

end

def denied_or_dismissed?
%w[denied dismissed].include? disposition
end

def assigned_to
@assigned_to ||= (denied_or_dismissed? ? prev_motions_attorney : User.find_by(id: params[:assigned_to_id]))
def judge_user
task.assigned_to
end

def attorney_user
@attorney_user ||= denied_or_dismissed? ? prev_motions_attorney : User.find_by(id: params[:assigned_to_id])
end

def prev_motions_attorney
mtv_mail_task.assigned_to
task.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.

I think this is clear enough without making a method for it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok newbie question but why are we changing it from mtv_mail_task to task.parent

Copy link
Contributor Author

@leikkisa leikkisa Feb 4, 2020

Choose a reason for hiding this comment

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

@Sjones352 Not a newbie question! This was a completely unnecessary change, I just thought that task.parent.assigned_to was just as clear and the mtv_mail_task method was unneeded.

If you think it's harder to read now, I can put it back.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this works. My original inclination was to keep mtv_mail_task if task.parent is mentioned anywhere else in this file, but the one other place is in create_abstract_task which isn't too topical to mtv_mail_task.

end

def prev_motions_attorney_or_org
prev_motions_attorney.inactive? ? LitigationSupport.singleton : prev_motions_attorney
end

def mtv_mail_task
task.parent
end
end
8 changes: 4 additions & 4 deletions client/app/queue/mtv/MTVJudgeDisposition.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const formatInstructions = ({ disposition, vacateType, hyperlink, instructions }

switch (disposition) {
case 'granted':
case 'partial':
case 'partially_granted':
parts.push(`This will be a ${vacateTypeText(vacateType)}`);
parts.push(instructions);
break;
Expand All @@ -53,7 +53,7 @@ const formatInstructions = ({ disposition, vacateType, hyperlink, instructions }
return parts.join('\n');
};

const grantTypes = ['granted', 'partial'];
const grantTypes = ['granted', 'partially_granted'];

const dispositionStrings = {
denied: 'denial',
Expand Down Expand Up @@ -113,7 +113,7 @@ export const MTVJudgeDisposition = ({
!disposition ||
!instructions ||
(isGrantType() && !vacateType) ||
(disposition === 'partial' && !issueIds.length) ||
(disposition === 'partially_granted' && !issueIds.length) ||
(disposition === 'dismissed' && !hyperlink)
);
};
Expand All @@ -140,7 +140,7 @@ export const MTVJudgeDisposition = ({
value={disposition}
/>

{disposition && disposition === 'partial' && (
{disposition && disposition === 'partially_granted' && (
<MTVIssueSelection
issues={appeal.decisionIssues}
onChange={({ issueIds: newIssueIds }) => setIssueIds(newIssueIds)}
Expand Down
4 changes: 2 additions & 2 deletions client/constants/MOTION_TO_VACATE.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@
],
"PARTIAL_GRANT_OPTION": {
"displayText": "Grant partial vacatur",
"value": "partial",
"value": "partially_granted",
"help": "e.g. if you do not want to vacate all the issues the veteran is requesting"
},
"DISPOSITION_TEXT": {
"granted": "grant or partial vacatur",
"partial": "partial vacatur",
"partially_granted": "partial vacatur",
"denied": "denial of all issues for vacatur",
"dismissed": "dismissal"
}
Expand Down
Loading