From ecd4dd4e0b08f5b8934fd2cde426e7d1059681c6 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 6 Apr 2016 04:37:01 -0700
Subject: [PATCH] Expose column positions via maniphest.edit

Summary: Ref T5214. Fixes T10486. Ref T6027. This exposes the `TYPE_COLUMNS` transaction in a usable way via API, and fixes the interactions via prefilling.

Test Plan:
  - Created tasks directly into columns via API.
  - Moved tasks between columns via API.
  - Used `?column=...` to try to create a template task with valid and bogus column PHIDs.

Reviewers: chad

Reviewed By: chad

Subscribers: AmyLewis

Maniphest Tasks: T5214, T6027, T10486

Differential Revision: https://secure.phabricator.com/D15636
---
 src/__phutil_library_map__.php                |  4 ++
 .../ConduitColumnsParameterType.php           | 38 ++++++++++++
 .../maniphest/editor/ManiphestEditEngine.php  | 58 +++++++++++++++++--
 .../editfield/PhabricatorColumnsEditField.php | 21 +++++++
 .../PhabricatorApplicationTransaction.php     |  1 -
 5 files changed, 117 insertions(+), 5 deletions(-)
 create mode 100644 src/applications/conduit/parametertype/ConduitColumnsParameterType.php
 create mode 100644 src/applications/transactions/editfield/PhabricatorColumnsEditField.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 13e4b98341..b1e36442c3 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -253,6 +253,7 @@ phutil_register_library_map(array(
     'ConduitBoolParameterType' => 'applications/conduit/parametertype/ConduitBoolParameterType.php',
     'ConduitCall' => 'applications/conduit/call/ConduitCall.php',
     'ConduitCallTestCase' => 'applications/conduit/call/__tests__/ConduitCallTestCase.php',
+    'ConduitColumnsParameterType' => 'applications/conduit/parametertype/ConduitColumnsParameterType.php',
     'ConduitConnectConduitAPIMethod' => 'applications/conduit/method/ConduitConnectConduitAPIMethod.php',
     'ConduitEpochParameterType' => 'applications/conduit/parametertype/ConduitEpochParameterType.php',
     'ConduitException' => 'applications/conduit/protocol/exception/ConduitException.php',
@@ -1984,6 +1985,7 @@ phutil_register_library_map(array(
     'PhabricatorChunkedFileStorageEngine' => 'applications/files/engine/PhabricatorChunkedFileStorageEngine.php',
     'PhabricatorClusterConfigOptions' => 'applications/config/option/PhabricatorClusterConfigOptions.php',
     'PhabricatorColumnProxyInterface' => 'applications/project/interface/PhabricatorColumnProxyInterface.php',
+    'PhabricatorColumnsEditField' => 'applications/transactions/editfield/PhabricatorColumnsEditField.php',
     'PhabricatorCommentEditEngineExtension' => 'applications/transactions/engineextension/PhabricatorCommentEditEngineExtension.php',
     'PhabricatorCommentEditField' => 'applications/transactions/editfield/PhabricatorCommentEditField.php',
     'PhabricatorCommentEditType' => 'applications/transactions/edittype/PhabricatorCommentEditType.php',
@@ -4380,6 +4382,7 @@ phutil_register_library_map(array(
     'ConduitBoolParameterType' => 'ConduitListParameterType',
     'ConduitCall' => 'Phobject',
     'ConduitCallTestCase' => 'PhabricatorTestCase',
+    'ConduitColumnsParameterType' => 'ConduitParameterType',
     'ConduitConnectConduitAPIMethod' => 'ConduitAPIMethod',
     'ConduitEpochParameterType' => 'ConduitListParameterType',
     'ConduitException' => 'Exception',
@@ -6383,6 +6386,7 @@ phutil_register_library_map(array(
     'PhabricatorChatLogQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'PhabricatorChunkedFileStorageEngine' => 'PhabricatorFileStorageEngine',
     'PhabricatorClusterConfigOptions' => 'PhabricatorApplicationConfigOptions',
+    'PhabricatorColumnsEditField' => 'PhabricatorPHIDListEditField',
     'PhabricatorCommentEditEngineExtension' => 'PhabricatorEditEngineExtension',
     'PhabricatorCommentEditField' => 'PhabricatorEditField',
     'PhabricatorCommentEditType' => 'PhabricatorEditType',
diff --git a/src/applications/conduit/parametertype/ConduitColumnsParameterType.php b/src/applications/conduit/parametertype/ConduitColumnsParameterType.php
new file mode 100644
index 0000000000..c6669fae06
--- /dev/null
+++ b/src/applications/conduit/parametertype/ConduitColumnsParameterType.php
@@ -0,0 +1,38 @@
+<?php
+
+final class ConduitColumnsParameterType
+  extends ConduitParameterType {
+
+  protected function getParameterValue(array $request, $key) {
+    // We don't do any meaningful validation here because the transaction
+    // itself validates everything and the input format is flexible.
+    return parent::getParameterValue($request, $key);
+  }
+
+  protected function getParameterTypeName() {
+    return 'columns';
+  }
+
+  protected function getParameterDefault() {
+    return array();
+  }
+
+  protected function getParameterFormatDescriptions() {
+    return array(
+      pht('Single column PHID.'),
+      pht('List of column PHIDs.'),
+      pht('List of position dictionaries.'),
+      pht('List with a mixture of PHIDs and dictionaries.'),
+    );
+  }
+
+  protected function getParameterExamples() {
+    return array(
+      '"PHID-PCOL-1111"',
+      '["PHID-PCOL-2222", "PHID-PCOL-3333"]',
+      '[{"columnPHID": "PHID-PCOL-4444", "afterPHID": "PHID-TASK-5555"}]',
+      '[{"columnPHID": "PHID-PCOL-4444", "beforePHID": "PHID-TASK-6666"}]',
+    );
+  }
+
+}
diff --git a/src/applications/maniphest/editor/ManiphestEditEngine.php b/src/applications/maniphest/editor/ManiphestEditEngine.php
index 6c7d26ca12..42822868f3 100644
--- a/src/applications/maniphest/editor/ManiphestEditEngine.php
+++ b/src/applications/maniphest/editor/ManiphestEditEngine.php
@@ -81,6 +81,57 @@ final class ManiphestEditEngine
       $owner_value = array($this->getViewer()->getPHID());
     }
 
+    $column_documentation = pht(<<<EODOCS
+You can use this transaction type to create a task into a particular workboard
+column, or move an existing task between columns.
+
+The transaction value can be specified in several forms. Some are simpler but
+less powerful, while others are more complex and more powerful.
+
+The simplest valid value is a single column PHID:
+
+```lang=json
+"PHID-PCOL-1111"
+```
+
+This will move the task into that column, or create the task into that column
+if you are creating a new task. If the task is currently on the board, it will
+be moved out of any exclusive columns. If the task is not currently on the
+board, it will be added to the board.
+
+You can also perform multiple moves at the same time by passing a list of
+PHIDs:
+
+```lang=json
+["PHID-PCOL-2222", "PHID-PCOL-3333"]
+```
+
+This is equivalent to performing each move individually.
+
+The most complex and most powerful form uses a dictionary to provide additional
+information about the move, including an optional specific position within the
+column.
+
+The target column should be identified as `columnPHID`, and you may select a
+position by passing either `beforePHID` or `afterPHID`, specifying the PHID of
+a task currently in the column that you want to move this task before or after:
+
+```lang=json
+[
+  {
+    "columnPHID": "PHID-PCOL-4444",
+    "beforePHID": "PHID-TASK-5555"
+  }
+]
+```
+
+Note that this affects only the "natural" position of the task. The task
+position when the board is sorted by some other attribute (like priority)
+depends on that attribute value: change a task's priority to move it on
+priority-sorted boards.
+EODOCS
+      );
+
     $fields = array(
       id(new PhabricatorHandlesEditField())
         ->setKey('parent')
@@ -95,18 +146,17 @@ final class ManiphestEditEngine
         ->setIsReorderable(false)
         ->setIsDefaultable(false)
         ->setIsLockable(false),
-      id(new PhabricatorHandlesEditField())
+      id(new PhabricatorColumnsEditField())
         ->setKey('column')
         ->setLabel(pht('Column'))
         ->setDescription(pht('Create a task in a workboard column.'))
         ->setConduitDescription(
           pht('Move a task to one or more workboard columns.'))
         ->setConduitTypeDescription(
-          pht('PHID or PHIDs of workboard columns.'))
+          pht('List of columns to move the task to.'))
+        ->setConduitDocumentation($column_documentation)
         ->setAliases(array('columnPHID', 'columns', 'columnPHIDs'))
         ->setTransactionType(PhabricatorTransactions::TYPE_COLUMNS)
-        ->setSingleValue(null)
-        ->setIsInvisible(true)
         ->setIsReorderable(false)
         ->setIsDefaultable(false)
         ->setIsLockable(false),
diff --git a/src/applications/transactions/editfield/PhabricatorColumnsEditField.php b/src/applications/transactions/editfield/PhabricatorColumnsEditField.php
new file mode 100644
index 0000000000..79fb25b86a
--- /dev/null
+++ b/src/applications/transactions/editfield/PhabricatorColumnsEditField.php
@@ -0,0 +1,21 @@
+<?php
+
+final class PhabricatorColumnsEditField
+  extends PhabricatorPHIDListEditField {
+
+  protected function newControl() {
+    $control = id(new AphrontFormHandlesControl());
+    $control->setIsInvisible(true);
+
+    return $control;
+  }
+
+  protected function newHTTPParameterType() {
+    return new AphrontPHIDListHTTPParameterType();
+  }
+
+  protected function newConduitParameterType() {
+    return new ConduitColumnsParameterType();
+  }
+
+}
diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
index 071b94a933..0e533cfa40 100644
--- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
+++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
@@ -504,7 +504,6 @@ abstract class PhabricatorApplicationTransaction
         case PhabricatorTransactions::TYPE_EDIT_POLICY:
         case PhabricatorTransactions::TYPE_JOIN_POLICY:
         case PhabricatorTransactions::TYPE_SPACE:
-        case PhabricatorTransactions::TYPE_COLUMNS:
           break;
         default:
           $old = $this->getOldValue();