Support HTTP parameter prefilling in EditEngine forms for CustomFields

Summary:
Ref T9132. This allows you to prefill custom fields with `?custom.x.y=value`, for most types of custom fields.

Dates (which are substantially more complicated) aren't supported. I'll just do those once the dust settles. Other types should work, I think.

Test Plan:
  - Verified custom fields appear on "HTTP Parameters" help UI.
  - Used `?x=y` to prefill custom fields on edit form.
  - Performed various normal edits.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9132

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

src/__phutil_library_map__.php

@@ -103,6 +103,7 @@ phutil_register_library_map(array(
     'AphrontAjaxResponse' => 'aphront/response/AphrontAjaxResponse.php',
     'AphrontApplicationConfiguration' => 'aphront/configuration/AphrontApplicationConfiguration.php',
     'AphrontBarView' => 'view/widget/bars/AphrontBarView.php',
+    'AphrontBoolHTTPParameterType' => 'aphront/httpparametertype/AphrontBoolHTTPParameterType.php',
     'AphrontCSRFException' => 'aphront/exception/AphrontCSRFException.php',
     'AphrontCalendarEventView' => 'applications/calendar/view/AphrontCalendarEventView.php',
     'AphrontController' => 'aphront/AphrontController.php',
@@ -139,6 +140,7 @@ phutil_register_library_map(array(
     'AphrontHTTPProxyResponse' => 'aphront/response/AphrontHTTPProxyResponse.php',
     'AphrontHTTPSink' => 'aphront/sink/AphrontHTTPSink.php',
     'AphrontHTTPSinkTestCase' => 'aphront/sink/__tests__/AphrontHTTPSinkTestCase.php',
+    'AphrontIntHTTPParameterType' => 'aphront/httpparametertype/AphrontIntHTTPParameterType.php',
     'AphrontIsolatedDatabaseConnectionTestCase' => 'infrastructure/storage/__tests__/AphrontIsolatedDatabaseConnectionTestCase.php',
     'AphrontIsolatedHTTPSink' => 'aphront/sink/AphrontIsolatedHTTPSink.php',
     'AphrontJSONResponse' => 'aphront/response/AphrontJSONResponse.php',
@@ -3907,6 +3909,7 @@ phutil_register_library_map(array(
     'AphrontAjaxResponse' => 'AphrontResponse',
     'AphrontApplicationConfiguration' => 'Phobject',
     'AphrontBarView' => 'AphrontView',
+    'AphrontBoolHTTPParameterType' => 'AphrontHTTPParameterType',
     'AphrontCSRFException' => 'AphrontException',
     'AphrontCalendarEventView' => 'AphrontView',
     'AphrontController' => 'Phobject',
@@ -3946,6 +3949,7 @@ phutil_register_library_map(array(
     'AphrontHTTPProxyResponse' => 'AphrontResponse',
     'AphrontHTTPSink' => 'Phobject',
     'AphrontHTTPSinkTestCase' => 'PhabricatorTestCase',
+    'AphrontIntHTTPParameterType' => 'AphrontHTTPParameterType',
     'AphrontIsolatedDatabaseConnectionTestCase' => 'PhabricatorTestCase',
     'AphrontIsolatedHTTPSink' => 'AphrontHTTPSink',
     'AphrontJSONResponse' => 'AphrontResponse',

src/aphront/httpparametertype/AphrontBoolHTTPParameterType.php

@@ -0,0 +1,29 @@
+<?php
+
+final class AphrontBoolHTTPParameterType
+  extends AphrontHTTPParameterType {
+
+  protected function getParameterValue(AphrontRequest $request, $key) {
+    return $request->getBool($key);
+  }
+
+  protected function getParameterTypeName() {
+    return 'bool';
+  }
+
+  protected function getParameterFormatDescriptions() {
+    return array(
+      pht('A boolean value (true or false).'),
+    );
+  }
+
+  protected function getParameterExamples() {
+    return array(
+      'v=true',
+      'v=false',
+      'v=1',
+      'v=0',
+    );
+  }
+
+}

src/aphront/httpparametertype/AphrontIntHTTPParameterType.php

@@ -0,0 +1,26 @@
+<?php
+
+final class AphrontIntHTTPParameterType
+  extends AphrontHTTPParameterType {
+
+  protected function getParameterValue(AphrontRequest $request, $key) {
+    return $request->getInt($key);
+  }
+
+  protected function getParameterTypeName() {
+    return 'int';
+  }
+
+  protected function getParameterFormatDescriptions() {
+    return array(
+      pht('An integer.'),
+    );
+  }
+
+  protected function getParameterExamples() {
+    return array(
+      'v=123',
+    );
+  }
+
+}

src/applications/transactions/editengine/PhabricatorEditEngine.php

@@ -685,10 +685,6 @@ abstract class PhabricatorEditEngine
 
           $field->readValueFromRequest($request);
         }
-      } else {
-        foreach ($fields as $field) {
-          $field->readValueFromObject($object);
-        }
       }
     }
 

src/applications/transactions/editfield/PhabricatorEditField.php

@@ -299,24 +299,27 @@ abstract class PhabricatorEditField extends Phobject {
   }
 
   public function readValueFromRequest(AphrontRequest $request) {
-    $check = array_merge(array($this->getKey()), $this->getAliases());
+    $check = $this->getAllReadValueFromRequestKeys();
     foreach ($check as $key) {
       if (!$this->getValueExistsInRequest($request, $key)) {
         continue;
       }
 
       $this->value = $this->getValueFromRequest($request, $key);
-      return;
+      break;
     }
-
-    $this->readValueFromObject($this->getObject());
-
     return $this;
   }
 
-  public function readValueFromObject($object) {
-    $this->value = $this->getValueFromObject($object);
-    return $this;
+  public function getAllReadValueFromRequestKeys() {
+    $keys = array();
+
+    $keys[] = $this->getKey();
+    foreach ($this->getAliases() as $alias) {
+      $keys[] = $alias;
+    }
+
+    return $keys;
   }
 
   public function readDefaultValueFromConfiguration($value) {
@@ -337,11 +340,11 @@ abstract class PhabricatorEditField extends Phobject {
   }
 
   protected function getValueExistsInRequest(AphrontRequest $request, $key) {
-    return $this->getValueExistsInSubmit($request, $key);
+    return $this->getHTTPParameterValueExists($request, $key);
   }
 
   protected function getValueFromRequest(AphrontRequest $request, $key) {
-    return $this->getValueFromSubmit($request, $key);
+    return $this->getHTTPParameterValue($request, $key);
   }
 
 
@@ -385,6 +388,16 @@ abstract class PhabricatorEditField extends Phobject {
   }
 
   protected function getValueExistsInSubmit(AphrontRequest $request, $key) {
+    return $this->getHTTPParameterValueExists($request, $key);
+  }
+
+  protected function getValueFromSubmit(AphrontRequest $request, $key) {
+    return $this->getHTTPParameterValue($request, $key);
+  }
+
+  protected function getHTTPParameterValueExists(
+    AphrontRequest $request,
+    $key) {
     $type = $this->getHTTPParameterType();
 
     if ($type) {
@@ -394,8 +407,14 @@ abstract class PhabricatorEditField extends Phobject {
     return false;
   }
 
-  protected function getValueFromSubmit(AphrontRequest $request, $key) {
-    return $this->getHTTPParameterType()->getValue($request, $key);
+  protected function getHTTPParameterValue($request, $key) {
+    $type = $this->getHTTPParameterType();
+
+    if ($type) {
+      return $type->getValue($request, $key);
+    }
+
+    return null;
   }
 
   protected function getDefaultValue() {

src/applications/transactions/view/PhabricatorApplicationEditHTTPParameterHelpView.php

@@ -95,7 +95,7 @@ EOTEXT
     foreach ($fields as $field) {
       $rows[] = array(
         $field->getLabel(),
-        $field->getKey(),
+        head($field->getAllReadValueFromRequestKeys()),
         $field->getHTTPParameterType()->getTypeName(),
         $field->getDescription(),
       );
@@ -150,7 +150,7 @@ EOTEXT
 
     $rows = array();
     foreach ($fields as $field) {
-      $aliases = $field->getAliases();
+      $aliases = array_slice($field->getAllReadValueFromRequestKeys(), 1);
       if (!$aliases) {
         continue;
       }

src/infrastructure/customfield/editor/PhabricatorCustomFieldEditEngineExtension.php

@@ -34,7 +34,10 @@ final class PhabricatorCustomFieldEditEngineExtension
       PhabricatorCustomField::ROLE_EDIT);
 
     $field_list->setViewer($viewer);
-    $field_list->readFieldsFromStorage($object);
+
+    if (!$engine->getIsCreate()) {
+      $field_list->readFieldsFromStorage($object);
+    }
 
     $results = array();
     foreach ($field_list->getFields() as $field) {

src/infrastructure/customfield/editor/PhabricatorCustomFieldEditField.php

@@ -4,6 +4,7 @@ final class PhabricatorCustomFieldEditField
   extends PhabricatorEditField {
 
   private $customField;
+  private $httpParameterType;
 
   public function setCustomField(PhabricatorCustomField $custom_field) {
     $this->customField = $custom_field;
@@ -14,14 +15,22 @@ final class PhabricatorCustomFieldEditField
     return $this->customField;
   }
 
+  public function setCustomFieldHTTPParameterType(
+    AphrontHTTPParameterType $type) {
+    $this->httpParameterType = $type;
+    return $this;
+  }
+
+  public function getCustomFieldHTTPParameterType() {
+    return $this->httpParameterType;
+  }
+
   protected function buildControl() {
     $field = $this->getCustomField();
     $clone = clone $field;
 
-    if ($this->getIsSubmittedForm()) {
-      $value = $this->getValue();
-      $clone->setValueFromApplicationTransactions($value);
-    }
+    $value = $this->getValue();
+    $clone->setValueFromApplicationTransactions($value);
 
     return $clone->renderEditControl(array());
   }
@@ -42,11 +51,6 @@ final class PhabricatorCustomFieldEditField
     return $clone->getNewValueForApplicationTransactions();
   }
 
-  protected function getValueExistsInRequest(AphrontRequest $request, $key) {
-    // For now, never read these out of the request.
-    return false;
-  }
-
   protected function getValueExistsInSubmit(AphrontRequest $request, $key) {
     return true;
   }
@@ -66,8 +70,31 @@ final class PhabricatorCustomFieldEditField
   }
 
   protected function newHTTPParameterType() {
-    // TODO: For now, don't support custom fields for HTTP prefill.
+    $type = $this->getCustomFieldHTTPParameterType();
+
+    if ($type) {
+      return clone $type;
+    }
+
     return null;
   }
 
+  public function getAllReadValueFromRequestKeys() {
+    $keys = array();
+
+    // NOTE: This piece of complexity is so we can expose a reasonable key in
+    // the UI ("custom.x") instead of a crufty internal key ("std:app:x").
+    // Perhaps we can simplify this some day.
+
+    // In the parent, this is just getKey(), but that returns a cumbersome
+    // key in EditFields. Use the simpler edit type key instead.
+    $keys[] = $this->getEditTypeKey();
+
+    foreach ($this->getAliases() as $alias) {
+      $keys[] = $alias;
+    }
+
+    return $keys;
+  }
+
 }

src/infrastructure/customfield/field/PhabricatorCustomField.php

@@ -1091,8 +1091,15 @@ abstract class PhabricatorCustomField extends Phobject {
   }
 
   protected function newEditField() {
-    return id(new PhabricatorCustomFieldEditField())
+    $field = id(new PhabricatorCustomFieldEditField())
       ->setCustomField($this);
+
+    $http_type = $this->getHTTPParameterType();
+    if ($http_type) {
+      $field->setCustomFieldHTTPParameterType($http_type);
+    }
+
+    return $field;
   }
 
   protected function newStandardEditField() {
@@ -1109,6 +1116,13 @@ abstract class PhabricatorCustomField extends Phobject {
       ->setValue($this->getNewValueForApplicationTransactions());
   }
 
+  protected function getHTTPParameterType() {
+    if ($this->proxy) {
+      return $this->proxy->getHTTPParameterType();
+    }
+    return null;
+  }
+
   /**
    * @task edit
    */

src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldBool.php

@@ -129,4 +129,8 @@ final class PhabricatorStandardCustomFieldBool
     );
   }
 
+  protected function getHTTPParameterType() {
+    return new AphrontBoolHTTPParameterType();
+  }
+
 }

src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldInt.php

@@ -112,5 +112,9 @@ final class PhabricatorStandardCustomFieldInt
     }
   }
 
+  protected function getHTTPParameterType() {
+    return new AphrontIntHTTPParameterType();
+  }
+
 
 }

src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldLink.php

@@ -80,4 +80,8 @@ final class PhabricatorStandardCustomFieldLink
     );
   }
 
+  protected function getHTTPParameterType() {
+    return new AphrontStringHTTPParameterType();
+  }
+
 }

src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php

@@ -34,14 +34,21 @@ abstract class PhabricatorStandardCustomFieldPHIDs
   }
 
   public function setValueFromStorage($value) {
+    // NOTE: We're accepting either a JSON string (a real storage value) or
+    // an array (from HTTP parameter prefilling). This is a little hacky, but
+    // should hold until this can get cleaned up more thoroughly.
+    // TODO: Clean this up.
+
     $result = array();
-    if ($value) {
+    if (!is_array($value)) {
       $value = json_decode($value, true);
       if (is_array($value)) {
         $result = array_values($value);
       }
     }
+
     $this->setFieldValue($value);
+
     return $this;
   }
 
@@ -213,4 +220,8 @@ abstract class PhabricatorStandardCustomFieldPHIDs
     return $value;
   }
 
+  protected function getHTTPParameterType() {
+    return new AphrontPHIDListHTTPParameterType();
+  }
+
 }

src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldRemarkup.php

@@ -95,5 +95,9 @@ final class PhabricatorStandardCustomFieldRemarkup
     );
   }
 
+  protected function getHTTPParameterType() {
+    return new AphrontStringHTTPParameterType();
+  }
+
 
 }

src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldSelect.php

@@ -136,4 +136,8 @@ final class PhabricatorStandardCustomFieldSelect
       ->setValueMap($this->getOptions());
   }
 
+  protected function getHTTPParameterType() {
+    return new AphrontSelectHTTPParameterType();
+  }
+
 }

src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldText.php

@@ -63,4 +63,8 @@ final class PhabricatorStandardCustomFieldText
     );
   }
 
+  protected function getHTTPParameterType() {
+    return new AphrontStringHTTPParameterType();
+  }
+
 }

src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldUsers.php

@@ -11,4 +11,8 @@ final class PhabricatorStandardCustomFieldUsers
     return new PhabricatorPeopleDatasource();
   }
 
+  protected function getHTTPParameterType() {
+    return new AphrontUserListHTTPParameterType();
+  }
+
 }