From 4d8707df132dea42716f3f02490c321db24bc5dc Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 4 Oct 2013 18:31:41 -0700 Subject: [PATCH] Use status list UI to show reviewers in Differential Summary: Ref T1279. No logical changes, just updates the reviewer display style. We currently keep track of only "requested changes". Test Plan: See screenshot. Reviewers: btrahan, chad Reviewed By: chad CC: aran Maniphest Tasks: T1279 Differential Revision: https://secure.phabricator.com/D7228 --- ...ifferentialReviewersFieldSpecification.php | 42 ++++++++++++++++++- .../storage/DifferentialReviewer.php | 16 +------ .../controller/DiffusionCommitController.php | 4 +- 3 files changed, 45 insertions(+), 17 deletions(-) diff --git a/src/applications/differential/field/specification/DifferentialReviewersFieldSpecification.php b/src/applications/differential/field/specification/DifferentialReviewersFieldSpecification.php index 853d99a3ef..ca7bde3e87 100644 --- a/src/applications/differential/field/specification/DifferentialReviewersFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialReviewersFieldSpecification.php @@ -19,7 +19,47 @@ final class DifferentialReviewersFieldSpecification } public function renderValueForRevisionView() { - return $this->renderUserList($this->getReviewerPHIDs()); + if (!$this->getReviewerPHIDs()) { + // Renders "None". + return $this->renderUserList(array()); + } + + $revision = $this->getRevision(); + $reviewers = $revision->getReviewerStatus(); + + $diff = $revision->loadActiveDiff(); + if ($diff) { + $diff = $diff->getID(); + } + + $view = new PHUIStatusListView(); + $handles = $this->getLoadedHandles(); + foreach ($reviewers as $reviewer) { + $phid = $reviewer->getReviewerPHID(); + + $item = new PHUIStatusItemView(); + + switch ($reviewer->getStatus()) { + case DifferentialReviewerStatus::STATUS_ADDED: + $item->setIcon('open-dark', pht('Review Requested')); + break; + case DifferentialReviewerStatus::STATUS_REJECTED: + if ($reviewer->getDiffID() == $diff) { + $item->setIcon( + 'reject-red', + pht('Requested Changes')); + } else { + $item->setIcon( + 'reject-dark', + pht('Requested Changes to Prior Diff')); + } + break; + } + + $item->setTarget($handles[$phid]->renderLink()); + $view->addItem($item); + } + return $view; } private function getReviewerPHIDs() { diff --git a/src/applications/differential/storage/DifferentialReviewer.php b/src/applications/differential/storage/DifferentialReviewer.php index 142db9c38a..6dd13b1cbc 100644 --- a/src/applications/differential/storage/DifferentialReviewer.php +++ b/src/applications/differential/storage/DifferentialReviewer.php @@ -8,7 +8,8 @@ final class DifferentialReviewer { public function __construct($reviewer_phid, $status, $diff_id = null) { $this->reviewerPHID = $reviewer_phid; - $this->setStatus($status, $diff_id); + $this->status = $status; + $this->diffID = $diff_id; } public function getReviewerPHID() { @@ -23,17 +24,4 @@ final class DifferentialReviewer { return $this->diffID; } - public function setStatus($status, $diff_id = null) { - if ($status == DifferentialReviewerStatus::STATUS_REJECTED - && $diff_id === null) { - - throw new Exception('STATUS_REJECTED must have a diff_id set'); - } - - $this->status = $status; - $this->diffID = $diff_id; - - return $this; - } - } diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index e88e321566..5a3cdf2d9d 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -1004,10 +1004,10 @@ final class DiffusionCommitController extends DiffusionController { $item->setIcon('warning-dark', pht('Audit Requested')); break; case PhabricatorAuditStatusConstants::RESIGNED: - $item->setIcon('open-dark', pht('Accepted')); + $item->setIcon('open-dark', pht('Resigned')); break; case PhabricatorAuditStatusConstants::CLOSED: - $item->setIcon('accept-blue', pht('Accepted')); + $item->setIcon('accept-blue', pht('Closed')); break; case PhabricatorAuditStatusConstants::CC: $item->setIcon('info-dark', pht('Subscribed'));