Index: src/applications/differential/controller/DifferentialRevisionViewController.php =================================================================== --- src/applications/differential/controller/DifferentialRevisionViewController.php +++ src/applications/differential/controller/DifferentialRevisionViewController.php @@ -132,38 +132,6 @@ $aux_field->setHandles(array_select_keys($handles, $aux_phids[$key])); } - $reviewer_warning = null; - if ($revision->getStatus() == - ArcanistDifferentialRevisionStatus::NEEDS_REVIEW) { - $has_live_reviewer = false; - foreach ($revision->getReviewers() as $reviewer) { - if (!$handles[$reviewer]->isDisabled()) { - $has_live_reviewer = true; - break; - } - } - if (!$has_live_reviewer) { - $reviewer_warning = new AphrontErrorView(); - $reviewer_warning->setSeverity(AphrontErrorView::SEVERITY_WARNING); - $reviewer_warning->setTitle(pht('No Active Reviewers')); - if ($revision->getReviewers()) { - $reviewer_warning->appendChild( - phutil_tag( - 'p', - array(), - pht('All specified reviewers are disabled and this revision '. - 'needs review. You may want to add some new reviewers.'))); - } else { - $reviewer_warning->appendChild( - phutil_tag( - 'p', - array(), - pht('This revision has no specified reviewers and needs '. - 'review. You may want to add some reviewers.'))); - } - } - } - $request_uri = $request->getRequestURI(); $limit = 100; @@ -258,6 +226,13 @@ $revision_detail->setActions($actions); $revision_detail->setUser($user); + $revision_detail_box = $revision_detail->render(); + + $revision_warnings = $this->buildRevisionWarnings($revision, $handles); + if ($revision_warnings) { + $revision_detail_box->setErrorView($revision_warnings); + } + $comment_view = $this->buildTransactions( $revision, $diff_vs ? $diffs[$diff_vs] : $target, @@ -420,9 +395,8 @@ ->setNavigationMarker(true); $content = array( - $reviewer_warning, $top_anchor, - $revision_detail, + $revision_detail_box, $page_pane, ); @@ -941,4 +915,34 @@ return $timeline; } + private function buildRevisionWarnings( + DifferentialRevision $revision, + array $handles) { + + $status_needs_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; + if ($revision->getStatus() != $status_needs_review) { + return; + } + + foreach ($revision->getReviewers() as $reviewer) { + if (!$handles[$reviewer]->isDisabled()) { + return; + } + } + + $warnings = array(); + if ($revision->getReviewers()) { + $warnings[] = pht( + 'This revision needs review, but all specified reviewers are '. + 'disabled or inactive.'); + } else { + $warnings[] = pht( + 'This revision needs review, but there are no reviewers specified.'); + } + + return id(new AphrontErrorView()) + ->setSeverity(AphrontErrorView::SEVERITY_WARNING) + ->setErrors($warnings); + } + }