From a6632f8c188a0b0b460b35003ccc0e41a45bb90e Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 6 Dec 2018 11:42:53 -0800 Subject: [PATCH] Allow "maniphest.subtypes" to configure which options are presented by "Create Subtask" Summary: Ref T13222. Ref T12588. See PHI683. After D19853, "Create Subtask" may pop a dialog to let you choose between multiple forms. Allow users to configure which forms are available by using `maniphest.subtypes` to choose available children for each subtype. Users may either specify particular subtypes or specific forms. Test Plan: Configured "Quest" tasks to have "Objective" children, got appropriate prompt behavior. Used "subtypes" and "forms" to select forms; used "forms" to reorder forms. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13222, T12588 Differential Revision: https://secure.phabricator.com/D19854 --- .../PhabricatorManiphestConfigOptions.php | 50 +++++++++++++++++ .../PhabricatorEditEngineSubtype.php | 54 +++++++++++++++++++ .../PhabricatorEditEngineSubtypeMap.php | 14 +++-- ...torEditEngineConfigurationSearchEngine.php | 33 +++++++++--- 4 files changed, 135 insertions(+), 16 deletions(-) diff --git a/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php b/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php index 4458dc3636..609db0d1b6 100644 --- a/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php +++ b/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php @@ -338,6 +338,8 @@ dictionary with these keys: - `tag` //Optional string.// Tag text for this subtype. - `color` //Optional string.// Display color for this subtype. - `icon` //Optional string.// Icon for the subtype. + - `children` //Optional map.// Configure options shown to the user when + they "Create Subtask". See below. Each subtype must have a unique key, and you must define a subtype with the key "%s", which is used as a default subtype. @@ -345,6 +347,54 @@ the key "%s", which is used as a default subtype. The tag text (`tag`) is used to set the text shown in the subtype tag on list views and workboards. If you do not configure it, the default subtype will have no subtype tag and other subtypes will use their name as tag text. + +The `children` key allows you to configure which options are presented to the +user when they "Create Subtask" from a task of this subtype. You can specify +these keys: + + - `subtypes`: //Optional list.// Show users creation forms for these + task subtypes. + - `forms`: //Optional list.// Show users these specific forms, + in order. + +If you don't specify either constraint, users will be shown creation forms +for the same subtype. + +For example, if you have a "quest" subtype and do not configure `children`, +users who click "Create Subtask" will be presented with all create forms for +"quest" tasks. + +If you want to present them with forms for a different task subtype or set of +subtypes instead, use `subtypes`: + +``` + { + ... + "children": { + "subtypes": ["objective", "boss", "reward"] + } + ... + } +``` + +If you want to present them with specific forms, use `forms` and specify form +IDs: + +``` + { + ... + "children": { + "forms": [12, 16] + } + ... + } +``` + +When specifying forms by ID explicitly, the order you specify the forms in will +be used when presenting options to the user. + +If only one option would be presented, the user will be taken directly to the +appropriate form instead of being prompted to choose a form. EOTEXT , $subtype_default_key)); diff --git a/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php b/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php index 23fac106d7..7aad1c9509 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php @@ -11,6 +11,8 @@ final class PhabricatorEditEngineSubtype private $icon; private $tagText; private $color; + private $childSubtypes = array(); + private $childIdentifiers = array(); public function setKey($key) { $this->key = $key; @@ -57,6 +59,24 @@ final class PhabricatorEditEngineSubtype return $this->color; } + public function setChildSubtypes(array $child_subtypes) { + $this->childSubtypes = $child_subtypes; + return $this; + } + + public function getChildSubtypes() { + return $this->childSubtypes; + } + + public function setChildFormIdentifiers(array $child_identifiers) { + $this->childIdentifiers = $child_identifiers; + return $this; + } + + public function getChildFormIdentifiers() { + return $this->childIdentifiers; + } + public function hasTagView() { return (bool)strlen($this->getTagText()); } @@ -118,6 +138,7 @@ final class PhabricatorEditEngineSubtype 'tag' => 'optional string', 'color' => 'optional string', 'icon' => 'optional string', + 'children' => 'optional map', )); $key = $value['key']; @@ -141,6 +162,27 @@ final class PhabricatorEditEngineSubtype 'no name. Subtypes must have a name.', $key)); } + + $children = idx($value, 'children'); + if ($children) { + PhutilTypeSpec::checkMap( + $children, + array( + 'subtypes' => 'optional list', + 'forms' => 'optional list', + )); + + $child_subtypes = idx($children, 'subtypes'); + $child_forms = idx($children, 'forms'); + + if ($child_subtypes && $child_forms) { + throw new Exception( + pht( + 'Subtype configuration is invalid: subtype with key "%s" '. + 'specifies both child subtypes and child forms. Specify one '. + 'or the other, but not both.')); + } + } } if (!isset($map[self::SUBTYPE_DEFAULT])) { @@ -179,6 +221,18 @@ final class PhabricatorEditEngineSubtype $subtype->setColor($color); } + $children = idx($entry, 'children', array()); + $child_subtypes = idx($children, 'subtypes'); + $child_forms = idx($children, 'forms'); + + if ($child_subtypes) { + $subtype->setChildSubtypes($child_subtypes); + } + + if ($child_forms) { + $subtype->setChildFormIdentifiers($child_forms); + } + $map[$key] = $subtype; } diff --git a/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php b/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php index c76911dbc3..638b665184 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php @@ -46,16 +46,14 @@ final class PhabricatorEditEngineSubtypeMap $subtype_key = $object->getEditEngineSubtype(); $subtype = $this->getSubtype($subtype_key); - // TODO: Allow subtype configuration to specify that children should be - // created from particular forms or subtypes. - $select_ids = array(); - $select_subtypes = array(); + $select_identifiers = $subtype->getChildFormIdentifiers(); + $select_subtypes = $subtype->getChildSubtypes(); $query = $edit_engine->newConfigurationQuery() ->withIsDisabled(false); - if ($select_ids) { - $query->withIDs($select_ids); + if ($select_identifiers) { + $query->withIdentifiers($select_identifiers); } else { // If we're selecting by subtype rather than selecting specific forms, // only select create forms. @@ -73,8 +71,8 @@ final class PhabricatorEditEngineSubtypeMap // If we're selecting by ID, respect the order specified in the // constraint. Otherwise, use the create form sort order. - if ($select_ids) { - $forms = array_select_keys($forms, $select_ids) + $forms; + if ($select_identifiers) { + $forms = array_select_keys($forms, $select_identifiers) + $forms; } else { $forms = msort($forms, 'getCreateSortKey'); } diff --git a/src/applications/transactions/query/PhabricatorEditEngineConfigurationSearchEngine.php b/src/applications/transactions/query/PhabricatorEditEngineConfigurationSearchEngine.php index 52d48f528b..694b4c48b6 100644 --- a/src/applications/transactions/query/PhabricatorEditEngineConfigurationSearchEngine.php +++ b/src/applications/transactions/query/PhabricatorEditEngineConfigurationSearchEngine.php @@ -115,14 +115,6 @@ final class PhabricatorEditEngineConfigurationSearchEngine ->setHeader($config->getDisplayName()); $id = $config->getID(); - if ($id) { - $item->addIcon('fa-file-text-o bluegrey', pht('Form %d', $id)); - $key = $id; - } else { - $item->addIcon('fa-file-text bluegrey', pht('Builtin')); - $key = $config->getBuiltinKey(); - } - $item->setHref("/transactions/editengine/{$engine_key}/view/{$key}/"); if ($config->getIsDefault()) { $item->addAttribute(pht('Default Create Form')); @@ -139,6 +131,31 @@ final class PhabricatorEditEngineConfigurationSearchEngine $item->setStatusIcon('fa-file-text-o green', pht('Enabled')); } + $subtype_key = $config->getSubtype(); + if ($subtype_key !== PhabricatorEditEngineSubtype::SUBTYPE_DEFAULT) { + $engine = $config->getEngine(); + if ($engine->supportsSubtypes()) { + $map = $engine->newSubtypeMap(); + if ($map->isValidSubtype($subtype_key)) { + $subtype = $map->getSubtype($subtype_key); + + $icon = $subtype->getIcon(); + $color = $subtype->getColor(); + + $item->addIcon("{$icon} {$color}", $subtype->getName()); + } + } + } + + if ($id) { + $item->setObjectName(pht('Form %d', $id)); + $key = $id; + } else { + $item->addIcon('fa-file-text bluegrey', pht('Builtin')); + $key = $config->getBuiltinKey(); + } + $item->setHref("/transactions/editengine/{$engine_key}/view/{$key}/"); + $list->addItem($item); }