Page MenuHomePhabricator
Files F14407499

D17513.diff
No OneTemporary
Actions

D17513.diff
View Options

diff --git a/src/applications/differential/conduit/DifferentialCreateCommentConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialCreateCommentConduitAPIMethod.php
--- a/src/applications/differential/conduit/DifferentialCreateCommentConduitAPIMethod.php
+++ b/src/applications/differential/conduit/DifferentialCreateCommentConduitAPIMethod.php
@@ -56,11 +56,28 @@
$xactions = array();
+ $modular_map = array(
+ 'accept' => DifferentialRevisionAcceptTransaction::TRANSACTIONTYPE,
+ 'reject' => DifferentialRevisionRejectTransaction::TRANSACTIONTYPE,
+ 'resign' => DifferentialRevisionResignTransaction::TRANSACTIONTYPE,
+ );
+
$action = $request->getValue('action');
- if ($action && ($action != 'comment') && ($action != 'none')) {
+ if (isset($modular_map[$action])) {
$xactions[] = id(new DifferentialTransaction())
- ->setTransactionType(DifferentialTransaction::TYPE_ACTION)
- ->setNewValue($action);
+ ->setTransactionType($modular_map[$action])
+ ->setNewValue(true);
+ } else if ($action) {
+ switch ($action) {
+ case 'comment':
+ case 'none':
+ break;
+ default:
+ $xactions[] = id(new DifferentialTransaction())
+ ->setTransactionType(DifferentialTransaction::TYPE_ACTION)
+ ->setNewValue($action);
+ break;
+ }
}
$content = $request->getValue('message');
diff --git a/src/applications/differential/constants/DifferentialAction.php b/src/applications/differential/constants/DifferentialAction.php
--- a/src/applications/differential/constants/DifferentialAction.php
+++ b/src/applications/differential/constants/DifferentialAction.php
@@ -119,37 +119,4 @@
return $title;
}
- public static function getActionVerb($action) {
- $verbs = array(
- self::ACTION_COMMENT => pht('Comment'),
- self::ACTION_ACCEPT => pht("Accept Revision \xE2\x9C\x94"),
- self::ACTION_REJECT => pht("Request Changes \xE2\x9C\x98"),
- self::ACTION_RETHINK => pht("Plan Changes \xE2\x9C\x98"),
- self::ACTION_ABANDON => pht('Abandon Revision'),
- self::ACTION_REQUEST => pht('Request Review'),
- self::ACTION_RECLAIM => pht('Reclaim Revision'),
- self::ACTION_RESIGN => pht('Resign as Reviewer'),
- self::ACTION_ADDREVIEWERS => pht('Add Reviewers'),
- self::ACTION_ADDCCS => pht('Add Subscribers'),
- self::ACTION_CLOSE => pht('Close Revision'),
- self::ACTION_CLAIM => pht('Commandeer Revision'),
- self::ACTION_REOPEN => pht('Reopen'),
- );
-
- if (!empty($verbs[$action])) {
- return $verbs[$action];
- } else {
- return pht('brazenly %s', $action);
- }
- }
-
- public static function allowReviewers($action) {
- if ($action == self::ACTION_ADDREVIEWERS ||
- $action == self::ACTION_REQUEST ||
- $action == self::ACTION_RESIGN) {
- return true;
- }
- return false;
- }
-
}
diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -130,33 +130,6 @@
$action_type = $xaction->getNewValue();
switch ($action_type) {
- case DifferentialAction::ACTION_ACCEPT:
- case DifferentialAction::ACTION_REJECT:
- if ($action_type == DifferentialAction::ACTION_ACCEPT) {
- $new_status = DifferentialReviewerStatus::STATUS_ACCEPTED;
- } else {
- $new_status = DifferentialReviewerStatus::STATUS_REJECTED;
- }
-
- $actor = $this->getActor();
-
- // These transactions can cause effects in two ways: by altering the
- // status of an existing reviewer; or by adding the actor as a new
- // reviewer.
-
- $will_add_reviewer = true;
- foreach ($object->getReviewerStatus() as $reviewer) {
- if ($reviewer->hasAuthority($actor)) {
- if ($reviewer->getStatus() != $new_status) {
- return true;
- }
- }
- if ($reviewer->getReviewerPHID() == $actor_phid) {
- $will_add_reviwer = false;
- }
- }
-
- return $will_add_reviewer;
case DifferentialAction::ACTION_CLOSE:
return ($object->getStatus() != $status_closed);
case DifferentialAction::ACTION_ABANDON:
@@ -169,13 +142,6 @@
return ($object->getStatus() != $status_plan);
case DifferentialAction::ACTION_REQUEST:
return ($object->getStatus() != $status_review);
- case DifferentialAction::ACTION_RESIGN:
- foreach ($object->getReviewerStatus() as $reviewer) {
- if ($reviewer->getReviewerPHID() == $actor_phid) {
- return true;
- }
- }
- return false;
case DifferentialAction::ACTION_CLAIM:
return ($actor_phid != $object->getAuthorPHID());
}
@@ -222,12 +188,6 @@
return;
case DifferentialTransaction::TYPE_ACTION:
switch ($xaction->getNewValue()) {
- case DifferentialAction::ACTION_RESIGN:
- case DifferentialAction::ACTION_ACCEPT:
- case DifferentialAction::ACTION_REJECT:
- // These have no direct effects, and affect review status only
- // indirectly by altering reviewers with TYPE_EDGE transactions.
- return;
case DifferentialAction::ACTION_ABANDON:
$object->setStatus(ArcanistDifferentialRevisionStatus::ABANDONED);
return;
@@ -482,59 +442,9 @@
$action_type = $xaction->getNewValue();
switch ($action_type) {
- case DifferentialAction::ACTION_ACCEPT:
- case DifferentialAction::ACTION_REJECT:
- if ($action_type == DifferentialAction::ACTION_ACCEPT) {
- $data = array(
- 'status' => DifferentialReviewerStatus::STATUS_ACCEPTED,
- );
- } else {
- $data = array(
- 'status' => DifferentialReviewerStatus::STATUS_REJECTED,
- );
- }
-
- $edits = array();
-
- foreach ($object->getReviewerStatus() as $reviewer) {
- if ($reviewer->hasAuthority($actor)) {
- $edits[$reviewer->getReviewerPHID()] = array(
- 'data' => $data,
- );
- }
- }
-
- // Also either update or add the actor themselves as a reviewer.
- $edits[$actor_phid] = array(
- 'data' => $data,
- );
-
- $results[] = id(new DifferentialTransaction())
- ->setTransactionType($type_edge)
- ->setMetadataValue('edge:type', $edge_reviewer)
- ->setIgnoreOnNoEffect(true)
- ->setNewValue(array('+' => $edits));
- break;
-
case DifferentialAction::ACTION_CLAIM:
$is_commandeer = true;
break;
- case DifferentialAction::ACTION_RESIGN:
- // If the user is resigning, add a separate reviewer edit
- // transaction which removes them as a reviewer.
-
- $results[] = id(new DifferentialTransaction())
- ->setTransactionType($type_edge)
- ->setMetadataValue('edge:type', $edge_reviewer)
- ->setIgnoreOnNoEffect(true)
- ->setNewValue(
- array(
- '-' => array(
- $actor_phid => $actor_phid,
- ),
- ));
-
- break;
}
break;
}
@@ -925,60 +835,6 @@
$status_closed = ArcanistDifferentialRevisionStatus::CLOSED;
switch ($action) {
- case DifferentialAction::ACTION_ACCEPT:
- if ($actor_is_author && !$allow_self_accept) {
- return pht(
- 'You can not accept this revision because you are the owner.');
- }
-
- if ($revision_status == $status_abandoned) {
- return pht(
- 'You can not accept this revision because it has been '.
- 'abandoned.');
- }
-
- if ($revision_status == $status_closed) {
- return pht(
- 'You can not accept this revision because it has already been '.
- 'closed.');
- }
-
- // TODO: It would be nice to make this generic at some point.
- $signatures = DifferentialRequiredSignaturesField::loadForRevision(
- $revision);
- foreach ($signatures as $phid => $signed) {
- if (!$signed) {
- return pht(
- 'You can not accept this revision because the author has '.
- 'not signed all of the required legal documents.');
- }
- }
-
- break;
-
- case DifferentialAction::ACTION_REJECT:
- if ($actor_is_author) {
- return pht('You can not request changes to your own revision.');
- }
-
- if ($revision_status == $status_abandoned) {
- return pht(
- 'You can not request changes to this revision because it has been '.
- 'abandoned.');
- }
-
- if ($revision_status == $status_closed) {
- return pht(
- 'You can not request changes to this revision because it has '.
- 'already been closed.');
- }
- break;
-
- case DifferentialAction::ACTION_RESIGN:
- // You can always resign from a revision if you're a reviewer. If you
- // aren't, this is a no-op rather than invalid.
- break;
-
case DifferentialAction::ACTION_CLAIM:
// You can claim a revision if you're not the owner. If you are, this
// is a no-op rather than invalid.

File Metadata

Mime Type
text/plain
Expires
Wed, Dec 25, 3:01 AM (8 h, 38 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6924620
Default Alt Text
D17513.diff (9 KB)

Event Timeline

Phabricator · Built by Phacility