Consolidate transaction generation in EditType objects

Summary:
Ref T9132. This is a bit more cleanup to make adding CustomField support easier.

Right now, both `EditField` and `EditType` can actually generate a transaction. This doesn't matter too much in practice today, but gets a little more complicated a couple of diffs from now with CustomField stuff.

Instead, always use `EditType` to generate the transaction. In the future, this should give us less total code and make more things work cleanly by default.

Test Plan: Used web UI and Conduit to make various edits to pastes, including doing race-condition tests on "Projects".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9132

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

src/applications/transactions/editengine/PhabricatorEditEngine.php

@@ -630,13 +630,22 @@ abstract class PhabricatorEditEngine
 
       $xactions = array();
       foreach ($fields as $field) {
-        $xaction = $field->generateTransaction(clone $template);
+        $types = $field->getWebEditTypes();
+        foreach ($types as $type) {
+          $type_xactions = $type->generateTransactions(
+            clone $template,
+            array(
+              'value' => $field->getValueForTransaction(),
+            ));
 
-        if (!$xaction) {
+          if (!$type_xactions) {
-          continue;
+            continue;
+          }
+
+          foreach ($type_xactions as $type_xaction) {
+            $xactions[] = $type_xaction;
+          }
         }
-
-        $xactions[] = $xaction;
       }
 
       $editor = $object->getApplicationTransactionEditor()
@@ -941,7 +950,7 @@ abstract class PhabricatorEditEngine
 
     $fields = $this->buildEditFields($object);
 
-    $types = $this->getAllEditTypesFromFields($fields);
+    $types = $this->getConduitEditTypesFromFields($fields);
     $template = $object->getApplicationTransactionTemplate();
 
     $xactions = $this->getConduitTransactions($request, $types, $template);
@@ -1031,9 +1040,13 @@ abstract class PhabricatorEditEngine
     foreach ($xactions as $xaction) {
       $type = $types[$xaction['type']];
 
-      $results[] = $type->generateTransaction(
+      $type_xactions = $type->generateTransactions(
         clone $template,
         $xaction);
+
+      foreach ($type_xactions as $type_xaction) {
+        $results[] = $type_xaction;
+      }
     }
 
     return $results;
@@ -1044,10 +1057,10 @@ abstract class PhabricatorEditEngine
    * @return map<string, PhabricatorEditType>
    * @task conduit
    */
-  private function getAllEditTypesFromFields(array $fields) {
+  private function getConduitEditTypesFromFields(array $fields) {
     $types = array();
     foreach ($fields as $field) {
-      $field_types = $field->getEditTransactionTypes();
+      $field_types = $field->getConduitEditTypes();
 
       if ($field_types === null) {
         continue;
@@ -1061,7 +1074,7 @@ abstract class PhabricatorEditEngine
     return $types;
   }
 
-  public function getAllEditTypes() {
+  public function getConduitEditTypes() {
     $config = $this->loadEditEngineConfiguration(null);
     if (!$config) {
       return array();
@@ -1069,7 +1082,7 @@ abstract class PhabricatorEditEngine
 
     $object = $this->newEditableObject();
     $fields = $this->buildEditFields($object);
-    return $this->getAllEditTypesFromFields($fields);
+    return $this->getConduitEditTypesFromFields($fields);
   }
 
   final public static function getAllEditEngines() {

src/applications/transactions/editengine/PhabricatorEditEngineAPIMethod.php

@@ -44,7 +44,7 @@ abstract class PhabricatorEditEngineAPIMethod
     $engine = $this->newEditEngine()
       ->setViewer($viewer);
 
-    $types = $engine->getAllEditTypes();
+    $types = $engine->getConduitEditTypes();
 
     $out = array();
 

src/applications/transactions/editfield/PhabricatorCommentEditField.php

@@ -11,15 +11,4 @@ final class PhabricatorCommentEditField
     return new PhabricatorCommentEditType();
   }
 
-  public function generateTransaction(
-    PhabricatorApplicationTransaction $xaction) {
-
-    $spec = array(
-      'value' => $this->getValueForTransaction(),
-    );
-
-    return head($this->getEditTransactionTypes())
-      ->generateTransaction($xaction, $spec);
-  }
-
 }

src/applications/transactions/editfield/PhabricatorEditField.php

@@ -268,24 +268,6 @@ abstract class PhabricatorEditField extends Phobject {
     return $this;
   }
 
-  public function generateTransaction(
-    PhabricatorApplicationTransaction $xaction) {
-
-    if (!$this->getTransactionType()) {
-      return null;
-    }
-
-    $xaction
-      ->setTransactionType($this->getTransactionType())
-      ->setNewValue($this->getValueForTransaction());
-
-    foreach ($this->metadata as $key => $value) {
-      $xaction->setMetadataValue($key, $value);
-    }
-
-    return $xaction;
-  }
-
   public function setMetadataValue($key, $value) {
     $this->metadata[$key] = $value;
     return $this;
@@ -295,7 +277,7 @@ abstract class PhabricatorEditField extends Phobject {
     return $this->metadata;
   }
 
-  protected function getValueForTransaction() {
+  public function getValueForTransaction() {
     return $this->getValue();
   }
 
@@ -449,7 +431,7 @@ abstract class PhabricatorEditField extends Phobject {
       ->setValueType($this->getHTTPParameterType()->getTypeName());
   }
 
-  protected function getEditTransactionType() {
+  protected function getEditType() {
     $transaction_type = $this->getTransactionType();
 
     if ($transaction_type === null) {
@@ -465,8 +447,18 @@ abstract class PhabricatorEditField extends Phobject {
       ->setMetadata($this->getMetadata());
   }
 
-  public function getEditTransactionTypes() {
+  public function getConduitEditTypes() {
-    $edit_type = $this->getEditTransactionType();
+    $edit_type = $this->getEditType();
+
+    if ($edit_type === null) {
+      return null;
+    }
+
+    return array($edit_type);
+  }
+
+  public function getWebEditTypes() {
+    $edit_type = $this->getEditType();
 
     if ($edit_type === null) {
       return null;

src/applications/transactions/editfield/PhabricatorPHIDListEditField.php

@@ -28,7 +28,7 @@ abstract class PhabricatorPHIDListEditField
     return new AphrontPHIDListHTTPParameterType();
   }
 
-  protected function getValueForTransaction() {
+  public function getValueForTransaction() {
     $new = parent::getValueForTransaction();
 
     if (!$this->getUseEdgeTransactions()) {
@@ -71,9 +71,9 @@ abstract class PhabricatorPHIDListEditField
     return parent::newEditType();
   }
 
-  public function getEditTransactionTypes() {
+  public function getConduitEditTypes() {
     if (!$this->getUseEdgeTransactions()) {
-      return parent::getEditTransactionTypes();
+      return parent::getConduitEditTypes();
     }
 
     $transaction_type = $this->getTransactionType();
@@ -84,7 +84,7 @@ abstract class PhabricatorPHIDListEditField
     $type_key = $this->getEditTypeKey();
     $strings = $this->transactionDescriptions;
 
-    $base = $this->getEditTransactionType();
+    $base = $this->getEditType();
 
     $add = id(clone $base)
       ->setEditType($type_key.'.add')

src/applications/transactions/edittype/PhabricatorCommentEditType.php

@@ -6,22 +6,17 @@ final class PhabricatorCommentEditType extends PhabricatorEditType {
     return id(new AphrontStringHTTPParameterType())->getTypeName();
   }
 
-  public function generateTransaction(
+  public function generateTransactions(
     PhabricatorApplicationTransaction $template,
     array $spec) {
 
     $comment = $template->getApplicationTransactionCommentObject()
       ->setContent(idx($spec, 'value'));
 
-    $template
+    $xaction = $this->newTransaction($template)
-      ->setTransactionType($this->getTransactionType())
       ->attachComment($comment);
 
-    foreach ($this->getMetadata() as $key => $value) {
+    return array($xaction);
-      $template->setMetadataValue($key, $value);
-    }
-
-    return $template;
   }
 
   public function getValueDescription() {

src/applications/transactions/edittype/PhabricatorEdgeEditType.php

@@ -18,25 +18,23 @@ final class PhabricatorEdgeEditType extends PhabricatorEditType {
     return 'list<phid>';
   }
 
-  public function generateTransaction(
+  public function generateTransactions(
     PhabricatorApplicationTransaction $template,
     array $spec) {
 
     $value = idx($spec, 'value');
-    $value = array_fuse($value);
-    $value = array(
-      $this->getEdgeOperation() => $value,
-    );
 
-    $template
+    if ($this->getEdgeOperation() !== null) {
-      ->setTransactionType($this->getTransactionType())
+      $value = array_fuse($value);
-      ->setNewValue($value);
+      $value = array(
-
+        $this->getEdgeOperation() => $value,
-    foreach ($this->getMetadata() as $key => $value) {
+      );
-      $template->setMetadataValue($key, $value);
     }
 
-    return $template;
+    $xaction = $this->newTransaction($template)
+      ->setNewValue($value);
+
+    return array($xaction);
   }
 
   public function setValueDescription($value_description) {

src/applications/transactions/edittype/PhabricatorEditType.php

@@ -66,11 +66,24 @@ abstract class PhabricatorEditType extends Phobject {
     return $this->transactionType;
   }
 
-  abstract public function generateTransaction(
+  abstract public function generateTransactions(
     PhabricatorApplicationTransaction $template,
     array $spec);
 
   abstract public function getValueType();
   abstract public function getValueDescription();
 
+  protected function newTransaction(
+    PhabricatorApplicationTransaction $template) {
+
+    $xaction = id(clone $template)
+      ->setTransactionType($this->getTransactionType());
+
+    foreach ($this->getMetadata() as $key => $value) {
+      $xaction->setMetadataValue($key, $value);
+    }
+
+    return $xaction;
+  }
+
 }

src/applications/transactions/edittype/PhabricatorSimpleEditType.php

@@ -14,19 +14,14 @@ final class PhabricatorSimpleEditType extends PhabricatorEditType {
     return $this->valueType;
   }
 
-  public function generateTransaction(
+  public function generateTransactions(
     PhabricatorApplicationTransaction $template,
     array $spec) {
 
-    $template
+    $edit = $this->newTransaction($template)
-      ->setTransactionType($this->getTransactionType())
       ->setNewValue(idx($spec, 'value'));
 
-    foreach ($this->getMetadata() as $key => $value) {
+    return array($edit);
-      $template->setMetadataValue($key, $value);
-    }
-
-    return $template;
   }
 
   public function setValueDescription($value_description) {