Maniphest - remove "attach file" action

Summary: Fixes T4655. Basically leaves the display code intact for legacy installs but removes the option from the UI and removes "create" code.

Test Plan:
tried to attach file and the action was not in the dropdown!
made a new task and it worked!
commented on an old task and it worked!

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4655

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

src/applications/maniphest/conduit/ConduitAPI_maniphest_Method.php

@@ -38,7 +38,6 @@ abstract class ConduitAPI_maniphest_Method extends ConduitAPIMethod {
       'ccPHIDs'       => 'optional list<phid>',
       'priority'      => 'optional int',
       'projectPHIDs'  => 'optional list<phid>',
-      'filePHIDs'     => 'optional list<phid>',
       'auxiliary'     => 'optional dict',
     );
 
@@ -126,18 +125,6 @@ abstract class ConduitAPI_maniphest_Method extends ConduitAPIMethod {
       $changes[ManiphestTransaction::TYPE_PROJECTS] = $project_phids;
     }
 
-    $file_phids = $request->getValue('filePHIDs');
-    if ($file_phids !== null) {
-      $this->validatePHIDList($file_phids,
-                              PhabricatorFilePHIDTypeFile::TYPECONST,
-                              'filePHIDS');
-      $file_map = array_fill_keys($file_phids, true);
-      $attached = $task->getAttached();
-      $attached[PhabricatorFilePHIDTypeFile::TYPECONST] = $file_map;
-
-      $changes[ManiphestTransaction::TYPE_ATTACH] = $attached;
-    }
-
     $template = new ManiphestTransaction();
 
     $transactions = array();

src/applications/maniphest/controller/ManiphestTaskDetailController.php

@@ -155,7 +155,6 @@ final class ManiphestTaskDetailController extends ManiphestController {
       ManiphestTransaction::TYPE_OWNER      => pht('Reassign / Claim'),
       ManiphestTransaction::TYPE_CCS        => pht('Add CCs'),
       ManiphestTransaction::TYPE_PRIORITY   => pht('Change Priority'),
-      ManiphestTransaction::TYPE_ATTACH     => pht('Upload File'),
       ManiphestTransaction::TYPE_PROJECTS   => pht('Associate Projects'),
     );
 
@@ -304,7 +303,6 @@ final class ManiphestTaskDetailController extends ManiphestController {
       ManiphestTransaction::TYPE_CCS      => 'ccs',
       ManiphestTransaction::TYPE_PRIORITY => 'priority',
       ManiphestTransaction::TYPE_PROJECTS => 'projects',
-      ManiphestTransaction::TYPE_ATTACH   => 'file',
     );
 
     $tokenizer_map = array(

src/applications/maniphest/controller/ManiphestTaskEditController.php

@@ -25,7 +25,6 @@ final class ManiphestTaskEditController extends ManiphestController {
     $can_edit_status = $this->hasApplicationCapability(
       ManiphestCapabilityEditStatus::CAPABILITY);
 
-    $files = array();
     $parent_task = null;
     $template_id = null;
 
@@ -108,24 +107,6 @@ final class ManiphestTaskEditController extends ManiphestController {
         }
       }
 
-      $file_phids = $request->getArr('files', array());
-      if (!$file_phids) {
-        // Allow a single 'file' key instead, mostly since Mac OS X urlencodes
-        // square brackets in URLs when passed to 'open', so you can't 'open'
-        // a URL like '?files[]=xyz' and have PHP interpret it correctly.
-        $phid = $request->getStr('file');
-        if ($phid) {
-          $file_phids = array($phid);
-        }
-      }
-
-      if ($file_phids) {
-        $files = id(new PhabricatorFileQuery())
-          ->setViewer($user)
-          ->withPHIDs($file_phids)
-          ->execute();
-      }
-
       $template_id = $request->getInt('template');
 
       // You can only have a parent task if you're creating a new task.
@@ -267,14 +248,6 @@ final class ManiphestTaskEditController extends ManiphestController {
             $request->getStr('editPolicy');
         }
 
-        if ($files) {
-          $file_map = mpull($files, 'getPHID');
-          $file_map = array_fill_keys($file_map, array());
-          $changes[ManiphestTransaction::TYPE_ATTACH] = array(
-            PhabricatorFilePHIDTypeFile::TYPECONST => $file_map,
-          );
-        }
-
         $template = new ManiphestTransaction();
         $transactions = array();
 
@@ -674,21 +647,6 @@ final class ManiphestTaskEditController extends ManiphestController {
       'tokenizerID' => $project_tokenizer_id,
     ));
 
-    if ($files) {
-      $file_display = mpull($files, 'getName');
-      $file_display = phutil_implode_html(phutil_tag('br'), $file_display);
-
-      $form->appendChild(
-        id(new AphrontFormMarkupControl())
-          ->setLabel(pht('Files'))
-          ->setValue($file_display));
-
-      foreach ($files as $ii => $file) {
-        $form->addHiddenInput('files['.$ii.']', $file->getPHID());
-      }
-    }
-
-
     $description_control = new PhabricatorRemarkupControl();
     // "Upsell" creating tasks via email in create flows if the instance is
     // configured for this awesomeness.

src/applications/maniphest/controller/ManiphestTransactionSaveController.php

@@ -20,55 +20,6 @@ final class ManiphestTransactionSaveController extends ManiphestController {
 
     $action = $request->getStr('action');
 
-    // If we have drag-and-dropped files, attach them first in a separate
-    // transaction. These can come in on any transaction type, which is why we
-    // handle them separately.
-    $files = array();
-
-    // Look for drag-and-drop uploads first.
-    $file_phids = $request->getArr('files');
-    if ($file_phids) {
-      $files = id(new PhabricatorFileQuery())
-        ->setViewer($user)
-        ->withPHIDs(array($file_phids))
-        ->execute();
-    }
-
-    // This means "attach a file" even though we store other types of data
-    // as 'attached'.
-    if ($action == ManiphestTransaction::TYPE_ATTACH) {
-      if (!empty($_FILES['file'])) {
-        $err = idx($_FILES['file'], 'error');
-        if ($err != UPLOAD_ERR_NO_FILE) {
-          $file = PhabricatorFile::newFromPHPUpload(
-            $_FILES['file'],
-            array(
-              'authorPHID' => $user->getPHID(),
-            ));
-          $files[] = $file;
-        }
-      }
-    }
-
-    // If we had explicit or drag-and-drop files, create a transaction
-    // for those before we deal with whatever else might have happened.
-    $file_transaction = null;
-    if ($files) {
-      $files = mpull($files, 'getPHID', 'getPHID');
-      $new = $task->getAttached();
-      foreach ($files as $phid) {
-        if (empty($new[PhabricatorFilePHIDTypeFile::TYPECONST])) {
-          $new[PhabricatorFilePHIDTypeFile::TYPECONST] = array();
-        }
-        $new[PhabricatorFilePHIDTypeFile::TYPECONST][$phid] = array();
-      }
-      $transaction = new ManiphestTransaction();
-      $transaction
-        ->setTransactionType(ManiphestTransaction::TYPE_ATTACH);
-      $transaction->setNewValue($new);
-      $transactions[] = $transaction;
-    }
-
     // Compute new CCs added by @mentions. Several things can cause CCs to
     // be added as side effects: mentions, explicit CCs, users who aren't
     // CC'd interacting with the task, and ownership changes. We build up a
@@ -114,10 +65,6 @@ final class ManiphestTransactionSaveController extends ManiphestController {
       case ManiphestTransaction::TYPE_PRIORITY:
         $transaction->setNewValue($request->getInt('priority'));
         break;
-      case ManiphestTransaction::TYPE_ATTACH:
-        // Nuke this, we created it above.
-        $transaction = null;
-        break;
       case PhabricatorTransactions::TYPE_COMMENT:
         // Nuke this, we're going to create it below.
         $transaction = null;

src/applications/maniphest/editor/ManiphestTransactionEditor.php

@@ -16,7 +16,6 @@ final class ManiphestTransactionEditor
     $types[] = ManiphestTransaction::TYPE_OWNER;
     $types[] = ManiphestTransaction::TYPE_CCS;
     $types[] = ManiphestTransaction::TYPE_PROJECTS;
-    $types[] = ManiphestTransaction::TYPE_ATTACH;
     $types[] = ManiphestTransaction::TYPE_EDGE;
     $types[] = ManiphestTransaction::TYPE_SUBPRIORITY;
     $types[] = ManiphestTransaction::TYPE_PROJECT_COLUMN;
@@ -57,8 +56,6 @@ final class ManiphestTransactionEditor
         return array_values(array_unique($object->getCCPHIDs()));
       case ManiphestTransaction::TYPE_PROJECTS:
         return array_values(array_unique($object->getProjectPHIDs()));
-      case ManiphestTransaction::TYPE_ATTACH:
-        return $object->getAttached();
       case ManiphestTransaction::TYPE_EDGE:
       case ManiphestTransaction::TYPE_PROJECT_COLUMN:
         // These are pre-populated.
@@ -84,7 +81,6 @@ final class ManiphestTransactionEditor
       case ManiphestTransaction::TYPE_STATUS:
       case ManiphestTransaction::TYPE_TITLE:
       case ManiphestTransaction::TYPE_DESCRIPTION:
-      case ManiphestTransaction::TYPE_ATTACH:
       case ManiphestTransaction::TYPE_EDGE:
       case ManiphestTransaction::TYPE_SUBPRIORITY:
       case ManiphestTransaction::TYPE_PROJECT_COLUMN:
@@ -155,8 +151,6 @@ final class ManiphestTransactionEditor
         return $object->setCCPHIDs($xaction->getNewValue());
       case ManiphestTransaction::TYPE_PROJECTS:
         return $object->setProjectPHIDs($xaction->getNewValue());
-      case ManiphestTransaction::TYPE_ATTACH:
-        return $object->setAttached($xaction->getNewValue());
       case ManiphestTransaction::TYPE_EDGE:
         // These are a weird, funky mess and are already being applied by the
         // time we reach this.

src/applications/maniphest/storage/ManiphestTransaction.php

@@ -11,10 +11,13 @@ final class ManiphestTransaction
   const TYPE_PROJECTS = 'projects';
   const TYPE_PRIORITY = 'priority';
   const TYPE_EDGE = 'edge';
-  const TYPE_ATTACH = 'attach';
   const TYPE_SUBPRIORITY = 'subpriority';
   const TYPE_PROJECT_COLUMN = 'projectcolumn';
 
+  // NOTE: this type is deprecated. Keep it around for legacy installs
+  // so any transactions render correctly.
+  const TYPE_ATTACH = 'attach';
+
   public function getApplicationName() {
     return 'maniphest';
   }