Make Herald Rules sticky in X-Herald-Rules

Summary:
See T354. List every rule which has ever been applied in X-Herald-Rules, not
just the ones which most recently triggered.

Also some random fixes while I was debugging this:

  - When conduit methods throw non-conduit exceptions, make sure they get
logged.
  - Trigger the Facebook "tasks" backcompat block only if we were going to fail
(this should reduce the shakniess of the transition).
  - Fix some log spew from the new field stuff.

Test Plan:
  - Created a rule (ID #3) "No Zebras" which triggers for revisions without
"zebra" in the title.
  - Created a revision without "zebra" in the title, got X-Herald-Rules: <2>,
<3>
  - Updated revision to have "zebra" in the title, verified rule did not trigger
in Herald transcript.
  - Verified X-Herald-Rules is still: <2>, <3>

Reviewed By: aran
Reviewers: aran, jungejason, tuomaspelkonen
CC: aran, epriestley
Differential Revision: 817

src/applications/conduit/controller/api/PhabricatorConduitAPIController.php

@@ -124,6 +124,7 @@ class PhabricatorConduitAPIController
         list($error_code, $error_info) = $auth_error;
       }
     } catch (Exception $ex) {
+      phlog($ex);
       $result = null;
       $error_code = 'ERR-CONDUIT-CORE';
       $error_info = $ex->getMessage();

src/applications/conduit/controller/api/__init__.php

@@ -17,6 +17,7 @@ phutil_require_module('phabricator', 'storage/queryfx');
 phutil_require_module('phabricator', 'view/control/table');
 phutil_require_module('phabricator', 'view/layout/panel');
 
+phutil_require_module('phutil', 'error');
 phutil_require_module('phutil', 'markup');
 phutil_require_module('phutil', 'parser/json');
 phutil_require_module('phutil', 'utils');

src/applications/differential/editor/revision/DifferentialRevisionEditor.php

@@ -83,15 +83,13 @@ class DifferentialRevisionEditor {
     $aux_fields = mpull($aux_fields, null, 'getCommitMessageKey');
 
     foreach ($fields as $field => $value) {
-
-      if ($field == 'tasks') {
-        // TODO: Deprecate once this can be fully supported with custom fields.
-        // This is just to prevent a backcompat break for Facebook.
-        $this->setTasks($value);
-        continue;
-      }
-
       if (empty($aux_fields[$field])) {
+        if ($field == 'tasks') {
+          // TODO: Deprecate once this can be fully supported with custom
+          // fields. This is just to prevent a backcompat break for Facebook.
+          $this->setTasks($value);
+          continue;
+        }
         throw new Exception(
           "Parsed commit message contains unrecognized field '{$field}'.");
       }
@@ -253,7 +251,7 @@ class DifferentialRevisionEditor {
       $xscript_phid = $xscript->getPHID();
       $xscript_header = $xscript->getXHeraldRulesHeader();
 
-      HeraldTranscript::saveXHeraldRulesHeader(
+      $xscript_header = HeraldTranscript::saveXHeraldRulesHeader(
         $revision->getPHID(),
         $xscript_header);
 

src/applications/differential/field/specification/ccs/DifferentialCCsFieldSpecification.php

@@ -19,7 +19,7 @@
 final class DifferentialCCsFieldSpecification
   extends DifferentialFieldSpecification {
 
-  private $ccs;
+  private $ccs = array();
 
   public function shouldAppearOnRevisionView() {
     return true;

src/applications/differential/field/specification/summary/DifferentialSummaryFieldSpecification.php

@@ -19,7 +19,7 @@
 final class DifferentialSummaryFieldSpecification
   extends DifferentialFieldSpecification {
 
-  private $summary;
+  private $summary = '';
 
   public function shouldAppearOnEdit() {
     $this->summary = $this->getRevision()->getSummary();
@@ -51,7 +51,7 @@ final class DifferentialSummaryFieldSpecification
   }
 
   public function setValueFromParsedCommitMessage($value) {
-    $this->summary = $value;
+    $this->summary = (string)$value;
     return $this;
   }
 

src/applications/herald/storage/transcript/base/HeraldTranscript.php

@@ -60,6 +60,13 @@ class HeraldTranscript extends HeraldDAO {
   }
 
   public static function saveXHeraldRulesHeader($phid, $header) {
+
+    // Combine any existing header with the new header, listing all rules
+    // which have ever triggered for this object.
+    $header = self::combineXHeraldRulesHeaders(
+      self::loadXHeraldRulesHeader($phid),
+      $header);
+
     queryfx(
       id(new HeraldTranscript())->establishConnection('w'),
       'INSERT INTO %T (phid, header) VALUES (%s, %s)
@@ -67,6 +74,16 @@ class HeraldTranscript extends HeraldDAO {
       self::TABLE_SAVED_HEADER,
       $phid,
       $header);
+
+    return $header;
+  }
+
+  private static function combineXHeraldRulesHeaders($u, $v) {
+    $u = preg_split('/[, ]+/', $u);
+    $v = preg_split('/[, ]+/', $v);
+
+    $combined = array_unique(array_filter(array_merge($u, $v)));
+    return implode(', ', $combined);
   }
 
   public static function loadXHeraldRulesHeader($phid) {