Allow Differential custom fields to appear on edit and view interfaces

Summary: Depends on D798. Extends custom fields and makes the vaguely useful:
they can appear on the edit and view interfaces. This does not integrate them
with commit messages yet; that's more complicated but I plan to do it shortly.
Test Plan: Implemented a custom field per P123, it correctly appears on the edit
interface, persists, validates, and shows up when viewing the revision.
Reviewed By: jungejason
Reviewers: jungejason, tuomaspelkonen, aran
CC: aran, epriestley, jungejason
Differential Revision: 800

src/__phutil_library_map__.php

@@ -163,6 +163,8 @@ phutil_register_library_map(array(
     'DifferentialExceptionMail' => 'applications/differential/mail/exception',
     'DifferentialFieldSelector' => 'applications/differential/field/selector/base',
     'DifferentialFieldSpecification' => 'applications/differential/field/specification/base',
+    'DifferentialFieldSpecificationIncompleteException' => 'applications/differential/field/exception/incomplete',
+    'DifferentialFieldValidationException' => 'applications/differential/field/exception/validation',
     'DifferentialHunk' => 'applications/differential/storage/hunk',
     'DifferentialInlineComment' => 'applications/differential/storage/inlinecomment',
     'DifferentialInlineCommentEditController' => 'applications/differential/controller/inlinecommentedit',

src/applications/differential/controller/revisionedit/DifferentialRevisionEditController.php

@@ -41,6 +41,8 @@ class DifferentialRevisionEditController extends DifferentialController {
       $revision = new DifferentialRevision();
     }
 
+    $aux_fields = $this->loadAuxiliaryFields($revision);
+
     $diff_id = $request->getInt('diffID');
     if ($diff_id) {
       $diff = id(new DifferentialDiff())->load($diff_id);
@@ -66,8 +68,6 @@ class DifferentialRevisionEditController extends DifferentialController {
       $revision->setTitle($request->getStr('title'));
       $revision->setSummary($request->getStr('summary'));
       $revision->setTestPlan($request->getStr('testplan'));
-      $revision->setBlameRevision($request->getStr('blame'));
-      $revision->setRevertPlan($request->getStr('revert'));
 
       if (!strlen(trim($revision->getTitle()))) {
         $errors[] = 'You must provide a title.';
@@ -90,11 +90,21 @@ class DifferentialRevisionEditController extends DifferentialController {
         $e_reviewers = 'Invalid';
       }
 
+      foreach ($aux_fields as $aux_field) {
+        $aux_field->setValueFromRequest($request);
+        try {
+          $aux_field->validateField();
+        } catch (DifferentialFieldValidationException $ex) {
+          $errors[] = $ex->getMessage();
+        }
+      }
+
       if (!$errors) {
         $editor = new DifferentialRevisionEditor($revision, $user_phid);
         if ($diff) {
           $editor->addDiff($diff, $request->getStr('comments'));
         }
+        $editor->setAuxiliaryFields($aux_fields);
         $editor->setCCPHIDs($request->getArr('cc'));
         $editor->setReviewers($request->getArr('reviewers'));
         $editor->save();
@@ -185,20 +195,14 @@ class DifferentialRevisionEditController extends DifferentialController {
           ->setLabel('CC')
           ->setName('cc')
           ->setDatasource('/typeahead/common/mailable/')
-          ->setValue($cc_map))
+          ->setValue($cc_map));
-      ->appendChild(
+
-        id(new AphrontFormTextControl())
+    foreach ($aux_fields as $aux_field) {
-          ->setLabel('Blame Revision')
+      $control = $aux_field->renderEditControl();
-          ->setName('blame')
+      if ($control) {
-          ->setValue($revision->getBlameRevision())
+        $form->appendChild($control);
-          ->setCaption('Revision which broke the stuff which this '.
+      }
-                       'change fixes.'))
+    }
-      ->appendChild(
-        id(new AphrontFormTextAreaControl())
-          ->setLabel('Revert Plan')
-          ->setName('revert')
-          ->setValue($revision->getRevertPlan())
-          ->setCaption('Special steps required to safely revert this change.'));
 
     $submit = id(new AphrontFormSubmitControl())
       ->setValue('Save');
@@ -231,4 +235,19 @@ class DifferentialRevisionEditController extends DifferentialController {
       ));
   }
 
+  private function loadAuxiliaryFields(DifferentialRevision $revision) {
+
+    $aux_fields = DifferentialFieldSelector::newSelector()
+      ->getFieldSpecifications();
+    foreach ($aux_fields as $key => $aux_field) {
+      if (!$aux_field->shouldAppearOnEdit()) {
+        unset($aux_fields[$key]);
+      }
+    }
+
+    return DifferentialAuxiliaryField::loadFromStorage(
+      $revision,
+      $aux_fields);
+  }
+
 }

src/applications/differential/controller/revisionedit/__init__.php

@@ -10,13 +10,14 @@ phutil_require_module('phabricator', 'aphront/response/404');
 phutil_require_module('phabricator', 'aphront/response/redirect');
 phutil_require_module('phabricator', 'applications/differential/controller/base');
 phutil_require_module('phabricator', 'applications/differential/editor/revision');
+phutil_require_module('phabricator', 'applications/differential/field/selector/base');
+phutil_require_module('phabricator', 'applications/differential/storage/auxiliaryfield');
 phutil_require_module('phabricator', 'applications/differential/storage/diff');
 phutil_require_module('phabricator', 'applications/differential/storage/revision');
 phutil_require_module('phabricator', 'applications/phid/handle/data');
 phutil_require_module('phabricator', 'view/form/base');
 phutil_require_module('phabricator', 'view/form/control/divider');
 phutil_require_module('phabricator', 'view/form/control/submit');
-phutil_require_module('phabricator', 'view/form/control/text');
 phutil_require_module('phabricator', 'view/form/control/textarea');
 phutil_require_module('phabricator', 'view/form/control/tokenizer');
 phutil_require_module('phabricator', 'view/form/error');

src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php

@@ -43,6 +43,8 @@ class DifferentialRevisionViewController extends DifferentialController {
         "This revision has no diffs. Something has gone quite wrong.");
     }
 
+    $aux_fields = $this->loadAuxiliaryFields($revision);
+
     $diff_vs = $request->getInt('vs');
 
     $target = end($diffs);
@@ -153,6 +155,7 @@ class DifferentialRevisionViewController extends DifferentialController {
 
     $revision_detail = new DifferentialRevisionDetailView();
     $revision_detail->setRevision($revision);
+    $revision_detail->setAuxiliaryFields($aux_fields);
 
     $custom_renderer_class = PhabricatorEnv::getEnvConfig(
       'differential.revision-custom-detail-renderer');
@@ -715,4 +718,19 @@ class DifferentialRevisionViewController extends DifferentialController {
         ->setViewTime(time())
         ->replace();
   }
+
+  private function loadAuxiliaryFields(DifferentialRevision $revision) {
+    $aux_fields = DifferentialFieldSelector::newSelector()
+      ->getFieldSpecifications();
+    foreach ($aux_fields as $key => $aux_field) {
+      if (!$aux_field->shouldAppearOnRevisionView()) {
+        unset($aux_fields[$key]);
+      }
+    }
+
+    return DifferentialAuxiliaryField::loadFromStorage(
+      $revision,
+      $aux_fields);
+  }
+
 }

src/applications/differential/controller/revisionview/__init__.php

@@ -12,7 +12,9 @@ phutil_require_module('phabricator', 'applications/differential/constants/revisi
 phutil_require_module('phabricator', 'applications/differential/constants/unitstatus');
 phutil_require_module('phabricator', 'applications/differential/constants/unittestresult');
 phutil_require_module('phabricator', 'applications/differential/controller/base');
+phutil_require_module('phabricator', 'applications/differential/field/selector/base');
 phutil_require_module('phabricator', 'applications/differential/parser/changeset');
+phutil_require_module('phabricator', 'applications/differential/storage/auxiliaryfield');
 phutil_require_module('phabricator', 'applications/differential/storage/changeset');
 phutil_require_module('phabricator', 'applications/differential/storage/comment');
 phutil_require_module('phabricator', 'applications/differential/storage/diffproperty');

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

@@ -33,6 +33,8 @@ class DifferentialRevisionEditor {
   protected $silentUpdate;
   protected $tasks = null;
 
+  private $auxiliaryFields = array();
+
   public function __construct(DifferentialRevision $revision, $actor_phid) {
     $this->revision = $revision;
     $this->actorPHID = $actor_phid;
@@ -79,6 +81,11 @@ class DifferentialRevisionEditor {
     $this->setTasks($fields['tasks']);
   }
 
+  public function setAuxiliaryFields(array $auxiliary_fields) {
+    $this->auxiliaryFields = $auxiliary_fields;
+    return $this;
+  }
+
   public function getRevision() {
     return $this->revision;
   }
@@ -289,6 +296,8 @@ class DifferentialRevisionEditor {
       array_keys($add['ccs']),
       $this->actorPHID);
 
+    $this->updateAuxiliaryFields();
+
     // Add the author and users included from Herald rules to the relevant set
     // of users so they get a copy of the email.
     if (!$this->silentUpdate) {
@@ -644,5 +653,42 @@ class DifferentialRevisionEditor {
     }
   }
 
+  private function updateAuxiliaryFields() {
+    $aux_map = array();
+    foreach ($this->auxiliaryFields as $aux_field) {
+      $key = $aux_field->getStorageKey();
+      $val = $aux_field->getValueForStorage();
+
+      $aux_map[$key] = $val;
+    }
+
+    if (!$aux_map) {
+      return;
+    }
+
+    $revision = $this->revision;
+
+    $fields = id(new DifferentialAuxiliaryField())->loadAllWhere(
+      'revisionPHID = %s AND name IN (%Ls)',
+      $revision->getPHID(),
+      array_keys($aux_map));
+    $fields = mpull($fields, null, 'getName');
+
+    foreach ($aux_map as $key => $val) {
+      $obj = idx($fields, $key);
+      if (!$obj) {
+        $obj = new DifferentialAuxiliaryField();
+        $obj->setRevisionPHID($revision->getPHID());
+        $obj->setName($key);
+      }
+
+      if ($obj->getValue() !== $val) {
+        $obj->setValue($val);
+        $obj->save();
+      }
+    }
+  }
+
+
 }
 

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

@@ -10,6 +10,7 @@ phutil_require_module('phabricator', 'applications/differential/constants/action
 phutil_require_module('phabricator', 'applications/differential/constants/revisionstatus');
 phutil_require_module('phabricator', 'applications/differential/mail/ccwelcome');
 phutil_require_module('phabricator', 'applications/differential/mail/newdiff');
+phutil_require_module('phabricator', 'applications/differential/storage/auxiliaryfield');
 phutil_require_module('phabricator', 'applications/differential/storage/comment');
 phutil_require_module('phabricator', 'applications/differential/storage/revision');
 phutil_require_module('phabricator', 'applications/feed/constants/story');

src/applications/differential/field/exception/incomplete/DifferentialFieldSpecificationIncompleteException.php

@@ -0,0 +1,31 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+class DifferentialFieldSpecificationIncompleteException extends Exception {
+
+  public function __construct(DifferentialFieldSpecification $spec, $claim) {
+    $key = $spec->getStorageKey();
+    $class = get_class($spec);
+
+    parent::__construct(
+      "Differential field specification for '{$key}' (of class '{$class}') is ".
+      "incompletely implemented: it claims it should appear in a context but ".
+      "does not implement all the required methods for that context.");
+  }
+
+}

src/applications/differential/field/exception/incomplete/__init__.php

@@ -0,0 +1,10 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+
+phutil_require_source('DifferentialFieldSpecificationIncompleteException.php');

src/applications/differential/field/exception/validation/DifferentialFieldValidationException.php

@@ -0,0 +1,21 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+class DifferentialFieldValidationException extends Exception {
+
+}

src/applications/differential/field/exception/validation/__init__.php

@@ -0,0 +1,10 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+
+phutil_require_source('DifferentialFieldValidationException.php');

src/applications/differential/field/specification/base/DifferentialFieldSpecification.php

@@ -16,6 +16,119 @@
  * limitations under the License.
  */
 
+/**
+ * Describes and implements the behavior for a custom field on Differential
+ * revisions. Along with other configuration, you can extend this class to add
+ * custom fields to Differential revisions and commit messages.
+ *
+ * Generally, you should implement all methods from the storage task and then
+ * the methods from one or more interface tasks.
+ *
+ * @task storage Field Storage
+ * @task edit Extending the Revision Edit Interface
+ * @task view Extending the Revision View Interface
+ */
 abstract class DifferentialFieldSpecification {
 
+
+/* -(  Storage  )------------------------------------------------------------ */
+
+
+  /**
+   * Return a unique string used to key storage of this field's value, like
+   * "mycompany.fieldname" or similar.
+   *
+   * @return string Unique key which identifies this field in auxiliary field
+   *                storage. Maximum length is 32.
+   * @group storage
+   */
+  abstract public function getStorageKey();
+
+
+  /**
+   * Return a serialized representation of the field value, appropriate for
+   * storing in auxiliary field storage.
+   *
+   * @return string Serialized field value.
+   * @group storage
+   */
+  abstract public function getValueForStorage();
+
+
+  /**
+   * Set the field's value given a serialized storage value. This is called
+   * when the field is loaded; if no data is available, the value will be
+   * null.
+   *
+   * @param string|null Serialized field representation (from
+   *                    getValueForStorage) or null if no value has ever been
+   *                    stored.
+   * @return this
+   * @group storage
+   */
+  abstract public function setValueFromStorage($value);
+
+
+/* -(  Extending the Revision Edit Interface  )------------------------------ */
+
+
+  /**
+   * @task edit
+   */
+  public function shouldAppearOnEdit() {
+    return false;
+  }
+
+
+  /**
+   * @task edit
+   */
+  public function setValueFromRequest(AphrontRequest $request) {
+    throw new DifferentialFieldSpecificationIncompleteException($this);
+  }
+
+
+  /**
+   * @task edit
+   */
+  public function renderEditControl() {
+    throw new DifferentialFieldSpecificationIncompleteException($this);
+  }
+
+
+  /**
+   * @task edit
+   */
+  public function validateField() {
+    throw new DifferentialFieldSpecificationIncompleteException($this);
+  }
+
+
+/* -(  Extending the Revision View Interface  )------------------------------ */
+
+
+  /**
+   * @task view
+   */
+  public function shouldAppearOnRevisionView() {
+    return false;
+  }
+
+
+  /**
+   * @task view
+   */
+  public function renderLabelForRevisionView() {
+    throw new DifferentialFieldSpecificationIncompleteException($this);
+  }
+
+
+  /**
+   * @task view
+   */
+  public function renderValueForRevisionView() {
+    throw new DifferentialFieldSpecificationIncompleteException($this);
+  }
+
+
 }

src/applications/differential/field/specification/base/__init__.php

@@ -6,5 +6,7 @@
 
 
 
+phutil_require_module('phabricator', 'applications/differential/field/exception/incomplete');
+
 
 phutil_require_source('DifferentialFieldSpecification.php');

src/applications/differential/storage/auxiliaryfield/DifferentialAuxiliaryField.php

@@ -32,4 +32,28 @@ final class DifferentialAuxiliaryField extends DifferentialDAO {
     return $this;
   }
 
+  public static function loadFromStorage(
+    DifferentialRevision $revision,
+    array $aux_fields) {
+
+    $storage_keys = array_filter(mpull($aux_fields, 'getStorageKey'));
+    $field_data = array();
+    if ($storage_keys) {
+      $field_data = id(new DifferentialAuxiliaryField())->loadAllWhere(
+        'revisionPHID = %s AND name IN (%Ls)',
+        $revision->getPHID(),
+        $storage_keys);
+      $field_data = mpull($field_data, 'getValue', 'getName');
+    }
+
+    foreach ($aux_fields as $aux_field) {
+      $key = $aux_field->getStorageKey();
+      if ($key) {
+        $aux_field->setValueFromStorage(idx($field_data, $key));
+      }
+    }
+
+    return $aux_fields;
+  }
+
 }

src/applications/differential/storage/auxiliaryfield/__init__.php

@@ -8,5 +8,7 @@
 
 phutil_require_module('phabricator', 'applications/differential/storage/base');
 
+phutil_require_module('phutil', 'utils');
+
 
 phutil_require_source('DifferentialAuxiliaryField.php');

src/applications/differential/view/revisiondetail/DifferentialRevisionDetailView.php

@@ -22,6 +22,7 @@ final class DifferentialRevisionDetailView extends AphrontView {
   private $properties;
   private $actions;
   private $user;
+  private $auxiliaryFields = array();
 
   public function setRevision($revision) {
     $this->revision = $revision;
@@ -43,6 +44,11 @@ final class DifferentialRevisionDetailView extends AphrontView {
     return $this;
   }
 
+  public function setAuxiliaryFields(array $fields) {
+    $this->auxiliaryFields = $fields;
+    return $this;
+  }
+
   public function render() {
 
     require_celerity_resource('differential-core-view-css');
@@ -59,6 +65,18 @@ final class DifferentialRevisionDetailView extends AphrontView {
         '</tr>';
     }
 
+    foreach ($this->auxiliaryFields as $field) {
+      $value = $field->renderValueForRevisionView();
+      if ($value !== null) {
+        $label = $field->renderLabelForRevisionView();
+        $rows[] =
+          '<tr>'.
+            '<th>'.$label.'</th>'.
+            '<td>'.$value.'</td>'.
+          '</tr>';
+      }
+    }
+
     $properties =
       '<table class="differential-revision-properties">'.
         implode("\n", $rows).