Page MenuHomePhabricator
Files F17817894

D17113.id41155.diff
No OneTemporary
Actions

D17113.id41155.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
@@ -506,6 +506,7 @@
'DifferentialReviewersView' => 'applications/differential/view/DifferentialReviewersView.php',
'DifferentialRevision' => 'applications/differential/storage/DifferentialRevision.php',
'DifferentialRevisionAbandonTransaction' => 'applications/differential/xaction/DifferentialRevisionAbandonTransaction.php',
+ 'DifferentialRevisionAcceptTransaction' => 'applications/differential/xaction/DifferentialRevisionAcceptTransaction.php',
'DifferentialRevisionActionTransaction' => 'applications/differential/xaction/DifferentialRevisionActionTransaction.php',
'DifferentialRevisionAffectedFilesHeraldField' => 'applications/differential/herald/DifferentialRevisionAffectedFilesHeraldField.php',
'DifferentialRevisionAuthorHeraldField' => 'applications/differential/herald/DifferentialRevisionAuthorHeraldField.php',
@@ -545,6 +546,7 @@
'DifferentialRevisionPlanChangesTransaction' => 'applications/differential/xaction/DifferentialRevisionPlanChangesTransaction.php',
'DifferentialRevisionQuery' => 'applications/differential/query/DifferentialRevisionQuery.php',
'DifferentialRevisionReclaimTransaction' => 'applications/differential/xaction/DifferentialRevisionReclaimTransaction.php',
+ 'DifferentialRevisionRejectTransaction' => 'applications/differential/xaction/DifferentialRevisionRejectTransaction.php',
'DifferentialRevisionRelationship' => 'applications/differential/relationships/DifferentialRevisionRelationship.php',
'DifferentialRevisionRelationshipSource' => 'applications/search/relationship/DifferentialRevisionRelationshipSource.php',
'DifferentialRevisionReopenTransaction' => 'applications/differential/xaction/DifferentialRevisionReopenTransaction.php',
@@ -553,6 +555,7 @@
'DifferentialRevisionRepositoryTransaction' => 'applications/differential/xaction/DifferentialRevisionRepositoryTransaction.php',
'DifferentialRevisionRequestReviewTransaction' => 'applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php',
'DifferentialRevisionRequiredActionResultBucket' => 'applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php',
+ 'DifferentialRevisionResignTransaction' => 'applications/differential/xaction/DifferentialRevisionResignTransaction.php',
'DifferentialRevisionResultBucket' => 'applications/differential/query/DifferentialRevisionResultBucket.php',
'DifferentialRevisionReviewersHeraldField' => 'applications/differential/herald/DifferentialRevisionReviewersHeraldField.php',
'DifferentialRevisionReviewersTransaction' => 'applications/differential/xaction/DifferentialRevisionReviewersTransaction.php',
@@ -5180,6 +5183,7 @@
'PhabricatorConduitResultInterface',
),
'DifferentialRevisionAbandonTransaction' => 'DifferentialRevisionActionTransaction',
+ 'DifferentialRevisionAcceptTransaction' => 'DifferentialRevisionActionTransaction',
'DifferentialRevisionActionTransaction' => 'DifferentialRevisionTransactionType',
'DifferentialRevisionAffectedFilesHeraldField' => 'DifferentialRevisionHeraldField',
'DifferentialRevisionAuthorHeraldField' => 'DifferentialRevisionHeraldField',
@@ -5219,6 +5223,7 @@
'DifferentialRevisionPlanChangesTransaction' => 'DifferentialRevisionActionTransaction',
'DifferentialRevisionQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'DifferentialRevisionReclaimTransaction' => 'DifferentialRevisionActionTransaction',
+ 'DifferentialRevisionRejectTransaction' => 'DifferentialRevisionActionTransaction',
'DifferentialRevisionRelationship' => 'PhabricatorObjectRelationship',
'DifferentialRevisionRelationshipSource' => 'PhabricatorObjectRelationshipSource',
'DifferentialRevisionReopenTransaction' => 'DifferentialRevisionActionTransaction',
@@ -5227,6 +5232,7 @@
'DifferentialRevisionRepositoryTransaction' => 'DifferentialRevisionTransactionType',
'DifferentialRevisionRequestReviewTransaction' => 'DifferentialRevisionActionTransaction',
'DifferentialRevisionRequiredActionResultBucket' => 'DifferentialRevisionResultBucket',
+ 'DifferentialRevisionResignTransaction' => 'DifferentialRevisionActionTransaction',
'DifferentialRevisionResultBucket' => 'PhabricatorSearchResultBucket',
'DifferentialRevisionReviewersHeraldField' => 'DifferentialRevisionHeraldField',
'DifferentialRevisionReviewersTransaction' => 'DifferentialRevisionTransactionType',
diff --git a/src/applications/differential/constants/DifferentialReviewerStatus.php b/src/applications/differential/constants/DifferentialReviewerStatus.php
--- a/src/applications/differential/constants/DifferentialReviewerStatus.php
+++ b/src/applications/differential/constants/DifferentialReviewerStatus.php
@@ -9,6 +9,7 @@
const STATUS_COMMENTED = 'commented';
const STATUS_ACCEPTED_OLDER = 'accepted-older';
const STATUS_REJECTED_OLDER = 'rejected-older';
+ const STATUS_RESIGNED = 'resigned';
/**
* Returns the relative strength of a status, used to pick a winner when a
@@ -34,6 +35,7 @@
self::STATUS_ACCEPTED => 5,
self::STATUS_REJECTED => 5,
+ self::STATUS_RESIGNED => 5,
);
return idx($map, $constant, 0);
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
@@ -38,7 +38,8 @@
protected function newObjectQuery() {
return id(new DifferentialRevisionQuery())
->needActiveDiffs(true)
- ->needReviewerStatus(true);
+ ->needReviewerStatus(true)
+ ->needReviewerAuthority(true);
}
protected function getObjectCreateTitleText($object) {
diff --git a/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php
@@ -0,0 +1,77 @@
+<?php
+
+final class DifferentialRevisionAcceptTransaction
+ extends DifferentialRevisionActionTransaction {
+
+ const TRANSACTIONTYPE = 'differential.revision.accept';
+ const ACTIONKEY = 'accept';
+
+ protected function getRevisionActionLabel() {
+ return pht("Accept Revision \xE2\x9C\x94");
+ }
+
+ protected function getRevisionActionDescription() {
+ return pht('These changes will be approved.');
+ }
+
+ public function getIcon() {
+ return 'fa-check-circle-o';
+ }
+
+ public function getColor() {
+ return 'green';
+ }
+
+ public function generateOldValue($object) {
+ $actor = $this->getActor();
+ return $this->isViewerAcceptingReviewer($object, $actor);
+ }
+
+ public function applyExternalEffects($object, $value) {
+ $status = DifferentialReviewerStatus::STATUS_ACCEPTED;
+ $actor = $this->getActor();
+ $this->applyReviewerEffect($object, $actor, $value, $status);
+ }
+
+ protected function validateAction($object, PhabricatorUser $viewer) {
+ if ($object->isClosed()) {
+ throw new Exception(
+ pht(
+ 'You can not accept this revision because it has already been '.
+ 'closed. Only open revisions can be accepted.'));
+ }
+
+ $config_key = 'differential.allow-self-accept';
+ if (!PhabricatorEnv::getEnvConfig($config_key)) {
+ if ($this->isViewerRevisionAuthor($object, $viewer)) {
+ throw new Exception(
+ pht(
+ 'You can not accept this revision because you are the revision '.
+ 'author. You can only accept revisions you do not own. You can '.
+ 'change this behavior by adjusting the "%s" setting in Config.',
+ $config_key));
+ }
+ }
+
+ if ($this->isViewerAcceptingReviewer($object, $viewer)) {
+ throw new Exception(
+ pht(
+ 'You can not accept this revision because you have already '.
+ 'accepted it.'));
+ }
+ }
+
+ public function getTitle() {
+ return pht(
+ '%s accepted this revision.',
+ $this->renderAuthor());
+ }
+
+ public function getTitleForFeed() {
+ return pht(
+ '%s accepted %s.',
+ $this->renderAuthor(),
+ $this->renderObject());
+ }
+
+}
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
@@ -41,6 +41,116 @@
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) {
diff --git a/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php b/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php
@@ -0,0 +1,74 @@
+<?php
+
+final class DifferentialRevisionRejectTransaction
+ extends DifferentialRevisionActionTransaction {
+
+ const TRANSACTIONTYPE = 'differential.revision.reject';
+ const ACTIONKEY = 'reject';
+
+ protected function getRevisionActionLabel() {
+ return pht("Request Changes \xE2\x9C\x98");
+ }
+
+ protected function getRevisionActionDescription() {
+ return pht('This revision will be returned to the author for updates.');
+ }
+
+ public function getIcon() {
+ return 'fa-times-circle-o';
+ }
+
+ public function getColor() {
+ return 'red';
+ }
+
+ public function generateOldValue($object) {
+ $actor = $this->getActor();
+ return $this->isViewerRejectingReviewer($object, $actor);
+ }
+
+ public function applyExternalEffects($object, $value) {
+ $status = DifferentialReviewerStatus::STATUS_REJECTED;
+ $actor = $this->getActor();
+ $this->applyReviewerEffect($object, $actor, $value, $status);
+ }
+
+ protected function validateAction($object, PhabricatorUser $viewer) {
+ if ($object->isClosed()) {
+ throw new Exception(
+ pht(
+ 'You can not request changes to this revision because it has '.
+ 'already been closed. You can only request changes to open '.
+ 'revisions.'));
+ }
+
+ if ($this->isViewerRevisionAuthor($object, $viewer)) {
+ throw new Exception(
+ pht(
+ 'You can not request changes to this revision because you are the '.
+ 'revision author. You can only request changes to revisions you do '.
+ 'not own.'));
+ }
+
+ if ($this->isViewerRejectingReviewer($object, $viewer)) {
+ throw new Exception(
+ pht(
+ 'You can not request changes to this revision because you have '.
+ 'already requested changes.'));
+ }
+ }
+
+ public function getTitle() {
+ return pht(
+ '%s requested changes to this revision.',
+ $this->renderAuthor());
+ }
+
+ public function getTitleForFeed() {
+ return pht(
+ '%s requested changes to %s.',
+ $this->renderAuthor(),
+ $this->renderObject());
+ }
+
+}
diff --git a/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php b/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/xaction/DifferentialRevisionResignTransaction.php
@@ -0,0 +1,66 @@
+<?php
+
+final class DifferentialRevisionResignTransaction
+ extends DifferentialRevisionActionTransaction {
+
+ const TRANSACTIONTYPE = 'differential.revision.resign';
+ const ACTIONKEY = 'resign';
+
+ protected function getRevisionActionLabel() {
+ return pht('Resign as Reviewer');
+ }
+
+ protected function getRevisionActionDescription() {
+ return pht('You will resign as a reviewer for this change.');
+ }
+
+ public function getIcon() {
+ return 'fa-flag';
+ }
+
+ public function getColor() {
+ return 'orange';
+ }
+
+ public function generateOldValue($object) {
+ $actor = $this->getActor();
+ return !$this->isViewerAnyReviewer($object, $actor);
+ }
+
+ public function applyExternalEffects($object, $value) {
+ $status = DifferentialReviewerStatus::STATUS_RESIGNED;
+ $actor = $this->getActor();
+ $this->applyReviewerEffect($object, $actor, $value, $status);
+ }
+
+ protected function validateAction($object, PhabricatorUser $viewer) {
+ if ($object->isClosed()) {
+ throw new Exception(
+ pht(
+ 'You can not resign from this revision because it has already '.
+ 'been closed. You can only resign from open revisions.'));
+ }
+
+ if (!$this->isViewerAnyReviewer($object, $viewer)) {
+ throw new Exception(
+ pht(
+ 'You can not resign from this revision because you are not a '.
+ 'reviewer. You can only resign from revisions where you are a '.
+ 'reviewer.'));
+ }
+ }
+
+ public function getTitle() {
+ return pht(
+ '%s resigned from this revision.',
+ $this->renderAuthor());
+ }
+
+ public function getTitleForFeed() {
+ return pht(
+ '%s resigned from %s.',
+ $this->renderAuthor(),
+ $this->renderObject());
+ }
+
+}

File Metadata

Mime Type
text/plain
Expires
Sat, Jul 26, 11:57 PM (4 d, 8 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
8550466
Default Alt Text
D17113.id41155.diff (16 KB)

Event Timeline

Phabricator · Built by Phacility