-
Notifications
You must be signed in to change notification settings - Fork 19
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
Enable all tasks to be placed on timed hold #10728
Conversation
Code Climate has analyzed commit 9d272ae and detected 0 issues on this pull request. View more on Code Climate. |
multi_transaction do | ||
if parent_task.is_a?(Task) | ||
parent_task.update!(instructions: [parent_task.instructions, instructions].flatten.compact) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adds instructions to the task that is being placed on hold so that the instructions display in the task snapshot.
@@ -10,17 +10,22 @@ class TimedHoldTask < GenericTask | |||
after_create :cancel_active_siblings | |||
|
|||
attr_accessor :days_on_hold | |||
validates :days_on_hold, presence: true, inclusion: { in: 1..100 }, on: :create | |||
validates :days_on_hold, presence: true, inclusion: { in: 1..120 }, on: :create |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
120 days is a preset option for the colocated on hold length, so this matches that.
}); | ||
} | ||
|
||
render = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Majority of this render function is copied from the existing ColocatedPlaceHoldView
component that this modal will eventually replace entirely.
I didn't bother to factor out any of this into shared functions because the old component will be going away in short order as part of #9207.
@@ -62,4 +62,80 @@ | |||
end | |||
end | |||
end | |||
|
|||
describe "timed holds" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Front-end tests for this new feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙆♂️looks good to me!
@@ -58,6 +61,8 @@ def available_actions(user) | |||
|
|||
[] | |||
end | |||
# rubocop:enable Metrics/AbcSize | |||
# rubocop:enable Metrics/MethodLength |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was the reason for disabling Rubocop here? Why not extract the different arrays into well-named methods that represent the different action types? For example:
def available_actions(user)
return [] unless user
return actions_when_task_assigned_to_user if assigned_to == user
if task_is_assigned_to_user_within_organization?(user)
return actions_when_task_is_assigned_to_user_within_organization
end
if task_is_assigned_to_users_organization?(user)
return actions_when_task_is_assigned_to_users_organization
end
[]
end
def actions_when_task_is_assigned_to_user
[
Constants.TASK_ACTIONS.ASSIGN_TO_TEAM.to_h,
Constants.TASK_ACTIONS.REASSIGN_TO_PERSON.to_h,
appropriate_timed_hold_task_action,
Constants.TASK_ACTIONS.MARK_COMPLETE.to_h,
Constants.TASK_ACTIONS.CANCEL_TASK.to_h
]
end
def actions_when_task_is_assigned_to_user_within_organization
[
Constants.TASK_ACTIONS.REASSIGN_TO_PERSON.to_h
]
end
def actions_when_task_is_assigned_to_users_organization
[
Constants.TASK_ACTIONS.ASSIGN_TO_TEAM.to_h,
Constants.TASK_ACTIONS.ASSIGN_TO_PERSON.to_h,
Constants.TASK_ACTIONS.MARK_COMPLETE.to_h,
Constants.TASK_ACTIONS.CANCEL_TASK.to_h
]
end
Connects #9207.
ColocatedTask
sColocatedTask
s