Allow the "Create Subtask" workflow to prompt for a subtype selection, and prepare for customizable options

Summary:
Ref T13222. Ref T12588. See PHI683. Currently, "Create Subtask" always uses the first edit form that the user has access to for the same task subtype. (For example, if you "Create Subtask" from a "Bug", you get the first edit form for "Bugs".)

I didn't want to go too crazy with the initial subtype implementation, but it seems like we're generally on firm ground and it's working fairly well: user requests are for more flexibility in using the system as implemented, not changes to the system or confusion/difficulty with any of the tradeoffs. Thus, I'm generally comfortable continuing to build it out in the same direction. To improve flexibility, I want to make the options from "Create Subtask" more flexible/configurable.

I plan to let you specify that a given subtype (say, "Quest") prompts you with creation options for a set of other subtypes (say, "Objective"), or prompts you with a particular set of forms.

If we end up with a single option, we just go into the current flow (directly to the edit form). If we end up with more than one option, we prompt the user to choose between them.

This change is a first step toward this:

  - When building "Create Subtask", query for multiple forms.
  - The default behavior is now "prompt user to choose among create forms of the same subtype". Previously, it was "use the first edit form of the same subtype". This is a behavioral change.
  - The next change will make the selected forms configurable.
  - (I also plan to make the dialog itself less rough.)

Test Plan: {F6051067}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13222, T12588

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

src/__phutil_library_map__.php

@@ -1760,6 +1760,7 @@ phutil_register_library_map(array(
     'ManiphestTaskStatusTestCase' => 'applications/maniphest/constants/__tests__/ManiphestTaskStatusTestCase.php',
     'ManiphestTaskStatusTransaction' => 'applications/maniphest/xaction/ManiphestTaskStatusTransaction.php',
     'ManiphestTaskSubpriorityTransaction' => 'applications/maniphest/xaction/ManiphestTaskSubpriorityTransaction.php',
+    'ManiphestTaskSubtaskController' => 'applications/maniphest/controller/ManiphestTaskSubtaskController.php',
     'ManiphestTaskSubtypeDatasource' => 'applications/maniphest/typeahead/ManiphestTaskSubtypeDatasource.php',
     'ManiphestTaskTestCase' => 'applications/maniphest/__tests__/ManiphestTaskTestCase.php',
     'ManiphestTaskTitleHeraldField' => 'applications/maniphest/herald/ManiphestTaskTitleHeraldField.php',
@@ -7347,6 +7348,7 @@ phutil_register_library_map(array(
     'ManiphestTaskStatusTestCase' => 'PhabricatorTestCase',
     'ManiphestTaskStatusTransaction' => 'ManiphestTaskTransactionType',
     'ManiphestTaskSubpriorityTransaction' => 'ManiphestTaskTransactionType',
+    'ManiphestTaskSubtaskController' => 'ManiphestController',
     'ManiphestTaskSubtypeDatasource' => 'PhabricatorTypeaheadDatasource',
     'ManiphestTaskTestCase' => 'PhabricatorTestCase',
     'ManiphestTaskTitleHeraldField' => 'ManiphestTaskHeraldField',

src/applications/maniphest/application/PhabricatorManiphestApplication.php

@@ -52,6 +52,7 @@ final class PhabricatorManiphestApplication extends PhabricatorApplication {
         'task/' => array(
           $this->getEditRoutePattern('edit/')
             => 'ManiphestTaskEditController',
+          'subtask/(?P<id>[1-9]\d*)/' => 'ManiphestTaskSubtaskController',
         ),
         'subpriority/' => 'ManiphestSubpriorityController',
       ),

src/applications/maniphest/controller/ManiphestTaskDetailController.php

@@ -281,29 +281,39 @@ final class ManiphestTaskDetailController extends ManiphestController {
         ->setDisabled(!$can_edit)
         ->setWorkflow($workflow_edit));
 
-    $edit_config = $edit_engine->loadDefaultEditConfiguration($task);
-    $can_create = (bool)$edit_config;
+    $subtype_map = $task->newEditEngineSubtypeMap();
+    $subtask_options = $subtype_map->getCreateFormsForSubtype(
+      $edit_engine,
+      $task);
 
-    if ($can_create) {
-      $form_key = $edit_config->getIdentifier();
-      $edit_uri = id(new PhutilURI("/task/edit/form/{$form_key}/"))
+    // If no forms are available, we want to show the user an error.
+    // If one form is available, we take them user directly to the form.
+    // If two or more forms are available, we give the user a choice.
+
+    // The "subtask" controller handles the first case (no forms) and the
+    // third case (more than one form). In the case of one form, we link
+    // directly to the form.
+    $subtask_uri = "/task/subtask/{$id}/";
+    $subtask_workflow = true;
+
+    if (count($subtask_options) == 1) {
+      $subtask_form = head($subtask_options);
+      $form_key = $subtask_form->getIdentifier();
+      $subtask_uri = id(new PhutilURI("/task/edit/form/{$form_key}/"))
         ->setQueryParam('parent', $id)
         ->setQueryParam('template', $id)
         ->setQueryParam('status', ManiphestTaskStatus::getDefaultStatus());
-      $edit_uri = $this->getApplicationURI($edit_uri);
-    } else {
-      // TODO: This will usually give us a somewhat-reasonable error page, but
-      // could be a bit cleaner.
-      $edit_uri = "/task/edit/{$id}/";
-      $edit_uri = $this->getApplicationURI($edit_uri);
+      $subtask_workflow = false;
     }
 
+    $subtask_uri = $this->getApplicationURI($subtask_uri);
+
     $subtask_item = id(new PhabricatorActionView())
       ->setName(pht('Create Subtask'))
-      ->setHref($edit_uri)
+      ->setHref($subtask_uri)
       ->setIcon('fa-level-down')
-      ->setDisabled(!$can_create)
-      ->setWorkflow(!$can_create);
+      ->setDisabled(!$subtask_options)
+      ->setWorkflow($subtask_workflow);
 
     $relationship_list = PhabricatorObjectRelationshipList::newForObject(
       $viewer,

src/applications/maniphest/controller/ManiphestTaskSubtaskController.php

@@ -0,0 +1,76 @@
+<?php
+
+final class ManiphestTaskSubtaskController
+  extends ManiphestController {
+
+  public function handleRequest(AphrontRequest $request) {
+    $viewer = $this->getViewer();
+    $id = $request->getURIData('id');
+
+    $task = id(new ManiphestTaskQuery())
+      ->setViewer($viewer)
+      ->withIDs(array($id))
+      ->executeOne();
+    if (!$task) {
+      return new Aphront404Response();
+    }
+
+    $cancel_uri = $task->getURI();
+
+    $edit_engine = id(new ManiphestEditEngine())
+      ->setViewer($viewer)
+      ->setTargetObject($task);
+
+    $subtype_map = $task->newEditEngineSubtypeMap();
+
+    $subtype_options = $subtype_map->getCreateFormsForSubtype(
+      $edit_engine,
+      $task);
+
+    if (!$subtype_options) {
+      return $this->newDialog()
+        ->setTitle(pht('No Forms'))
+        ->appendParagraph(
+          pht(
+            'You do not have access to any forms which can be used to '.
+            'create a subtask.'))
+        ->addCancelButton($cancel_uri, pht('Close'));
+    }
+
+    if ($request->isFormPost()) {
+      $form_key = $request->getStr('formKey');
+      if (isset($subtype_options[$form_key])) {
+        $subtask_uri = id(new PhutilURI("/task/edit/form/{$form_key}/"))
+          ->setQueryParam('parent', $id)
+          ->setQueryParam('template', $id)
+          ->setQueryParam('status', ManiphestTaskStatus::getDefaultStatus());
+        $subtask_uri = $this->getApplicationURI($subtask_uri);
+
+        return id(new AphrontRedirectResponse())
+          ->setURI($subtask_uri);
+      }
+    }
+
+    $control = id(new AphrontFormRadioButtonControl())
+      ->setName('formKey')
+      ->setLabel(pht('Subtype'));
+
+    foreach ($subtype_options as $key => $subtype_form) {
+      $control->addButton(
+        $key,
+        $subtype_form->getDisplayName(),
+        null);
+    }
+
+    $form = id(new AphrontFormView())
+      ->setViewer($viewer)
+      ->appendControl($control);
+
+    return $this->newDialog()
+      ->setTitle(pht('Choose Subtype'))
+      ->appendForm($form)
+      ->addSubmitButton(pht('Continue'))
+      ->addCancelButton($cancel_uri);
+  }
+
+}

src/applications/transactions/editengine/PhabricatorEditEngine.php

@@ -358,7 +358,7 @@ abstract class PhabricatorEditEngine
     return $this->editEngineConfiguration;
   }
 
-  private function newConfigurationQuery() {
+  public function newConfigurationQuery() {
     return id(new PhabricatorEditEngineConfigurationQuery())
       ->setViewer($this->getViewer())
       ->withEngineKeys(array($this->getEngineKey()));

src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php

@@ -39,4 +39,47 @@ final class PhabricatorEditEngineSubtypeMap
     return $this->subtypes[$subtype_key];
   }
 
+  public function getCreateFormsForSubtype(
+    PhabricatorEditEngine $edit_engine,
+    PhabricatorEditEngineSubtypeInterface $object) {
+
+    $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();
+
+    $query = $edit_engine->newConfigurationQuery()
+      ->withIsDisabled(false);
+
+    if ($select_ids) {
+      $query->withIDs($select_ids);
+    } else {
+      // If we're selecting by subtype rather than selecting specific forms,
+      // only select create forms.
+      $query->withIsDefault(true);
+
+      if ($select_subtypes) {
+        $query->withSubtypes($select_subtypes);
+      } else {
+        $query->withSubtypes(array($subtype_key));
+      }
+    }
+
+    $forms = $query->execute();
+    $forms = mpull($forms, null, 'getIdentifier');
+
+    // 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;
+    } else {
+      $forms = msort($forms, 'getCreateSortKey');
+    }
+
+    return $forms;
+  }
+
 }