Page MenuHomestyx hydra
Files F128411

No OneTemporary
Actions

View Options

diff --git a/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php b/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php
index 5f5f4008db..711f70afb3 100644
--- a/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php
+++ b/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php
@@ -1,298 +1,298 @@
<?php
final class DifferentialRevisionRequiredActionResultBucket
extends DifferentialRevisionResultBucket {
const BUCKETKEY = 'action';
const KEY_MUSTREVIEW = 'must-review';
const KEY_SHOULDREVIEW = 'should-review';
private $objects;
public function getResultBucketName() {
return pht('Bucket by Required Action');
}
protected function buildResultGroups(
PhabricatorSavedQuery $query,
array $objects) {
$this->objects = $objects;
$phids = $query->getEvaluatedParameter('responsiblePHIDs');
if (!$phids) {
throw new Exception(
pht(
'You can not bucket results by required action without '.
'specifying "Responsible Users".'));
}
$phids = array_fuse($phids);
// Before continuing, throw away any revisions which responsible users
// have explicitly resigned from.
// The goal is to allow users to resign from revisions they don't want to
// review to get these revisions off their dashboard, even if there are
// other project or package reviewers which they have authority over.
$this->filterResigned($phids);
// We also throw away draft revisions which you aren't the author of.
$this->filterOtherDrafts($phids);
$groups = array();
$groups[] = $this->newGroup()
->setName(pht('Must Review'))
->setKey(self::KEY_MUSTREVIEW)
->setNoDataString(pht('No revisions are blocked on your review.'))
->setObjects($this->filterMustReview($phids));
$groups[] = $this->newGroup()
->setName(pht('Ready to Review'))
->setKey(self::KEY_SHOULDREVIEW)
->setNoDataString(pht('No revisions are waiting on you to review them.'))
->setObjects($this->filterShouldReview($phids));
$groups[] = $this->newGroup()
->setName(pht('Ready to Land'))
->setNoDataString(pht('No revisions are ready to land.'))
->setObjects($this->filterShouldLand($phids));
$groups[] = $this->newGroup()
->setName(pht('Ready to Update'))
->setNoDataString(pht('No revisions are waiting for updates.'))
->setObjects($this->filterShouldUpdate($phids));
$groups[] = $this->newGroup()
->setName(pht('Drafts'))
->setNoDataString(pht('You have no draft revisions.'))
->setObjects($this->filterDrafts($phids));
$groups[] = $this->newGroup()
->setName(pht('Waiting on Review'))
->setNoDataString(pht('None of your revisions are waiting on review.'))
->setObjects($this->filterWaitingForReview($phids));
$groups[] = $this->newGroup()
->setName(pht('Waiting on Authors'))
->setNoDataString(pht('No revisions are waiting on author action.'))
->setObjects($this->filterWaitingOnAuthors($phids));
$groups[] = $this->newGroup()
->setName(pht('Waiting on Other Reviewers'))
->setNoDataString(pht('No revisions are waiting for other reviewers.'))
->setObjects($this->filterWaitingOnOtherReviewers($phids));
// Because you can apply these buckets to queries which include revisions
// that have been closed, add an "Other" bucket if we still have stuff
// that didn't get filtered into any of the previous buckets.
if ($this->objects) {
$groups[] = $this->newGroup()
->setName(pht('Other Revisions'))
->setObjects($this->objects);
}
return $groups;
}
private function filterMustReview(array $phids) {
$blocking = array(
DifferentialReviewerStatus::STATUS_BLOCKING,
DifferentialReviewerStatus::STATUS_REJECTED,
DifferentialReviewerStatus::STATUS_REJECTED_OLDER,
);
$blocking = array_fuse($blocking);
$objects = $this->getRevisionsUnderReview($this->objects, $phids);
$results = array();
foreach ($objects as $key => $object) {
if (!$this->hasReviewersWithStatus($object, $phids, $blocking)) {
continue;
}
$results[$key] = $object;
unset($this->objects[$key]);
}
return $results;
}
private function filterShouldReview(array $phids) {
$reviewing = array(
DifferentialReviewerStatus::STATUS_ADDED,
DifferentialReviewerStatus::STATUS_COMMENTED,
// If an author has used "Request Review" to put an accepted revision
// back into the "Needs Review" state, include "Accepted" reviewers
// whose reviews have been voided in the "Should Review" bucket.
// If we don't do this, they end up in "Waiting on Other Reviewers",
// even if there are no other reviewers.
DifferentialReviewerStatus::STATUS_ACCEPTED,
);
$reviewing = array_fuse($reviewing);
$objects = $this->getRevisionsUnderReview($this->objects, $phids);
$results = array();
foreach ($objects as $key => $object) {
- if (!$this->hasReviewersWithStatus($object, $phids, $reviewing, false)) {
+ if (!$this->hasReviewersWithStatus($object, $phids, $reviewing, true)) {
continue;
}
$results[$key] = $object;
unset($this->objects[$key]);
}
return $results;
}
private function filterShouldLand(array $phids) {
$objects = $this->getRevisionsAuthored($this->objects, $phids);
$results = array();
foreach ($objects as $key => $object) {
if (!$object->isAccepted()) {
continue;
}
$results[$key] = $object;
unset($this->objects[$key]);
}
return $results;
}
private function filterShouldUpdate(array $phids) {
$statuses = array(
DifferentialRevisionStatus::NEEDS_REVISION,
DifferentialRevisionStatus::CHANGES_PLANNED,
);
$statuses = array_fuse($statuses);
$objects = $this->getRevisionsAuthored($this->objects, $phids);
$results = array();
foreach ($objects as $key => $object) {
if (empty($statuses[$object->getModernRevisionStatus()])) {
continue;
}
$results[$key] = $object;
unset($this->objects[$key]);
}
return $results;
}
private function filterWaitingForReview(array $phids) {
$objects = $this->getRevisionsAuthored($this->objects, $phids);
$results = array();
foreach ($objects as $key => $object) {
if (!$object->isNeedsReview()) {
continue;
}
$results[$key] = $object;
unset($this->objects[$key]);
}
return $results;
}
private function filterWaitingOnAuthors(array $phids) {
$statuses = array(
DifferentialRevisionStatus::ACCEPTED,
DifferentialRevisionStatus::NEEDS_REVISION,
DifferentialRevisionStatus::CHANGES_PLANNED,
);
$statuses = array_fuse($statuses);
$objects = $this->getRevisionsNotAuthored($this->objects, $phids);
$results = array();
foreach ($objects as $key => $object) {
if (empty($statuses[$object->getModernRevisionStatus()])) {
continue;
}
$results[$key] = $object;
unset($this->objects[$key]);
}
return $results;
}
private function filterWaitingOnOtherReviewers(array $phids) {
$objects = $this->getRevisionsNotAuthored($this->objects, $phids);
$results = array();
foreach ($objects as $key => $object) {
if (!$object->isNeedsReview()) {
continue;
}
$results[$key] = $object;
unset($this->objects[$key]);
}
return $results;
}
private function filterResigned(array $phids) {
$resigned = array(
DifferentialReviewerStatus::STATUS_RESIGNED,
);
$resigned = array_fuse($resigned);
$objects = $this->getRevisionsNotAuthored($this->objects, $phids);
$results = array();
foreach ($objects as $key => $object) {
if (!$this->hasReviewersWithStatus($object, $phids, $resigned)) {
continue;
}
$results[$key] = $object;
unset($this->objects[$key]);
}
return $results;
}
private function filterOtherDrafts(array $phids) {
$objects = $this->getRevisionsNotAuthored($this->objects, $phids);
$results = array();
foreach ($objects as $key => $object) {
if (!$object->isDraft()) {
continue;
}
$results[$key] = $object;
unset($this->objects[$key]);
}
return $results;
}
private function filterDrafts(array $phids) {
$objects = $this->getRevisionsAuthored($this->objects, $phids);
$results = array();
foreach ($objects as $key => $object) {
if (!$object->isDraft()) {
continue;
}
$results[$key] = $object;
unset($this->objects[$key]);
}
return $results;
}
}
diff --git a/src/applications/differential/query/DifferentialRevisionResultBucket.php b/src/applications/differential/query/DifferentialRevisionResultBucket.php
index a0769ac349..c5cc5c0e6c 100644
--- a/src/applications/differential/query/DifferentialRevisionResultBucket.php
+++ b/src/applications/differential/query/DifferentialRevisionResultBucket.php
@@ -1,87 +1,86 @@
<?php
abstract class DifferentialRevisionResultBucket
extends PhabricatorSearchResultBucket {
public static function getAllResultBuckets() {
return id(new PhutilClassMapQuery())
->setAncestorClass(__CLASS__)
->setUniqueMethod('getResultBucketKey')
->execute();
}
protected function getRevisionsUnderReview(array $objects, array $phids) {
$results = array();
$objects = $this->getRevisionsNotAuthored($objects, $phids);
foreach ($objects as $key => $object) {
if (!$object->isNeedsReview()) {
continue;
}
$results[$key] = $object;
}
return $results;
}
protected function getRevisionsAuthored(array $objects, array $phids) {
$results = array();
foreach ($objects as $key => $object) {
if (isset($phids[$object->getAuthorPHID()])) {
$results[$key] = $object;
}
}
return $results;
}
protected function getRevisionsNotAuthored(array $objects, array $phids) {
$results = array();
foreach ($objects as $key => $object) {
if (empty($phids[$object->getAuthorPHID()])) {
$results[$key] = $object;
}
}
return $results;
}
protected function hasReviewersWithStatus(
DifferentialRevision $revision,
array $phids,
array $statuses,
- $current = null) {
+ $include_voided = null) {
foreach ($revision->getReviewers() as $reviewer) {
$reviewer_phid = $reviewer->getReviewerPHID();
if (empty($phids[$reviewer_phid])) {
continue;
}
$status = $reviewer->getReviewerStatus();
if (empty($statuses[$status])) {
continue;
}
- if ($current !== null) {
+ if ($include_voided !== null) {
if ($status == DifferentialReviewerStatus::STATUS_ACCEPTED) {
- $diff_phid = $revision->getActiveDiffPHID();
- $is_current = $reviewer->isAccepted($diff_phid);
- if ($is_current !== $current) {
+ $is_voided = (bool)$reviewer->getVoidedPHID();
+ if ($is_voided !== $include_voided) {
continue;
}
}
}
return true;
}
return false;
}
}
diff --git a/src/applications/differential/view/DifferentialReviewersView.php b/src/applications/differential/view/DifferentialReviewersView.php
index 33aad25289..f88669e539 100644
--- a/src/applications/differential/view/DifferentialReviewersView.php
+++ b/src/applications/differential/view/DifferentialReviewersView.php
@@ -1,186 +1,192 @@
<?php
final class DifferentialReviewersView extends AphrontView {
private $reviewers;
private $handles;
private $diff;
public function setReviewers(array $reviewers) {
assert_instances_of($reviewers, 'DifferentialReviewer');
$this->reviewers = $reviewers;
return $this;
}
public function setHandles(array $handles) {
assert_instances_of($handles, 'PhabricatorObjectHandle');
$this->handles = $handles;
return $this;
}
public function setActiveDiff(DifferentialDiff $diff) {
$this->diff = $diff;
return $this;
}
public function render() {
$viewer = $this->getUser();
$reviewers = $this->reviewers;
$view = new PHUIStatusListView();
// Move resigned reviewers to the bottom.
$head = array();
$tail = array();
foreach ($reviewers as $key => $reviewer) {
if ($reviewer->isResigned()) {
$tail[$key] = $reviewer;
} else {
$head[$key] = $reviewer;
}
}
$reviewers = $head + $tail;
foreach ($reviewers as $reviewer) {
$phid = $reviewer->getReviewerPHID();
$handle = $this->handles[$phid];
$action_phid = $reviewer->getLastActionDiffPHID();
$is_current_action = $this->isCurrent($action_phid);
+ $is_voided = (bool)$reviewer->getVoidedPHID();
$comment_phid = $reviewer->getLastCommentDiffPHID();
$is_current_comment = $this->isCurrent($comment_phid);
$item = new PHUIStatusItemView();
$item->setHighlighted($reviewer->hasAuthority($viewer));
// If someone other than the reviewer acted on the reviewer's behalf,
// show who is responsible for the current state. This is usually a
// user accepting for a package or project.
$authority_phid = $reviewer->getLastActorPHID();
if ($authority_phid && ($authority_phid !== $phid)) {
$authority_name = $viewer->renderHandle($authority_phid)
->setAsText(true);
} else {
$authority_name = null;
}
switch ($reviewer->getReviewerStatus()) {
case DifferentialReviewerStatus::STATUS_ADDED:
if ($comment_phid) {
if ($is_current_comment) {
$icon = 'fa-comment';
$color = 'blue';
$label = pht('Commented');
} else {
$icon = 'fa-comment-o';
$color = 'bluegrey';
$label = pht('Commented Previously');
}
} else {
$icon = PHUIStatusItemView::ICON_OPEN;
$color = 'bluegrey';
$label = pht('Review Requested');
}
break;
case DifferentialReviewerStatus::STATUS_ACCEPTED:
- if ($is_current_action) {
+ if ($is_current_action && !$is_voided) {
$icon = PHUIStatusItemView::ICON_ACCEPT;
$color = 'green';
if ($authority_name !== null) {
$label = pht('Accepted (by %s)', $authority_name);
} else {
$label = pht('Accepted');
}
} else {
$icon = 'fa-check-circle-o';
$color = 'bluegrey';
- if ($authority_name !== null) {
+
+ if (!$is_current_action && $is_voided) {
+ // The reviewer accepted the revision, but later the author
+ // used "Request Review" to request an updated review.
+ $label = pht('Accepted Earlier');
+ } else if ($authority_name !== null) {
$label = pht('Accepted Prior Diff (by %s)', $authority_name);
} else {
$label = pht('Accepted Prior Diff');
}
}
break;
case DifferentialReviewerStatus::STATUS_REJECTED:
if ($is_current_action) {
$icon = PHUIStatusItemView::ICON_REJECT;
$color = 'red';
if ($authority_name !== null) {
$label = pht('Requested Changes (by %s)', $authority_name);
} else {
$label = pht('Requested Changes');
}
} else {
$icon = 'fa-times-circle-o';
$color = 'red';
if ($authority_name !== null) {
$label = pht(
'Requested Changes to Prior Diff (by %s)',
$authority_name);
} else {
$label = pht('Requested Changes to Prior Diff');
}
}
break;
case DifferentialReviewerStatus::STATUS_BLOCKING:
$icon = PHUIStatusItemView::ICON_MINUS;
$color = 'red';
$label = pht('Blocking Review');
break;
case DifferentialReviewerStatus::STATUS_RESIGNED:
$icon = 'fa-times';
$color = 'grey';
$label = pht('Resigned');
break;
default:
$icon = PHUIStatusItemView::ICON_QUESTION;
$color = 'bluegrey';
$label = pht('Unknown ("%s")', $reviewer->getReviewerStatus());
break;
}
$item->setIcon($icon, $color, $label);
$item->setTarget($handle->renderHovercardLink());
if ($reviewer->isPackage()) {
if (!$reviewer->getChangesets()) {
$item->setNote(pht('(Owns No Changed Paths)'));
}
}
$view->addItem($item);
}
return $view;
}
private function isCurrent($action_phid) {
if (!$this->diff) {
return true;
}
if (!$action_phid) {
return true;
}
$diff_phid = $this->diff->getPHID();
if (!$diff_phid) {
return true;
}
if ($diff_phid == $action_phid) {
return true;
}
return false;
}
}

File Metadata

Mime Type
text/x-diff
Expires
Wed, Apr 30, 3:02 PM (1 d, 4 h)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
108736
Default Alt Text
(17 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!)