1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-20 13:52:40 +01:00

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
This commit is contained in:
epriestley 2015-02-11 13:00:59 -08:00
parent 2a2b47326c
commit 187836b8a9
5 changed files with 20 additions and 19 deletions

View file

@ -40,25 +40,25 @@ abstract class PhabricatorSetupCheck {
$this->executeChecks(); $this->executeChecks();
} }
final public static function getOpenSetupIssueCount() { final public static function getOpenSetupIssueKeys() {
$cache = PhabricatorCaches::getSetupCache(); $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 = 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'); assert_instances_of($all_issues, 'PhabricatorSetupIssue');
$count = 0; $keys = array();
foreach ($all_issues as $issue) { foreach ($all_issues as $issue) {
if (!$issue->getIsIgnored()) { if (!$issue->getIsIgnored()) {
$count++; $keys[] = $issue->getIssueKey();
} }
} }
return $count; return $keys;
} }
final public static function getConfigNeedsRepair() { final public static function getConfigNeedsRepair() {
@ -76,13 +76,13 @@ abstract class PhabricatorSetupCheck {
$cache->deleteKeys( $cache->deleteKeys(
array( array(
'phabricator.setup.needs-repair', 'phabricator.setup.needs-repair',
'phabricator.setup.issues', 'phabricator.setup.issue-keys',
)); ));
} }
final public static function willProcessRequest() { final public static function willProcessRequest() {
$issue_count = self::getOpenSetupIssueCount(); $issue_keys = self::getOpenSetupIssueKeys();
if ($issue_count === null) { if ($issue_keys === null) {
$issues = self::runAllChecks(); $issues = self::runAllChecks();
foreach ($issues as $issue) { foreach ($issues as $issue) {
if ($issue->getIsFatal()) { if ($issue->getIsFatal()) {
@ -92,7 +92,7 @@ abstract class PhabricatorSetupCheck {
->setView($view); ->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. // Try to repair configuration unless we have a clean bill of health on it.

View file

@ -11,8 +11,8 @@ final class PhabricatorConfigIssueListController
$nav->selectFilter('issue/'); $nav->selectFilter('issue/');
$issues = PhabricatorSetupCheck::runAllChecks(); $issues = PhabricatorSetupCheck::runAllChecks();
PhabricatorSetupCheck::setOpenSetupIssueCount( PhabricatorSetupCheck::setOpenSetupIssueKeys(
PhabricatorSetupCheck::countUnignoredIssues($issues)); PhabricatorSetupCheck::getUnignoredIssueKeys($issues));
$important = $this->buildIssueList( $important = $this->buildIssueList(
$issues, PhabricatorSetupCheck::GROUP_IMPORTANT); $issues, PhabricatorSetupCheck::GROUP_IMPORTANT);

View file

@ -14,8 +14,8 @@ final class PhabricatorConfigIssueViewController
$user = $request->getUser(); $user = $request->getUser();
$issues = PhabricatorSetupCheck::runAllChecks(); $issues = PhabricatorSetupCheck::runAllChecks();
PhabricatorSetupCheck::setOpenSetupIssueCount( PhabricatorSetupCheck::setOpenSetupIssueKeys(
PhabricatorSetupCheck::countUnignoredIssues($issues)); PhabricatorSetupCheck::getUnignoredIssueKeys($issues));
if (empty($issues[$this->issueKey])) { if (empty($issues[$this->issueKey])) {
$content = id(new PHUIErrorView()) $content = id(new PHUIErrorView())

View file

@ -46,7 +46,7 @@ final class PhabricatorConfigWelcomeController
true, true,
$content); $content);
$issues_resolved = !PhabricatorSetupCheck::getOpenSetupIssueCount(); $issues_resolved = !PhabricatorSetupCheck::getOpenSetupIssueKeys();
$setup_href = PhabricatorEnv::getURI('/config/issue/'); $setup_href = PhabricatorEnv::getURI('/config/issue/');
if ($issues_resolved) { if ($issues_resolved) {

View file

@ -335,7 +335,7 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView {
// Render the "you have unresolved setup issues..." warning. // Render the "you have unresolved setup issues..." warning.
$setup_warning = null; $setup_warning = null;
if ($user && $user->getIsAdmin()) { if ($user && $user->getIsAdmin()) {
$open = PhabricatorSetupCheck::getOpenSetupIssueCount(); $open = PhabricatorSetupCheck::getOpenSetupIssueKeys();
if ($open) { if ($open) {
$setup_warning = phutil_tag_div( $setup_warning = phutil_tag_div(
'setup-warning-callout', 'setup-warning-callout',
@ -343,8 +343,9 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView {
'a', 'a',
array( array(
'href' => '/config/issue/', '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))));
} }
} }