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 @@ -557,6 +557,7 @@ 'DifferentialRevisionRequiredActionResultBucket' => 'applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php', 'DifferentialRevisionResignTransaction' => 'applications/differential/xaction/DifferentialRevisionResignTransaction.php', 'DifferentialRevisionResultBucket' => 'applications/differential/query/DifferentialRevisionResultBucket.php', + 'DifferentialRevisionReviewTransaction' => 'applications/differential/xaction/DifferentialRevisionReviewTransaction.php', 'DifferentialRevisionReviewersHeraldField' => 'applications/differential/herald/DifferentialRevisionReviewersHeraldField.php', 'DifferentialRevisionReviewersTransaction' => 'applications/differential/xaction/DifferentialRevisionReviewersTransaction.php', 'DifferentialRevisionSearchConduitAPIMethod' => 'applications/differential/conduit/DifferentialRevisionSearchConduitAPIMethod.php', @@ -2554,6 +2555,7 @@ 'PhabricatorEditEngineAPIMethod' => 'applications/transactions/editengine/PhabricatorEditEngineAPIMethod.php', 'PhabricatorEditEngineColumnsCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEngineColumnsCommentAction.php', 'PhabricatorEditEngineCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEngineCommentAction.php', + 'PhabricatorEditEngineCommentActionGroup' => 'applications/transactions/commentaction/PhabricatorEditEngineCommentActionGroup.php', 'PhabricatorEditEngineConfiguration' => 'applications/transactions/storage/PhabricatorEditEngineConfiguration.php', 'PhabricatorEditEngineConfigurationDefaultCreateController' => 'applications/transactions/controller/PhabricatorEditEngineConfigurationDefaultCreateController.php', 'PhabricatorEditEngineConfigurationDefaultsController' => 'applications/transactions/controller/PhabricatorEditEngineConfigurationDefaultsController.php', @@ -5183,7 +5185,7 @@ 'PhabricatorConduitResultInterface', ), 'DifferentialRevisionAbandonTransaction' => 'DifferentialRevisionActionTransaction', - 'DifferentialRevisionAcceptTransaction' => 'DifferentialRevisionActionTransaction', + 'DifferentialRevisionAcceptTransaction' => 'DifferentialRevisionReviewTransaction', 'DifferentialRevisionActionTransaction' => 'DifferentialRevisionTransactionType', 'DifferentialRevisionAffectedFilesHeraldField' => 'DifferentialRevisionHeraldField', 'DifferentialRevisionAuthorHeraldField' => 'DifferentialRevisionHeraldField', @@ -5223,7 +5225,7 @@ 'DifferentialRevisionPlanChangesTransaction' => 'DifferentialRevisionActionTransaction', 'DifferentialRevisionQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'DifferentialRevisionReclaimTransaction' => 'DifferentialRevisionActionTransaction', - 'DifferentialRevisionRejectTransaction' => 'DifferentialRevisionActionTransaction', + 'DifferentialRevisionRejectTransaction' => 'DifferentialRevisionReviewTransaction', 'DifferentialRevisionRelationship' => 'PhabricatorObjectRelationship', 'DifferentialRevisionRelationshipSource' => 'PhabricatorObjectRelationshipSource', 'DifferentialRevisionReopenTransaction' => 'DifferentialRevisionActionTransaction', @@ -5232,8 +5234,9 @@ 'DifferentialRevisionRepositoryTransaction' => 'DifferentialRevisionTransactionType', 'DifferentialRevisionRequestReviewTransaction' => 'DifferentialRevisionActionTransaction', 'DifferentialRevisionRequiredActionResultBucket' => 'DifferentialRevisionResultBucket', - 'DifferentialRevisionResignTransaction' => 'DifferentialRevisionActionTransaction', + 'DifferentialRevisionResignTransaction' => 'DifferentialRevisionReviewTransaction', 'DifferentialRevisionResultBucket' => 'PhabricatorSearchResultBucket', + 'DifferentialRevisionReviewTransaction' => 'DifferentialRevisionActionTransaction', 'DifferentialRevisionReviewersHeraldField' => 'DifferentialRevisionHeraldField', 'DifferentialRevisionReviewersTransaction' => 'DifferentialRevisionTransactionType', 'DifferentialRevisionSearchConduitAPIMethod' => 'PhabricatorSearchEngineAPIMethod', @@ -7536,6 +7539,7 @@ 'PhabricatorEditEngineAPIMethod' => 'ConduitAPIMethod', 'PhabricatorEditEngineColumnsCommentAction' => 'PhabricatorEditEngineCommentAction', 'PhabricatorEditEngineCommentAction' => 'Phobject', + 'PhabricatorEditEngineCommentActionGroup' => 'Phobject', 'PhabricatorEditEngineConfiguration' => array( 'PhabricatorSearchDAO', 'PhabricatorApplicationTransactionInterface', diff --git a/src/applications/differential/editor/DifferentialRevisionEditEngine.php b/src/applications/differential/editor/DifferentialRevisionEditEngine.php --- a/src/applications/differential/editor/DifferentialRevisionEditEngine.php +++ b/src/applications/differential/editor/DifferentialRevisionEditEngine.php @@ -9,6 +9,9 @@ const KEY_UPDATE = 'update'; + const ACTIONGROUP_REVIEW = 'review'; + const ACTIONGROUP_REVISION = 'revision'; + public function getEngineName() { return pht('Revisions'); } @@ -87,6 +90,17 @@ return $this->diff; } + protected function newCommentActionGroups() { + return array( + id(new PhabricatorEditEngineCommentActionGroup()) + ->setKey(self::ACTIONGROUP_REVIEW) + ->setLabel(pht('Review Actions')), + id(new PhabricatorEditEngineCommentActionGroup()) + ->setKey(self::ACTIONGROUP_REVISION) + ->setLabel(pht('Revision Actions')), + ); + } + protected function buildCustomEditFields($object) { $viewer = $this->getViewer(); diff --git a/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php --- a/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php @@ -1,7 +1,7 @@ getPHID() === $revision->getAuthorPHID()); } - protected function isViewerAnyReviewer( - DifferentialRevision $revision, - PhabricatorUser $viewer) { - return ($this->getViewerReviewerStatus($revision, $viewer) !== null); - } - - protected function isViewerAcceptingReviewer( - DifferentialRevision $revision, - PhabricatorUser $viewer) { - return $this->isViewerReviewerStatusAmong( - $revision, - $viewer, - array( - DifferentialReviewerStatus::STATUS_ACCEPTED, - )); - } - - protected function isViewerRejectingReviewer( - DifferentialRevision $revision, - PhabricatorUser $viewer) { - return $this->isViewerReviewerStatusAmong( - $revision, - $viewer, - array( - DifferentialReviewerStatus::STATUS_REJECTED, - )); - } - - protected function getViewerReviewerStatus( - DifferentialRevision $revision, - PhabricatorUser $viewer) { - - if (!$viewer->getPHID()) { - return null; - } - - foreach ($revision->getReviewerStatus() as $reviewer) { - if ($reviewer->getReviewerPHID() != $viewer->getPHID()) { - continue; - } - - return $reviewer->getStatus(); - } - - return null; - } - - protected function isViewerReviewerStatusAmong( - DifferentialRevision $revision, - PhabricatorUser $viewer, - array $status_list) { - - $status = $this->getViewerReviewerStatus($revision, $viewer); - if ($status === null) { - return false; - } - - $status_map = array_fuse($status_list); - return isset($status_map[$status]); - } - - protected function applyReviewerEffect( - DifferentialRevision $revision, - PhabricatorUser $viewer, - $value, - $status) { - - $map = array(); - - // When you accept or reject, you may accept or reject on behalf of all - // reviewers you have authority for. When you resign, you only affect - // yourself. - $with_authority = ($status != DifferentialReviewerStatus::STATUS_RESIGNED); - if ($with_authority) { - foreach ($revision->getReviewerStatus() as $reviewer) { - if ($reviewer->hasAuthority($viewer)) { - $map[$reviewer->getReviewerPHID()] = $status; - } - } - } - - // In all cases, you affect yourself. - $map[$viewer->getPHID()] = $status; - - // Convert reviewer statuses into edge data. - foreach ($map as $reviewer_phid => $reviewer_status) { - $map[$reviewer_phid] = array( - 'data' => array( - 'status' => $reviewer_status, - ), - ); - } - - $src_phid = $revision->getPHID(); - $edge_type = DifferentialRevisionHasReviewerEdgeType::EDGECONST; - - $editor = new PhabricatorEdgeEditor(); - foreach ($map as $dst_phid => $edge_data) { - if ($status == DifferentialReviewerStatus::STATUS_RESIGNED) { - // TODO: For now, we just remove these reviewers. In the future, we will - // store resignations explicitly. - $editor->removeEdge($src_phid, $edge_type, $dst_phid); - } else { - $editor->addEdge($src_phid, $edge_type, $dst_phid, $edge_data); - } - } - - $editor->save(); - } - public function newEditField( DifferentialRevision $revision, PhabricatorUser $viewer) { @@ -167,6 +61,9 @@ $description = $this->getRevisionActionDescription(); $field->setActionDescription($description); + + $group_key = $this->getRevisionActionGroupKey(); + $field->setCommentActionGroupKey($group_key); } } diff --git a/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php b/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php --- a/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php @@ -1,7 +1,7 @@ getPhobjectClassConstant('ACTIONKEY', 32); - } - - public function isActionAvailable($object, PhabricatorUser $viewer) { - try { - $this->validateAction($object, $viewer); - return true; - } catch (Exception $ex) { - return false; - } - } - - abstract protected function validateAction($object, PhabricatorUser $viewer); - abstract protected function getRevisionActionLabel(); - - protected function getRevisionActionDescription() { - return null; - } - - public static function loadAllActions() { - return id(new PhutilClassMapQuery()) - ->setAncestorClass(__CLASS__) - ->setUniqueMethod('getRevisionActionKey') - ->execute(); - } - - protected function isViewerRevisionAuthor( - DifferentialRevision $revision, - PhabricatorUser $viewer) { - - if (!$viewer->getPHID()) { - return false; - } - - return ($viewer->getPHID() === $revision->getAuthorPHID()); + protected function getRevisionActionGroupKey() { + return DifferentialRevisionEditEngine::ACTIONGROUP_REVIEW; } protected function isViewerAnyReviewer( @@ -151,48 +117,4 @@ $editor->save(); } - public function newEditField( - DifferentialRevision $revision, - PhabricatorUser $viewer) { - - $field = id(new PhabricatorApplyEditField()) - ->setKey($this->getRevisionActionKey()) - ->setTransactionType($this->getTransactionTypeConstant()) - ->setValue(true); - - if ($this->isActionAvailable($revision, $viewer)) { - $label = $this->getRevisionActionLabel(); - if ($label !== null) { - $field->setCommentActionLabel($label); - - $description = $this->getRevisionActionDescription(); - $field->setActionDescription($description); - } - } - - return $field; - } - - public function validateTransactions($object, array $xactions) { - $errors = array(); - $actor = $this->getActor(); - - $action_exception = null; - try { - $this->validateAction($object, $actor); - } catch (Exception $ex) { - $action_exception = $ex; - } - - foreach ($xactions as $xaction) { - if ($action_exception) { - $errors[] = $this->newInvalidError( - $action_exception->getMessage(), - $xaction); - } - } - - return $errors; - } - } diff --git a/src/applications/transactions/commentaction/PhabricatorEditEngineCommentAction.php b/src/applications/transactions/commentaction/PhabricatorEditEngineCommentAction.php --- a/src/applications/transactions/commentaction/PhabricatorEditEngineCommentAction.php +++ b/src/applications/transactions/commentaction/PhabricatorEditEngineCommentAction.php @@ -7,6 +7,7 @@ private $value; private $initialValue; private $order; + private $groupKey; abstract public function getPHUIXControlType(); abstract public function getPHUIXControlSpecification(); @@ -20,6 +21,15 @@ return $this->key; } + public function setGroupKey($group_key) { + $this->groupKey = $group_key; + return $this; + } + + public function getGroupKey() { + return $this->groupKey; + } + public function setLabel($label) { $this->label = $label; return $this; diff --git a/src/applications/transactions/commentaction/PhabricatorEditEngineCommentActionGroup.php b/src/applications/transactions/commentaction/PhabricatorEditEngineCommentActionGroup.php new file mode 100644 --- /dev/null +++ b/src/applications/transactions/commentaction/PhabricatorEditEngineCommentActionGroup.php @@ -0,0 +1,27 @@ +key = $key; + return $this; + } + + public function getKey() { + return $this->key; + } + + public function setLabel($label) { + $this->label = $label; + return $this; + } + + public function getLabel() { + return $this->label; + } + +} diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -1549,6 +1549,9 @@ $view->setCommentActions($comment_actions); + $comment_groups = $this->newCommentActionGroups(); + $view->setCommentActionGroups($comment_groups); + return $view; } @@ -2157,6 +2160,10 @@ return $request->getURIData('editAction'); } + protected function newCommentActionGroups() { + return array(); + } + /* -( Form Pages )--------------------------------------------------------- */ diff --git a/src/applications/transactions/editfield/PhabricatorEditField.php b/src/applications/transactions/editfield/PhabricatorEditField.php --- a/src/applications/transactions/editfield/PhabricatorEditField.php +++ b/src/applications/transactions/editfield/PhabricatorEditField.php @@ -25,6 +25,7 @@ private $commentActionLabel; private $commentActionValue; + private $commentActionGroupKey; private $commentActionOrder = 1000; private $hasCommentActionValue; @@ -245,6 +246,15 @@ return $this->commentActionLabel; } + public function setCommentActionGroupKey($key) { + $this->commentActionGroupKey = $key; + return $this; + } + + public function getCommentActionGroupKey() { + return $this->commentActionGroupKey; + } + public function setCommentActionOrder($order) { $this->commentActionOrder = $order; return $this; @@ -719,7 +729,8 @@ ->setKey($this->getKey()) ->setLabel($label) ->setValue($this->getValueForCommentAction($value)) - ->setOrder($this->getCommentActionOrder()); + ->setOrder($this->getCommentActionOrder()) + ->setGroupKey($this->getCommentActionGroupKey()); return $action; } diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php --- a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php +++ b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php @@ -24,6 +24,7 @@ private $currentVersion; private $versionedDraft; private $commentActions; + private $commentActionGroups = array(); private $transactionTimeline; public function setObjectPHID($object_phid) { @@ -118,6 +119,16 @@ return $this->commentActions; } + public function setCommentActionGroups(array $groups) { + assert_instances_of($groups, 'PhabricatorEditEngineCommentActionGroup'); + $this->commentActionGroups = $groups; + return $this; + } + + public function getCommentActionGroups() { + return $this->commentActionGroups; + } + public function setNoPermission($no_permission) { $this->noPermission = $no_permission; return $this; @@ -279,16 +290,13 @@ 'type' => $comment_action->getPHUIXControlType(), 'spec' => $comment_action->getPHUIXControlSpecification(), 'initialValue' => $comment_action->getInitialValue(), + 'groupKey' => $comment_action->getGroupKey(), ); $type_map[$key] = $comment_action; } - $options = array(); - $options['+'] = pht('Add Action...'); - foreach ($action_map as $key => $item) { - $options[$key] = $item['label']; - } + $options = $this->newCommentActionOptions($action_map); $action_id = celerity_generate_unique_node_id(); $input_id = celerity_generate_unique_node_id(); @@ -424,4 +432,46 @@ return $this->commentID; } + private function newCommentActionOptions(array $action_map) { + $options = array(); + $options['+'] = pht('Add Action...'); + + // Merge options into groups. + $groups = array(); + foreach ($action_map as $key => $item) { + $group_key = $item['groupKey']; + if (!isset($groups[$group_key])) { + $groups[$group_key] = array(); + } + $groups[$group_key][$key] = $item; + } + + $group_specs = $this->getCommentActionGroups(); + $group_labels = mpull($group_specs, 'getLabel', 'getKey'); + + // Reorder groups to put them in the same order as the recognized + // group definitions. + $groups = array_select_keys($groups, array_keys($group_labels)) + $groups; + + // Move options with no group to the end. + $default_group = idx($groups, ''); + if ($default_group) { + unset($groups['']); + $groups[''] = $default_group; + } + + foreach ($groups as $group_key => $group_items) { + if (strlen($group_key)) { + $group_label = idx($group_labels, $group_key, $group_key); + $options[$group_label] = ipull($group_items, 'label'); + } else { + foreach ($group_items as $key => $item) { + $options[$key] = $item['label']; + } + } + } + + return $options; + } + }