From c1ae5321d745064340fdc7eba3d1c5e2267bda40 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 2 Dec 2015 05:55:31 -0800 Subject: [PATCH] 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 | 4 ++ .../AphrontBoolHTTPParameterType.php | 29 ++++++++++++ .../AphrontIntHTTPParameterType.php | 26 ++++++++++ .../editengine/PhabricatorEditEngine.php | 4 -- .../editfield/PhabricatorEditField.php | 43 ++++++++++++----- ...orApplicationEditHTTPParameterHelpView.php | 4 +- ...bricatorCustomFieldEditEngineExtension.php | 5 +- .../PhabricatorCustomFieldEditField.php | 47 +++++++++++++++---- .../field/PhabricatorCustomField.php | 16 ++++++- .../PhabricatorStandardCustomFieldBool.php | 4 ++ .../PhabricatorStandardCustomFieldInt.php | 4 ++ .../PhabricatorStandardCustomFieldLink.php | 4 ++ .../PhabricatorStandardCustomFieldPHIDs.php | 13 ++++- ...PhabricatorStandardCustomFieldRemarkup.php | 4 ++ .../PhabricatorStandardCustomFieldSelect.php | 4 ++ .../PhabricatorStandardCustomFieldText.php | 4 ++ .../PhabricatorStandardCustomFieldUsers.php | 4 ++ 17 files changed, 188 insertions(+), 31 deletions(-) create mode 100644 src/aphront/httpparametertype/AphrontBoolHTTPParameterType.php create mode 100644 src/aphront/httpparametertype/AphrontIntHTTPParameterType.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 2da6090204..0475b40d03 100644 --- a/src/__phutil_library_map__.php +++ b/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', diff --git a/src/aphront/httpparametertype/AphrontBoolHTTPParameterType.php b/src/aphront/httpparametertype/AphrontBoolHTTPParameterType.php new file mode 100644 index 0000000000..4fc758ddda --- /dev/null +++ b/src/aphront/httpparametertype/AphrontBoolHTTPParameterType.php @@ -0,0 +1,29 @@ +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', + ); + } + +} diff --git a/src/aphront/httpparametertype/AphrontIntHTTPParameterType.php b/src/aphront/httpparametertype/AphrontIntHTTPParameterType.php new file mode 100644 index 0000000000..e32614da02 --- /dev/null +++ b/src/aphront/httpparametertype/AphrontIntHTTPParameterType.php @@ -0,0 +1,26 @@ +getInt($key); + } + + protected function getParameterTypeName() { + return 'int'; + } + + protected function getParameterFormatDescriptions() { + return array( + pht('An integer.'), + ); + } + + protected function getParameterExamples() { + return array( + 'v=123', + ); + } + +} diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php index 484f7015c4..94284b2caa 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -685,10 +685,6 @@ abstract class PhabricatorEditEngine $field->readValueFromRequest($request); } - } else { - foreach ($fields as $field) { - $field->readValueFromObject($object); - } } } diff --git a/src/applications/transactions/editfield/PhabricatorEditField.php b/src/applications/transactions/editfield/PhabricatorEditField.php index c4d7494c5b..973f57d35d 100644 --- a/src/applications/transactions/editfield/PhabricatorEditField.php +++ b/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() { diff --git a/src/applications/transactions/view/PhabricatorApplicationEditHTTPParameterHelpView.php b/src/applications/transactions/view/PhabricatorApplicationEditHTTPParameterHelpView.php index e8397e6a65..9200bdcf1f 100644 --- a/src/applications/transactions/view/PhabricatorApplicationEditHTTPParameterHelpView.php +++ b/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; } diff --git a/src/infrastructure/customfield/editor/PhabricatorCustomFieldEditEngineExtension.php b/src/infrastructure/customfield/editor/PhabricatorCustomFieldEditEngineExtension.php index 2ae4f56d3c..07618aab67 100644 --- a/src/infrastructure/customfield/editor/PhabricatorCustomFieldEditEngineExtension.php +++ b/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) { diff --git a/src/infrastructure/customfield/editor/PhabricatorCustomFieldEditField.php b/src/infrastructure/customfield/editor/PhabricatorCustomFieldEditField.php index 00dd8f3895..3aba613e87 100644 --- a/src/infrastructure/customfield/editor/PhabricatorCustomFieldEditField.php +++ b/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; + } + } diff --git a/src/infrastructure/customfield/field/PhabricatorCustomField.php b/src/infrastructure/customfield/field/PhabricatorCustomField.php index 8fd071092d..18082c8fce 100644 --- a/src/infrastructure/customfield/field/PhabricatorCustomField.php +++ b/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 */ diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldBool.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldBool.php index 29192e5c17..e31b6bc6e8 100644 --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldBool.php +++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldBool.php @@ -129,4 +129,8 @@ final class PhabricatorStandardCustomFieldBool ); } + protected function getHTTPParameterType() { + return new AphrontBoolHTTPParameterType(); + } + } diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldInt.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldInt.php index cbb89275f9..8c54b45c3d 100644 --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldInt.php +++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldInt.php @@ -112,5 +112,9 @@ final class PhabricatorStandardCustomFieldInt } } + protected function getHTTPParameterType() { + return new AphrontIntHTTPParameterType(); + } + } diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldLink.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldLink.php index cd16c930d7..91352e3d60 100644 --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldLink.php +++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldLink.php @@ -80,4 +80,8 @@ final class PhabricatorStandardCustomFieldLink ); } + protected function getHTTPParameterType() { + return new AphrontStringHTTPParameterType(); + } + } diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php index 4c3159a621..216c7ccb63 100644 --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldPHIDs.php +++ b/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(); + } + } diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldRemarkup.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldRemarkup.php index e1b3a158cd..894d48dbb3 100644 --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldRemarkup.php +++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldRemarkup.php @@ -95,5 +95,9 @@ final class PhabricatorStandardCustomFieldRemarkup ); } + protected function getHTTPParameterType() { + return new AphrontStringHTTPParameterType(); + } + } diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldSelect.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldSelect.php index 7870d2acbd..06b5d5e01b 100644 --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldSelect.php +++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldSelect.php @@ -136,4 +136,8 @@ final class PhabricatorStandardCustomFieldSelect ->setValueMap($this->getOptions()); } + protected function getHTTPParameterType() { + return new AphrontSelectHTTPParameterType(); + } + } diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldText.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldText.php index ca519b0c79..4f6590b6a4 100644 --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldText.php +++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldText.php @@ -63,4 +63,8 @@ final class PhabricatorStandardCustomFieldText ); } + protected function getHTTPParameterType() { + return new AphrontStringHTTPParameterType(); + } + } diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldUsers.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldUsers.php index 2f1e6db53a..17b082e583 100644 --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldUsers.php +++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldUsers.php @@ -11,4 +11,8 @@ final class PhabricatorStandardCustomFieldUsers return new PhabricatorPeopleDatasource(); } + protected function getHTTPParameterType() { + return new AphrontUserListHTTPParameterType(); + } + }