Replace "Edit" controller with "EditPro" controller

Summary: Ref T2222. Remove the old controller and swap in the new ApplicationTransactions one.

Test Plan: Made a pile of edits.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2222

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

src/__phutil_library_map__.php

@@ -466,7 +466,6 @@ phutil_register_library_map(array(
     'DifferentialRevisionControlSystem' => 'applications/differential/constants/DifferentialRevisionControlSystem.php',
     'DifferentialRevisionDetailView' => 'applications/differential/view/DifferentialRevisionDetailView.php',
     'DifferentialRevisionEditController' => 'applications/differential/controller/DifferentialRevisionEditController.php',
-    'DifferentialRevisionEditControllerPro' => 'applications/differential/controller/DifferentialRevisionEditControllerPro.php',
     'DifferentialRevisionEditor' => 'applications/differential/editor/DifferentialRevisionEditor.php',
     'DifferentialRevisionIDFieldParserTestCase' => 'applications/differential/field/specification/__tests__/DifferentialRevisionIDFieldParserTestCase.php',
     'DifferentialRevisionIDFieldSpecification' => 'applications/differential/field/specification/DifferentialRevisionIDFieldSpecification.php',
@@ -3034,7 +3033,6 @@ phutil_register_library_map(array(
     ),
     'DifferentialRevisionDetailView' => 'AphrontView',
     'DifferentialRevisionEditController' => 'DifferentialController',
-    'DifferentialRevisionEditControllerPro' => 'DifferentialController',
     'DifferentialRevisionEditor' => 'PhabricatorEditor',
     'DifferentialRevisionIDFieldParserTestCase' => 'PhabricatorTestCase',
     'DifferentialRevisionIDFieldSpecification' => 'DifferentialFieldSpecification',

src/applications/differential/application/PhabricatorApplicationDifferential.php

@@ -49,8 +49,6 @@ final class PhabricatorApplicationDifferential extends PhabricatorApplication {
         'changeset/' => 'DifferentialChangesetViewController',
         'revision/edit/(?:(?P<id>[1-9]\d*)/)?'
           => 'DifferentialRevisionEditController',
-        'revision/editpro/(?:(?P<id>[1-9]\d*)/)?'
-          => 'DifferentialRevisionEditControllerPro',
         'revision/land/(?:(?P<id>[1-9]\d*))/(?P<strategy>[^/]+)/'
           => 'DifferentialRevisionLandController',
         'comment/' => array(

src/applications/differential/controller/DifferentialRevisionEditController.php

@@ -1,6 +1,7 @@
 <?php
 
-final class DifferentialRevisionEditController extends DifferentialController {
+final class DifferentialRevisionEditController
+  extends DifferentialController {
 
   private $id;
 
@@ -22,6 +23,7 @@ final class DifferentialRevisionEditController extends DifferentialController {
         ->withIDs(array($this->id))
         ->needRelationships(true)
         ->needReviewerStatus(true)
+        ->needActiveDiffs(true)
         ->requireCapabilities(
           array(
             PhabricatorPolicyCapability::CAN_VIEW,
@@ -33,10 +35,9 @@ final class DifferentialRevisionEditController extends DifferentialController {
       }
     } else {
       $revision = DifferentialRevision::initializeNewRevision($viewer);
+      $revision->attachReviewerStatus(array());
     }
 
-    $aux_fields = $this->loadAuxiliaryFields($revision);
-
     $diff_id = $request->getInt('diffID');
     if ($diff_id) {
       $diff = id(new DifferentialDiffQuery())
@@ -54,47 +55,57 @@ final class DifferentialRevisionEditController extends DifferentialController {
       $diff = null;
     }
 
-    $errors = array();
+    if (!$diff) {
+      if (!$revision->getID()) {
+        throw new Exception(
+          pht('You can not create a new revision without a diff!'));
+      }
+    } else {
+      // TODO: It would be nice to show the diff being attached in the UI.
+    }
 
+    $field_list = PhabricatorCustomField::getObjectFields(
+      $revision,
+      PhabricatorCustomField::ROLE_EDIT);
+    $field_list
+      ->setViewer($viewer)
+      ->readFieldsFromStorage($revision);
 
+    $validation_exception = null;
     if ($request->isFormPost() && !$request->getStr('viaDiffView')) {
-      foreach ($aux_fields as $aux_field) {
+      $xactions = $field_list->buildFieldTransactionsFromRequest(
-        $aux_field->setValueFromRequest($request);
+        new DifferentialTransaction(),
-        try {
+        $request);
-          $aux_field->validateField();
-        } catch (DifferentialFieldValidationException $ex) {
-          $errors[] = $ex->getMessage();
-        }
-      }
 
-      if (!$errors) {
-        $is_new = !$revision->getID();
-        $user = $request->getUser();
-
-        $editor = new DifferentialRevisionEditor($revision);
-        $editor->setActor($request->getUser());
       if ($diff) {
-          $editor->addDiff($diff, $request->getStr('comments'));
+        $xactions[] = id(new DifferentialTransaction())
+          ->setTransactionType(DifferentialTransaction::TYPE_UPDATE)
+          ->setNewValue($diff->getPHID());
       }
-        $editor->setAuxiliaryFields($aux_fields);
-        $editor->setAphrontRequestForEventDispatch($request);
-        $editor->save();
 
-        return id(new AphrontRedirectResponse())
+      $comments = $request->getStr('comments');
-          ->setURI('/D'.$revision->getID());
+      if (strlen($comments)) {
+        $xactions[] = id(new DifferentialTransaction())
+          ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
+          ->attachComment(
+            id(new DifferentialTransactionComment())
+              ->setContent($comments));
+      }
+
+      $editor = id(new DifferentialTransactionEditor())
+        ->setActor($viewer)
+        ->setContentSourceFromRequest($request)
+        ->setContinueOnNoEffect(true);
+
+      try {
+        $editor->applyTransactions($revision, $xactions);
+        $revision_uri = '/D'.$revision->getID();
+        return id(new AphrontRedirectResponse())->setURI($revision_uri);
+      } catch (PhabricatorApplicationTransactionValidationException $ex) {
+        $validation_exception = $ex;
       }
     }
 
-    $aux_phids = array();
-    foreach ($aux_fields as $key => $aux_field) {
-      $aux_phids[$key] = $aux_field->getRequiredHandlePHIDsForRevisionEdit();
-    }
-    $phids = array_mergev($aux_phids);
-    $phids = array_unique($phids);
-    $handles = $this->loadViewerHandles($phids);
-    foreach ($aux_fields as $key => $aux_field) {
-      $aux_field->setHandles(array_select_keys($handles, $aux_phids[$key]));
-    }
 
     $form = new AphrontFormView();
     $form->setUser($request->getUser());
@@ -123,14 +134,7 @@ final class DifferentialRevisionEditController extends DifferentialController {
           id(new AphrontFormDividerControl()));
     }
 
-    $preview = array();
+    $field_list->appendFieldsToForm($form);
-    foreach ($aux_fields as $aux_field) {
-      $control = $aux_field->renderEditControl();
-      if ($control) {
-        $form->appendChild($control);
-      }
-      $preview[] = $aux_field->renderEditPreview();
-    }
 
     $submit = id(new AphrontFormSubmitControl())
       ->setValue('Save');
@@ -161,7 +165,7 @@ final class DifferentialRevisionEditController extends DifferentialController {
 
     $form_box = id(new PHUIObjectBoxView())
       ->setHeaderText($title)
-      ->setFormErrors($errors)
+      ->setValidationException($validation_exception)
       ->setForm($form);
 
     $crumbs->addTextCrumb($title);
@@ -170,31 +174,11 @@ final class DifferentialRevisionEditController extends DifferentialController {
       array(
         $crumbs,
         $form_box,
-        $preview),
+      ),
       array(
         'title' => $title,
         'device' => true,
       ));
   }
 
-  private function loadAuxiliaryFields(DifferentialRevision $revision) {
-
-    $user = $this->getRequest()->getUser();
-
-    $aux_fields = DifferentialFieldSelector::newSelector()
-      ->getFieldSpecifications();
-    foreach ($aux_fields as $key => $aux_field) {
-      $aux_field->setRevision($revision);
-      if (!$aux_field->shouldAppearOnEdit()) {
-        unset($aux_fields[$key]);
-      } else {
-        $aux_field->setUser($user);
-      }
-    }
-
-    return DifferentialAuxiliaryField::loadFromStorage(
-      $revision,
-      $aux_fields);
-  }
-
 }

src/applications/differential/controller/DifferentialRevisionEditControllerPro.php

@@ -1,185 +0,0 @@
-<?php
-
-final class DifferentialRevisionEditControllerPro
-  extends DifferentialController {
-
-  private $id;
-
-  public function willProcessRequest(array $data) {
-    $this->id = idx($data, 'id');
-  }
-
-  public function processRequest() {
-    $request = $this->getRequest();
-    $viewer = $request->getUser();
-
-    if (!$this->id) {
-      $this->id = $request->getInt('revisionID');
-    }
-
-    if ($this->id) {
-      $revision = id(new DifferentialRevisionQuery())
-        ->setViewer($viewer)
-        ->withIDs(array($this->id))
-        ->needRelationships(true)
-        ->needReviewerStatus(true)
-        ->needActiveDiffs(true)
-        ->requireCapabilities(
-          array(
-            PhabricatorPolicyCapability::CAN_VIEW,
-            PhabricatorPolicyCapability::CAN_EDIT,
-          ))
-        ->executeOne();
-      if (!$revision) {
-        return new Aphront404Response();
-      }
-    } else {
-      $revision = DifferentialRevision::initializeNewRevision($viewer);
-      $revision->attachReviewerStatus(array());
-    }
-
-    $diff_id = $request->getInt('diffID');
-    if ($diff_id) {
-      $diff = id(new DifferentialDiffQuery())
-        ->setViewer($viewer)
-        ->withIDs(array($diff_id))
-        ->executeOne();
-      if (!$diff) {
-        return new Aphront404Response();
-      }
-      if ($diff->getRevisionID()) {
-        // TODO: Redirect?
-        throw new Exception("This diff is already attached to a revision!");
-      }
-    } else {
-      $diff = null;
-    }
-
-    if (!$diff) {
-      if (!$revision->getID()) {
-        throw new Exception(
-          pht('You can not create a new revision without a diff!'));
-      }
-    } else {
-      // TODO: It would be nice to show the diff being attached in the UI.
-    }
-
-    $field_list = PhabricatorCustomField::getObjectFields(
-      $revision,
-      PhabricatorCustomField::ROLE_EDIT);
-    $field_list
-      ->setViewer($viewer)
-      ->readFieldsFromStorage($revision);
-
-    $validation_exception = null;
-    if ($request->isFormPost() && !$request->getStr('viaDiffView')) {
-      $xactions = $field_list->buildFieldTransactionsFromRequest(
-        new DifferentialTransaction(),
-        $request);
-
-      if ($diff) {
-        $xactions[] = id(new DifferentialTransaction())
-          ->setTransactionType(DifferentialTransaction::TYPE_UPDATE)
-          ->setNewValue($diff->getPHID());
-      }
-
-      $comments = $request->getStr('comments');
-      if (strlen($comments)) {
-        $xactions[] = id(new DifferentialTransaction())
-          ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
-          ->attachComment(
-            id(new DifferentialTransactionComment())
-              ->setContent($comments));
-      }
-
-      $editor = id(new DifferentialTransactionEditor())
-        ->setActor($viewer)
-        ->setContentSourceFromRequest($request)
-        ->setContinueOnNoEffect(true);
-
-      try {
-        $editor->applyTransactions($revision, $xactions);
-        $revision_uri = '/D'.$revision->getID();
-        return id(new AphrontRedirectResponse())->setURI($revision_uri);
-      } catch (PhabricatorApplicationTransactionValidationException $ex) {
-        $validation_exception = $ex;
-      }
-    }
-
-
-    $form = new AphrontFormView();
-    $form->setUser($request->getUser());
-    if ($diff) {
-      $form->addHiddenInput('diffID', $diff->getID());
-    }
-
-    if ($revision->getID()) {
-      $form->setAction(
-        '/differential/revision/editpro/'.$revision->getID().'/');
-    } else {
-      $form->setAction('/differential/revision/editpro/');
-    }
-
-    if ($diff && $revision->getID()) {
-      $form
-        ->appendChild(
-          id(new AphrontFormTextAreaControl())
-            ->setLabel(pht('Comments'))
-            ->setName('comments')
-            ->setCaption(pht("Explain what's new in this diff."))
-            ->setValue($request->getStr('comments')))
-        ->appendChild(
-          id(new AphrontFormSubmitControl())
-            ->setValue(pht('Save')))
-        ->appendChild(
-          id(new AphrontFormDividerControl()));
-    }
-
-    $field_list->appendFieldsToForm($form);
-
-    $submit = id(new AphrontFormSubmitControl())
-      ->setValue('Save');
-    if ($diff) {
-      $submit->addCancelButton('/differential/diff/'.$diff->getID().'/');
-    } else {
-      $submit->addCancelButton('/D'.$revision->getID());
-    }
-
-    $form->appendChild($submit);
-
-    $crumbs = $this->buildApplicationCrumbs();
-    if ($revision->getID()) {
-      if ($diff) {
-        $title = pht('Update Differential Revision');
-        $crumbs->addTextCrumb(
-          'D'.$revision->getID(),
-          '/differential/diff/'.$diff->getID().'/');
-      } else {
-        $title = pht('Edit Differential Revision');
-        $crumbs->addTextCrumb(
-          'D'.$revision->getID(),
-          '/D'.$revision->getID());
-      }
-    } else {
-      $title = pht('Create New Differential Revision');
-    }
-
-    $form_box = id(new PHUIObjectBoxView())
-      ->setHeaderText($title)
-      ->setValidationException($validation_exception)
-      ->setForm($form);
-
-    $crumbs->addTextCrumb($title);
-
-    return $this->buildApplicationPage(
-      array(
-        $crumbs,
-        $form_box,
-      ),
-      array(
-        'title' => $title,
-        'device' => true,
-      ));
-  }
-
-}