End holds set with timed hold tasks #10687
Conversation
Freeing up space in models/task.rb which had gone over its line limit. Also removed timeline_details method from models/task.rb which was no longer being used.
Also, hide TimedHoldTasks from the queue table view.
ghost
assigned 
Generated by 🚫 Danger |
|
Code Climate has analyzed commit 71df9b7 and detected 0 issues on this pull request. View more on Code Climate. |
tomas-nava
changed the title
Not just when parent's status is updated, but when any parameter of the parent (aside from placed_on_hold_at) is updated.
| title: timeline_title, | ||
| date: closed_at | ||
| } | ||
| end |
There was a problem hiding this comment.
Removed because this function was never called?
There was a problem hiding this comment.
Yep, it was defined and used in November '18, then the usage was removed in January but the definition never was.
| @@ -0,0 +1,226 @@ | |||
| # frozen_string_literal: true | |||
|
|
|||
| class TaskActionRepository | |||
There was a problem hiding this comment.
I love this! Such a clean way to separate this stuff out!
| @@ -433,6 +433,10 @@ | |||
| "NON_COMP_ADDRESS_MESSAGE": "The Veteran/appellant has filed a Notice of Disagreement at the Board of Veterans' Appeals. In order to decide that appeal, the Board will need the complete records from", | |||
| "INTAKE_EDIT_TITLE": "Edit contention title", | |||
|
|
|||
| "END_HOLD_MODAL_TITLE": "End hold early", | |||
| "END_HOLD_MODAL_BODY": "Do you want to end the hold early?", | |||
| "END_HOLD_SUCCESS_MESSAGE_TITLE": "Success! The hold has been ended early.", | |||
| @@ -22,6 +22,7 @@ class Task < ApplicationRecord | |||
| before_update :set_timestamps | |||
| after_update :update_parent_status, if: :task_just_closed_and_has_parent? | |||
| after_update :update_children_status_after_closed, if: :task_just_closed? | |||
| after_update :cancel_timed_hold, unless: :task_just_placed_on_hold? | |||
There was a problem hiding this comment.
If we create a child task for a task that is on a timed hold, will we cancel the existing timed hold? If not, I think it is fine to defer that work to a follow on PR since this looks good (and big!) as is and we aren't creating timed holds yet.
There was a problem hiding this comment.
We will; I added a test in this commit.
We may revisit this idea later, possibly as a validation on the TaskTimer object instead of the TimedHoldTask, due to the way in which TaskTimers are instantiated via the concern.
tomas-nava
added
the
Ready-to-Merge
Connects #9207
Description
TimedHoldTasksTaskActionRepositoryclassTimedHoldTaskTimedHoldTaskis invalid if it has more than oneTaskTimer