Generate file attachment transactions for explicit Remarkup attachments on common edit pathways

Summary:
Ref T13603. On common edit pathways, extract explicit file attachments from Remarkup. These pathways are affected:

  - Objects that use EditEngine and expose a remarkup area via "RemarkupEditField".
  - Objects that use EditEngine to generate a comment area.

This is the vast majority of pathways, but not entirely exhaustive.

Test Plan: Created and commented on a task, explicitly attaching images. Saw images attach properly.

Maniphest Tasks: T13603

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

src/__phutil_library_map__.php

@@ -239,6 +239,7 @@ phutil_register_library_map(array(
     'AphrontIsolatedDatabaseConnection' => 'infrastructure/storage/connection/AphrontIsolatedDatabaseConnection.php',
     'AphrontIsolatedDatabaseConnectionTestCase' => 'infrastructure/storage/__tests__/AphrontIsolatedDatabaseConnectionTestCase.php',
     'AphrontIsolatedHTTPSink' => 'aphront/sink/AphrontIsolatedHTTPSink.php',
+    'AphrontJSONHTTPParameterType' => 'aphront/httpparametertype/AphrontJSONHTTPParameterType.php',
     'AphrontJSONResponse' => 'aphront/response/AphrontJSONResponse.php',
     'AphrontJavelinView' => 'view/AphrontJavelinView.php',
     'AphrontKeyboardShortcutsAvailableView' => 'view/widget/AphrontKeyboardShortcutsAvailableView.php',
@@ -272,6 +273,7 @@ phutil_register_library_map(array(
     'AphrontRedirectResponse' => 'aphront/response/AphrontRedirectResponse.php',
     'AphrontRedirectResponseTestCase' => 'aphront/response/__tests__/AphrontRedirectResponseTestCase.php',
     'AphrontReloadResponse' => 'aphront/response/AphrontReloadResponse.php',
+    'AphrontRemarkupHTTPParameterType' => 'aphront/httpparametertype/AphrontRemarkupHTTPParameterType.php',
     'AphrontRequest' => 'aphront/AphrontRequest.php',
     'AphrontRequestExceptionHandler' => 'aphront/handler/AphrontRequestExceptionHandler.php',
     'AphrontRequestStream' => 'aphront/requeststream/AphrontRequestStream.php',
@@ -5858,6 +5860,7 @@ phutil_register_library_map(array(
     'QueryFormattingTestCase' => 'infrastructure/storage/__tests__/QueryFormattingTestCase.php',
     'QueryFuture' => 'infrastructure/storage/future/QueryFuture.php',
     'RemarkupProcessConduitAPIMethod' => 'applications/remarkup/conduit/RemarkupProcessConduitAPIMethod.php',
+    'RemarkupValue' => 'applications/remarkup/RemarkupValue.php',
     'RepositoryConduitAPIMethod' => 'applications/repository/conduit/RepositoryConduitAPIMethod.php',
     'RepositoryQueryConduitAPIMethod' => 'applications/repository/conduit/RepositoryQueryConduitAPIMethod.php',
     'ShellLogView' => 'applications/harbormaster/view/ShellLogView.php',
@@ -6207,6 +6210,7 @@ phutil_register_library_map(array(
     'AphrontIsolatedDatabaseConnection' => 'AphrontDatabaseConnection',
     'AphrontIsolatedDatabaseConnectionTestCase' => 'PhabricatorTestCase',
     'AphrontIsolatedHTTPSink' => 'AphrontHTTPSink',
+    'AphrontJSONHTTPParameterType' => 'AphrontHTTPParameterType',
     'AphrontJSONResponse' => 'AphrontResponse',
     'AphrontJavelinView' => 'AphrontView',
     'AphrontKeyboardShortcutsAvailableView' => 'AphrontView',
@@ -6243,6 +6247,7 @@ phutil_register_library_map(array(
     'AphrontRedirectResponse' => 'AphrontResponse',
     'AphrontRedirectResponseTestCase' => 'PhabricatorTestCase',
     'AphrontReloadResponse' => 'AphrontRedirectResponse',
+    'AphrontRemarkupHTTPParameterType' => 'AphrontHTTPParameterType',
     'AphrontRequest' => 'Phobject',
     'AphrontRequestExceptionHandler' => 'Phobject',
     'AphrontRequestStream' => 'Phobject',
@@ -12747,6 +12752,7 @@ phutil_register_library_map(array(
     'QueryFormattingTestCase' => 'PhabricatorTestCase',
     'QueryFuture' => 'Future',
     'RemarkupProcessConduitAPIMethod' => 'ConduitAPIMethod',
+    'RemarkupValue' => 'Phobject',
     'RepositoryConduitAPIMethod' => 'ConduitAPIMethod',
     'RepositoryQueryConduitAPIMethod' => 'RepositoryConduitAPIMethod',
     'ShellLogView' => 'AphrontView',

src/aphront/httpparametertype/AphrontJSONHTTPParameterType.php

@@ -0,0 +1,31 @@
+<?php
+
+final class AphrontJSONHTTPParameterType
+  extends AphrontHTTPParameterType {
+
+  protected function getParameterDefault() {
+    return array();
+  }
+
+  protected function getParameterValue(AphrontRequest $request, $key) {
+    $str = $request->getStr($key);
+    return phutil_json_decode($str);
+  }
+
+  protected function getParameterTypeName() {
+    return 'string (json object)';
+  }
+
+  protected function getParameterFormatDescriptions() {
+    return array(
+      pht('A JSON-encoded object.'),
+    );
+  }
+
+  protected function getParameterExamples() {
+    return array(
+      'v={...}',
+    );
+  }
+
+}

src/aphront/httpparametertype/AphrontRemarkupHTTPParameterType.php

@@ -0,0 +1,50 @@
+<?php
+
+final class AphrontRemarkupHTTPParameterType
+  extends AphrontHTTPParameterType {
+
+  protected function getParameterDefault() {
+    return $this->newRemarkupValue();
+  }
+
+  protected function getParameterValue(AphrontRequest $request, $key) {
+    $corpus_key = $key;
+    $corpus_type = new AphrontStringHTTPParameterType();
+    $corpus_value = $this->getValueWithType(
+      $corpus_type,
+      $request,
+      $corpus_key);
+
+    $metadata_key = $key.'_metadata';
+    $metadata_type = new AphrontJSONHTTPParameterType();
+    $metadata_value = $this->getValueWithType(
+      $metadata_type,
+      $request,
+      $metadata_key);
+
+    return $this->newRemarkupValue()
+      ->setCorpus($corpus_value)
+      ->setMetadata($metadata_value);
+  }
+
+  protected function getParameterTypeName() {
+    return 'string (remarkup)';
+  }
+
+  protected function getParameterFormatDescriptions() {
+    return array(
+      pht('Remarkup text.'),
+    );
+  }
+
+  protected function getParameterExamples() {
+    return array(
+      'v=Lorem...',
+    );
+  }
+
+  private function newRemarkupValue() {
+    return new RemarkupValue();
+  }
+
+}

src/applications/remarkup/RemarkupValue.php

@@ -0,0 +1,27 @@
+<?php
+
+final class RemarkupValue
+  extends Phobject {
+
+  private $corpus;
+  private $metadata;
+
+  public function setCorpus($corpus) {
+    $this->corpus = $corpus;
+    return $this;
+  }
+
+  public function getCorpus() {
+    return $this->corpus;
+  }
+
+  public function setMetadata(array $metadata) {
+    $this->metadata = $metadata;
+    return $this;
+  }
+
+  public function getMetadata() {
+    return $this->metadata;
+  }
+
+}

src/applications/transactions/data/PhabricatorTransactionChange.php

@@ -3,6 +3,7 @@
 abstract class PhabricatorTransactionChange extends Phobject {
 
   private $transaction;
+  private $metadata = array();
   private $oldValue;
   private $newValue;
 
@@ -34,4 +35,13 @@ abstract class PhabricatorTransactionChange extends Phobject {
     return $this->newValue;
   }
 
+  final public function setMetadata(array $metadata) {
+    $this->metadata = $metadata;
+    return $this;
+  }
+
+  final public function getMetadata() {
+    return $this->metadata;
+  }
+
 }

src/applications/transactions/editengine/PhabricatorEditEngine.php

@@ -2012,6 +2012,7 @@ abstract class PhabricatorEditEngine
     if (strlen($comment_text) || !$xactions) {
       $xactions[] = id(clone $template)
         ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
+        ->setMetadataValue('remarkup.control', $comment_metadata)
         ->attachComment(
           id(clone $comment_template)
             ->setContent($comment_text));
@@ -2079,6 +2080,26 @@ abstract class PhabricatorEditEngine
     }
   }
 
+  public static function newTransactionsFromRemarkupMetadata(
+    PhabricatorApplicationTransaction $template,
+    array $metadata) {
+
+    $xactions = array();
+
+    $attached_phids = idx($metadata, 'attachedFilePHIDs');
+    if (is_array($attached_phids) && $attached_phids) {
+      $attachment_map = array_fill_keys(
+        $attached_phids,
+        PhabricatorFileAttachment::MODE_ATTACH);
+
+      $xactions[] = id(clone $template)
+        ->setTransactionType(PhabricatorTransactions::TYPE_FILE)
+        ->setNewValue($attachment_map);
+    }
+
+    return $xactions;
+  }
+
   protected function newDraftEngine($object) {
     $viewer = $this->getViewer();
 

src/applications/transactions/editfield/PhabricatorRemarkupEditField.php

@@ -7,6 +7,10 @@ final class PhabricatorRemarkupEditField
     return new PhabricatorRemarkupControl();
   }
 
+  protected function newHTTPParameterType() {
+    return new AphrontRemarkupHTTPParameterType();
+  }
+
   protected function newConduitParameterType() {
     return new ConduitStringParameterType();
   }
@@ -15,4 +19,29 @@ final class PhabricatorRemarkupEditField
     return new BulkRemarkupParameterType();
   }
 
+  public function getValueForTransaction() {
+    $value = $this->getValue();
+
+    if ($value instanceof RemarkupValue) {
+      $value = $value->getCorpus();
+    }
+
+    return $value;
+  }
+
+  public function getMetadata() {
+    $defaults = array();
+
+    $value = $this->getValue();
+    if ($value instanceof RemarkupValue) {
+      $defaults['remarkup.control'] = $value->getMetadata();
+    }
+
+    $metadata = parent::getMetadata();
+    $metadata = $metadata + $defaults;
+
+    return $metadata;
+  }
+
+
 }

src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php

@@ -2225,7 +2225,8 @@ abstract class PhabricatorApplicationTransactionEditor
 
     $file_xaction = $this->newFileTransaction(
       $object,
-      $xactions);
+      $xactions,
+      $changes);
     if ($file_xaction) {
       $xactions[] = $file_xaction;
     }
@@ -2236,19 +2237,33 @@ abstract class PhabricatorApplicationTransactionEditor
 
   private function newFileTransaction(
     PhabricatorLiskDAO $object,
-    array $xactions) {
+    array $xactions,
+    array $remarkup_changes) {
+
+    assert_instances_of(
+      $remarkup_changes,
+      'PhabricatorTransactionRemarkupChange');
 
     $new_map = array();
 
-    $file_phids = $this->extractFilePHIDs($object, $xactions);
+    foreach ($remarkup_changes as $remarkup_change) {
-    if (!$file_phids) {
+      $metadata = $remarkup_change->getMetadata();
-      return null;
+
+      $attached_phids = idx($metadata, 'attachedFilePHIDs');
+      foreach ($attached_phids as $file_phid) {
+        $new_map[$file_phid] = PhabricatorFileAttachment::MODE_ATTACH;
+      }
     }
 
+    $file_phids = $this->extractFilePHIDs($object, $xactions);
     foreach ($file_phids as $file_phid) {
       $new_map[$file_phid] = PhabricatorFileAttachment::MODE_ATTACH;
     }
 
+    if (!$new_map) {
+      return null;
+    }
+
     $xaction = $object->getApplicationTransactionTemplate()
       ->setTransactionType(PhabricatorTransactions::TYPE_FILE)
       ->setNewValue($new_map);

src/applications/transactions/storage/PhabricatorApplicationTransaction.php

@@ -244,6 +244,13 @@ abstract class PhabricatorApplicationTransaction
         ->setNewValue($new_value);
     }
 
+    $metadata = $this->getMetadataValue('remarkup.control', array());
+    foreach ($changes as $change) {
+      if (!$change->getMetadata()) {
+        $change->setMetadata($metadata);
+      }
+    }
+
     return $changes;
   }
 

src/view/form/control/PhabricatorRemarkupControl.php

@@ -1,6 +1,7 @@
 <?php
 
-final class PhabricatorRemarkupControl extends AphrontFormTextAreaControl {
+final class PhabricatorRemarkupControl
+  extends AphrontFormTextAreaControl {
 
   private $disableMacro = false;
   private $disableFullScreen = false;
@@ -45,6 +46,15 @@ final class PhabricatorRemarkupControl extends AphrontFormTextAreaControl {
     return $this->remarkupMetadata;
   }
 
+  public function setValue($value) {
+    if ($value instanceof RemarkupValue) {
+      $this->setRemarkupMetadata($value->getMetadata());
+      $value = $value->getCorpus();
+    }
+
+    return parent::setValue($value);
+  }
+
   protected function renderInput() {
     $id = $this->getID();
     if (!$id) {