Page MenuHomestyx hydra
Files F83582

No OneTemporary
Actions

View Options

diff --git a/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php
index df4e3b4d33..3a64a44767 100644
--- a/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php
@@ -1,103 +1,103 @@
<?php
final class DifferentialRevisionAcceptTransaction
extends DifferentialRevisionReviewTransaction {
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';
}
protected function getRevisionActionOrder() {
return 500;
}
public function getActionName() {
return pht('Accepted');
}
public function getCommandKeyword() {
$accept_key = 'differential.enable-email-accept';
$allow_email_accept = PhabricatorEnv::getEnvConfig($accept_key);
if (!$allow_email_accept) {
return null;
}
return 'accept';
}
public function getCommandAliases() {
return array();
}
public function getCommandSummary() {
return pht('Accept a revision.');
}
public function generateOldValue($object) {
$actor = $this->getActor();
- return $this->isViewerAcceptingReviewer($object, $actor);
+ return $this->isViewerFullyAccepted($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)) {
+ if ($this->isViewerFullyAccepted($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/DifferentialRevisionRejectTransaction.php b/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php
index 96812de06b..b4c71209aa 100644
--- a/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php
@@ -1,96 +1,96 @@
<?php
final class DifferentialRevisionRejectTransaction
extends DifferentialRevisionReviewTransaction {
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';
}
protected function getRevisionActionOrder() {
return 600;
}
public function getActionName() {
return pht('Requested Changes');
}
public function getCommandKeyword() {
return 'request';
}
public function getCommandAliases() {
return array(
'reject',
);
}
public function getCommandSummary() {
return pht('Request changes to a revision.');
}
public function generateOldValue($object) {
$actor = $this->getActor();
- return $this->isViewerRejectingReviewer($object, $actor);
+ return $this->isViewerFullyRejected($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)) {
+ if ($this->isViewerFullyRejected($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/DifferentialRevisionReviewTransaction.php b/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
index de75977220..7e0c60a7b1 100644
--- a/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
@@ -1,120 +1,136 @@
<?php
abstract class DifferentialRevisionReviewTransaction
extends DifferentialRevisionActionTransaction {
protected function getRevisionActionGroupKey() {
return DifferentialRevisionEditEngine::ACTIONGROUP_REVIEW;
}
protected function isViewerAnyReviewer(
DifferentialRevision $revision,
PhabricatorUser $viewer) {
return ($this->getViewerReviewerStatus($revision, $viewer) !== null);
}
- protected function isViewerAcceptingReviewer(
+ protected function isViewerFullyAccepted(
DifferentialRevision $revision,
PhabricatorUser $viewer) {
- return $this->isViewerReviewerStatusAmong(
+ return $this->isViewerReviewerStatusFullyAmong(
$revision,
$viewer,
array(
DifferentialReviewerStatus::STATUS_ACCEPTED,
));
}
- protected function isViewerRejectingReviewer(
+ protected function isViewerFullyRejected(
DifferentialRevision $revision,
PhabricatorUser $viewer) {
- return $this->isViewerReviewerStatusAmong(
+ return $this->isViewerReviewerStatusFullyAmong(
$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(
+ protected function isViewerReviewerStatusFullyAmong(
DifferentialRevision $revision,
PhabricatorUser $viewer,
array $status_list) {
+ // If the user themselves is not a reviewer, the reviews they have
+ // authority over can not all be in any set of states since their own
+ // personal review has no state.
$status = $this->getViewerReviewerStatus($revision, $viewer);
if ($status === null) {
return false;
}
+ // Otherwise, check that all reviews they have authority over are in
+ // the desired set of states.
$status_map = array_fuse($status_list);
- return isset($status_map[$status]);
+ foreach ($revision->getReviewerStatus() as $reviewer) {
+ if (!$reviewer->hasAuthority($viewer)) {
+ continue;
+ }
+
+ $status = $reviewer->getStatus();
+ if (!isset($status_map[$status])) {
+ return false;
+ }
+ }
+
+ return true;
}
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();
}
}

File Metadata

Mime Type
text/x-diff
Expires
Sat, Mar 15, 2:38 PM (1 d, 25 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
72099
Default Alt Text
(10 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!)