From 0558d53273e57f93b929dc79197a0bfa4870143d Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 19 Sep 2013 11:56:15 -0700 Subject: [PATCH] Convert maniphest to use standard fields Summary: Ref T3794. Drop auxiliary field, use standard field. Test Plan: Performed migration, field seemed to survive it intact. Edited and viewed tasks. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T3794 Differential Revision: https://secure.phabricator.com/D7036 --- resources/sql/patches/20130919.mfieldconf.php | 66 +++++++++++++++++++ src/__phutil_library_map__.php | 6 ++ .../PhabricatorSetupCheckExtraConfig.php | 5 ++ ...hestAuxiliaryFieldDefaultSpecification.php | 27 +------- .../PhabricatorManiphestConfigOptions.php | 6 +- .../ManiphestTaskEditController.php | 18 ++++- .../field/ManiphestConfiguredCustomField.php | 22 +++++++ .../maniphest/field/ManiphestCustomField.php | 9 ++- .../user/userguide/maniphest_custom.diviner | 23 ++----- .../patch/PhabricatorBuiltinPatchList.php | 4 ++ 10 files changed, 136 insertions(+), 50 deletions(-) create mode 100644 resources/sql/patches/20130919.mfieldconf.php create mode 100644 src/applications/maniphest/field/ManiphestConfiguredCustomField.php diff --git a/resources/sql/patches/20130919.mfieldconf.php b/resources/sql/patches/20130919.mfieldconf.php new file mode 100644 index 0000000000..099e7c0813 --- /dev/null +++ b/resources/sql/patches/20130919.mfieldconf.php @@ -0,0 +1,66 @@ + $spec) { + $new_spec = array(); + + foreach ($spec as $key => $value) { + switch ($key) { + case 'label': + $new_spec['name'] = $value; + break; + case 'required': + case 'default': + case 'caption': + case 'options': + $new_spec[$key] = $value; + break; + case 'checkbox-label': + $new_spec['strings']['edit.checkbox'] = $value; + break; + case 'checkbox-value': + $new_spec['strings']['view.yes'] = $value; + break; + case 'type': + switch ($value) { + case 'string': + $value = 'text'; + break; + case 'user': + $value = 'users'; + $new_spec['limit'] = 1; + break; + } + $new_spec['type'] = $value; + break; + case 'copy': + $new_spec['copy'] = $value; + break; + } + } + + $new[$field_key] = $new_spec; +} + +PhabricatorConfigEntry::loadConfigEntry($new_key) + ->setIsDeleted(0) + ->setValue($new) + ->save(); + +echo "Done.\n"; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 3c2106b883..8a7cff2f68 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -688,6 +688,7 @@ phutil_register_library_map(array( 'ManiphestAuxiliaryFieldDefaultSpecification' => 'applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldDefaultSpecification.php', 'ManiphestAuxiliaryFieldSpecification' => 'applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldSpecification.php', 'ManiphestBatchEditController' => 'applications/maniphest/controller/ManiphestBatchEditController.php', + 'ManiphestConfiguredCustomField' => 'applications/maniphest/field/ManiphestConfiguredCustomField.php', 'ManiphestConstants' => 'applications/maniphest/constants/ManiphestConstants.php', 'ManiphestController' => 'applications/maniphest/controller/ManiphestController.php', 'ManiphestCreateMailReceiver' => 'applications/maniphest/mail/ManiphestCreateMailReceiver.php', @@ -2765,6 +2766,11 @@ phutil_register_library_map(array( 1 => 'PhabricatorMarkupInterface', ), 'ManiphestBatchEditController' => 'ManiphestController', + 'ManiphestConfiguredCustomField' => + array( + 0 => 'ManiphestCustomField', + 1 => 'PhabricatorStandardCustomFieldInterface', + ), 'ManiphestController' => 'PhabricatorController', 'ManiphestCreateMailReceiver' => 'PhabricatorMailReceiver', 'ManiphestCustomField' => 'PhabricatorCustomField', diff --git a/src/applications/config/check/PhabricatorSetupCheckExtraConfig.php b/src/applications/config/check/PhabricatorSetupCheckExtraConfig.php index a0634df705..9f0ae4a7ce 100644 --- a/src/applications/config/check/PhabricatorSetupCheckExtraConfig.php +++ b/src/applications/config/check/PhabricatorSetupCheckExtraConfig.php @@ -150,6 +150,11 @@ final class PhabricatorSetupCheckExtraConfig extends PhabricatorSetupCheck { pht( 'Maniphest fields are now loaded automatically. You can configure '. 'them with `maniphest.fields`.'), + 'maniphest.custom-fields' => + pht( + 'Maniphest fields are now defined in '. + '`maniphest.custom-field-definitions`. Existing definitions have '. + 'been migrated.'), ); return $ancient_config; diff --git a/src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldDefaultSpecification.php b/src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldDefaultSpecification.php index 6b0cbc8671..f5be110ae2 100644 --- a/src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldDefaultSpecification.php +++ b/src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldDefaultSpecification.php @@ -26,32 +26,7 @@ class ManiphestAuxiliaryFieldDefaultSpecification const TYPE_HEADER = 'header'; public function createFields() { - $fields = PhabricatorEnv::getEnvConfig('maniphest.custom-fields'); - $specs = array(); - foreach ($fields as $aux => $info) { - $spec = new ManiphestAuxiliaryFieldDefaultSpecification(); - $spec->setAuxiliaryKey($aux); - $spec->setLabel(idx($info, 'label')); - $spec->setCaption(idx($info, 'caption')); - $spec->setFieldType(idx($info, 'type')); - $spec->setRequired(idx($info, 'required')); - - $spec->setCheckboxLabel(idx($info, 'checkbox-label')); - $spec->setCheckboxValue(idx($info, 'checkbox-value', 1)); - - if ($spec->getFieldType() == - ManiphestAuxiliaryFieldDefaultSpecification::TYPE_SELECT) { - $spec->setSelectOptions(idx($info, 'options')); - } - - $spec->setShouldCopyWhenCreatingSimilarTask(idx($info, 'copy')); - - $spec->setDefaultValue(idx($info, 'default')); - - $specs[] = $spec; - } - - return $specs; + return array(); } public function getFieldType() { diff --git a/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php b/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php index f516c8aeae..4d871d4ee5 100644 --- a/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php +++ b/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php @@ -59,7 +59,7 @@ final class PhabricatorManiphestConfigOptions $custom_field_type = 'custom:PhabricatorCustomFieldConfigOptionType'; return array( - $this->newOption('maniphest.custom-fields', 'wild', array()) + $this->newOption('maniphest.custom-field-definitions', 'wild', array()) ->setSummary(pht("Custom Maniphest fields.")) ->setDescription( pht( @@ -67,9 +67,9 @@ final class PhabricatorManiphestConfigOptions "adding custom fields to Maniphest, see 'Maniphest User Guide: ". "Adding Custom Fields'.")) ->addExample( - '{"mycompany:estimated-hours": {"label": "Estimated Hours", '. + '{"mycompany:estimated-hours": {"name": "Estimated Hours", '. '"type": "int", "caption": "Estimated number of hours this will '. - 'take.", "required": false}}', + 'take."}}', pht('Valid Setting')), $this->newOption('maniphest.fields', $custom_field_type, $default_fields) ->setCustomData(id(new ManiphestTask())->getCustomFieldBaseClass()) diff --git a/src/applications/maniphest/controller/ManiphestTaskEditController.php b/src/applications/maniphest/controller/ManiphestTaskEditController.php index fe33126384..359bf95e05 100644 --- a/src/applications/maniphest/controller/ManiphestTaskEditController.php +++ b/src/applications/maniphest/controller/ManiphestTaskEditController.php @@ -229,9 +229,21 @@ final class ManiphestTaskEditController extends ManiphestController { ManiphestTransactionType::TYPE_AUXILIARY); $aux_key = $aux_field->getFieldKey(); $transaction->setMetadataValue('aux:key', $aux_key); - $transaction->setOldValue(idx($old_values, $aux_key)); - $transaction->setNewValue( - $aux_field->getNewValueForApplicationTransactions()); + $old = idx($old_values, $aux_key); + $new = $aux_field->getNewValueForApplicationTransactions(); + + // TODO: This is a ghetto check for transactions with no effect. + if (!is_array($old) && !is_array($new)) { + if ((string)$old === (string)$new) { + continue; + } + } else if ($old == $new) { + continue; + } + + $transaction->setOldValue($old); + $transaction->setNewValue($new); + $transactions[] = $transaction; } } diff --git a/src/applications/maniphest/field/ManiphestConfiguredCustomField.php b/src/applications/maniphest/field/ManiphestConfiguredCustomField.php new file mode 100644 index 0000000000..bbf409ddbd --- /dev/null +++ b/src/applications/maniphest/field/ManiphestConfiguredCustomField.php @@ -0,0 +1,22 @@ +getOldValue(); + $new = $transaction->getNewValue(); + return pht( + 'updated field %s from %s to %s', + $this->getFieldName(), + $old, + $new); } public function getMarkupFields() { return array(); } - } diff --git a/src/docs/user/userguide/maniphest_custom.diviner b/src/docs/user/userguide/maniphest_custom.diviner index 09a000d862..0193d52002 100644 --- a/src/docs/user/userguide/maniphest_custom.diviner +++ b/src/docs/user/userguide/maniphest_custom.diviner @@ -22,36 +22,34 @@ If you don't need complicated display controls or sophisticated validation, you can add simple fields. These allow you to attach things like strings, numbers, and dropdown menus to the task template. -Customize Maniphest fields by setting ##maniphest.custom-fields## in your -configuration. For example, suppose you want to add "Estimated Hours" and +Customize Maniphest fields by setting `maniphest.custom-field-definitions` in +your configuration. For example, suppose you want to add "Estimated Hours" and "Actual Hours" fields. To do this, set your configuration like this: 'maniphest.custom-fields' => array( 'mycompany:estimated-hours' => array( - 'label' => 'Estimated Hours', + 'name' => 'Estimated Hours', 'type' => 'int', 'caption' => 'Estimated number of hours this will take.', - 'required' => false, + 'required' => true, ), 'mycompany:actual-hours' => array( - 'label' => 'Actual Hours', + 'name' => 'Actual Hours', 'type' => 'int', - 'required' => false, ), ) Each array key must be unique, and is used to organize the internal storage of the field. These options are available: - - **label**: Display label for the field on the edit and detail interfaces. + - **name**: Display label for the field on the edit and detail interfaces. - **type**: Field type. The supported field types are: - **int**: An integer, rendered as a text field. - - **string**: A string, rendered as a text field. + - **text**: A string, rendered as a text field. - **bool**: A boolean value, rendered as a checkbox. - **select**: Allows the user to select from several options, rendered as a dropdown. - **remarkup**: A text area which allows the user to enter markup. - - **user**: A single user typeahead. - **users**: A typeahead which allows multiple users to be input. - **date**: A date/time picker. - **header**: Renders a visual divider which you can use to group fields. @@ -59,14 +57,7 @@ the field. These options are available: - **required**: True if the user should be required to provide a value. - **options**: If type is set to **select**, provide options for the dropdown as a dictionary. - - **checkbox-label**: If type is set to **bool**, an optional string to - show next to the checkbox. - - **checkbox-value**: If type is set to **bool**, the value to show on - the detail view when the checkbox is selected. - **default**: Default field value. - - For **date**, you can use a string like `"July 4, 1990"`, `"5PM today"`, - or any other valid input to `strtotime()`. - - For **user** and **users**, you can use an array of user PHIDs. - **copy**: When a user creates a task, the UI gives them an option to "Create Another Similar Task". Some fields from the original task are copied into the new task, while others are not; by default, fields are not copied. diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index 906f5735c2..b25cc5e17f 100644 --- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php @@ -1604,6 +1604,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'php', 'name' => $this->getPatchPath('20130915.maniphestmigrate.php'), ), + '20130919.mfieldconf.php' => array( + 'type' => 'php', + 'name' => $this->getPatchPath('20130919.mfieldconf.php'), + ), ); } }