Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15283305
D17114.id41157.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
21 KB
Referenced Files
None
Subscribers
None
D17114.id41157.diff
View Options
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
Details
Attached
Mime Type
text/plain
Expires
Wed, Mar 5, 6:43 AM (2 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7223793
Default Alt Text
D17114.id41157.diff (21 KB)
Attached To
Mode
D17114: Allow comment actions to be grouped; group Differential "Review" and "Revision" actions
Attached
Detach File
Event Timeline
Log In to Comment