Improve resolution process for nonfatal setup issues

Summary:
  - When a setup issue is nonfatal (i.e., a warning), instruct the user to edit the value from the web UI instead of using `bin/config`.
  - When the user edits configuration in response to a setup issue, send them back to the issue when they're done.
  - When an issue relates to PHP configuration, link to the PHP documentation on configuration.
  - Add new-style setup check for timezone issues.

Test Plan: Mucked with my timezone config, resolved the issues I created.

Reviewers: codeblock, btrahan, vrana

Reviewed By: codeblock

CC: aran

Maniphest Tasks: T2221, T2228

Differential Revision: https://secure.phabricator.com/D4298

src/__celerity_resource_map__.php

@@ -3141,7 +3141,7 @@ celerity_register_resource_map(array(
   ),
   'setup-issue-css' =>
   array(
-    'uri' => '/res/4214704f/rsrc/css/application/config/setup-issue.css',
+    'uri' => '/res/d642d4e5/rsrc/css/application/config/setup-issue.css',
     'type' => 'css',
     'requires' =>
     array(

src/__phutil_library_map__.php

@@ -1143,6 +1143,7 @@ phutil_register_library_map(array(
     'PhabricatorSetup' => 'infrastructure/PhabricatorSetup.php',
     'PhabricatorSetupCheck' => 'applications/config/check/PhabricatorSetupCheck.php',
     'PhabricatorSetupCheckAPC' => 'applications/config/check/PhabricatorSetupCheckAPC.php',
+    'PhabricatorSetupCheckTimezone' => 'applications/config/check/PhabricatorSetupCheckTimezone.php',
     'PhabricatorSetupIssue' => 'applications/config/issue/PhabricatorSetupIssue.php',
     'PhabricatorSetupIssueView' => 'applications/config/view/PhabricatorSetupIssueView.php',
     'PhabricatorSlowvoteChoice' => 'applications/slowvote/storage/PhabricatorSlowvoteChoice.php',
@@ -2426,6 +2427,7 @@ phutil_register_library_map(array(
     'PhabricatorSettingsPanelSSHKeys' => 'PhabricatorSettingsPanel',
     'PhabricatorSettingsPanelSearchPreferences' => 'PhabricatorSettingsPanel',
     'PhabricatorSetupCheckAPC' => 'PhabricatorSetupCheck',
+    'PhabricatorSetupCheckTimezone' => 'PhabricatorSetupCheck',
     'PhabricatorSetupIssueView' => 'AphrontView',
     'PhabricatorSlowvoteChoice' => 'PhabricatorSlowvoteDAO',
     'PhabricatorSlowvoteComment' => 'PhabricatorSlowvoteDAO',

src/applications/cache/PhabricatorCaches.php

@@ -28,9 +28,9 @@ final class PhabricatorCaches {
     static $cache;
     if (!$cache) {
       $caches = self::buildSetupCaches();
+      $caches = self::addProfilerToCaches($caches);
       $cache = id(new PhutilKeyValueCacheStack())
-        ->setCaches($caches)
-        ->setProfiler(PhutilServiceProfiler::getInstance());
+        ->setCaches($caches);
     }
     return $cache;
   }
@@ -160,4 +160,13 @@ final class PhabricatorCaches {
     return true;
   }
 
+  private static function addProfilerToCaches(array $caches) {
+    foreach ($caches as $key => $cache) {
+      $pcache = new PhutilKeyValueCacheProfiler($cache);
+      $pcache->setProfiler(PhutilServiceProfiler::getInstance());
+      $caches[$key] = $pcache;
+    }
+    return $caches;
+  }
+
 }

src/applications/cache/PhabricatorKeyValueDatabaseCache.php

@@ -7,17 +7,6 @@ final class PhabricatorKeyValueDatabaseCache
   const CACHE_FORMAT_DEFLATE    = 'deflate';
 
   public function setKeys(array $keys, $ttl = null) {
-    $call_id = null;
-    if ($this->getProfiler()) {
-      $call_id = $this->getProfiler()->beginServiceCall(
-        array(
-          'type' => 'kvcache-set',
-          'name' => 'phabricator-db',
-          'keys' => array_keys($keys),
-          'ttl'  => $ttl,
-        ));
-    }
-
     if ($keys) {
       $map = $this->digestKeys(array_keys($keys));
       $conn_w = $this->establishConnection('w');
@@ -58,24 +47,10 @@ final class PhabricatorKeyValueDatabaseCache
       unset($guard);
     }
 
-    if ($call_id) {
-      $this->getProfiler()->endServiceCall($call_id, array());
-    }
-
     return $this;
   }
 
   public function getKeys(array $keys) {
-    $call_id = null;
-    if ($this->getProfiler()) {
-      $call_id = $this->getProfiler()->beginServiceCall(
-        array(
-          'type' => 'kvcache-get',
-          'name' => 'phabricator-db',
-          'keys' => $keys,
-        ));
-    }
-
     $results = array();
     if ($keys) {
       $map = $this->digestKeys($keys);
@@ -109,28 +84,10 @@ final class PhabricatorKeyValueDatabaseCache
       }
     }
 
-    if ($call_id) {
-      $this->getProfiler()->endServiceCall(
-        $call_id,
-        array(
-          'hits' => array_keys($results),
-        ));
-    }
-
     return $results;
   }
 
   public function deleteKeys(array $keys) {
-    $call_id = null;
-    if ($this->getProfiler()) {
-      $call_id = $this->getProfiler()->beginServiceCall(
-        array(
-          'type' => 'kvcache-del',
-          'name' => 'phabricator-db',
-          'keys' => $keys,
-        ));
-    }
-
     if ($keys) {
       $map = $this->digestKeys($keys);
       queryfx(
@@ -140,10 +97,6 @@ final class PhabricatorKeyValueDatabaseCache
         $keys);
     }
 
-    if ($call_id) {
-      $this->getProfiler()->endServiceCall($call_id, array());
-    }
-
     return $this;
   }
 

src/applications/config/check/PhabricatorSetupCheckTimezone.php

@@ -0,0 +1,31 @@
+<?php
+
+final class PhabricatorSetupCheckTimezone extends PhabricatorSetupCheck {
+
+  protected function executeChecks() {
+    $timezone = nonempty(
+      PhabricatorEnv::getEnvConfig('phabricator.timezone'),
+      ini_get('date.timezone'));
+    if ($timezone) {
+      return;
+    }
+
+    $summary = pht(
+      "Without a configured timezone, PHP will emit warnings when working ".
+      "with dates, and dates and times may not display correctly.");
+
+    $message = pht(
+      "Your configuration fails to specify a server timezone. You can either ".
+      "set the PHP configuration value 'date.timezone' or the Phabricator ".
+      "configuration value 'phabricator.timezone' to specify one.");
+
+    $this
+      ->newIssue('config.timezone')
+      ->setShortName(pht('Timezone'))
+      ->setName(pht('Server Timezone Not Configured'))
+      ->setSummary($summary)
+      ->setMessage($message)
+      ->addPHPConfig('date.timezone')
+      ->addPhabricatorConfig('phabricator.timezone');
+  }
+}

src/applications/config/controller/PhabricatorConfigEditController.php

@@ -22,6 +22,14 @@ final class PhabricatorConfigEditController
     $group = $option->getGroup();
     $group_uri = $this->getApplicationURI('group/'.$group->getKey().'/');
 
+    $issue = $request->getStr('issue');
+    if ($issue) {
+      // If the user came here from an open setup issue, send them back.
+      $done_uri = $this->getApplicationURI('issue/'.$issue.'/');
+    } else {
+      $done_uri = $group_uri;
+    }
+
     // TODO: This isn't quite correct -- we should read from the entire
     // configuration stack, ignoring database configuration. For now, though,
     // it's a reasonable approximation.
@@ -62,7 +70,7 @@ final class PhabricatorConfigEditController
       } else {
         // TODO: When we do Transactions, make this just set isDeleted = 1
         $config_entry->delete();
-        return id(new AphrontRedirectResponse())->setURI($group_uri);
+        return id(new AphrontRedirectResponse())->setURI($done_uri);
       }
 
       $config_entry->setValue($value);
@@ -70,7 +78,7 @@ final class PhabricatorConfigEditController
 
       if (!$errors) {
         $config_entry->save();
-        return id(new AphrontRedirectResponse())->setURI($group_uri);
+        return id(new AphrontRedirectResponse())->setURI($done_uri);
       }
     }
 
@@ -88,6 +96,7 @@ final class PhabricatorConfigEditController
 
     $form
       ->setUser($user)
+      ->addHiddenInput('issue', $request->getStr('issue'))
       ->appendChild(
         id(new AphrontFormTextAreaControl())
           ->setLabel('JSON Value')
@@ -98,7 +107,7 @@ final class PhabricatorConfigEditController
         ->setName('value'))
       ->appendChild(
         id(new AphrontFormSubmitControl())
-          ->addCancelButton($group_uri)
+          ->addCancelButton($done_uri)
           ->setValue(pht('Save Config Entry')))
       ->appendChild(
         phutil_render_tag(

src/applications/config/view/PhabricatorSetupIssueView.php

@@ -23,16 +23,16 @@ final class PhabricatorSetupIssueView extends AphrontView {
       ),
       nl2br(phutil_escape_html($issue->getMessage())));
 
-    $configs = $issue->getPhabricatorConfig();
-    if ($configs) {
-      $description .= $this->renderPhabricatorConfig($configs);
-    }
-
     $configs = $issue->getPHPConfig();
     if ($configs) {
       $description .= $this->renderPHPConfig($configs);
     }
 
+    $configs = $issue->getPhabricatorConfig();
+    if ($configs) {
+      $description .= $this->renderPhabricatorConfig($configs);
+    }
+
     $commands = $issue->getCommands();
     if ($commands) {
       $run_these = pht("Run these %d command(s):", count($commands));
@@ -111,6 +111,8 @@ final class PhabricatorSetupIssueView extends AphrontView {
   }
 
   private function renderPhabricatorConfig(array $configs) {
+    $issue = $this->getIssue();
+
     $table_info = phutil_render_tag(
       'p',
       array(),
@@ -141,10 +143,10 @@ final class PhabricatorSetupIssueView extends AphrontView {
     $table = phutil_render_tag(
       'table',
       array(
-
       ),
       implode("\n", $table));
 
+    if ($this->getIssue()->getIsFatal()) {
       $update_info = phutil_render_tag(
         'p',
         array(),
@@ -161,6 +163,23 @@ final class PhabricatorSetupIssueView extends AphrontView {
         $update[] = $cmd;
       }
       $update = phutil_render_tag('pre', array(), implode("\n", $update));
+    } else {
+      $update_info = phutil_render_tag(
+        'p',
+        array(),
+        pht("You can update these %d value(s) here:", count($configs)));
+      $update = array();
+      foreach ($configs as $config) {
+        $link = phutil_render_tag(
+          'a',
+          array(
+            'href' => '/config/edit/'.$config.'/?issue='.$issue->getIssueKey(),
+          ),
+          pht('Edit %s', phutil_escape_html($config)));
+        $update[] = '<li>'.$link.'</li>';
+      }
+      $update = '<ul>'.implode("\n", $update).'</ul>';
+    }
 
     return phutil_render_tag(
       'div',
@@ -276,8 +295,21 @@ final class PhabricatorSetupIssueView extends AphrontView {
       'p',
       array(),
       pht(
-        "After editing the PHP configuration, <strong>restart your webserver ".
-        "for the changes to take effect</strong>."));
+        "You can find more information about PHP configuration values in the ".
+        "%s.",
+        phutil_render_tag(
+          'a',
+          array(
+            'href' => 'http://php.net/manual/ini.list.php',
+          ),
+          pht('PHP Documentation'))));
+
+    $info .= phutil_render_tag(
+      'p',
+      array(),
+      pht(
+        "After editing the PHP configuration, <strong>restart your ".
+        "webserver for the changes to take effect</strong>."));
 
     return phutil_render_tag(
       'div',

src/applications/paste/query/PhabricatorPasteQuery.php

@@ -124,8 +124,10 @@ final class PhabricatorPasteQuery
   }
 
   private function loadContent(array $pastes) {
-    $cache = id(new PhabricatorKeyValueDatabaseCache())
-      ->setProfiler(PhutilServiceProfiler::getInstance());
+    $cache = new PhabricatorKeyValueDatabaseCache();
+
+    $cache = new PhutilKeyValueCacheProfiler($cache);
+    $cache->setProfiler(PhutilServiceProfiler::getInstance());
 
     $keys = array();
     foreach ($pastes as $paste) {

webroot/rsrc/css/application/config/setup-issue.css

@@ -84,3 +84,9 @@
   margin: 15px 0;
   padding: 0 20px;
 }
+
+.setup-issue ul {
+  width: 90%;
+  margin: 1em auto;
+  list-style: circle;
+}