From b701313e0ef23c05eb27ac7558fb7e320c44335c Mon Sep 17 00:00:00 2001
From: Chad Little <chad@chadsdomain.com>
Date: Tue, 10 Feb 2015 12:53:00 -0800
Subject: [PATCH] Split Setup Issues into Groups

Summary: Groups setup issues into Important, PHP, MySQL, and Base for easier parsing on initial installations.

Test Plan:
Test my internal server and various issues.

{F289699}

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7207

Differential Revision: https://secure.phabricator.com/D11726
---
 .../config/check/PhabricatorAPCSetupCheck.php |  4 +
 .../check/PhabricatorAuthSetupCheck.php       |  4 +
 .../check/PhabricatorBaseURISetupCheck.php    |  4 +
 .../check/PhabricatorBinariesSetupCheck.php   |  3 +
 .../check/PhabricatorDaemonsSetupCheck.php    |  4 +
 .../check/PhabricatorDatabaseSetupCheck.php   |  4 +
 .../check/PhabricatorElasticSetupCheck.php    |  4 +
 .../check/PhabricatorExtensionsSetupCheck.php |  4 +
 .../PhabricatorExtraConfigSetupCheck.php      |  4 +
 .../check/PhabricatorFileinfoSetupCheck.php   |  4 +
 .../config/check/PhabricatorGDSetupCheck.php  |  4 +
 .../PhabricatorImagemagickSetupCheck.php      |  4 +
 .../PhabricatorInvalidConfigSetupCheck.php    |  4 +
 .../check/PhabricatorMailSetupCheck.php       |  4 +
 .../check/PhabricatorMySQLSetupCheck.php      |  4 +
 .../check/PhabricatorPHPConfigSetupCheck.php  |  4 +
 .../check/PhabricatorPathSetupCheck.php       |  4 +
 .../check/PhabricatorPygmentSetupCheck.php    |  4 +
 .../PhabricatorRepositoriesSetupCheck.php     |  4 +
 .../check/PhabricatorSecuritySetupCheck.php   |  4 +
 .../config/check/PhabricatorSetupCheck.php    |  6 ++
 .../check/PhabricatorStorageSetupCheck.php    |  4 +
 .../check/PhabricatorTimezoneSetupCheck.php   |  4 +
 .../PhabricatorConfigIssueListController.php  | 80 ++++++++++++++-----
 .../config/issue/PhabricatorSetupIssue.php    | 14 ++++
 25 files changed, 167 insertions(+), 20 deletions(-)

diff --git a/src/applications/config/check/PhabricatorAPCSetupCheck.php b/src/applications/config/check/PhabricatorAPCSetupCheck.php
index e6efa7d76d..62925bbf90 100644
--- a/src/applications/config/check/PhabricatorAPCSetupCheck.php
+++ b/src/applications/config/check/PhabricatorAPCSetupCheck.php
@@ -2,6 +2,10 @@
 
 final class PhabricatorAPCSetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_OTHER;
+  }
+
   protected function executeChecks() {
     if (!extension_loaded('apc')) {
       $message = pht(
diff --git a/src/applications/config/check/PhabricatorAuthSetupCheck.php b/src/applications/config/check/PhabricatorAuthSetupCheck.php
index 82cae4d962..5d4f731867 100644
--- a/src/applications/config/check/PhabricatorAuthSetupCheck.php
+++ b/src/applications/config/check/PhabricatorAuthSetupCheck.php
@@ -2,6 +2,10 @@
 
 final class PhabricatorAuthSetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_IMPORTANT;
+  }
+
   protected function executeChecks() {
     // NOTE: We're not actually building these providers. Building providers
     // can require additional configuration to be present (e.g., to build
diff --git a/src/applications/config/check/PhabricatorBaseURISetupCheck.php b/src/applications/config/check/PhabricatorBaseURISetupCheck.php
index ccbadf7bbb..ae22476fdf 100644
--- a/src/applications/config/check/PhabricatorBaseURISetupCheck.php
+++ b/src/applications/config/check/PhabricatorBaseURISetupCheck.php
@@ -2,6 +2,10 @@
 
 final class PhabricatorBaseURISetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_IMPORTANT;
+  }
+
   protected function executeChecks() {
     $base_uri = PhabricatorEnv::getEnvConfig('phabricator.base-uri');
 
diff --git a/src/applications/config/check/PhabricatorBinariesSetupCheck.php b/src/applications/config/check/PhabricatorBinariesSetupCheck.php
index 3df43e8546..a63c6b5500 100644
--- a/src/applications/config/check/PhabricatorBinariesSetupCheck.php
+++ b/src/applications/config/check/PhabricatorBinariesSetupCheck.php
@@ -2,6 +2,9 @@
 
 final class PhabricatorBinariesSetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_OTHER;
+  }
 
   protected function executeChecks() {
 
diff --git a/src/applications/config/check/PhabricatorDaemonsSetupCheck.php b/src/applications/config/check/PhabricatorDaemonsSetupCheck.php
index d7c93dc03b..52f4fe2f37 100644
--- a/src/applications/config/check/PhabricatorDaemonsSetupCheck.php
+++ b/src/applications/config/check/PhabricatorDaemonsSetupCheck.php
@@ -2,6 +2,10 @@
 
 final class PhabricatorDaemonsSetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_IMPORTANT;
+  }
+
   protected function executeChecks() {
 
     $task_daemon = id(new PhabricatorDaemonLogQuery())
diff --git a/src/applications/config/check/PhabricatorDatabaseSetupCheck.php b/src/applications/config/check/PhabricatorDatabaseSetupCheck.php
index 16b8764294..3f7af1bdb4 100644
--- a/src/applications/config/check/PhabricatorDatabaseSetupCheck.php
+++ b/src/applications/config/check/PhabricatorDatabaseSetupCheck.php
@@ -2,6 +2,10 @@
 
 final class PhabricatorDatabaseSetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_IMPORTANT;
+  }
+
   public function getExecutionOrder() {
     // This must run after basic PHP checks, but before most other checks.
     return 0.5;
diff --git a/src/applications/config/check/PhabricatorElasticSetupCheck.php b/src/applications/config/check/PhabricatorElasticSetupCheck.php
index e1ee4f4dc0..7b60444a88 100644
--- a/src/applications/config/check/PhabricatorElasticSetupCheck.php
+++ b/src/applications/config/check/PhabricatorElasticSetupCheck.php
@@ -2,6 +2,10 @@
 
 final class PhabricatorElasticSetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_OTHER;
+  }
+
   protected function executeChecks() {
     if (PhabricatorDefaultSearchEngineSelector::shouldUseElasticSearch()) {
       $engine = PhabricatorSearchEngineSelector::newSelector()->newEngine();
diff --git a/src/applications/config/check/PhabricatorExtensionsSetupCheck.php b/src/applications/config/check/PhabricatorExtensionsSetupCheck.php
index 8d2de44d19..fa723cc7da 100644
--- a/src/applications/config/check/PhabricatorExtensionsSetupCheck.php
+++ b/src/applications/config/check/PhabricatorExtensionsSetupCheck.php
@@ -2,6 +2,10 @@
 
 final class PhabricatorExtensionsSetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_PHP;
+  }
+
   public function getExecutionOrder() {
     return 0;
   }
diff --git a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php
index 0b3e427953..c6f5b8b694 100644
--- a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php
+++ b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php
@@ -2,6 +2,10 @@
 
 final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_OTHER;
+  }
+
   protected function executeChecks() {
     $ancient_config = self::getAncientConfig();
 
diff --git a/src/applications/config/check/PhabricatorFileinfoSetupCheck.php b/src/applications/config/check/PhabricatorFileinfoSetupCheck.php
index 4bbfeed4c0..4f5013c033 100644
--- a/src/applications/config/check/PhabricatorFileinfoSetupCheck.php
+++ b/src/applications/config/check/PhabricatorFileinfoSetupCheck.php
@@ -2,6 +2,10 @@
 
 final class PhabricatorFileinfoSetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_OTHER;
+  }
+
   protected function executeChecks() {
     if (!extension_loaded('fileinfo')) {
       $message = pht(
diff --git a/src/applications/config/check/PhabricatorGDSetupCheck.php b/src/applications/config/check/PhabricatorGDSetupCheck.php
index 556c38e2e3..f738715bce 100644
--- a/src/applications/config/check/PhabricatorGDSetupCheck.php
+++ b/src/applications/config/check/PhabricatorGDSetupCheck.php
@@ -2,6 +2,10 @@
 
 final class PhabricatorGDSetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_OTHER;
+  }
+
   protected function executeChecks() {
     if (!extension_loaded('gd')) {
       $message = pht(
diff --git a/src/applications/config/check/PhabricatorImagemagickSetupCheck.php b/src/applications/config/check/PhabricatorImagemagickSetupCheck.php
index 0f0ddfc553..55c538f3a2 100644
--- a/src/applications/config/check/PhabricatorImagemagickSetupCheck.php
+++ b/src/applications/config/check/PhabricatorImagemagickSetupCheck.php
@@ -2,6 +2,10 @@
 
 final class PhabricatorImagemagickSetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_OTHER;
+  }
+
   protected function executeChecks() {
     $imagemagick = PhabricatorEnv::getEnvConfig('files.enable-imagemagick');
     if ($imagemagick) {
diff --git a/src/applications/config/check/PhabricatorInvalidConfigSetupCheck.php b/src/applications/config/check/PhabricatorInvalidConfigSetupCheck.php
index 0eb9908b70..576ff21f2e 100644
--- a/src/applications/config/check/PhabricatorInvalidConfigSetupCheck.php
+++ b/src/applications/config/check/PhabricatorInvalidConfigSetupCheck.php
@@ -2,6 +2,10 @@
 
 final class PhabricatorInvalidConfigSetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_OTHER;
+  }
+
   protected function executeChecks() {
     $groups = PhabricatorApplicationConfigOptions::loadAll();
     foreach ($groups as $group) {
diff --git a/src/applications/config/check/PhabricatorMailSetupCheck.php b/src/applications/config/check/PhabricatorMailSetupCheck.php
index cb9fedb367..ff789d4aee 100644
--- a/src/applications/config/check/PhabricatorMailSetupCheck.php
+++ b/src/applications/config/check/PhabricatorMailSetupCheck.php
@@ -2,6 +2,10 @@
 
 final class PhabricatorMailSetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_OTHER;
+  }
+
   protected function executeChecks() {
     $adapter = PhabricatorEnv::getEnvConfig('metamta.mail-adapter');
 
diff --git a/src/applications/config/check/PhabricatorMySQLSetupCheck.php b/src/applications/config/check/PhabricatorMySQLSetupCheck.php
index b198af1412..053f31394b 100644
--- a/src/applications/config/check/PhabricatorMySQLSetupCheck.php
+++ b/src/applications/config/check/PhabricatorMySQLSetupCheck.php
@@ -2,6 +2,10 @@
 
 final class PhabricatorMySQLSetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_MYSQL;
+  }
+
   public static function loadRawConfigValue($key) {
     $conn_raw = id(new PhabricatorUser())->establishConnection('w');
 
diff --git a/src/applications/config/check/PhabricatorPHPConfigSetupCheck.php b/src/applications/config/check/PhabricatorPHPConfigSetupCheck.php
index 50da517f31..57d97226a6 100644
--- a/src/applications/config/check/PhabricatorPHPConfigSetupCheck.php
+++ b/src/applications/config/check/PhabricatorPHPConfigSetupCheck.php
@@ -2,6 +2,10 @@
 
 final class PhabricatorPHPConfigSetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_PHP;
+  }
+
   public function getExecutionOrder() {
     return 0;
   }
diff --git a/src/applications/config/check/PhabricatorPathSetupCheck.php b/src/applications/config/check/PhabricatorPathSetupCheck.php
index 52cddd95d2..4833e4861a 100644
--- a/src/applications/config/check/PhabricatorPathSetupCheck.php
+++ b/src/applications/config/check/PhabricatorPathSetupCheck.php
@@ -2,6 +2,10 @@
 
 final class PhabricatorPathSetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_OTHER;
+  }
+
   protected function executeChecks() {
     // NOTE: We've already appended `environment.append-paths`, so we don't
     // need to explicitly check for it.
diff --git a/src/applications/config/check/PhabricatorPygmentSetupCheck.php b/src/applications/config/check/PhabricatorPygmentSetupCheck.php
index bfb5f7e39e..26f1f46332 100644
--- a/src/applications/config/check/PhabricatorPygmentSetupCheck.php
+++ b/src/applications/config/check/PhabricatorPygmentSetupCheck.php
@@ -2,6 +2,10 @@
 
 final class PhabricatorPygmentSetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_OTHER;
+  }
+
   protected function executeChecks() {
     $pygment = PhabricatorEnv::getEnvConfig('pygments.enabled');
 
diff --git a/src/applications/config/check/PhabricatorRepositoriesSetupCheck.php b/src/applications/config/check/PhabricatorRepositoriesSetupCheck.php
index de78709a9a..89967f7eaa 100644
--- a/src/applications/config/check/PhabricatorRepositoriesSetupCheck.php
+++ b/src/applications/config/check/PhabricatorRepositoriesSetupCheck.php
@@ -2,6 +2,10 @@
 
 final class PhabricatorRepositoriesSetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_OTHER;
+  }
+
   protected function executeChecks() {
 
     $cluster_services = id(new AlmanacServiceQuery())
diff --git a/src/applications/config/check/PhabricatorSecuritySetupCheck.php b/src/applications/config/check/PhabricatorSecuritySetupCheck.php
index 654f0d5f44..d7ae7db53a 100644
--- a/src/applications/config/check/PhabricatorSecuritySetupCheck.php
+++ b/src/applications/config/check/PhabricatorSecuritySetupCheck.php
@@ -2,6 +2,10 @@
 
 final class PhabricatorSecuritySetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_OTHER;
+  }
+
   protected function executeChecks() {
 
     // This checks for a version of bash with the "Shellshock" vulnerability.
diff --git a/src/applications/config/check/PhabricatorSetupCheck.php b/src/applications/config/check/PhabricatorSetupCheck.php
index 90e599e883..85a9152274 100644
--- a/src/applications/config/check/PhabricatorSetupCheck.php
+++ b/src/applications/config/check/PhabricatorSetupCheck.php
@@ -6,6 +6,11 @@ abstract class PhabricatorSetupCheck {
 
   abstract protected function executeChecks();
 
+  const GROUP_OTHER       = 'other';
+  const GROUP_MYSQL       = 'mysql';
+  const GROUP_PHP         = 'php';
+  const GROUP_IMPORTANT   = 'important';
+
   public function getExecutionOrder() {
     return 1;
   }
@@ -14,6 +19,7 @@ abstract class PhabricatorSetupCheck {
     $issue = id(new PhabricatorSetupIssue())
       ->setIssueKey($key);
     $this->issues[$key] = $issue;
+    $issue->setGroup($this->getDefaultGroup());
 
     return $issue;
   }
diff --git a/src/applications/config/check/PhabricatorStorageSetupCheck.php b/src/applications/config/check/PhabricatorStorageSetupCheck.php
index 377729c364..12296f03a9 100644
--- a/src/applications/config/check/PhabricatorStorageSetupCheck.php
+++ b/src/applications/config/check/PhabricatorStorageSetupCheck.php
@@ -2,6 +2,10 @@
 
 final class PhabricatorStorageSetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_OTHER;
+  }
+
   /**
    * @phutil-external-symbol class PhabricatorStartup
    */
diff --git a/src/applications/config/check/PhabricatorTimezoneSetupCheck.php b/src/applications/config/check/PhabricatorTimezoneSetupCheck.php
index 7f304a3f7e..5fcf74a294 100644
--- a/src/applications/config/check/PhabricatorTimezoneSetupCheck.php
+++ b/src/applications/config/check/PhabricatorTimezoneSetupCheck.php
@@ -2,6 +2,10 @@
 
 final class PhabricatorTimezoneSetupCheck extends PhabricatorSetupCheck {
 
+  public function getDefaultGroup() {
+    return self::GROUP_OTHER;
+  }
+
   protected function executeChecks() {
     $php_value = ini_get('date.timezone');
     if ($php_value) {
diff --git a/src/applications/config/controller/PhabricatorConfigIssueListController.php b/src/applications/config/controller/PhabricatorConfigIssueListController.php
index 99bb4d8ff7..d1ebf85134 100644
--- a/src/applications/config/controller/PhabricatorConfigIssueListController.php
+++ b/src/applications/config/controller/PhabricatorConfigIssueListController.php
@@ -14,18 +14,49 @@ final class PhabricatorConfigIssueListController
     PhabricatorSetupCheck::setOpenSetupIssueCount(
       PhabricatorSetupCheck::countUnignoredIssues($issues));
 
-    $list = $this->buildIssueList($issues);
-    $list->setNoDataString(pht('There are no open setup issues.'));
-    $list->setStackable(true);
+    $important = $this->buildIssueList(
+      $issues, PhabricatorSetupCheck::GROUP_IMPORTANT);
+    $php = $this->buildIssueList(
+      $issues, PhabricatorSetupCheck::GROUP_PHP);
+    $mysql = $this->buildIssueList(
+      $issues, PhabricatorSetupCheck::GROUP_MYSQL);
+    $other = $this->buildIssueList(
+      $issues, PhabricatorSetupCheck::GROUP_OTHER);
 
-    $box = id(new PHUIObjectBoxView())
-      ->setHeaderText(pht('Open Phabricator Setup Issues'))
-      ->appendChild($list);
+    $setup_issues = array();
+    if ($important) {
+      $setup_issues[] = id(new PHUIObjectBoxView())
+        ->setHeaderText(pht('Important Setup Issues'))
+        ->appendChild($important);
+    }
 
-    $nav->appendChild(
-      array(
-        $box,
-      ));
+    if ($php) {
+      $setup_issues[] = id(new PHUIObjectBoxView())
+        ->setHeaderText(pht('PHP Setup Issues'))
+        ->appendChild($php);
+    }
+
+    if ($mysql) {
+      $setup_issues[] = id(new PHUIObjectBoxView())
+        ->setHeaderText(pht('MySQL Setup Issues'))
+        ->appendChild($mysql);
+    }
+
+    if ($other) {
+      $setup_issues[] = id(new PHUIObjectBoxView())
+        ->setHeaderText(pht('Other Setup Issues'))
+        ->appendChild($other);
+    }
+
+    if (empty($setup_issues)) {
+      $setup_issues[] = id(new PHUIErrorView())
+        ->setTitle(pht('No Issues'))
+        ->appendChild(
+          pht('Your install has no current setup issues to resolve.'))
+        ->setSeverity(PHUIErrorView::SEVERITY_NOTICE);
+    }
+
+    $nav->appendChild($setup_issues);
 
     $title = pht('Setup Issues');
 
@@ -42,25 +73,30 @@ final class PhabricatorConfigIssueListController
       ));
   }
 
-  private function buildIssueList(array $issues) {
+  private function buildIssueList(array $issues, $group) {
     assert_instances_of($issues, 'PhabricatorSetupIssue');
     $list = new PHUIObjectItemListView();
+    $list->setStackable(true);
     $ignored_items = array();
+    $items = 0;
 
     foreach ($issues as $issue) {
+      if ($issue->getGroup() == $group) {
+        $items++;
         $href = $this->getApplicationURI('/issue/'.$issue->getIssueKey().'/');
         $item = id(new PHUIObjectItemView())
           ->setHeader($issue->getName())
           ->setHref($href)
           ->addAttribute($issue->getSummary());
-      if (!$issue->getIsIgnored()) {
-        $item->setBarColor('yellow');
-        $list->addItem($item);
-      } else {
-        $item->addIcon('fa-eye-slash', pht('Ignored'));
-        $item->setDisabled(true);
-        $item->setBarColor('none');
-        $ignored_items[] = $item;
+        if (!$issue->getIsIgnored()) {
+          $item->setBarColor('yellow');
+          $list->addItem($item);
+        } else {
+          $item->addIcon('fa-eye-slash', pht('Ignored'));
+          $item->setDisabled(true);
+          $item->setBarColor('none');
+          $ignored_items[] = $item;
+        }
       }
     }
 
@@ -68,7 +104,11 @@ final class PhabricatorConfigIssueListController
       $list->addItem($item);
     }
 
-    return $list;
+    if ($items == 0) {
+      return null;
+    } else {
+      return $list;
+    }
   }
 
 }
diff --git a/src/applications/config/issue/PhabricatorSetupIssue.php b/src/applications/config/issue/PhabricatorSetupIssue.php
index 133f55e017..f1415fec05 100644
--- a/src/applications/config/issue/PhabricatorSetupIssue.php
+++ b/src/applications/config/issue/PhabricatorSetupIssue.php
@@ -8,6 +8,7 @@ final class PhabricatorSetupIssue {
   private $isFatal;
   private $summary;
   private $shortName;
+  private $group;
 
   private $isIgnored = false;
   private $phpExtensions = array();
@@ -40,6 +41,19 @@ final class PhabricatorSetupIssue {
     return $this->shortName;
   }
 
+  public function setGroup($group) {
+    $this->group = $group;
+    return $this;
+  }
+
+  public function getGroup() {
+    if ($this->group) {
+      return $this->group;
+    } else {
+      return PhabricatorSetupCheck::GROUP_OTHER;
+    }
+  }
+
   public function setName($name) {
     $this->name = $name;
     return $this;