From 187836b8a92504828eb3158f3d87df57f3447a53 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 11 Feb 2015 13:00:59 -0800 Subject: [PATCH] Show open setup issue keys in "title" attribute of setup issues warning Summary: Ref T7184. I managed to write a phantom setup issue which fails normally and succeeds when looked at carefully, so clicking "you have open issues..." always cleared them. This made it very difficult to figure out what the problem was. Show issue keys in the "title" attribute to make this sort of thing easier to deal with. Test Plan: Moused over "You have issues..." text, saw issue key, quickly fixed issue with new information. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T7184 Differential Revision: https://secure.phabricator.com/D11743 --- .../config/check/PhabricatorSetupCheck.php | 24 +++++++++---------- .../PhabricatorConfigIssueListController.php | 4 ++-- .../PhabricatorConfigIssueViewController.php | 4 ++-- .../PhabricatorConfigWelcomeController.php | 2 +- src/view/page/PhabricatorStandardPageView.php | 5 ++-- 5 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/applications/config/check/PhabricatorSetupCheck.php b/src/applications/config/check/PhabricatorSetupCheck.php index 59a7b35b07..c5e6fae94e 100644 --- a/src/applications/config/check/PhabricatorSetupCheck.php +++ b/src/applications/config/check/PhabricatorSetupCheck.php @@ -40,25 +40,25 @@ abstract class PhabricatorSetupCheck { $this->executeChecks(); } - final public static function getOpenSetupIssueCount() { + final public static function getOpenSetupIssueKeys() { $cache = PhabricatorCaches::getSetupCache(); - return $cache->getKey('phabricator.setup.issues'); + return $cache->getKey('phabricator.setup.issue-keys'); } - final public static function setOpenSetupIssueCount($count) { + final public static function setOpenSetupIssueKeys(array $keys) { $cache = PhabricatorCaches::getSetupCache(); - $cache->setKey('phabricator.setup.issues', $count); + $cache->setKey('phabricator.setup.issue-keys', $keys); } - final public static function countUnignoredIssues(array $all_issues) { + final public static function getUnignoredIssueKeys(array $all_issues) { assert_instances_of($all_issues, 'PhabricatorSetupIssue'); - $count = 0; + $keys = array(); foreach ($all_issues as $issue) { if (!$issue->getIsIgnored()) { - $count++; + $keys[] = $issue->getIssueKey(); } } - return $count; + return $keys; } final public static function getConfigNeedsRepair() { @@ -76,13 +76,13 @@ abstract class PhabricatorSetupCheck { $cache->deleteKeys( array( 'phabricator.setup.needs-repair', - 'phabricator.setup.issues', + 'phabricator.setup.issue-keys', )); } final public static function willProcessRequest() { - $issue_count = self::getOpenSetupIssueCount(); - if ($issue_count === null) { + $issue_keys = self::getOpenSetupIssueKeys(); + if ($issue_keys === null) { $issues = self::runAllChecks(); foreach ($issues as $issue) { if ($issue->getIsFatal()) { @@ -92,7 +92,7 @@ abstract class PhabricatorSetupCheck { ->setView($view); } } - self::setOpenSetupIssueCount(self::countUnignoredIssues($issues)); + self::setOpenSetupIssueKeys(self::getUnignoredIssueKeys($issues)); } // Try to repair configuration unless we have a clean bill of health on it. diff --git a/src/applications/config/controller/PhabricatorConfigIssueListController.php b/src/applications/config/controller/PhabricatorConfigIssueListController.php index d1ebf85134..7a63a22a1a 100644 --- a/src/applications/config/controller/PhabricatorConfigIssueListController.php +++ b/src/applications/config/controller/PhabricatorConfigIssueListController.php @@ -11,8 +11,8 @@ final class PhabricatorConfigIssueListController $nav->selectFilter('issue/'); $issues = PhabricatorSetupCheck::runAllChecks(); - PhabricatorSetupCheck::setOpenSetupIssueCount( - PhabricatorSetupCheck::countUnignoredIssues($issues)); + PhabricatorSetupCheck::setOpenSetupIssueKeys( + PhabricatorSetupCheck::getUnignoredIssueKeys($issues)); $important = $this->buildIssueList( $issues, PhabricatorSetupCheck::GROUP_IMPORTANT); diff --git a/src/applications/config/controller/PhabricatorConfigIssueViewController.php b/src/applications/config/controller/PhabricatorConfigIssueViewController.php index dc0b68a452..6ee28bc833 100644 --- a/src/applications/config/controller/PhabricatorConfigIssueViewController.php +++ b/src/applications/config/controller/PhabricatorConfigIssueViewController.php @@ -14,8 +14,8 @@ final class PhabricatorConfigIssueViewController $user = $request->getUser(); $issues = PhabricatorSetupCheck::runAllChecks(); - PhabricatorSetupCheck::setOpenSetupIssueCount( - PhabricatorSetupCheck::countUnignoredIssues($issues)); + PhabricatorSetupCheck::setOpenSetupIssueKeys( + PhabricatorSetupCheck::getUnignoredIssueKeys($issues)); if (empty($issues[$this->issueKey])) { $content = id(new PHUIErrorView()) diff --git a/src/applications/config/controller/PhabricatorConfigWelcomeController.php b/src/applications/config/controller/PhabricatorConfigWelcomeController.php index 352d4d50b4..63078c784b 100644 --- a/src/applications/config/controller/PhabricatorConfigWelcomeController.php +++ b/src/applications/config/controller/PhabricatorConfigWelcomeController.php @@ -46,7 +46,7 @@ final class PhabricatorConfigWelcomeController true, $content); - $issues_resolved = !PhabricatorSetupCheck::getOpenSetupIssueCount(); + $issues_resolved = !PhabricatorSetupCheck::getOpenSetupIssueKeys(); $setup_href = PhabricatorEnv::getURI('/config/issue/'); if ($issues_resolved) { diff --git a/src/view/page/PhabricatorStandardPageView.php b/src/view/page/PhabricatorStandardPageView.php index 46c2df38ee..9016fa1138 100644 --- a/src/view/page/PhabricatorStandardPageView.php +++ b/src/view/page/PhabricatorStandardPageView.php @@ -335,7 +335,7 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView { // Render the "you have unresolved setup issues..." warning. $setup_warning = null; if ($user && $user->getIsAdmin()) { - $open = PhabricatorSetupCheck::getOpenSetupIssueCount(); + $open = PhabricatorSetupCheck::getOpenSetupIssueKeys(); if ($open) { $setup_warning = phutil_tag_div( 'setup-warning-callout', @@ -343,8 +343,9 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView { 'a', array( 'href' => '/config/issue/', + 'title' => implode(', ', $open), ), - pht('You have %d unresolved setup issue(s)...', $open))); + pht('You have %d unresolved setup issue(s)...', count($open)))); } }