Replace workboard task creation with EditEngine

Summary: Ref T9908. This is the last of the things that need to swap over. Test Plan: - Created tasks from a workboard. - Created tasks in different columns. - Edited tasks. - Used `?parent=..`. - Verified that default edit form config now affects comment actions. - No more weird comment thing on forms, at least for now. Reviewers: chad Reviewed By: chad Maniphest Tasks: T9908 Differential Revision: https://secure.phabricator.com/D14715
2025-03-28 12:08:14 +01:00 · 2015-12-08 16:30:49 -08:00 · 2015-12-08 16:30:49 -08:00 · eef2572508
commit eef2572508
parent a13ef20a0c
11 changed files with 233 additions and 21 deletions
--- a/src/applications/config/controller/PhabricatorConfigWelcomeController.php
+++ b/src/applications/config/controller/PhabricatorConfigWelcomeController.php
@ -265,7 +265,7 @@ final class PhabricatorConfigWelcomeController


    $maniphest_uri = PhabricatorEnv::getURI('/maniphest/');
-    $maniphest_create_uri = PhabricatorEnv::getURI('/maniphest/task/create/');
+    $maniphest_create_uri = PhabricatorEnv::getURI('/maniphest/editpro/');
    $maniphest_all_uri = PhabricatorEnv::getURI('/maniphest/query/all/');
    $quick[] = $this->newItem(
      $request,
--- a/src/applications/maniphest/editor/ManiphestEditEngine.php
+++ b/src/applications/maniphest/editor/ManiphestEditEngine.php
@ -75,7 +75,22 @@ final class ManiphestEditEngine
        ->setDescription(pht('Task to make this a subtask of.'))
        ->setAliases(array('parentPHID'))
        ->setTransactionType(ManiphestTransaction::TYPE_PARENT)
-        ->setSingleValue(null),
+        ->setHandleParameterType(new ManiphestTaskListHTTPParameterType())
+        ->setSingleValue(null)
+        ->setIsReorderable(false)
+        ->setIsDefaultable(false)
+        ->setIsLockable(false),
+      id(new PhabricatorHandlesEditField())
+        ->setKey('column')
+        ->setLabel(pht('Column'))
+        ->setDescription(pht('Workboard column to create this task into.'))
+        ->setAliases(array('columnPHID'))
+        ->setTransactionType(ManiphestTransaction::TYPE_COLUMN)
+        ->setSingleValue(null)
+        ->setIsInvisible(true)
+        ->setIsReorderable(false)
+        ->setIsDefaultable(false)
+        ->setIsLockable(false),
      id(new PhabricatorTextEditField())
        ->setKey('title')
        ->setLabel(pht('Title'))
@ -213,7 +228,7 @@ final class ManiphestEditEngine

    }

-    return parent::newEditResponse();
+    return parent::newEditResponse($request, $object, $xactions);
  }

  private function buildListResponse(ManiphestTask $task) {
--- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php
+++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php
@ -27,6 +27,7 @@ final class ManiphestTransactionEditor
    $types[] = ManiphestTransaction::TYPE_MERGED_FROM;
    $types[] = ManiphestTransaction::TYPE_UNBLOCK;
    $types[] = ManiphestTransaction::TYPE_PARENT;
+    $types[] = ManiphestTransaction::TYPE_COLUMN;
    $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
    $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;

@ -69,6 +70,7 @@ final class ManiphestTransactionEditor
      case ManiphestTransaction::TYPE_MERGED_FROM:
        return null;
      case ManiphestTransaction::TYPE_PARENT:
+      case ManiphestTransaction::TYPE_COLUMN:
        return null;
    }
  }
@ -92,6 +94,7 @@ final class ManiphestTransactionEditor
      case ManiphestTransaction::TYPE_UNBLOCK:
        return $xaction->getNewValue();
      case ManiphestTransaction::TYPE_PARENT:
+      case ManiphestTransaction::TYPE_COLUMN:
        return $xaction->getNewValue();
    }
  }
@ -159,8 +162,8 @@ final class ManiphestTransactionEditor
        $object->setStatus(ManiphestTaskStatus::getDuplicateStatus());
        return;
      case ManiphestTransaction::TYPE_MERGED_FROM:
-        return;
      case ManiphestTransaction::TYPE_PARENT:
+      case ManiphestTransaction::TYPE_COLUMN:
        return;
    }
  }
@ -757,14 +760,71 @@ final class ManiphestTransactionEditor
        }
        break;
      case ManiphestTransaction::TYPE_PARENT:
-        if ($xactions && !$this->getIsNewObject()) {
-          $error = new PhabricatorApplicationTransactionValidationError(
+        $with_effect = array();
+        foreach ($xactions as $xaction) {
+          $task_phid = $xaction->getNewValue();
+          if (!$task_phid) {
+            continue;
+          }
+
+          $with_effect[] = $xaction;
+
+          $task = id(new ManiphestTaskQuery())
+            ->setViewer($this->getActor())
+            ->withPHIDs(array($task_phid))
+            ->executeOne();
+          if (!$task) {
+            $errors[] = new PhabricatorApplicationTransactionValidationError(
+              $type,
+              pht('Invalid'),
+              pht(
+                'Parent task identifier "%s" does not identify a visible '.
+                'task.',
+                $task_phid),
+              $xaction);
+          }
+        }
+
+        if ($with_effect && !$this->getIsNewObject()) {
+          $errors[] = new PhabricatorApplicationTransactionValidationError(
            $type,
            pht('Invalid'),
            pht(
              'You can only select a parent task when creating a '.
              'transaction for the first time.'),
-            last($xactions));
+            last($with_effect));
+        }
+        break;
+      case ManiphestTransaction::TYPE_COLUMN:
+        $with_effect = array();
+        foreach ($xactions as $xaction) {
+          $column_phid = $xaction->getNewValue();
+          if (!$column_phid) {
+            continue;
+          }
+
+          $with_effect[] = $xaction;
+
+          $column = $this->loadProjectColumn($column_phid);
+          if (!$column) {
+            $errors[] = new PhabricatorApplicationTransactionValidationError(
+              $type,
+              pht('Invalid'),
+              pht(
+                'Column PHID "%s" does not identify a visible column.',
+                $column_phid),
+              $xaction);
+          }
+        }
+
+        if ($with_effect && !$this->getIsNewObject()) {
+          $errors[] = new PhabricatorApplicationTransactionValidationError(
+            $type,
+            pht('Invalid'),
+            pht(
+              'You can only put a task into an initial column during task '.
+              'creation.'),
+            last($with_effect));
        }
        break;
    }
@ -841,6 +901,28 @@ final class ManiphestTransactionEditor
    $results = parent::expandTransaction($object, $xaction);

    switch ($xaction->getTransactionType()) {
+      case ManiphestTransaction::TYPE_COLUMN:
+        $column_phid = $xaction->getNewValue();
+        if (!$column_phid) {
+          break;
+        }
+
+        // When a task is created into a column, we also generate a transaction
+        // to actually put it in that column.
+        $column = $this->loadProjectColumn($column_phid);
+        $results[] = id(new ManiphestTransaction())
+          ->setTransactionType(ManiphestTransaction::TYPE_PROJECT_COLUMN)
+          ->setOldValue(
+            array(
+              'projectPHID' => $column->getProjectPHID(),
+              'columnPHIDs' => array(),
+            ))
+          ->setNewValue(
+            array(
+              'projectPHID' => $column->getProjectPHID(),
+              'columnPHIDs' => array($column->getPHID()),
+            ));
+        break;
      case ManiphestTransaction::TYPE_OWNER:
        // When a task is reassigned, move the old owner to the subscriber
        // list so they're still in the loop.
@ -860,5 +942,12 @@ final class ManiphestTransactionEditor
    return $results;
  }

+  private function loadProjectColumn($column_phid) {
+    return id(new PhabricatorProjectColumnQuery())
+      ->setViewer($this->getActor())
+      ->withPHIDs(array($column_phid))
+      ->executeOne();
+  }
+

 }
--- a/src/applications/maniphest/storage/ManiphestTransaction.php
+++ b/src/applications/maniphest/storage/ManiphestTransaction.php
@ -15,6 +15,7 @@ final class ManiphestTransaction
  const TYPE_MERGED_FROM = 'mergedfrom';
  const TYPE_UNBLOCK = 'unblock';
  const TYPE_PARENT = 'parent';
+  const TYPE_COLUMN = 'column';

  // NOTE: this type is deprecated. Keep it around for legacy installs
  // so any transactions render correctly.
@ -149,6 +150,7 @@ final class ManiphestTransaction
        break;
      case self::TYPE_SUBPRIORITY:
      case self::TYPE_PARENT:
+      case self::TYPE_COLUMN:
        return true;
      case self::TYPE_PROJECT_COLUMN:
        $old_cols = idx($this->getOldValue(), 'columnPHIDs');
--- a/src/applications/project/controller/PhabricatorProjectBoardViewController.php
+++ b/src/applications/project/controller/PhabricatorProjectBoardViewController.php
@ -276,7 +276,7 @@ final class PhabricatorProjectBoardViewController
      'boardID' => $board_id,
      'projectPHID' => $project->getPHID(),
      'moveURI' => $this->getApplicationURI('move/'.$project->getID().'/'),
-      'createURI' => '/maniphest/task/create/',
+      'createURI' => $this->getCreateURI(),
      'order' => $this->sortKey,
    );
    $this->initBehavior(
@ -630,7 +630,7 @@ final class PhabricatorProjectBoardViewController
    $column_items[] = id(new PhabricatorActionView())
      ->setIcon('fa-plus')
      ->setName(pht('Create Task...'))
-      ->setHref('/maniphest/task/create/')
+      ->setHref($this->getCreateURI())
      ->addSigil('column-add-task')
      ->setMetadata(
        array(
@ -768,4 +768,22 @@ final class PhabricatorProjectBoardViewController
    return $base;
  }

+  private function getCreateURI() {
+    $viewer = $this->getViewer();
+
+    // TODO: This should be cleaned up, but maybe we're going to make options
+    // for each column or board?
+    $edit_config = id(new ManiphestEditEngine())
+      ->setViewer($viewer)
+      ->loadDefaultEditConfiguration();
+    if ($edit_config) {
+      $form_key = $edit_config->getIdentifier();
+      $create_uri = "/maniphest/editpro/form/{$form_key}/";
+    } else {
+      $create_uri = '/maniphest/editpro/';
+    }
+
+    return $create_uri;
+  }
+
 }
--- a/src/applications/search/engine/PhabricatorJumpNavHandler.php
+++ b/src/applications/search/engine/PhabricatorJumpNavHandler.php
@ -66,7 +66,7 @@ final class PhabricatorJumpNavHandler extends Phobject {
              return id(new AphrontRedirectResponse())->setURI($uri);
            case 'create-task':
              return id(new AphrontRedirectResponse())
-                ->setURI('/maniphest/task/create/?title='
+                ->setURI('/maniphest/editpro/?title='
                  .phutil_escape_uri($matches[1]));
            default:
              throw new Exception(pht("Unknown jump effect '%s'!", $effect));
--- a/src/applications/transactions/editengine/PhabricatorEditEngine.php
+++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php
@ -1137,7 +1137,10 @@ abstract class PhabricatorEditEngine

    $all_types = array();
    foreach ($fields as $field) {
-      // TODO: Load draft stuff.
+      if (!$this->isCommentField($field)) {
+        continue;
+      }
+
      $types = $field->getCommentEditTypes();
      foreach ($types as $type) {
        $all_types[] = $type;
@ -1329,9 +1332,8 @@ abstract class PhabricatorEditEngine
          $viewer->getPHID(),
          $current_version);

-        // TODO: This is just a proof of concept.
        $draft
-          ->setProperty('temporary.comment', $comment_text)
+          ->setProperty('comment', $comment_text)
          ->setProperty('actions', $actions)
          ->save();
      }
@ -1342,6 +1344,10 @@ abstract class PhabricatorEditEngine
    if ($actions) {
      $type_map = array();
      foreach ($fields as $field) {
+        if (!$this->isCommentField($field)) {
+          continue;
+        }
+
        $types = $field->getCommentEditTypes();
        foreach ($types as $type) {
          $type_map[$type->getEditType()] = array(
@ -1686,6 +1692,23 @@ abstract class PhabricatorEditEngine
      PhabricatorPolicyCapability::CAN_EDIT);
  }

+  private function isCommentField(PhabricatorEditField $field) {
+    // TODO: This is a little bit hacky.
+    if ($field->getKey() == 'comment') {
+      return true;
+    }
+
+    if ($field->getIsLocked()) {
+      return false;
+    }
+
+    if ($field->getIsHidden()) {
+      return false;
+    }
+
+    return true;
+  }
+

 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */

--- a/src/applications/transactions/editengineextension/PhabricatorCommentEditEngineExtension.php
+++ b/src/applications/transactions/editengineextension/PhabricatorCommentEditEngineExtension.php
@ -44,6 +44,9 @@ final class PhabricatorCommentEditEngineExtension
      ->setDescription(pht('Add comments.'))
      ->setAliases(array('comments'))
      ->setIsHidden(true)
+      ->setIsReorderable(false)
+      ->setIsDefaultable(false)
+      ->setIsLockable(false)
      ->setTransactionType($comment_type)
      ->setValue(null);

--- a/src/applications/transactions/editfield/PhabricatorHandlesEditField.php
+++ b/src/applications/transactions/editfield/PhabricatorHandlesEditField.php
@ -3,12 +3,45 @@
 final class PhabricatorHandlesEditField
  extends PhabricatorPHIDListEditField {

+  private $handleParameterType;
+  private $isInvisible;
+
+  public function setHandleParameterType(AphrontHTTPParameterType $type) {
+    $this->handleParameterType = $type;
+    return $this;
+  }
+
+  public function getHandleParameterType() {
+    return $this->handleParameterType;
+  }
+
+  public function setIsInvisible($is_invisible) {
+    $this->isInvisible = $is_invisible;
+    return $this;
+  }
+
+  public function getIsInvisible() {
+    return $this->isInvisible;
+  }
+
  protected function newControl() {
-    return id(new AphrontFormHandlesControl());
+    $control = id(new AphrontFormHandlesControl());
+
+    if ($this->getIsInvisible()) {
+      $control->setIsInvisible(true);
+    }
+
+    return $control;
  }

  protected function newHTTPParameterType() {
-    return new ManiphestTaskListHTTPParameterType();
+    $type = $this->getHandleParameterType();
+
+    if ($type) {
+      return $type;
+    }
+
+    return new AphrontPHIDListHTTPParameterType();
  }

 }
--- a/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php
+++ b/src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php
@ -196,7 +196,7 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView {

    $versioned_draft = $this->getVersionedDraft();
    if ($versioned_draft) {
-      $draft_comment = $versioned_draft->getProperty('temporary.comment', '');
+      $draft_comment = $versioned_draft->getProperty('comment', '');
    }

    if (!$this->getObjectPHID()) {
--- a/src/view/form/control/AphrontFormHandlesControl.php
+++ b/src/view/form/control/AphrontFormHandlesControl.php
@ -2,22 +2,50 @@

 final class AphrontFormHandlesControl extends AphrontFormControl {

+  private $isInvisible;
+
  protected function getCustomControlClass() {
    return 'aphront-form-control-handles';
  }

+  public function setIsInvisible($is_invisible) {
+    $this->isInvisible = $is_invisible;
+    return $this;
+  }
+
+  public function getIsInvisible() {
+    return $this->isInvisible;
+  }
+
  protected function shouldRender() {
    return (bool)$this->getValue();
  }

+  public function getLabel() {
+    // TODO: This is a bit funky and still rendering a few pixels of padding
+    // on the form, but there's currently no way to get a control to only emit
+    // hidden inputs. Clean this up eventually.
+
+    if ($this->getIsInvisible()) {
+      return null;
+    }
+
+    return parent::getLabel();
+  }
+
  protected function renderInput() {
    $value = $this->getValue();
    $viewer = $this->getUser();

-    $list = $viewer->renderHandleList($value);
-    $list = id(new PHUIBoxView())
-      ->addPadding(PHUI::PADDING_SMALL_TOP)
-      ->appendChild($list);
+    $out = array();
+
+    if (!$this->getIsInvisible()) {
+      $list = $viewer->renderHandleList($value);
+      $list = id(new PHUIBoxView())
+        ->addPadding(PHUI::PADDING_SMALL_TOP)
+        ->appendChild($list);
+      $out[] = $list;
+    }

    $inputs = array();
    foreach ($value as $phid) {
@ -29,8 +57,9 @@ final class AphrontFormHandlesControl extends AphrontFormControl {
          'value' => $phid,
        ));
    }
+    $out[] = $inputs;

-    return array($list, $inputs);
+    return $out;
  }

 }