mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-20 13:52:40 +01:00
Display Reviewed By instead of first Reviewer in revisions overview
Summary: Displaying reviewer who was by coincidence listed first is quite confusing, especially for committed revisions. This displays the one who really reviewed the revision if available. This implementation is pretty bad from performance perspective - O(N) queries to retrieve all comments. The page load still feels quite fast. Test Plan: /differential/filter/revisions/ Reviewers: epriestley CC: aran, epriestley Differential Revision: https://secure.phabricator.com/D1772
This commit is contained in:
parent
f2caa6888e
commit
87c60abbd0
1 changed files with 16 additions and 9 deletions
|
@ -19,6 +19,7 @@
|
|||
final class DifferentialReviewersFieldSpecification
|
||||
extends DifferentialFieldSpecification {
|
||||
|
||||
private $primaryReviewers = array();
|
||||
private $reviewers;
|
||||
private $error;
|
||||
|
||||
|
@ -149,15 +150,16 @@ final class DifferentialReviewersFieldSpecification
|
|||
}
|
||||
|
||||
public function renderValueForRevisionList(DifferentialRevision $revision) {
|
||||
$reviewer_phids = $revision->getReviewers();
|
||||
if ($reviewer_phids) {
|
||||
$first = reset($reviewer_phids);
|
||||
if (count($reviewer_phids) > 1) {
|
||||
$suffix = ' (+'.(count($reviewer_phids) - 1).')';
|
||||
if (isset($this->primaryReviewers[$revision->getID()])) {
|
||||
$primary_reviewer = $this->primaryReviewers[$revision->getID()];
|
||||
$other_reviewers = array_flip($revision->getReviewers());
|
||||
unset($other_reviewers[$primary_reviewer]);
|
||||
if ($other_reviewers) {
|
||||
$suffix = ' (+'.(count($other_reviewers)).')';
|
||||
} else {
|
||||
$suffix = null;
|
||||
}
|
||||
return $this->getHandle($first)->renderLink().$suffix;
|
||||
return $this->getHandle($primary_reviewer)->renderLink().$suffix;
|
||||
} else {
|
||||
return '<em>None</em>';
|
||||
}
|
||||
|
@ -165,9 +167,14 @@ final class DifferentialReviewersFieldSpecification
|
|||
|
||||
public function getRequiredHandlePHIDsForRevisionList(
|
||||
DifferentialRevision $revision) {
|
||||
$reviewer_phids = $revision->getReviewers();
|
||||
if ($reviewer_phids) {
|
||||
return array(reset($reviewer_phids));
|
||||
$primary_reviewer = $revision->loadReviewedBy();
|
||||
if (!$primary_reviewer) {
|
||||
$reviewer_phids = $revision->getReviewers();
|
||||
$primary_reviewer = reset($reviewer_phids);
|
||||
}
|
||||
if ($primary_reviewer) {
|
||||
$this->primaryReviewers[$revision->getID()] = $primary_reviewer;
|
||||
return array($primary_reviewer);
|
||||
}
|
||||
return array();
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue