Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15412464
D17522.id42145.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
21 KB
Referenced Files
None
Subscribers
None
D17522.id42145.diff
View Options
diff --git a/src/applications/differential/conduit/DifferentialCreateRevisionConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialCreateRevisionConduitAPIMethod.php
--- a/src/applications/differential/conduit/DifferentialCreateRevisionConduitAPIMethod.php
+++ b/src/applications/differential/conduit/DifferentialCreateRevisionConduitAPIMethod.php
@@ -53,7 +53,7 @@
}
$revision = DifferentialRevision::initializeNewRevision($viewer);
- $revision->attachReviewerStatus(array());
+ $revision->attachReviewers(array());
$result = $this->applyFieldEdit(
$request,
diff --git a/src/applications/differential/customfield/DifferentialProjectReviewersField.php b/src/applications/differential/customfield/DifferentialProjectReviewersField.php
--- a/src/applications/differential/customfield/DifferentialProjectReviewersField.php
+++ b/src/applications/differential/customfield/DifferentialProjectReviewersField.php
@@ -52,7 +52,7 @@
private function getProjectReviewers() {
$reviewers = array();
- foreach ($this->getObject()->getReviewerStatus() as $reviewer) {
+ foreach ($this->getObject()->getReviewers() as $reviewer) {
if (!$reviewer->isUser()) {
$reviewers[] = $reviewer;
}
diff --git a/src/applications/differential/customfield/DifferentialReviewersField.php b/src/applications/differential/customfield/DifferentialReviewersField.php
--- a/src/applications/differential/customfield/DifferentialReviewersField.php
+++ b/src/applications/differential/customfield/DifferentialReviewersField.php
@@ -17,7 +17,7 @@
protected function readValueFromRevision(
DifferentialRevision $revision) {
- return $revision->getReviewerStatus();
+ return $revision->getReviewers();
}
public function shouldAppearInPropertyView() {
@@ -53,7 +53,7 @@
private function getUserReviewers() {
$reviewers = array();
- foreach ($this->getObject()->getReviewerStatus() as $reviewer) {
+ foreach ($this->getObject()->getReviewers() as $reviewer) {
if ($reviewer->isUser()) {
$reviewers[] = $reviewer;
}
diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -326,9 +326,9 @@
// actually change the diff text.
$edits = array();
- foreach ($object->getReviewerStatus() as $reviewer) {
+ foreach ($object->getReviewers() as $reviewer) {
if ($downgrade_rejects) {
- if ($reviewer->getStatus() == $new_reject) {
+ if ($reviewer->getReviewerStatus() == $new_reject) {
$edits[$reviewer->getReviewerPHID()] = array(
'data' => array(
'status' => $old_reject,
@@ -338,7 +338,7 @@
}
if ($downgrade_accepts) {
- if ($reviewer->getStatus() == $new_accept) {
+ if ($reviewer->getReviewerStatus() == $new_accept) {
$edits[$reviewer->getReviewerPHID()] = array(
'data' => array(
'status' => $old_accept,
@@ -415,9 +415,9 @@
);
$edits = array();
- foreach ($object->getReviewerStatus() as $reviewer) {
+ foreach ($object->getReviewers() as $reviewer) {
if ($reviewer->getReviewerPHID() == $actor_phid) {
- if ($reviewer->getStatus() == $status_added) {
+ if ($reviewer->getReviewerStatus() == $status_added) {
$edits[$actor_phid] = array(
'data' => $data,
);
@@ -623,7 +623,7 @@
pht('Failed to load revision from transaction finalization.'));
}
- $object->attachReviewerStatus($new_revision->getReviewerStatus());
+ $object->attachReviewers($new_revision->getReviewers());
$object->attachActiveDiff($new_revision->getActiveDiff());
$object->attachRepository($new_revision->getRepository());
@@ -645,7 +645,11 @@
$status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION;
$status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW;
+ $is_sticky_accept = PhabricatorEnv::getEnvConfig(
+ 'differential.sticky-accept');
+
$old_status = $object->getStatus();
+ $active_diff = $object->getActiveDiff();
switch ($old_status) {
case $status_accepted:
case $status_revision:
@@ -661,11 +665,17 @@
$has_rejecting_reviewer = false;
$has_rejecting_older_reviewer = false;
$has_blocking_reviewer = false;
- foreach ($object->getReviewerStatus() as $reviewer) {
- $reviewer_status = $reviewer->getStatus();
+ foreach ($object->getReviewers() as $reviewer) {
+ $reviewer_status = $reviewer->getReviewerStatus();
switch ($reviewer_status) {
case DifferentialReviewerStatus::STATUS_REJECTED:
- $has_rejecting_reviewer = true;
+ $action_phid = $reviewer->getLastActionDiffPHID();
+ $active_phid = $active_diff->getPHID();
+ $is_current = ($action_phid == $active_phid);
+
+ if ($is_current) {
+ $has_rejecting_reviewer = true;
+ }
break;
case DifferentialReviewerStatus::STATUS_REJECTED_OLDER:
$has_rejecting_older_reviewer = true;
@@ -675,7 +685,13 @@
break;
case DifferentialReviewerStatus::STATUS_ACCEPTED:
if ($reviewer->isUser()) {
- $has_accepting_user = true;
+ $action_phid = $reviewer->getLastActionDiffPHID();
+ $active_phid = $active_diff->getPHID();
+ $is_current = ($action_phid == $active_phid);
+
+ if ($is_sticky_accept || $is_current) {
+ $has_accepting_user = true;
+ }
}
break;
}
@@ -1032,7 +1048,7 @@
protected function getMailTo(PhabricatorLiskDAO $object) {
$phids = array();
$phids[] = $object->getAuthorPHID();
- foreach ($object->getReviewerStatus() as $reviewer) {
+ foreach ($object->getReviewers() as $reviewer) {
$phids[] = $reviewer->getReviewerPHID();
}
return $phids;
@@ -1507,7 +1523,7 @@
// and both are needlessly complex. This logic should live in the normal
// transaction application pipeline. See T10967.
- $reviewers = $object->getReviewerStatus();
+ $reviewers = $object->getReviewers();
$reviewers = mpull($reviewers, null, 'getReviewerPHID');
if ($is_blocking) {
@@ -1528,7 +1544,7 @@
// If we're applying a stronger status (usually, upgrading a reviewer
// into a blocking reviewer), skip this check so we apply the change.
$old_strength = DifferentialReviewerStatus::getStatusStrength(
- $reviewers[$phid]->getStatus());
+ $reviewers[$phid]->getReviewerStatus());
if ($old_strength <= $new_strength) {
continue;
}
diff --git a/src/applications/differential/engineextension/DifferentialHovercardEngineExtension.php b/src/applications/differential/engineextension/DifferentialHovercardEngineExtension.php
--- a/src/applications/differential/engineextension/DifferentialHovercardEngineExtension.php
+++ b/src/applications/differential/engineextension/DifferentialHovercardEngineExtension.php
@@ -54,8 +54,7 @@
pht('Author'),
$viewer->renderHandle($revision->getAuthorPHID()));
- $reviewer_phids = $revision->getReviewerStatus();
- $reviewer_phids = mpull($reviewer_phids, 'getReviewerPHID');
+ $reviewer_phids = $revision->getReviewerPHIDs();
$hovercard->addField(
pht('Reviewers'),
diff --git a/src/applications/differential/field/DifferentialReviewedByCommitMessageField.php b/src/applications/differential/field/DifferentialReviewedByCommitMessageField.php
--- a/src/applications/differential/field/DifferentialReviewedByCommitMessageField.php
+++ b/src/applications/differential/field/DifferentialReviewedByCommitMessageField.php
@@ -37,10 +37,9 @@
}
$phids = array();
- foreach ($revision->getReviewerStatus() as $reviewer) {
- switch ($reviewer->getStatus()) {
+ foreach ($revision->getReviewers() as $reviewer) {
+ switch ($reviewer->getReviewerStatus()) {
case DifferentialReviewerStatus::STATUS_ACCEPTED:
- case DifferentialReviewerStatus::STATUS_ACCEPTED_OLDER:
$phids[] = $reviewer->getReviewerPHID();
break;
}
diff --git a/src/applications/differential/field/DifferentialReviewersCommitMessageField.php b/src/applications/differential/field/DifferentialReviewersCommitMessageField.php
--- a/src/applications/differential/field/DifferentialReviewersCommitMessageField.php
+++ b/src/applications/differential/field/DifferentialReviewersCommitMessageField.php
@@ -45,8 +45,8 @@
$status_blocking = DifferentialReviewerStatus::STATUS_BLOCKING;
$results = array();
- foreach ($revision->getReviewerStatus() as $reviewer) {
- if ($reviewer->getStatus() == $status_blocking) {
+ foreach ($revision->getReviewers() as $reviewer) {
+ if ($reviewer->getReviewerStatus() == $status_blocking) {
$suffixes = array('!' => '!');
} else {
$suffixes = array();
diff --git a/src/applications/differential/herald/DifferentialReviewersHeraldAction.php b/src/applications/differential/herald/DifferentialReviewersHeraldAction.php
--- a/src/applications/differential/herald/DifferentialReviewersHeraldAction.php
+++ b/src/applications/differential/herald/DifferentialReviewersHeraldAction.php
@@ -37,8 +37,7 @@
}
}
- $reviewers = $object->getReviewerStatus();
- $reviewers = mpull($reviewers, null, 'getReviewerPHID');
+ $reviewers = $object->getReviewers();
if ($is_blocking) {
$new_status = DifferentialReviewerStatus::STATUS_BLOCKING;
diff --git a/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php b/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php
--- a/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php
+++ b/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php
@@ -137,8 +137,7 @@
}
public function loadReviewers() {
- $reviewers = $this->getObject()->getReviewerStatus();
- return mpull($reviewers, 'getReviewerPHID');
+ return $this->getObject()->getReviewerPHIDs();
}
diff --git a/src/applications/differential/lipsum/PhabricatorDifferentialRevisionTestDataGenerator.php b/src/applications/differential/lipsum/PhabricatorDifferentialRevisionTestDataGenerator.php
--- a/src/applications/differential/lipsum/PhabricatorDifferentialRevisionTestDataGenerator.php
+++ b/src/applications/differential/lipsum/PhabricatorDifferentialRevisionTestDataGenerator.php
@@ -13,7 +13,7 @@
$author = $this->loadPhabricatorUser();
$revision = DifferentialRevision::initializeNewRevision($author);
- $revision->attachReviewerStatus(array());
+ $revision->attachReviewers(array());
$revision->attachActiveDiff(null);
// This could be a bit richer and more formal than it is.
diff --git a/src/applications/differential/query/DifferentialRevisionQuery.php b/src/applications/differential/query/DifferentialRevisionQuery.php
--- a/src/applications/differential/query/DifferentialRevisionQuery.php
+++ b/src/applications/differential/query/DifferentialRevisionQuery.php
@@ -974,7 +974,7 @@
$reviewers[$reviewer_phid] = $reviewer;
}
- $revision->attachReviewerStatus($reviewers);
+ $revision->attachReviewers($reviewers);
}
}
@@ -993,7 +993,6 @@
$project_type = PhabricatorProjectProjectPHIDType::TYPECONST;
$package_type = PhabricatorOwnersPackagePHIDType::TYPECONST;
- $edge_type = DifferentialRevisionHasReviewerEdgeType::EDGECONST;
foreach ($reviewers as $revision_phid => $reviewer_list) {
if (!$allow_self) {
if ($revision_map[$revision_phid]->getAuthorPHID() == $viewer_phid) {
diff --git a/src/applications/differential/query/DifferentialRevisionResultBucket.php b/src/applications/differential/query/DifferentialRevisionResultBucket.php
--- a/src/applications/differential/query/DifferentialRevisionResultBucket.php
+++ b/src/applications/differential/query/DifferentialRevisionResultBucket.php
@@ -56,13 +56,13 @@
array $phids,
array $statuses) {
- foreach ($revision->getReviewerStatus() as $reviewer) {
+ foreach ($revision->getReviewers() as $reviewer) {
$reviewer_phid = $reviewer->getReviewerPHID();
if (empty($phids[$reviewer_phid])) {
continue;
}
- $status = $reviewer->getStatus();
+ $status = $reviewer->getReviewerStatus();
if (empty($statuses[$status])) {
continue;
}
diff --git a/src/applications/differential/search/DifferentialRevisionFulltextEngine.php b/src/applications/differential/search/DifferentialRevisionFulltextEngine.php
--- a/src/applications/differential/search/DifferentialRevisionFulltextEngine.php
+++ b/src/applications/differential/search/DifferentialRevisionFulltextEngine.php
@@ -14,7 +14,7 @@
->executeOne();
// TODO: This isn't very clean, but custom fields currently rely on it.
- $object->attachReviewerStatus($revision->getReviewerStatus());
+ $object->attachReviewers($revision->getReviewers());
$document->setDocumentTitle($revision->getTitle());
@@ -36,8 +36,9 @@
// owner is the author (e.g., accepted, rejected, closed).
$status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW;
if ($revision->getStatus() == $status_review) {
- $reviewers = $revision->getReviewerStatus();
- $reviewers = mpull($reviewers, 'getReviewerPHID', 'getReviewerPHID');
+ $reviewers = $revision->getReviewerPHIDs();
+ $reviewers = array_fuse($reviewers);
+
if ($reviewers) {
foreach ($reviewers as $phid) {
$document->addRelationship(
diff --git a/src/applications/differential/storage/DifferentialReviewer.php b/src/applications/differential/storage/DifferentialReviewer.php
--- a/src/applications/differential/storage/DifferentialReviewer.php
+++ b/src/applications/differential/storage/DifferentialReviewer.php
@@ -27,12 +27,6 @@
) + parent::getConfiguration();
}
- public function getStatus() {
- // TODO: This is an older method for compatibility with some callers
- // which have not yet been cleaned up.
- return $this->getReviewerStatus();
- }
-
public function isUser() {
$user_type = PhabricatorPeopleUserPHIDType::TYPECONST;
return (phid_get_type($this->getReviewerPHID()) == $user_type);
diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php
--- a/src/applications/differential/storage/DifferentialRevision.php
+++ b/src/applications/differential/storage/DifferentialRevision.php
@@ -70,7 +70,7 @@
->setAuthorPHID($actor->getPHID())
->attachRepository(null)
->attachActiveDiff(null)
- ->attachReviewerStatus(array())
+ ->attachReviewers(array())
->setStatus(ArcanistDifferentialRevisionStatus::NEEDS_REVIEW);
}
@@ -332,30 +332,31 @@
);
}
- public function getReviewerStatus() {
+ public function getReviewers() {
return $this->assertAttached($this->reviewerStatus);
}
- public function attachReviewerStatus(array $reviewers) {
+ public function attachReviewers(array $reviewers) {
assert_instances_of($reviewers, 'DifferentialReviewer');
+ $reviewers = mpull($reviewers, null, 'getReviewerPHID');
$this->reviewerStatus = $reviewers;
return $this;
}
public function getReviewerPHIDs() {
- $reviewers = $this->getReviewerStatus();
+ $reviewers = $this->getReviewers();
return mpull($reviewers, 'getReviewerPHID');
}
public function getReviewerPHIDsForEdit() {
- $reviewers = $this->getReviewerStatus();
+ $reviewers = $this->getReviewers();
$status_blocking = DifferentialReviewerStatus::STATUS_BLOCKING;
$value = array();
foreach ($reviewers as $reviewer) {
$phid = $reviewer->getReviewerPHID();
- if ($reviewer->getStatus() == $status_blocking) {
+ if ($reviewer->getReviewerStatus() == $status_blocking) {
$value[] = 'blocking('.$phid.')';
} else {
$value[] = $phid;
@@ -480,9 +481,9 @@
->withPHIDs(array($this->getPHID()))
->needReviewers(true)
->executeOne()
- ->getReviewerStatus();
+ ->getReviewers();
} else {
- $reviewers = $this->getReviewerStatus();
+ $reviewers = $this->getReviewers();
}
foreach ($reviewers as $reviewer) {
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
@@ -21,7 +21,8 @@
$viewer,
array(
DifferentialReviewerStatus::STATUS_ACCEPTED,
- ));
+ ),
+ true);
}
protected function isViewerFullyRejected(
@@ -32,7 +33,8 @@
$viewer,
array(
DifferentialReviewerStatus::STATUS_REJECTED,
- ));
+ ),
+ true);
}
protected function getViewerReviewerStatus(
@@ -43,12 +45,12 @@
return null;
}
- foreach ($revision->getReviewerStatus() as $reviewer) {
+ foreach ($revision->getReviewers() as $reviewer) {
if ($reviewer->getReviewerPHID() != $viewer->getPHID()) {
continue;
}
- return $reviewer->getStatus();
+ return $reviewer->getReviewerStatus();
}
return null;
@@ -57,7 +59,8 @@
protected function isViewerReviewerStatusFullyAmong(
DifferentialRevision $revision,
PhabricatorUser $viewer,
- array $status_list) {
+ array $status_list,
+ $require_current) {
// 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
@@ -67,18 +70,26 @@
return false;
}
+ $active_phid = $this->getActiveDiffPHID($revision);
+
// Otherwise, check that all reviews they have authority over are in
// the desired set of states.
$status_map = array_fuse($status_list);
- foreach ($revision->getReviewerStatus() as $reviewer) {
+ foreach ($revision->getReviewers() as $reviewer) {
if (!$reviewer->hasAuthority($viewer)) {
continue;
}
- $status = $reviewer->getStatus();
+ $status = $reviewer->getReviewerStatus();
if (!isset($status_map[$status])) {
return false;
}
+
+ if ($require_current) {
+ if ($reviewer->getLastActionDiffPHID() != $active_phid) {
+ return false;
+ }
+ }
}
return true;
@@ -97,7 +108,7 @@
// yourself.
$with_authority = ($status != DifferentialReviewerStatus::STATUS_RESIGNED);
if ($with_authority) {
- foreach ($revision->getReviewerStatus() as $reviewer) {
+ foreach ($revision->getReviewers() as $reviewer) {
if ($reviewer->hasAuthority($viewer)) {
$map[$reviewer->getReviewerPHID()] = $status;
}
diff --git a/src/applications/differential/xaction/DifferentialRevisionReviewersTransaction.php b/src/applications/differential/xaction/DifferentialRevisionReviewersTransaction.php
--- a/src/applications/differential/xaction/DifferentialRevisionReviewersTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionReviewersTransaction.php
@@ -7,8 +7,8 @@
const EDITKEY = 'reviewers';
public function generateOldValue($object) {
- $reviewers = $object->getReviewerStatus();
- $reviewers = mpull($reviewers, 'getStatus', 'getReviewerPHID');
+ $reviewers = $object->getReviewers();
+ $reviewers = mpull($reviewers, 'getReviewerStatus', 'getReviewerPHID');
return $reviewers;
}
diff --git a/src/applications/differential/xaction/DifferentialRevisionTransactionType.php b/src/applications/differential/xaction/DifferentialRevisionTransactionType.php
--- a/src/applications/differential/xaction/DifferentialRevisionTransactionType.php
+++ b/src/applications/differential/xaction/DifferentialRevisionTransactionType.php
@@ -57,4 +57,13 @@
$xaction);
}
+ protected function getActiveDiffPHID(DifferentialRevision $revision) {
+ try {
+ $diff = $revision->getActiveDiff();
+ return $diff->getPHID();
+ } catch (Exception $ex) {
+ return null;
+ }
+ }
+
}
diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php
--- a/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php
+++ b/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php
@@ -165,7 +165,7 @@
$accepted_statuses = array_fuse($accepted_statuses);
$found_accept = false;
- foreach ($revision->getReviewerStatus() as $reviewer) {
+ foreach ($revision->getReviewers() as $reviewer) {
$reviewer_phid = $reviewer->getReviewerPHID();
// If this reviewer isn't a package owner, just ignore them.
@@ -175,7 +175,7 @@
// If this reviewer accepted the revision and owns the package, we're
// all clear and do not need to trigger an audit.
- if (isset($accepted_statuses[$reviewer->getStatus()])) {
+ if (isset($accepted_statuses[$reviewer->getReviewerStatus()])) {
$found_accept = true;
break;
}
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Thu, Mar 20, 1:08 PM (2 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7700250
Default Alt Text
D17522.id42145.diff (21 KB)
Attached To
Mode
D17522: Rename "getReviewerStatus()" to "getReviewers()"
Attached
Detach File
Event Timeline
Log In to Comment