From b383d2c338fac3b7d699c23b8a99ef3aa9529723 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 28 Feb 2014 16:49:30 -0800 Subject: [PATCH] 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 | 2 - .../PhabricatorApplicationDifferential.php | 2 - .../DifferentialRevisionEditController.php | 112 +++++------ .../DifferentialRevisionEditControllerPro.php | 185 ------------------ 4 files changed, 48 insertions(+), 253 deletions(-) delete mode 100644 src/applications/differential/controller/DifferentialRevisionEditControllerPro.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 470641551e..ea745663b1 100644 --- a/src/__phutil_library_map__.php +++ b/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', diff --git a/src/applications/differential/application/PhabricatorApplicationDifferential.php b/src/applications/differential/application/PhabricatorApplicationDifferential.php index 5e05074596..833f2f71e9 100644 --- a/src/applications/differential/application/PhabricatorApplicationDifferential.php +++ b/src/applications/differential/application/PhabricatorApplicationDifferential.php @@ -49,8 +49,6 @@ final class PhabricatorApplicationDifferential extends PhabricatorApplication { 'changeset/' => 'DifferentialChangesetViewController', 'revision/edit/(?:(?P[1-9]\d*)/)?' => 'DifferentialRevisionEditController', - 'revision/editpro/(?:(?P[1-9]\d*)/)?' - => 'DifferentialRevisionEditControllerPro', 'revision/land/(?:(?P[1-9]\d*))/(?P[^/]+)/' => 'DifferentialRevisionLandController', 'comment/' => array( diff --git a/src/applications/differential/controller/DifferentialRevisionEditController.php b/src/applications/differential/controller/DifferentialRevisionEditController.php index a070dea9c9..e88fe947eb 100644 --- a/src/applications/differential/controller/DifferentialRevisionEditController.php +++ b/src/applications/differential/controller/DifferentialRevisionEditController.php @@ -1,6 +1,7 @@ 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) { - $aux_field->setValueFromRequest($request); - try { - $aux_field->validateField(); - } catch (DifferentialFieldValidationException $ex) { - $errors[] = $ex->getMessage(); - } + $xactions = $field_list->buildFieldTransactionsFromRequest( + new DifferentialTransaction(), + $request); + + if ($diff) { + $xactions[] = id(new DifferentialTransaction()) + ->setTransactionType(DifferentialTransaction::TYPE_UPDATE) + ->setNewValue($diff->getPHID()); } - if (!$errors) { - $is_new = !$revision->getID(); - $user = $request->getUser(); + $comments = $request->getStr('comments'); + if (strlen($comments)) { + $xactions[] = id(new DifferentialTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) + ->attachComment( + id(new DifferentialTransactionComment()) + ->setContent($comments)); + } - $editor = new DifferentialRevisionEditor($revision); - $editor->setActor($request->getUser()); - if ($diff) { - $editor->addDiff($diff, $request->getStr('comments')); - } - $editor->setAuxiliaryFields($aux_fields); - $editor->setAphrontRequestForEventDispatch($request); - $editor->save(); + $editor = id(new DifferentialTransactionEditor()) + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->setContinueOnNoEffect(true); - return id(new AphrontRedirectResponse()) - ->setURI('/D'.$revision->getID()); + 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(); - foreach ($aux_fields as $aux_field) { - $control = $aux_field->renderEditControl(); - if ($control) { - $form->appendChild($control); - } - $preview[] = $aux_field->renderEditPreview(); - } + $field_list->appendFieldsToForm($form); $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); - } - } diff --git a/src/applications/differential/controller/DifferentialRevisionEditControllerPro.php b/src/applications/differential/controller/DifferentialRevisionEditControllerPro.php deleted file mode 100644 index 41a6a50d4a..0000000000 --- a/src/applications/differential/controller/DifferentialRevisionEditControllerPro.php +++ /dev/null @@ -1,185 +0,0 @@ -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, - )); - } - -}