Page MenuHomePhabricator

D17114.diff
No OneTemporary

D17114.diff

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 @@
<?php
final class DifferentialRevisionAcceptTransaction
- extends DifferentialRevisionActionTransaction {
+ extends DifferentialRevisionReviewTransaction {
const TRANSACTIONTYPE = 'differential.revision.accept';
const ACTIONKEY = 'accept';
diff --git a/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php b/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php
--- a/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php
@@ -19,6 +19,10 @@
abstract protected function validateAction($object, PhabricatorUser $viewer);
abstract protected function getRevisionActionLabel();
+ protected function getRevisionActionGroupKey() {
+ return DifferentialRevisionEditEngine::ACTIONGROUP_REVISION;
+ }
+
protected function getRevisionActionDescription() {
return null;
}
@@ -41,116 +45,6 @@
return ($viewer->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 @@
<?php
final class DifferentialRevisionRejectTransaction
- extends DifferentialRevisionActionTransaction {
+ extends DifferentialRevisionReviewTransaction {
const TRANSACTIONTYPE = 'differential.revision.reject';
const ACTIONKEY = 'reject';
diff --git a/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php b/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php
--- a/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php
@@ -1,7 +1,7 @@
<?php
final class DifferentialRevisionResignTransaction
- extends DifferentialRevisionActionTransaction {
+ extends DifferentialRevisionReviewTransaction {
const TRANSACTIONTYPE = 'differential.revision.resign';
const ACTIONKEY = 'resign';
diff --git a/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php b/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
copy from src/applications/differential/xaction/DifferentialRevisionActionTransaction.php
copy to src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
--- a/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
@@ -1,44 +1,10 @@
<?php
-abstract class DifferentialRevisionActionTransaction
- extends DifferentialRevisionTransactionType {
+abstract class DifferentialRevisionReviewTransaction
+ extends DifferentialRevisionActionTransaction {
- final public function getRevisionActionKey() {
- return $this->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 @@
+<?php
+
+final class PhabricatorEditEngineCommentActionGroup
+ extends Phobject {
+
+ private $key;
+ private $label;
+
+ public function setKey($key) {
+ $this->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;
+ }
+
}

File Metadata

Mime Type
text/plain
Expires
May 13 2024, 10:46 PM (9 w, 6 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6273603
Default Alt Text
D17114.diff (21 KB)

Event Timeline