diff --git a/resources/celerity/map.php b/resources/celerity/map.php --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -7,7 +7,7 @@ */ return array( 'names' => array( - 'core.pkg.css' => 'b7b8d101', + 'core.pkg.css' => '487f8476', 'core.pkg.js' => '6972d365', 'darkconsole.pkg.js' => 'e7393ebb', 'differential.pkg.css' => '7ba78475', @@ -145,7 +145,7 @@ 'rsrc/css/phui/phui-info-view.css' => '28efab79', 'rsrc/css/phui/phui-list.css' => '9da2aa00', 'rsrc/css/phui/phui-object-box.css' => '6b487c57', - 'rsrc/css/phui/phui-object-item-list-view.css' => '8d99e42b', + 'rsrc/css/phui/phui-object-item-list-view.css' => 'e6c99cb3', 'rsrc/css/phui/phui-pager.css' => 'bea33d23', 'rsrc/css/phui/phui-pinboard-view.css' => '2495140e', 'rsrc/css/phui/phui-profile-menu.css' => 'c8557f33', @@ -843,7 +843,7 @@ 'phui-inline-comment-view-css' => '5953c28e', 'phui-list-view-css' => '9da2aa00', 'phui-object-box-css' => '6b487c57', - 'phui-object-item-list-view-css' => '8d99e42b', + 'phui-object-item-list-view-css' => 'e6c99cb3', 'phui-pager-css' => 'bea33d23', 'phui-pinboard-view-css' => '2495140e', 'phui-profile-menu-css' => 'c8557f33', diff --git a/src/applications/differential/query/DifferentialRevisionSearchEngine.php b/src/applications/differential/query/DifferentialRevisionSearchEngine.php --- a/src/applications/differential/query/DifferentialRevisionSearchEngine.php +++ b/src/applications/differential/query/DifferentialRevisionSearchEngine.php @@ -20,7 +20,8 @@ ->needFlags(true) ->needDrafts(true) ->needRelationships(true) - ->needReviewerStatus(true); + ->needReviewerStatus(true) + ->needReviewerAuthority(true); } protected function buildQueryFromParameters(array $map) { diff --git a/src/applications/differential/view/DifferentialReviewersView.php b/src/applications/differential/view/DifferentialReviewersView.php --- a/src/applications/differential/view/DifferentialReviewersView.php +++ b/src/applications/differential/view/DifferentialReviewersView.php @@ -23,6 +23,115 @@ return $this; } + public function getReviewerIcon(DifferentialReviewer $reviewer) { + $is_current = $this->getIsCurrent($reviewer); + + switch ($reviewer->getStatus()) { + case DifferentialReviewerStatus::STATUS_ADDED: + return PHUIStatusItemView::ICON_OPEN; + case DifferentialReviewerStatus::STATUS_ACCEPTED: + if ($is_current) { + return PHUIStatusItemView::ICON_ACCEPT; + } else { + return 'fa-check-circle-o'; + } + case DifferentialReviewerStatus::STATUS_ACCEPTED_OLDER: + return 'fa-check-circle-o'; + case DifferentialReviewerStatus::STATUS_REJECTED: + if ($is_current) { + return PHUIStatusItemView::ICON_REJECT; + } else { + return 'fa-times-circle-o'; + } + case DifferentialReviewerStatus::STATUS_REJECTED_OLDER: + return 'fa-times-circle-o'; + case DifferentialReviewerStatus::STATUS_COMMENTED: + return 'fa-question-circle'; + case DifferentialReviewerStatus::STATUS_BLOCKING: + return PHUIStatusItemView::ICON_MINUS; + default: + return PHUIStatusItemView::ICON_QUESTION; + } + } + + public function getReviewerColor(DifferentialReviewer $reviewer) { + $is_current = $this->getIsCurrent($reviewer); + + switch ($reviewer->getStatus()) { + case DifferentialReviewerStatus::STATUS_ADDED: + return 'bluegrey'; + case DifferentialReviewerStatus::STATUS_ACCEPTED: + if ($is_current) { + return 'green'; + } else { + return 'bluegrey'; + } + case DifferentialReviewerStatus::STATUS_ACCEPTED_OLDER: + return 'bluegrey'; + case DifferentialReviewerStatus::STATUS_REJECTED: + if ($is_current) { + return 'red'; + } else { + return 'bluegrey'; + } + case DifferentialReviewerStatus::STATUS_REJECTED_OLDER: + return 'bluegrey'; + case DifferentialReviewerStatus::STATUS_COMMENTED: + return 'blue'; + case DifferentialReviewerStatus::STATUS_BLOCKING: + return 'red'; + default: + return 'bluegrey'; + } + } + + public function getReviewerLabel(DifferentialReviewer $reviewer) { + $is_current = $this->getIsCurrent($reviewer); + + switch ($reviewer->getStatus()) { + case DifferentialReviewerStatus::STATUS_ADDED: + return pht('Review Requested'); + case DifferentialReviewerStatus::STATUS_ACCEPTED: + if ($is_current) { + return pht('Accepted'); + } else { + return pht('Accepted Prior Diff'); + } + case DifferentialReviewerStatus::STATUS_ACCEPTED_OLDER: + return pht('Accepted Prior Diff'); + case DifferentialReviewerStatus::STATUS_REJECTED: + if ($is_current) { + return pht('Requested Changes'); + } else { + return pht('Requested Prior Changes'); + } + case DifferentialReviewerStatus::STATUS_REJECTED_OLDER: + return pht('Requested Prior Changes'); + case DifferentialReviewerStatus::STATUS_COMMENTED: + return pht('Commented'); + case DifferentialReviewerStatus::STATUS_BLOCKING: + return pht('Blocking Review'); + default: + return pht('Unknown Status ("%s")', $reviewer->getStatus()); + } + } + + private function getIsCurrent(DifferentialReviewer $reviewer) { + // If we're missing either the diff or action information for the + // reviewer, render information as current. + + if (!$this->diff) { + return true; + } + + if (!$reviewer->getDiffID()) { + return true; + } + + return ($this->diff->getID() == $reviewer->getDiffID()); + } + + public function render() { $viewer = $this->getUser(); @@ -31,95 +140,16 @@ $phid = $reviewer->getReviewerPHID(); $handle = $this->handles[$phid]; - // If we're missing either the diff or action information for the - // reviewer, render information as current. - $is_current = (!$this->diff) || - (!$reviewer->getDiffID()) || - ($this->diff->getID() == $reviewer->getDiffID()); $item = new PHUIStatusItemView(); $item->setHighlighted($reviewer->hasAuthority($viewer)); - switch ($reviewer->getStatus()) { - case DifferentialReviewerStatus::STATUS_ADDED: - $item->setIcon( - PHUIStatusItemView::ICON_OPEN, - 'bluegrey', - pht('Review Requested')); - break; - - case DifferentialReviewerStatus::STATUS_ACCEPTED: - if ($is_current) { - $item->setIcon( - PHUIStatusItemView::ICON_ACCEPT, - 'green', - pht('Accepted')); - } else { - $item->setIcon( - PHUIStatusItemView::ICON_ACCEPT, - 'bluegrey', - pht('Accepted Prior Diff')); - } - break; - - case DifferentialReviewerStatus::STATUS_ACCEPTED_OLDER: - $item->setIcon( - 'fa-check-circle-o', - 'bluegrey', - pht('Accepted Prior Diff')); - break; - - case DifferentialReviewerStatus::STATUS_REJECTED: - if ($is_current) { - $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_REJECTED_OLDER: - $item->setIcon( - 'fa-times-circle-o', - 'bluegrey', - pht('Rejected Prior Diff')); - break; - - case DifferentialReviewerStatus::STATUS_COMMENTED: - if ($is_current) { - $item->setIcon( - 'fa-question-circle', - 'blue', - pht('Commented')); - } else { - $item->setIcon( - 'fa-question-circle-o', - 'bluegrey', - pht('Commented Previously')); - } - break; - - case DifferentialReviewerStatus::STATUS_BLOCKING: - $item->setIcon( - PHUIStatusItemView::ICON_MINUS, - 'red', - pht('Blocking Review')); - break; - - default: - $item->setIcon( - PHUIStatusItemView::ICON_QUESTION, - 'bluegrey', - pht('%s?', $reviewer->getStatus())); - break; - - } + $icon = $this->getReviewerIcon($reviewer); + $color = $this->getReviewerColor($reviewer); + $label = $this->getReviewerLabel($reviewer); + + $item->setIcon($icon, $color, $label); $item->setTarget($handle->renderHovercardLink()); $view->addItem($item); diff --git a/src/applications/differential/view/DifferentialRevisionListView.php b/src/applications/differential/view/DifferentialRevisionListView.php --- a/src/applications/differential/view/DifferentialRevisionListView.php +++ b/src/applications/differential/view/DifferentialRevisionListView.php @@ -82,6 +82,8 @@ $this->initBehavior('phabricator-tooltips', array()); $this->requireResource('aphront-tooltip-css'); + $handles = $this->handles; + $list = new PHUIObjectItemListView(); foreach ($this->revisions as $revision) { @@ -128,9 +130,11 @@ $item->addHeadIcon($icons['flag']); } - $item->setObjectName('D'.$revision->getID()); + $monogram = $revision->getMonogram(); + + $item->setObjectName($monogram); $item->setHeader($revision->getTitle()); - $item->setHref('/D'.$revision->getID()); + $item->setHref('/'.$monogram); if (isset($icons['draft'])) { $draft = id(new PHUIIconView()) @@ -143,20 +147,60 @@ $item->addAttribute($draft); } - /* Most things 'Need Review', so accept it's the default */ - if ($status != ArcanistDifferentialRevisionStatus::NEEDS_REVIEW) { - $item->addAttribute($status_name); - } - // Author - $author_handle = $this->handles[$revision->getAuthorPHID()]; + $author_handle = $handles[$revision->getAuthorPHID()]; $item->addByline(pht('Author: %s', $author_handle->renderLink())); + // TODO: As above, this should be based on `getReviewerStatus()` only, + // but we don't always load it. $reviewers = array(); - // TODO: As above, this should be based on `getReviewerStatus()`. - foreach ($revision->getReviewers() as $reviewer) { - $reviewers[] = $this->handles[$reviewer]->renderLink(); + try { + $reviewers_view = new DifferentialReviewersView(); + + $with_authority = array(); + $without_authority = array(); + + foreach ($revision->getReviewerStatus() as $reviewer) { + $reviewer_phid = $reviewer->getReviewerPHID(); + + $has_authority = $reviewer->hasAuthority($viewer); + if ($has_authority) { + $reviewer_class = 'viewer-has-authority'; + } else { + $reviewer_class = null; + } + + $icon = $reviewers_view->getReviewerIcon($reviewer); + $color = $reviewers_view->getReviewerColor($reviewer); + + $icon_view = id(new PHUIIconView()) + ->setIcon($icon, $color); + + $reviewer_node = phutil_tag( + 'span', + array( + 'class' => $reviewer_class, + ), + array( + $icon_view, + ' ', + $handles[$reviewer_phid]->renderLink(), + )); + + if ($has_authority) { + $with_authority[] = $reviewer_node; + } else { + $without_authority[] = $reviewer_node; + } + } + + $reviewers = array_merge($with_authority, $without_authority); + } catch (Exception $ex) { + foreach ($revision->getReviewers() as $reviewer) { + $reviewers[] = $handles[$reviewer]->renderLink(); + } } + if (!$reviewers) { $reviewers = phutil_tag('em', array(), pht('None')); } else { diff --git a/webroot/rsrc/css/phui/phui-object-item-list-view.css b/webroot/rsrc/css/phui/phui-object-item-list-view.css --- a/webroot/rsrc/css/phui/phui-object-item-list-view.css +++ b/webroot/rsrc/css/phui/phui-object-item-list-view.css @@ -472,6 +472,11 @@ background: {$sh-yellowbackground}; } +.viewer-has-authority { + padding: 0 2px; + background: {$sh-yellowbackground}; +} + ul.phui-object-item-list-view .phui-object-item-highlighted .phui-object-item-frame { border-color: {$sh-yellowborder};