diff --git a/resources/sql/autopatches/20150724.herald.4.sql b/resources/sql/autopatches/20150724.herald.4.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20150724.herald.4.sql @@ -0,0 +1,13 @@ +UPDATE {$NAMESPACE}_herald.herald_actionrecord a + JOIN {$NAMESPACE}_herald.herald_rule r + ON a.ruleID = r.id + SET a.action = 'maniphest.assign.other' + WHERE r.ruleType != 'personal' + AND a.action = 'assigntask'; + +UPDATE {$NAMESPACE}_herald.herald_actionrecord a + JOIN {$NAMESPACE}_herald.herald_rule r + ON a.ruleID = r.id + SET a.action = 'maniphest.assign.self' + WHERE r.ruleType = 'personal' + AND a.action = 'assigntask'; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1013,6 +1013,7 @@ 'HeraldAdapter' => 'applications/herald/adapter/HeraldAdapter.php', 'HeraldAlwaysField' => 'applications/herald/field/HeraldAlwaysField.php', 'HeraldAnotherRuleField' => 'applications/herald/field/HeraldAnotherRuleField.php', + 'HeraldApplicationActionGroup' => 'applications/herald/action/HeraldApplicationActionGroup.php', 'HeraldApplyTranscript' => 'applications/herald/storage/transcript/HeraldApplyTranscript.php', 'HeraldBasicFieldGroup' => 'applications/herald/field/HeraldBasicFieldGroup.php', 'HeraldCommitAdapter' => 'applications/diffusion/herald/HeraldCommitAdapter.php', @@ -1186,6 +1187,9 @@ 'ManiphestStatusEmailCommand' => 'applications/maniphest/command/ManiphestStatusEmailCommand.php', 'ManiphestSubpriorityController' => 'applications/maniphest/controller/ManiphestSubpriorityController.php', 'ManiphestTask' => 'applications/maniphest/storage/ManiphestTask.php', + 'ManiphestTaskAssignHeraldAction' => 'applications/maniphest/herald/ManiphestTaskAssignHeraldAction.php', + 'ManiphestTaskAssignOtherHeraldAction' => 'applications/maniphest/herald/ManiphestTaskAssignOtherHeraldAction.php', + 'ManiphestTaskAssignSelfHeraldAction' => 'applications/maniphest/herald/ManiphestTaskAssignSelfHeraldAction.php', 'ManiphestTaskAssigneeHeraldField' => 'applications/maniphest/herald/ManiphestTaskAssigneeHeraldField.php', 'ManiphestTaskAuthorHeraldField' => 'applications/maniphest/herald/ManiphestTaskAuthorHeraldField.php', 'ManiphestTaskAuthorPolicyRule' => 'applications/maniphest/policyrule/ManiphestTaskAuthorPolicyRule.php', @@ -4710,6 +4714,7 @@ 'HeraldAdapter' => 'Phobject', 'HeraldAlwaysField' => 'HeraldField', 'HeraldAnotherRuleField' => 'HeraldField', + 'HeraldApplicationActionGroup' => 'HeraldActionGroup', 'HeraldApplyTranscript' => 'Phobject', 'HeraldBasicFieldGroup' => 'HeraldFieldGroup', 'HeraldCommitAdapter' => 'HeraldAdapter', @@ -4922,6 +4927,9 @@ 'PhabricatorProjectInterface', 'PhabricatorSpacesInterface', ), + 'ManiphestTaskAssignHeraldAction' => 'HeraldAction', + 'ManiphestTaskAssignOtherHeraldAction' => 'ManiphestTaskAssignHeraldAction', + 'ManiphestTaskAssignSelfHeraldAction' => 'ManiphestTaskAssignHeraldAction', 'ManiphestTaskAssigneeHeraldField' => 'ManiphestTaskHeraldField', 'ManiphestTaskAuthorHeraldField' => 'ManiphestTaskHeraldField', 'ManiphestTaskAuthorPolicyRule' => 'PhabricatorPolicyRule', diff --git a/src/applications/herald/action/HeraldApplicationActionGroup.php b/src/applications/herald/action/HeraldApplicationActionGroup.php new file mode 100644 --- /dev/null +++ b/src/applications/herald/action/HeraldApplicationActionGroup.php @@ -0,0 +1,15 @@ + pht('Trigger an Audit by'), - self::ACTION_ASSIGN_TASK => pht('Assign task to'), self::ACTION_ADD_REVIEWERS => pht('Add reviewers'), self::ACTION_ADD_BLOCKING_REVIEWERS => pht('Add blocking reviewers'), self::ACTION_APPLY_BUILD_PLANS => pht('Run build plans'), @@ -729,7 +727,6 @@ case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: $standard = array( self::ACTION_AUDIT => pht('Trigger an Audit by me'), - self::ACTION_ASSIGN_TASK => pht('Assign task to me'), self::ACTION_ADD_REVIEWERS => pht('Add me as a reviewer'), self::ACTION_ADD_BLOCKING_REVIEWERS => pht('Add me as a blocking reviewer'), @@ -772,7 +769,6 @@ if ($rule_type == HeraldRuleTypeConfig::RULE_TYPE_PERSONAL) { switch ($action->getAction()) { case self::ACTION_AUDIT: - case self::ACTION_ASSIGN_TASK: case self::ACTION_ADD_REVIEWERS: case self::ACTION_ADD_BLOCKING_REVIEWERS: // For personal rules, force these actions to target the rule owner. @@ -812,16 +808,12 @@ if ($is_personal) { switch ($action) { case self::ACTION_AUDIT: - case self::ACTION_ASSIGN_TASK: case self::ACTION_ADD_REVIEWERS: case self::ACTION_ADD_BLOCKING_REVIEWERS: return new HeraldEmptyFieldValue(); } } else { switch ($action) { - case self::ACTION_ASSIGN_TASK: - return $this->buildTokenizerFieldValue( - new PhabricatorPeopleDatasource()); case self::ACTION_AUDIT: case self::ACTION_ADD_REVIEWERS: case self::ACTION_ADD_BLOCKING_REVIEWERS: diff --git a/src/applications/herald/controller/HeraldTranscriptController.php b/src/applications/herald/controller/HeraldTranscriptController.php --- a/src/applications/herald/controller/HeraldTranscriptController.php +++ b/src/applications/herald/controller/HeraldTranscriptController.php @@ -233,7 +233,9 @@ $rule_list = id(new PHUIObjectItemListView()) ->setNoDataString(pht('No Herald rules applied to this object.')); - foreach ($xscript->getRuleTranscripts() as $rule_xscript) { + $rule_xscripts = $xscript->getRuleTranscripts(); + $rule_xscripts = msort($rule_xscripts, 'getRuleID'); + foreach ($rule_xscripts as $rule_xscript) { $rule_id = $rule_xscript->getRuleID(); $rule_item = id(new PHUIObjectItemView()) diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php --- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php +++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php @@ -506,23 +506,6 @@ ->setTask($object); } - protected function didApplyHeraldRules( - PhabricatorLiskDAO $object, - HeraldAdapter $adapter, - HeraldTranscript $transcript) { - - $xactions = array(); - - $assign_phid = $adapter->getAssignPHID(); - if ($assign_phid) { - $xactions[] = id(new ManiphestTransaction()) - ->setTransactionType(ManiphestTransaction::TYPE_OWNER) - ->setNewValue($assign_phid); - } - - return $xactions; - } - protected function requireCapabilities( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { diff --git a/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php b/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php --- a/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php +++ b/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php @@ -3,7 +3,6 @@ final class HeraldManiphestTaskAdapter extends HeraldAdapter { private $task; - private $assignPHID; protected function newObject() { return new ManiphestTask(); @@ -55,61 +54,12 @@ return $this->task; } - public function setAssignPHID($assign_phid) { - $this->assignPHID = $assign_phid; - return $this; - } - public function getAssignPHID() { - return $this->assignPHID; - } - public function getAdapterContentName() { return pht('Maniphest Tasks'); } - public function getActions($rule_type) { - switch ($rule_type) { - case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL: - return array_merge( - array( - self::ACTION_ASSIGN_TASK, - ), - parent::getActions($rule_type)); - case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: - return array_merge( - array( - self::ACTION_ASSIGN_TASK, - ), - parent::getActions($rule_type)); - } - } - public function getHeraldName() { return 'T'.$this->getTask()->getID(); } - public function applyHeraldEffects(array $effects) { - assert_instances_of($effects, 'HeraldEffect'); - - $result = array(); - foreach ($effects as $effect) { - $action = $effect->getAction(); - switch ($action) { - case self::ACTION_ASSIGN_TASK: - $target_array = $effect->getTarget(); - $assign_phid = reset($target_array); - $this->setAssignPHID($assign_phid); - $result[] = new HeraldApplyTranscript( - $effect, - true, - pht('Assigned task.')); - break; - default: - $result[] = $this->applyStandardEffect($effect); - break; - } - } - return $result; - } - } diff --git a/src/applications/maniphest/herald/ManiphestTaskAssignHeraldAction.php b/src/applications/maniphest/herald/ManiphestTaskAssignHeraldAction.php new file mode 100644 --- /dev/null +++ b/src/applications/maniphest/herald/ManiphestTaskAssignHeraldAction.php @@ -0,0 +1,116 @@ +logEffect(self::DO_EMPTY); + return; + } + + $adapter = $this->getAdapter(); + $object = $adapter->getObject(); + + if ($object->getOwnerPHID() == $phid) { + $this->logEffect(self::DO_ALREADY, array($phid)); + return; + } + + $user = id(new PhabricatorPeopleQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withPHIDs(array($phid)) + ->executeOne(); + if (!$user) { + $this->logEffect(self::DO_INVALID, array($phid)); + return; + } + + $can_view = PhabricatorPolicyFilter::hasCapability( + $user, + $object, + PhabricatorPolicyCapability::CAN_VIEW); + if (!$can_view) { + $this->logEffect(self::DO_PERMISSION, array($phid)); + return; + } + + $xaction = $adapter->newTransaction() + ->setTransactionType(ManiphestTransaction::TYPE_OWNER) + ->setNewValue($phid); + + $adapter->queueTransaction($xaction); + + $this->logEffect(self::DO_ASSIGN, array($phid)); + } + + protected function getActionEffectMap() { + return array( + self::DO_EMPTY => array( + 'icon' => 'fa-ban', + 'color' => 'grey', + 'name' => pht('Empty Action'), + ), + self::DO_ALREADY => array( + 'icon' => 'fa-user', + 'color' => 'grey', + 'name' => pht('Already Assigned'), + ), + self::DO_INVALID => array( + 'icon' => 'fa-ban', + 'color' => 'red', + 'name' => pht('Invalid Owner'), + ), + self::DO_PERMISSION => array( + 'icon' => 'fa-ban', + 'color' => 'red', + 'name' => pht('No Permission'), + ), + self::DO_ASSIGN => array( + 'icon' => 'fa-user', + 'color' => 'green', + 'name' => pht('Assigned Task'), + ), + ); + } + + public function renderActionEffectDescription($type, $data) { + switch ($type) { + case self::DO_EMPTY: + return pht('Action lists no user to assign.'); + case self::DO_ALREADY: + return pht( + 'User is already task owner: %s.', + $this->renderHandleList($data)); + case self::DO_INVALID: + return pht( + 'User is invalid: %s.', + $this->renderHandleList($data)); + case self::DO_PERMISSION: + return pht( + 'User does not have permission to see task: %s.', + $this->renderHandleList($data)); + case self::DO_ASSIGN: + return pht( + 'Assigned task to: %s.', + $this->renderHandleList($data)); + } + } + +} diff --git a/src/applications/maniphest/herald/ManiphestTaskAssignOtherHeraldAction.php b/src/applications/maniphest/herald/ManiphestTaskAssignOtherHeraldAction.php new file mode 100644 --- /dev/null +++ b/src/applications/maniphest/herald/ManiphestTaskAssignOtherHeraldAction.php @@ -0,0 +1,34 @@ +applyAssign($effect->getTarget()); + } + + public function getHeraldActionStandardType() { + return self::STANDARD_PHID_LIST; + } + + protected function getDatasource() { + // TODO: Eventually, it would be nice to get "limit = 1" exported from here + // up to the UI. + return new PhabricatorPeopleDatasource(); + } + + public function renderActionDescription($value) { + return pht('Assign task to: %s.', $this->renderHandleList($value)); + } + +} diff --git a/src/applications/maniphest/herald/ManiphestTaskAssignSelfHeraldAction.php b/src/applications/maniphest/herald/ManiphestTaskAssignSelfHeraldAction.php new file mode 100644 --- /dev/null +++ b/src/applications/maniphest/herald/ManiphestTaskAssignSelfHeraldAction.php @@ -0,0 +1,29 @@ +getRule()->getAuthorPHID(); + return $this->applyAssign(array($phid)); + } + + public function getHeraldActionStandardType() { + return self::STANDARD_NONE; + } + + public function renderActionDescription($value) { + return pht('Assign task to rule author.'); + } + +}