Page MenuHomestyx hydra
Files F83500

No OneTemporary
Actions

View Options

diff --git a/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php b/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php
index d37754227e..6d83d97a47 100644
--- a/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php
+++ b/src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php
@@ -1,232 +1,261 @@
<?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);
+
$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('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,
);
$reviewing = array_fuse($reviewing);
$objects = $this->getRevisionsUnderReview($this->objects, $phids);
$results = array();
foreach ($objects as $key => $object) {
if (!$this->hasReviewersWithStatus($object, $phids, $reviewing)) {
continue;
}
$results[$key] = $object;
unset($this->objects[$key]);
}
return $results;
}
private function filterShouldLand(array $phids) {
$status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED;
$objects = $this->getRevisionsAuthored($this->objects, $phids);
$results = array();
foreach ($objects as $key => $object) {
if ($object->getStatus() != $status_accepted) {
continue;
}
$results[$key] = $object;
unset($this->objects[$key]);
}
return $results;
}
private function filterShouldUpdate(array $phids) {
$statuses = array(
ArcanistDifferentialRevisionStatus::NEEDS_REVISION,
ArcanistDifferentialRevisionStatus::CHANGES_PLANNED,
ArcanistDifferentialRevisionStatus::IN_PREPARATION,
);
$statuses = array_fuse($statuses);
$objects = $this->getRevisionsAuthored($this->objects, $phids);
$results = array();
foreach ($objects as $key => $object) {
if (empty($statuses[$object->getStatus()])) {
continue;
}
$results[$key] = $object;
unset($this->objects[$key]);
}
return $results;
}
private function filterWaitingForReview(array $phids) {
$status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW;
$objects = $this->getRevisionsAuthored($this->objects, $phids);
$results = array();
foreach ($objects as $key => $object) {
if ($object->getStatus() != $status_review) {
continue;
}
$results[$key] = $object;
unset($this->objects[$key]);
}
return $results;
}
private function filterWaitingOnAuthors(array $phids) {
$statuses = array(
ArcanistDifferentialRevisionStatus::ACCEPTED,
ArcanistDifferentialRevisionStatus::NEEDS_REVISION,
ArcanistDifferentialRevisionStatus::CHANGES_PLANNED,
ArcanistDifferentialRevisionStatus::IN_PREPARATION,
);
$statuses = array_fuse($statuses);
$objects = $this->getRevisionsNotAuthored($this->objects, $phids);
$results = array();
foreach ($objects as $key => $object) {
if (empty($statuses[$object->getStatus()])) {
continue;
}
$results[$key] = $object;
unset($this->objects[$key]);
}
return $results;
}
private function filterWaitingOnOtherReviewers(array $phids) {
$statuses = array(
ArcanistDifferentialRevisionStatus::NEEDS_REVIEW,
);
$statuses = array_fuse($statuses);
$objects = $this->getRevisionsNotAuthored($this->objects, $phids);
$results = array();
foreach ($objects as $key => $object) {
if (!isset($statuses[$object->getStatus()])) {
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;
+ }
+
}
diff --git a/src/applications/differential/storage/DifferentialReviewer.php b/src/applications/differential/storage/DifferentialReviewer.php
index 08c9707225..836022c882 100644
--- a/src/applications/differential/storage/DifferentialReviewer.php
+++ b/src/applications/differential/storage/DifferentialReviewer.php
@@ -1,45 +1,50 @@
<?php
final class DifferentialReviewer
extends DifferentialDAO {
protected $revisionPHID;
protected $reviewerPHID;
protected $reviewerStatus;
protected $lastActionDiffPHID;
protected $lastCommentDiffPHID;
private $authority = array();
protected function getConfiguration() {
return array(
self::CONFIG_COLUMN_SCHEMA => array(
'reviewerStatus' => 'text64',
'lastActionDiffPHID' => 'phid?',
'lastCommentDiffPHID' => 'phid?',
),
self::CONFIG_KEY_SCHEMA => array(
'key_revision' => array(
'columns' => array('revisionPHID', 'reviewerPHID'),
'unique' => true,
),
),
) + parent::getConfiguration();
}
public function isUser() {
$user_type = PhabricatorPeopleUserPHIDType::TYPECONST;
return (phid_get_type($this->getReviewerPHID()) == $user_type);
}
public function attachAuthority(PhabricatorUser $user, $has_authority) {
$this->authority[$user->getCacheFragment()] = $has_authority;
return $this;
}
public function hasAuthority(PhabricatorUser $viewer) {
$cache_fragment = $viewer->getCacheFragment();
return $this->assertAttachedKey($this->authority, $cache_fragment);
}
+ public function isResigned() {
+ $status_resigned = DifferentialReviewerStatus::STATUS_RESIGNED;
+ return ($this->getReviewerStatus() == $status_resigned);
+ }
+
}
diff --git a/src/applications/differential/view/DifferentialReviewersView.php b/src/applications/differential/view/DifferentialReviewersView.php
index 54b7e35257..291f859d08 100644
--- a/src/applications/differential/view/DifferentialReviewersView.php
+++ b/src/applications/differential/view/DifferentialReviewersView.php
@@ -1,139 +1,160 @@
<?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();
- foreach ($this->reviewers as $reviewer) {
+
+ // 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);
$comment_phid = $reviewer->getLastCommentDiffPHID();
$is_current_comment = $this->isCurrent($comment_phid);
$item = new PHUIStatusItemView();
$item->setHighlighted($reviewer->hasAuthority($viewer));
switch ($reviewer->getReviewerStatus()) {
case DifferentialReviewerStatus::STATUS_ADDED:
if ($comment_phid) {
if ($is_current_comment) {
$item->setIcon(
'fa-comment',
'blue',
pht('Commented'));
} else {
$item->setIcon(
'fa-comment-o',
'bluegrey',
pht('Commented Previously'));
}
} else {
$item->setIcon(
PHUIStatusItemView::ICON_OPEN,
'bluegrey',
pht('Review Requested'));
}
break;
case DifferentialReviewerStatus::STATUS_ACCEPTED:
if ($is_current_action) {
$item->setIcon(
PHUIStatusItemView::ICON_ACCEPT,
'green',
pht('Accepted'));
} else {
$item->setIcon(
'fa-check-circle-o',
'bluegrey',
pht('Accepted Prior Diff'));
}
break;
case DifferentialReviewerStatus::STATUS_REJECTED:
if ($is_current_action) {
$item->setIcon(
PHUIStatusItemView::ICON_REJECT,
'red',
pht('Requested Changes'));
} else {
$item->setIcon(
'fa-times-circle-o',
'bluegrey',
pht('Requested Changes to Prior Diff'));
}
break;
case DifferentialReviewerStatus::STATUS_BLOCKING:
$item->setIcon(
PHUIStatusItemView::ICON_MINUS,
'red',
pht('Blocking Review'));
break;
+ case DifferentialReviewerStatus::STATUS_RESIGNED:
+ $item->setIcon(
+ 'fa-times',
+ 'grey',
+ pht('Resigned'));
+ break;
+
default:
$item->setIcon(
PHUIStatusItemView::ICON_QUESTION,
'bluegrey',
pht('%s?', $reviewer->getReviewerStatus()));
break;
}
$item->setTarget($handle->renderHovercardLink());
$view->addItem($item);
}
return $view;
}
private function isCurrent($action_phid) {
if (!$this->diff) {
echo "A\n";
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;
}
}
diff --git a/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php b/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
index c393ee51c5..42f644e8d1 100644
--- a/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php
@@ -1,197 +1,191 @@
<?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 isViewerFullyAccepted(
DifferentialRevision $revision,
PhabricatorUser $viewer) {
return $this->isViewerReviewerStatusFullyAmong(
$revision,
$viewer,
array(
DifferentialReviewerStatus::STATUS_ACCEPTED,
),
true);
}
protected function isViewerFullyRejected(
DifferentialRevision $revision,
PhabricatorUser $viewer) {
return $this->isViewerReviewerStatusFullyAmong(
$revision,
$viewer,
array(
DifferentialReviewerStatus::STATUS_REJECTED,
),
true);
}
protected function getViewerReviewerStatus(
DifferentialRevision $revision,
PhabricatorUser $viewer) {
if (!$viewer->getPHID()) {
return null;
}
foreach ($revision->getReviewers() as $reviewer) {
if ($reviewer->getReviewerPHID() != $viewer->getPHID()) {
continue;
}
return $reviewer->getReviewerStatus();
}
return null;
}
protected function isViewerReviewerStatusFullyAmong(
DifferentialRevision $revision,
PhabricatorUser $viewer,
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
// personal review has no state.
$status = $this->getViewerReviewerStatus($revision, $viewer);
if ($status === null) {
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->getReviewers() as $reviewer) {
if (!$reviewer->hasAuthority($viewer)) {
continue;
}
$status = $reviewer->getReviewerStatus();
if (!isset($status_map[$status])) {
return false;
}
if ($require_current) {
if ($reviewer->getLastActionDiffPHID() != $active_phid) {
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->getReviewers() 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,
),
);
}
// This is currently double-writing: to the old (edge) store and the new
// (reviewer) store. Do the old edge write first.
$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();
// Now, do the new write.
if ($map) {
$diff = $this->getEditor()->getActiveDiff($revision);
if ($diff) {
$diff_phid = $diff->getPHID();
} else {
$diff_phid = null;
}
$table = new DifferentialReviewer();
$reviewers = $table->loadAllWhere(
'revisionPHID = %s AND reviewerPHID IN (%Ls)',
$src_phid,
array_keys($map));
$reviewers = mpull($reviewers, null, 'getReviewerPHID');
foreach ($map as $dst_phid => $edge_data) {
$reviewer = idx($reviewers, $dst_phid);
if (!$reviewer) {
$reviewer = id(new DifferentialReviewer())
->setRevisionPHID($src_phid)
->setReviewerPHID($dst_phid);
}
$reviewer->setReviewerStatus($status);
if ($diff_phid) {
$reviewer->setLastActionDiffPHID($diff_phid);
}
- if ($status == DifferentialReviewerStatus::STATUS_RESIGNED) {
- if ($reviewer->getID()) {
- $reviewer->delete();
- }
- } else {
- try {
- $reviewer->save();
- } catch (AphrontDuplicateKeyQueryException $ex) {
- // At least for now, just ignore it if we lost a race.
- }
+ try {
+ $reviewer->save();
+ } catch (AphrontDuplicateKeyQueryException $ex) {
+ // At least for now, just ignore it if we lost a race.
}
}
}
}
}

File Metadata

Mime Type
text/x-diff
Expires
Fri, Mar 14, 4:34 PM (1 d, 10 h)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
72035
Default Alt Text
(20 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!)