Page MenuHomestyx hydra
Files F127147

No OneTemporary
Actions

View Options

diff --git a/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php b/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php
index 241678487a..b1f51e77e7 100644
--- a/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php
@@ -1,208 +1,219 @@
<?php
abstract class DifferentialRevisionActionTransaction
extends DifferentialRevisionTransactionType {
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(
DifferentialRevision $revision,
PhabricatorUser $viewer);
protected function validateOptionValue($object, $actor, array $value) {
return null;
}
public function getCommandKeyword() {
return null;
}
public function getCommandAliases() {
return array();
}
public function getCommandSummary() {
return null;
}
protected function getRevisionActionOrder() {
return 1000;
}
public function getActionStrength() {
return 300;
}
public function getRevisionActionOrderVector() {
return id(new PhutilSortVector())
->addInt($this->getRevisionActionOrder());
}
protected function getRevisionActionGroupKey() {
return DifferentialRevisionEditEngine::ACTIONGROUP_REVISION;
}
protected function getRevisionActionDescription(
DifferentialRevision $revision,
PhabricatorUser $viewer) {
return null;
}
protected function getRevisionActionSubmitButtonText(
DifferentialRevision $revision,
PhabricatorUser $viewer) {
return null;
}
+ protected function getRevisionActionMetadata(
+ DifferentialRevision $revision,
+ PhabricatorUser $viewer) {
+ return array();
+ }
+
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 getActionOptions(
PhabricatorUser $viewer,
DifferentialRevision $revision) {
return array(
array(),
array(),
);
}
public function newEditField(
DifferentialRevision $revision,
PhabricatorUser $viewer) {
// Actions in the "review" group, like "Accept Revision", do not require
// that the actor be able to edit the revision.
$group_review = DifferentialRevisionEditEngine::ACTIONGROUP_REVIEW;
$is_review = ($this->getRevisionActionGroupKey() == $group_review);
$field = id(new PhabricatorApplyEditField())
->setKey($this->getRevisionActionKey())
->setTransactionType($this->getTransactionTypeConstant())
->setCanApplyWithoutEditCapability($is_review)
->setValue(true);
if ($this->isActionAvailable($revision, $viewer)) {
$label = $this->getRevisionActionLabel($revision, $viewer);
if ($label !== null) {
$field->setCommentActionLabel($label);
$description = $this->getRevisionActionDescription($revision, $viewer);
$field->setActionDescription($description);
$group_key = $this->getRevisionActionGroupKey();
$field->setCommentActionGroupKey($group_key);
$button_text = $this->getRevisionActionSubmitButtonText(
$revision,
$viewer);
$field->setActionSubmitButtonText($button_text);
// Currently, every revision action conflicts with every other
// revision action: for example, you can not simultaneously Accept and
// Reject a revision.
// Under some configurations, some combinations of actions are sort of
// technically permissible. For example, you could reasonably Reject
// and Abandon a revision if "anyone can abandon anything" is enabled.
// It's not clear that these combinations are actually useful, so just
// keep things simple for now.
$field->setActionConflictKey('revision.action');
list($options, $value) = $this->getActionOptions($viewer, $revision);
// Show the options if the user can select on behalf of two or more
// reviewers, or can force-accept on behalf of one or more reviewers,
// or can accept on behalf of a reviewer other than themselves (see
// T12533).
$can_multi = (count($options) > 1);
$can_force = (count($value) < count($options));
$not_self = (head_key($options) != $viewer->getPHID());
if ($can_multi || $can_force || $not_self) {
$field->setOptions($options);
$field->setValue($value);
}
+
+ $metadata = $this->getRevisionActionMetadata($revision, $viewer);
+ foreach ($metadata as $metadata_key => $metadata_value) {
+ $field->setMetadataValue($metadata_key, $metadata_value);
+ }
}
}
return $field;
}
public function validateTransactions($object, array $xactions) {
$errors = array();
$actor = $this->getActor();
$action_exception = null;
foreach ($xactions as $xaction) {
// If this is a draft demotion action, let it skip all the normal
// validation. This is a little hacky and should perhaps move down
// into the actual action implementations, but currently we can not
// apply this rule in validateAction() because it doesn't operate on
// the actual transaction.
if ($xaction->getMetadataValue('draft.demote')) {
continue;
}
try {
$this->validateAction($object, $actor);
} catch (Exception $ex) {
$action_exception = $ex;
}
break;
}
foreach ($xactions as $xaction) {
if ($action_exception) {
$errors[] = $this->newInvalidError(
$action_exception->getMessage(),
$xaction);
continue;
}
$new = $xaction->getNewValue();
if (!is_array($new)) {
continue;
}
try {
$this->validateOptionValue($object, $actor, $new);
} catch (Exception $ex) {
$errors[] = $this->newInvalidError(
$ex->getMessage(),
$xaction);
}
}
return $errors;
}
}
diff --git a/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php b/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php
index 026b57b55c..32a3ab4a73 100644
--- a/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php
@@ -1,112 +1,196 @@
<?php
final class DifferentialRevisionRequestReviewTransaction
extends DifferentialRevisionActionTransaction {
const TRANSACTIONTYPE = 'differential.revision.request';
const ACTIONKEY = 'request-review';
+ const SOURCE_HARBORMASTER = 'harbormaster';
+ const SOURCE_AUTHOR = 'author';
+ const SOURCE_VIEWER = 'viewer';
+
protected function getRevisionActionLabel(
DifferentialRevision $revision,
PhabricatorUser $viewer) {
+
+ // See PHI1810. Allow non-authors to "Request Review" on draft revisions
+ // to promote them out of the draft state. This smoothes over the workflow
+ // where an author asks for review of an urgent change but has not used
+ // "Request Review" to skip builds.
+
+ if ($revision->isDraft()) {
+ if (!$this->isViewerRevisionAuthor($revision, $viewer)) {
+ return pht('Begin Review Now');
+ }
+ }
+
return pht('Request Review');
}
protected function getRevisionActionDescription(
DifferentialRevision $revision,
PhabricatorUser $viewer) {
if ($revision->isDraft()) {
- return pht('This revision will be submitted to reviewers for feedback.');
+ if (!$this->isViewerRevisionAuthor($revision, $viewer)) {
+ return pht(
+ 'This revision will be moved out of the draft state so you can '.
+ 'review it immediately.');
+ } else {
+ return pht(
+ 'This revision will be submitted to reviewers for feedback.');
+ }
} else {
return pht('This revision will be returned to reviewers for feedback.');
}
}
+ protected function getRevisionActionMetadata(
+ DifferentialRevision $revision,
+ PhabricatorUser $viewer) {
+ $map = array();
+
+ if ($revision->isDraft()) {
+ $action_source = $this->getActorSourceType(
+ $revision,
+ $viewer);
+ $map['promotion.source'] = $action_source;
+ }
+
+ return $map;
+ }
+
protected function getRevisionActionSubmitButtonText(
DifferentialRevision $revision,
PhabricatorUser $viewer) {
// See PHI975. When the action stack will promote the revision out of
// draft, change the button text from "Submit Quietly".
if ($revision->isDraft()) {
return pht('Publish Revision');
}
return null;
}
public function getColor() {
return 'sky';
}
protected function getRevisionActionOrder() {
return 200;
}
public function getActionName() {
return pht('Requested Review');
}
public function generateOldValue($object) {
return $object->isNeedsReview();
}
public function applyInternalEffects($object, $value) {
$status_review = DifferentialRevisionStatus::NEEDS_REVIEW;
$object
->setModernRevisionStatus($status_review)
->setShouldBroadcast(true);
}
protected function validateAction($object, PhabricatorUser $viewer) {
if ($object->isNeedsReview()) {
throw new Exception(
pht(
'You can not request review of this revision because this '.
'revision is already under review and the action would have '.
'no effect.'));
}
if ($object->isClosed()) {
throw new Exception(
pht(
'You can not request review of this revision because it has '.
'already been closed. You can only request review of open '.
'revisions.'));
}
- // When revisions automatically promote out of "Draft" after builds finish,
- // the viewer may be acting as the Harbormaster application.
- if (!$viewer->isOmnipotent()) {
- if (!$this->isViewerRevisionAuthor($object, $viewer)) {
- throw new Exception(
- pht(
- 'You can not request review of this revision because you are not '.
- 'the author of the revision.'));
- }
- }
+ $this->getActorSourceType($object, $viewer);
}
public function getTitle() {
- return pht(
- '%s requested review of this revision.',
- $this->renderAuthor());
+ $source = $this->getDraftPromotionSource();
+
+ switch ($source) {
+ case self::SOURCE_HARBORMASTER:
+ case self::SOURCE_VIEWER:
+ case self::SOURCE_AUTHOR:
+ return pht(
+ '%s published this revision for review.',
+ $this->renderAuthor());
+ default:
+ return pht(
+ '%s requested review of this revision.',
+ $this->renderAuthor());
+ }
}
public function getTitleForFeed() {
- return pht(
- '%s requested review of %s.',
- $this->renderAuthor(),
- $this->renderObject());
+ $source = $this->getDraftPromotionSource();
+
+ switch ($source) {
+ case self::SOURCE_HARBORMASTER:
+ case self::SOURCE_VIEWER:
+ case self::SOURCE_AUTHOR:
+ return pht(
+ '%s published %s for review.',
+ $this->renderAuthor(),
+ $this->renderObject());
+ default:
+ return pht(
+ '%s requested review of %s.',
+ $this->renderAuthor(),
+ $this->renderObject());
+ }
}
public function getTransactionTypeForConduit($xaction) {
return 'request-review';
}
public function getFieldValuesForConduit($object, $data) {
return array();
}
+ private function getDraftPromotionSource() {
+ return $this->getMetadataValue('promotion.source');
+ }
+
+ private function getActorSourceType(
+ DifferentialRevision $revision,
+ PhabricatorUser $viewer) {
+
+ $is_harbormaster = $viewer->isOmnipotent();
+ $is_author = $this->isViewerRevisionAuthor($revision, $viewer);
+ $is_draft = $revision->isDraft();
+
+ if ($is_harbormaster) {
+ // When revisions automatically promote out of "Draft" after builds
+ // finish, the viewer may be acting as the Harbormaster application.
+ $source = self::SOURCE_HARBORMASTER;
+ } else if ($is_author) {
+ $source = self::SOURCE_AUTHOR;
+ } else if ($is_draft) {
+ // Non-authors are allowed to "Request Review" on draft revisions, to
+ // force them into review immediately.
+ $source = self::SOURCE_VIEWER;
+ } else {
+ throw new Exception(
+ pht(
+ 'You can not request review of this revision because you are not '.
+ 'the author of the revision and it is not currently a draft.'));
+ }
+
+ return $source;
+ }
+
}

File Metadata

Mime Type
text/x-diff
Expires
Mon, Apr 28, 3:30 AM (9 h, 16 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
107728
Default Alt Text
(13 KB)

Event Timeline

styx copyright 2020 sirocyl / ellie (kymkdd) / ncommander / styx contributors
Use of other names and brands is not intended to reflect a claim of, or right to, use the name or brand.
styx is an operating system using the Linux kernel and components, and "styx Linux" as a phrase does not reflect the inclusion of the Linux brand or trademark in the styx project.
Linux® is the registered trademark of Linus Torvalds in the U.S. and other countries.
Terms and Conditions (TODO: fix this!)