Allow the new Differential EditEngine form to create/update diffs for revisions

Summary: Ref T11114. Much of this is around making the "comment-while-updating" flow work correctly.

Test Plan:
  - Created new diffs by copy/pasting, then:
    - used one to create a new revision;
    - used one to update an existing revision, with a comment.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11114

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

src/__phutil_library_map__.php

@@ -2514,6 +2514,7 @@ phutil_register_library_map(array(
     'PhabricatorDisabledUserController' => 'applications/auth/controller/PhabricatorDisabledUserController.php',
     'PhabricatorDisplayPreferencesSettingsPanel' => 'applications/settings/panel/PhabricatorDisplayPreferencesSettingsPanel.php',
     'PhabricatorDisqusAuthProvider' => 'applications/auth/provider/PhabricatorDisqusAuthProvider.php',
+    'PhabricatorDividerEditField' => 'applications/transactions/editfield/PhabricatorDividerEditField.php',
     'PhabricatorDividerProfileMenuItem' => 'applications/search/menuitem/PhabricatorDividerProfileMenuItem.php',
     'PhabricatorDivinerApplication' => 'applications/diviner/application/PhabricatorDivinerApplication.php',
     'PhabricatorDoorkeeperApplication' => 'applications/doorkeeper/application/PhabricatorDoorkeeperApplication.php',
@@ -3825,6 +3826,7 @@ phutil_register_library_map(array(
     'PhabricatorStreamingProtocolAdapter' => 'infrastructure/daemon/bot/adapter/PhabricatorStreamingProtocolAdapter.php',
     'PhabricatorStringListEditField' => 'applications/transactions/editfield/PhabricatorStringListEditField.php',
     'PhabricatorStringSetting' => 'applications/settings/setting/PhabricatorStringSetting.php',
+    'PhabricatorSubmitEditField' => 'applications/transactions/editfield/PhabricatorSubmitEditField.php',
     'PhabricatorSubscribableInterface' => 'applications/subscriptions/interface/PhabricatorSubscribableInterface.php',
     'PhabricatorSubscribedToObjectEdgeType' => 'applications/transactions/edges/PhabricatorSubscribedToObjectEdgeType.php',
     'PhabricatorSubscribersEditField' => 'applications/transactions/editfield/PhabricatorSubscribersEditField.php',
@@ -7468,6 +7470,7 @@ phutil_register_library_map(array(
     'PhabricatorDisabledUserController' => 'PhabricatorAuthController',
     'PhabricatorDisplayPreferencesSettingsPanel' => 'PhabricatorEditEngineSettingsPanel',
     'PhabricatorDisqusAuthProvider' => 'PhabricatorOAuth2AuthProvider',
+    'PhabricatorDividerEditField' => 'PhabricatorEditField',
     'PhabricatorDividerProfileMenuItem' => 'PhabricatorProfileMenuItem',
     'PhabricatorDivinerApplication' => 'PhabricatorApplication',
     'PhabricatorDoorkeeperApplication' => 'PhabricatorApplication',
@@ -9024,6 +9027,7 @@ phutil_register_library_map(array(
     'PhabricatorStreamingProtocolAdapter' => 'PhabricatorProtocolAdapter',
     'PhabricatorStringListEditField' => 'PhabricatorEditField',
     'PhabricatorStringSetting' => 'PhabricatorSetting',
+    'PhabricatorSubmitEditField' => 'PhabricatorEditField',
     'PhabricatorSubscribedToObjectEdgeType' => 'PhabricatorEdgeType',
     'PhabricatorSubscribersEditField' => 'PhabricatorTokenizerEditField',
     'PhabricatorSubscribersQuery' => 'PhabricatorQuery',

src/applications/differential/application/PhabricatorDifferentialApplication.php

@@ -69,6 +69,8 @@ final class PhabricatorDifferentialApplication extends PhabricatorApplication {
             => 'DifferentialRevisionEditController',
           $this->getEditRoutePattern('editpro/')
             => 'DifferentialRevisionEditProController',
+          $this->getEditRoutePattern('attach/(?P<diffID>[^/]+)/to/')
+            => 'DifferentialRevisionEditProController',
           'land/(?:(?P<id>[1-9]\d*))/(?P<strategy>[^/]+)/'
             => 'DifferentialRevisionLandController',
           'closedetails/(?P<phid>[^/]+)/'

src/applications/differential/controller/DifferentialRevisionEditProController.php

@@ -4,9 +4,53 @@ final class DifferentialRevisionEditProController
   extends DifferentialController {
 
   public function handleRequest(AphrontRequest $request) {
-    return id(new DifferentialRevisionEditEngine())
-      ->setController($this)
-      ->buildResponse();
+    $viewer = $this->getViewer();
+
+    // If we have a Diff ID, this is an "/attach/123/to/456/" action. The
+    // user just created a diff and is trying to use it to create or update
+    // a revision.
+    $diff_id = $request->getURIData('diffID');
+
+    if ($diff_id) {
+      $diff = id(new DifferentialDiffQuery())
+        ->setViewer($viewer)
+        ->withIDs(array($diff_id))
+        ->executeOne();
+      if (!$diff) {
+        return new Aphront404Response();
+      }
+
+      if ($diff->getRevisionID()) {
+        $revision = $diff->getRevision();
+        return $this->newDialog()
+          ->setTitle(pht('Diff Already Attached'))
+          ->appendParagraph(
+            pht(
+              'This diff is already attached to a revision.'))
+          ->addCancelButton($revision->getURI(), pht('Continue'));
+      }
+    } else {
+      $diff = null;
+    }
+
+    $revision_id = $request->getURIData('id');
+    if (!$diff && !$revision_id) {
+      return $this->newDialog()
+        ->setTitle(pht('Diff Required'))
+        ->appendParagraph(
+          pht(
+            'You can not create a revision without a diff.'))
+        ->addCancelButton($this->getApplicationURI());
+    }
+
+    $engine = id(new DifferentialRevisionEditEngine())
+      ->setController($this);
+
+    if ($diff) {
+      $engine->setDiff($diff);
+    }
+
+    return $engine->buildResponse();
   }
 
 }

src/applications/differential/editor/DifferentialRevisionEditEngine.php

@@ -3,6 +3,8 @@
 final class DifferentialRevisionEditEngine
   extends PhabricatorEditEngine {
 
+  private $diff;
+
   const ENGINECONST = 'differential.revision';
 
   public function getEngineName() {
@@ -33,6 +35,7 @@ final class DifferentialRevisionEditEngine
 
   protected function newObjectQuery() {
     return id(new DifferentialRevisionQuery())
+      ->needActiveDiffs(true)
       ->needReviewerStatus(true);
   }
 
@@ -41,7 +44,15 @@ final class DifferentialRevisionEditEngine
   }
 
   protected function getObjectEditTitleText($object) {
-    return pht('Edit Revision: %s', $object->getTitle());
+    $monogram = $object->getMonogram();
+    $title = $object->getTitle();
+
+    $diff = $this->getDiff();
+    if ($diff) {
+      return pht('Update Revision %s: %s', $monogram, $title);
+    } else {
+      return pht('Edit Revision %s: %s', $monogram, $title);
+    }
   }
 
   protected function getObjectEditShortText($object) {
@@ -60,6 +71,15 @@ final class DifferentialRevisionEditEngine
     return $object->getURI();
   }
 
+  public function setDiff(DifferentialDiff $diff) {
+    $this->diff = $diff;
+    return $this;
+  }
+
+  public function getDiff() {
+    return $this->diff;
+  }
+
   protected function buildCustomEditFields($object) {
 
     $plan_required = PhabricatorEnv::getEnvConfig(
@@ -68,7 +88,48 @@ final class DifferentialRevisionEditEngine
       $object,
       'differential:test-plan');
 
+    $diff = $this->getDiff();
+    if ($diff) {
+      $diff_phid = $diff->getPHID();
+    } else {
+      $diff_phid = null;
+    }
+
+    $is_update = ($diff && $object->getID());
+
     $fields = array();
+
+    $fields[] = id(new PhabricatorHandlesEditField())
+      ->setKey('update')
+      ->setLabel(pht('Update Diff'))
+      ->setDescription(pht('New diff to create or update the revision with.'))
+      ->setConduitDescription(pht('Create or update a revision with a diff.'))
+      ->setConduitTypeDescription(pht('PHID of the diff.'))
+      ->setTransactionType(DifferentialTransaction::TYPE_UPDATE)
+      ->setHandleParameterType(new AphrontPHIDListHTTPParameterType())
+      ->setSingleValue($diff_phid)
+      ->setIsReorderable(false)
+      ->setIsDefaultable(false)
+      ->setIsInvisible(true)
+      ->setIsLockable(false);
+
+    if ($is_update) {
+      $fields[] = id(new PhabricatorInstructionsEditField())
+        ->setKey('update.help')
+        ->setValue(pht('Describe the updates you have made to the diff.'));
+      $fields[] = id(new PhabricatorCommentEditField())
+        ->setKey('update.comment')
+        ->setLabel(pht('Comment'))
+        ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
+        ->setIsWebOnly(true)
+        ->setDescription(pht('Comments providing context for the update.'));
+      $fields[] = id(new PhabricatorSubmitEditField())
+        ->setKey('update.submit')
+        ->setValue($this->getObjectEditButtonText($object));
+      $fields[] = id(new PhabricatorDividerEditField())
+        ->setKey('update.note');
+    }
+
     $fields[] = id(new PhabricatorTextEditField())
       ->setKey('title')
       ->setLabel(pht('Title'))

src/applications/transactions/editfield/PhabricatorCommentEditField.php

@@ -3,6 +3,17 @@
 final class PhabricatorCommentEditField
   extends PhabricatorEditField {
 
+  private $isWebOnly;
+
+  public function setIsWebOnly($is_web_only) {
+    $this->isWebOnly = $is_web_only;
+    return $this;
+  }
+
+  public function getIsWebOnly() {
+    return $this->isWebOnly;
+  }
+
   protected function newControl() {
     return new PhabricatorRemarkupControl();
   }
@@ -12,15 +23,23 @@ final class PhabricatorCommentEditField
   }
 
   protected function newConduitParameterType() {
-    return new ConduitStringParameterType();
+    if ($this->getIsWebOnly()) {
+      return null;
+    } else {
+      return new ConduitStringParameterType();
+    }
   }
 
   public function shouldGenerateTransactionsFromSubmit() {
-    return false;
+    return !$this->isPrimaryCommentField();
   }
 
   public function shouldGenerateTransactionsFromComment() {
-    return true;
+    return $this->isPrimaryCommentField();
+  }
+
+  private function isPrimaryCommentField() {
+    return ($this->getKey() === 'comment');
   }
 
 }

src/applications/transactions/editfield/PhabricatorDividerEditField.php

@@ -0,0 +1,18 @@
+<?php
+
+final class PhabricatorDividerEditField
+  extends PhabricatorEditField {
+
+  protected function renderControl() {
+    return new AphrontFormDividerControl();
+  }
+
+  protected function newHTTPParameterType() {
+    return null;
+  }
+
+  protected function newConduitParameterType() {
+    return null;
+  }
+
+}

src/applications/transactions/editfield/PhabricatorSubmitEditField.php

@@ -0,0 +1,19 @@
+<?php
+
+final class PhabricatorSubmitEditField
+  extends PhabricatorEditField {
+
+  protected function renderControl() {
+    return id(new AphrontFormSubmitControl())
+      ->setValue($this->getValue());
+  }
+
+  protected function newHTTPParameterType() {
+    return null;
+  }
+
+  protected function newConduitParameterType() {
+    return null;
+  }
+
+}