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

[PART 1] Remove attorney task cancellation hook #14448

Merged
merged 17 commits into from
Jun 9, 2020

Conversation

hschallhorn
Copy link
Contributor

@hschallhorn hschallhorn commented Jun 2, 2020

FULL STACK

PART 1: Remove attorney task cancellation hook #14448
PART 2: Perform "caseflow" reassign of attorney tasks #14452
PART 3: Remove overtime status of appeal if case is reassigned #14453

THIS PR

Resolves #14181

PART 1 in stack to implement #14366
PART 2 #14452
PART 3 #14453

Description

Calls send_back_to_judge_assign! directly when cancelling an attorney task rather than relying on a callback.

Acceptance Criteria

  • Hook for sending a case back to judge reassign on attorney task cancellation is removed.

Testing Plan

  1. Log in as BVACASPER
  2. Go to any ama attorney task
  3. Select "Cancel Task & return to judge"
  4. Ensure the attorney and judge decision review task are cancelled and a judge assign task was opened
uuid = ""
Appeal.find_by_uuid(uuid).treee
                                  ┌────────────────────────────────────────────────────────────────────────┐
Appeal 682 (evidence_submission)   ID    STATUS     ASGN_BY      ASGN_TO      UPDATED_AT              
└── RootTask                       2027  on_hold                 Bva          2020-05-22 14:50:19 UTC 
    ├── JudgeDecisionReviewTask    2028  cancelled               BVAAABSHIRE  2020-06-02 18:19:56 UTC 
       └── AttorneyTask           2029  cancelled  BVAAABSHIRE  BVAEERDMAN   2020-06-02 18:19:55 UTC 
    └── JudgeAssignTask            2532  assigned                BVAAABSHIRE  2020-06-02 18:19:56 UTC 
                                  └────────────────────────────────────────────────────────────────────────┘
  1. Cancel any attorney task by hand
task = AttorneyTask.open.last
task.appeal.treee
Appeal 545 (direct_review) ────  ID    STATUS     ASGN_BY      ASGN_TO      UPDATED_AT              
└── RootTask                     1458  on_hold                 Bva          2020-05-22 14:47:39 UTC 
    ├── DistributionTask         1459  completed               Bva          2020-05-22 14:47:39 UTC 
    ├── JudgeAssignTask          1460  completed               BVAAABSHIRE  2020-06-01 14:34:38 UTC 
    ├── JudgeDecisionReviewTask  2493  cancelled  BVAAABSHIRE  BVAAABSHIRE  2020-06-01 14:52:39 UTC 
       └── AttorneyTask         2494  cancelled  BVAAABSHIRE  BVAEERDMAN   2020-06-01 14:52:39 UTC 
    ├── JudgeAssignTask          2495  cancelled               BVAAABSHIRE  2020-06-01 14:53:15 UTC 
    ├── JudgeAssignTask          2496  completed  BVAAABSHIRE  BVAAABSHIRE  2020-06-01 18:17:36 UTC 
    └── JudgeDecisionReviewTask  2523  on_hold    BVARDUNKLE   BVAAABSHIRE  2020-06-01 18:17:36 UTC 
        └── AttorneyTask         2524  assigned   BVARDUNKLE   BVAEERDMAN   2020-06-01 18:17:36 UTC 

=> nil
task.cancelled!
  1. Ensure this does not close the judge review task and does not open a judge assign task
task.appeal.reload.treee
Appeal 545 (direct_review)       ID   STATUS    ASGN_BY     ASGN_TO     UPDATED_AT
└── RootTask                     1458 on_hold               Bva         2020-05-22 14:47:39 UTC
    ├── DistributionTask         1459 completed             Bva         2020-05-22 14:47:39 UTC
    ├── JudgeAssignTask          1460 completed             BVAAABSHIRE 2020-06-01 14:34:38 UTC
    ├── JudgeDecisionReviewTask  2493 cancelled BVAAABSHIRE BVAAABSHIRE 2020-06-01 14:52:39 UTC
       └── AttorneyTask         2494 cancelled BVAAABSHIRE BVAEERDMAN  2020-06-01 14:52:39 UTC
    ├── JudgeAssignTask          2495 cancelled             BVAAABSHIRE 2020-06-01 14:53:15 UTC
    ├── JudgeAssignTask          2496 completed BVAAABSHIRE BVAAABSHIRE 2020-06-01 18:17:36 UTC
    └── JudgeDecisionReviewTask  2523 assigned  BVARDUNKLE  BVAAABSHIRE 2020-06-02 19:45:16 UTC
        └── AttorneyTask         2524 cancelled BVARDUNKLE  BVAEERDMAN  2020-06-02 19:45:16 UTC

User Facing Changes

  • None

if task.is_a?(AttorneyTask) && update_params[:status].eql?(Constants.TASK_STATUSES.cancelled)
task.verify_user_can_update!(current_user)

return task.send_back_to_judge_assign!
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Call send_back_to_judge_assign! directly rather than cancelling the attorney task and relying on the hook to cancel/open tasks

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 a bit grumpy at having this in controller. Thinking about it

Copy link
Contributor

Choose a reason for hiding this comment

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

can we instead override update_from_params inside of AttorneyTask, and do this logic there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes we can! Alec and I were fiddling with where this should go. Didn't want this to have any unintended consequences as we obviously struggled with when it was a cancellation hook.

@hschallhorn hschallhorn self-assigned this Jun 2, 2020
@codeclimate
Copy link

codeclimate bot commented Jun 2, 2020

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

View more on Code Climate.

@@ -246,7 +246,8 @@ def reassign_attorney_tasks
end

def reassign_attorney_task(task)
update_task_status_with_instructions(task, Constants.TASK_STATUSES.cancelled)
task.send_back_to_judge_assign!
task.update_with_instructions(instructions: reassignment_instructions(Constants.TASK_STATUSES.cancelled))
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 were previously relying on the cancellation hook here. Call the function instead.

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.

Some thoughts!

if task.is_a?(AttorneyTask) && update_params[:status].eql?(Constants.TASK_STATUSES.cancelled)
task.verify_user_can_update!(current_user)

return task.send_back_to_judge_assign!
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 a bit grumpy at having this in controller. Thinking about it

if task.is_a?(AttorneyTask) && update_params[:status].eql?(Constants.TASK_STATUSES.cancelled)
task.verify_user_can_update!(current_user)

return task.send_back_to_judge_assign!
Copy link
Contributor

Choose a reason for hiding this comment

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

can we instead override update_from_params inside of AttorneyTask, and do this logic there?

spec/factories/user.rb Show resolved Hide resolved
spec/controllers/tasks_controller_spec.rb Outdated Show resolved Hide resolved
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.

One last optional swap. Hurray unhooking stuff! ↪️ :shipit:

app/models/tasks/attorney_task.rb Outdated Show resolved Hide resolved
spec/factories/user.rb Show resolved Hide resolved
@hschallhorn hschallhorn changed the title Remove attorney task cancellation hook [PART 1] Remove attorney task cancellation hook Jun 4, 2020
* User proper reassign params when reassigning an attorney task

* Remove changes

* [PART 3] Remove overtime status of appeal if case is reassigned (#14453)

* Remove overtime status on judge or attorney reassignment

* Wip

* Properly set overtime to false when reassigning

* Undo changes

* lint

* Clear suvccess messages
@va-bot
Copy link
Collaborator

va-bot commented Jun 9, 2020

1 Warning
⚠️ This PR modifies React components — consider adding/updating corresponding Storybook file

Generated by 🚫 Danger

@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 Jun 9, 2020
@va-bot va-bot merged commit 69d7138 into master Jun 9, 2020
@va-bot va-bot deleted the hschallhorn/14366-remove-atty-task-cancellation-hook branch June 9, 2020 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove AttorneyTask cancellation hook
3 participants