Page MenuHomePhabricator
Files F13201589

D21403.id.diff
No OneTemporary
Actions

D21403.id.diff
View Options

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
@@ -66,6 +66,12 @@
return null;
}
+ protected function getRevisionActionMetadata(
+ DifferentialRevision $revision,
+ PhabricatorUser $viewer) {
+ return array();
+ }
+
public static function loadAllActions() {
return id(new PhutilClassMapQuery())
->setAncestorClass(__CLASS__)
@@ -150,6 +156,11 @@
$field->setOptions($options);
$field->setValue($value);
}
+
+ $metadata = $this->getRevisionActionMetadata($revision, $viewer);
+ foreach ($metadata as $metadata_key => $metadata_value) {
+ $field->setMetadataValue($metadata_key, $metadata_value);
+ }
}
}
diff --git a/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php b/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php
--- a/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php
@@ -6,9 +6,25 @@
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');
}
@@ -16,12 +32,34 @@
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) {
@@ -76,29 +114,43 @@
'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) {
@@ -109,4 +161,36 @@
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/plain
Expires
May 15 2024, 1:20 PM (5 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6296450
Default Alt Text
D21403.id.diff (6 KB)

Event Timeline

Phabricator · Built by Phacility