Page MenuHomePhabricator
Files F15422939

D17226.id.diff
No OneTemporary
Actions

D17226.id.diff
View Options

diff --git a/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php
--- a/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php
@@ -50,7 +50,7 @@
public function generateOldValue($object) {
$actor = $this->getActor();
- return $this->isViewerAcceptingReviewer($object, $actor);
+ return $this->isViewerFullyAccepted($object, $actor);
}
public function applyExternalEffects($object, $value) {
@@ -79,7 +79,7 @@
}
}
- if ($this->isViewerAcceptingReviewer($object, $viewer)) {
+ if ($this->isViewerFullyAccepted($object, $viewer)) {
throw new Exception(
pht(
'You can not accept this revision because you have already '.
diff --git a/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php b/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php
--- a/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionRejectTransaction.php
@@ -46,7 +46,7 @@
public function generateOldValue($object) {
$actor = $this->getActor();
- return $this->isViewerRejectingReviewer($object, $actor);
+ return $this->isViewerFullyRejected($object, $actor);
}
public function applyExternalEffects($object, $value) {
@@ -72,7 +72,7 @@
'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 '.
diff --git a/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php b/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
--- a/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
@@ -13,10 +13,10 @@
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(
@@ -24,10 +24,10 @@
));
}
- protected function isViewerRejectingReviewer(
+ protected function isViewerFullyRejected(
DifferentialRevision $revision,
PhabricatorUser $viewer) {
- return $this->isViewerReviewerStatusAmong(
+ return $this->isViewerReviewerStatusFullyAmong(
$revision,
$viewer,
array(
@@ -54,18 +54,34 @@
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(

File Metadata

Mime Type
text/plain
Expires
Sun, Mar 23, 11:32 AM (1 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7718406
Default Alt Text
D17226.id.diff (3 KB)

Event Timeline

Phabricator · Built by Phacility