diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -517,7 +517,6 @@ 'DifferentialRevisionControlSystem' => 'applications/differential/constants/DifferentialRevisionControlSystem.php', 'DifferentialRevisionDependedOnByRevisionEdgeType' => 'applications/differential/edge/DifferentialRevisionDependedOnByRevisionEdgeType.php', 'DifferentialRevisionDependsOnRevisionEdgeType' => 'applications/differential/edge/DifferentialRevisionDependsOnRevisionEdgeType.php', - 'DifferentialRevisionEditController' => 'applications/differential/controller/DifferentialRevisionEditController.php', 'DifferentialRevisionEditEngine' => 'applications/differential/editor/DifferentialRevisionEditEngine.php', 'DifferentialRevisionEditProController' => 'applications/differential/controller/DifferentialRevisionEditProController.php', 'DifferentialRevisionFulltextEngine' => 'applications/differential/search/DifferentialRevisionFulltextEngine.php', @@ -5173,7 +5172,6 @@ 'DifferentialRevisionControlSystem' => 'Phobject', 'DifferentialRevisionDependedOnByRevisionEdgeType' => 'PhabricatorEdgeType', 'DifferentialRevisionDependsOnRevisionEdgeType' => 'PhabricatorEdgeType', - 'DifferentialRevisionEditController' => 'DifferentialController', 'DifferentialRevisionEditEngine' => 'PhabricatorEditEngine', 'DifferentialRevisionEditProController' => 'DifferentialController', 'DifferentialRevisionFulltextEngine' => 'PhabricatorFulltextEngine', diff --git a/src/applications/differential/application/PhabricatorDifferentialApplication.php b/src/applications/differential/application/PhabricatorDifferentialApplication.php --- a/src/applications/differential/application/PhabricatorDifferentialApplication.php +++ b/src/applications/differential/application/PhabricatorDifferentialApplication.php @@ -65,8 +65,6 @@ ), 'changeset/' => 'DifferentialChangesetViewController', 'revision/' => array( - 'edit/(?:(?P[1-9]\d*)/)?' - => 'DifferentialRevisionEditController', $this->getEditRoutePattern('editpro/') => 'DifferentialRevisionEditProController', $this->getEditRoutePattern('attach/(?P[^/]+)/to/') diff --git a/src/applications/differential/controller/DifferentialDiffViewController.php b/src/applications/differential/controller/DifferentialDiffViewController.php --- a/src/applications/differential/controller/DifferentialDiffViewController.php +++ b/src/applications/differential/controller/DifferentialDiffViewController.php @@ -23,6 +23,20 @@ ->setURI('/D'.$diff->getRevisionID().'?id='.$diff->getID()); } + if ($request->isFormPost()) { + $diff_id = $diff->getID(); + $revision_id = $request->getInt('revisionID'); + if ($revision_id) { + $attach_uri = "/revision/attach/{$diff_id}/to/{$revision_id}/"; + } else { + $attach_uri = "/revision/attach/{$diff_id}/to/"; + } + $attach_uri = $this->getApplicationURI($attach_uri); + + return id(new AphrontRedirectResponse()) + ->setURI($attach_uri); + } + $diff_phid = $diff->getPHID(); $buildables = id(new HarbormasterBuildableQuery()) ->setViewer($viewer) @@ -78,13 +92,7 @@ $select); $form = id(new AphrontFormView()) - ->setUser($request->getUser()) - ->setAction('/differential/revision/edit/') - ->addHiddenInput('diffID', $diff->getID()) - ->addHiddenInput('viaDiffView', 1) - ->addHiddenInput( - id(new DifferentialRepositoryField())->getFieldKey(), - $diff->getRepositoryPHID()) + ->setViewer($viewer) ->appendRemarkupInstructions( pht( 'Review the diff for correctness. When you are satisfied, either '. @@ -98,7 +106,7 @@ ->setValue(pht('Continue'))); $props = id(new DifferentialDiffProperty())->loadAllWhere( - 'diffID = %d', + 'diffID = %d', $diff->getID()); $props = mpull($props, 'getData', 'getName'); diff --git a/src/applications/differential/controller/DifferentialRevisionEditController.php b/src/applications/differential/controller/DifferentialRevisionEditController.php deleted file mode 100644 --- a/src/applications/differential/controller/DifferentialRevisionEditController.php +++ /dev/null @@ -1,214 +0,0 @@ -getViewer(); - $id = $request->getURIData('id'); - - if (!$id) { - $id = $request->getInt('revisionID'); - } - - if ($id) { - $revision = id(new DifferentialRevisionQuery()) - ->setViewer($viewer) - ->withIDs(array($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( - pht('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); - - if ($request->getStr('viaDiffView') && $diff) { - $repo_key = id(new DifferentialRepositoryField())->getFieldKey(); - $repository_field = idx( - $field_list->getFields(), - $repo_key); - if ($repository_field) { - $repository_field->setValue($request->getStr($repo_key)); - } - $view_policy_key = id(new DifferentialViewPolicyField())->getFieldKey(); - $view_policy_field = idx( - $field_list->getFields(), - $view_policy_key); - if ($view_policy_field) { - $view_policy_field->setValue($diff->getViewPolicy()); - } - } - - $validation_exception = null; - if ($request->isFormPost() && !$request->getStr('viaDiffView')) { - - $editor = id(new DifferentialTransactionEditor()) - ->setActor($viewer) - ->setContentSourceFromRequest($request) - ->setContinueOnNoEffect(true); - - $xactions = $field_list->buildFieldTransactionsFromRequest( - new DifferentialTransaction(), - $request); - - if ($diff) { - $repository_phid = null; - $repository_tokenizer = $request->getArr( - id(new DifferentialRepositoryField())->getFieldKey()); - if ($repository_tokenizer) { - $repository_phid = reset($repository_tokenizer); - } - - $xactions[] = id(new DifferentialTransaction()) - ->setTransactionType(DifferentialTransaction::TYPE_UPDATE) - ->setNewValue($diff->getPHID()); - - $editor->setRepositoryPHIDOverride($repository_phid); - } - - $comments = $request->getStr('comments'); - if (strlen($comments)) { - $xactions[] = id(new DifferentialTransaction()) - ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) - ->attachComment( - id(new DifferentialTransactionComment()) - ->setContent($comments)); - } - - 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/edit/'.$revision->getID().'/'); - } else { - $form->setAction('/differential/revision/edit/'); - } - - 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) { - $header_icon = 'fa-upload'; - $title = pht('Update Revision'); - $crumbs->addTextCrumb( - 'D'.$revision->getID(), - '/differential/diff/'.$diff->getID().'/'); - } else { - $header_icon = 'fa-pencil'; - $title = pht('Edit Revision: %s', $revision->getTitle()); - $crumbs->addTextCrumb( - 'D'.$revision->getID(), - '/D'.$revision->getID()); - } - } else { - $header_icon = 'fa-plus-square'; - $title = pht('Create New Differential Revision'); - } - - $form_box = id(new PHUIObjectBoxView()) - ->setHeaderText('Revision') - ->setValidationException($validation_exception) - ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) - ->setForm($form); - - $crumbs->addTextCrumb($title); - $crumbs->setBorder(true); - - $header = id(new PHUIHeaderView()) - ->setHeader($title) - ->setHeaderIcon($header_icon); - - $view = id(new PHUITwoColumnView()) - ->setHeader($header) - ->setFooter($form_box); - - return $this->newPage() - ->setTitle($title) - ->setCrumbs($crumbs) - ->appendChild($view); - } - -} diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -570,7 +570,7 @@ $curtain->addAction( id(new PhabricatorActionView()) ->setIcon('fa-pencil') - ->setHref("/differential/revision/edit/{$revision_id}/") + ->setHref("/differential/revision/editpro/{$revision_id}/") ->setName(pht('Edit Revision')) ->setDisabled(!$can_edit) ->setWorkflow(!$can_edit)); diff --git a/src/applications/differential/customfield/DifferentialCoreCustomField.php b/src/applications/differential/customfield/DifferentialCoreCustomField.php --- a/src/applications/differential/customfield/DifferentialCoreCustomField.php +++ b/src/applications/differential/customfield/DifferentialCoreCustomField.php @@ -118,10 +118,6 @@ return true; } - public function shouldAppearInEditView() { - return true; - } - public function readValueFromObject(PhabricatorCustomFieldInterface $object) { if ($this->isCoreFieldRequired()) { $this->setFieldError(true); diff --git a/src/applications/differential/customfield/DifferentialManiphestTasksField.php b/src/applications/differential/customfield/DifferentialManiphestTasksField.php --- a/src/applications/differential/customfield/DifferentialManiphestTasksField.php +++ b/src/applications/differential/customfield/DifferentialManiphestTasksField.php @@ -15,10 +15,6 @@ return false; } - public function shouldAppearInEditView() { - return false; - } - public function getFieldName() { return pht('Maniphest Tasks'); } diff --git a/src/applications/differential/customfield/DifferentialProjectsField.php b/src/applications/differential/customfield/DifferentialProjectsField.php --- a/src/applications/differential/customfield/DifferentialProjectsField.php +++ b/src/applications/differential/customfield/DifferentialProjectsField.php @@ -19,10 +19,6 @@ return false; } - public function shouldAppearInEditView() { - return true; - } - public function shouldAppearInApplicationTransactions() { return true; } diff --git a/src/applications/differential/customfield/DifferentialRequiredSignaturesField.php b/src/applications/differential/customfield/DifferentialRequiredSignaturesField.php --- a/src/applications/differential/customfield/DifferentialRequiredSignaturesField.php +++ b/src/applications/differential/customfield/DifferentialRequiredSignaturesField.php @@ -19,10 +19,6 @@ return true; } - public function shouldAppearInEditView() { - return false; - } - protected function readValueFromRevision(DifferentialRevision $revision) { return self::loadForRevision($revision); } diff --git a/src/applications/differential/customfield/DifferentialReviewedByField.php b/src/applications/differential/customfield/DifferentialReviewedByField.php --- a/src/applications/differential/customfield/DifferentialReviewedByField.php +++ b/src/applications/differential/customfield/DifferentialReviewedByField.php @@ -23,10 +23,6 @@ return false; } - public function shouldAppearInEditView() { - return false; - } - public function canDisableField() { return true; } diff --git a/src/applications/differential/editor/DifferentialRevisionEditEngine.php b/src/applications/differential/editor/DifferentialRevisionEditEngine.php --- a/src/applications/differential/editor/DifferentialRevisionEditEngine.php +++ b/src/applications/differential/editor/DifferentialRevisionEditEngine.php @@ -95,23 +95,26 @@ $diff_phid = null; } - $is_update = ($diff && $object->getID()); + $is_create = $this->getIsCreate(); + $is_update = ($diff && !$is_create); $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 ($diff || $is_create) { + $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()) @@ -194,7 +197,7 @@ private function isCustomFieldEnabled(DifferentialRevision $revision, $key) { $field_list = PhabricatorCustomField::getObjectFields( $revision, - PhabricatorCustomField::ROLE_EDIT); + PhabricatorCustomField::ROLE_VIEW); $fields = $field_list->getFields(); return isset($fields[$key]); diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -70,6 +70,7 @@ ->setAuthorPHID($actor->getPHID()) ->attachRelationships(array()) ->attachRepository(null) + ->attachReviewerStatus(array()) ->setStatus(ArcanistDifferentialRevisionStatus::NEEDS_REVIEW); } diff --git a/src/infrastructure/customfield/editor/PhabricatorCustomFieldEditField.php b/src/infrastructure/customfield/editor/PhabricatorCustomFieldEditField.php --- a/src/infrastructure/customfield/editor/PhabricatorCustomFieldEditField.php +++ b/src/infrastructure/customfield/editor/PhabricatorCustomFieldEditField.php @@ -47,15 +47,14 @@ } protected function newEditType() { + $type = id(new PhabricatorCustomFieldEditType()) + ->setCustomField($this->getCustomField()); + $conduit_type = $this->newConduitParameterType(); - if (!$conduit_type) { - return null; + if ($conduit_type) { + $type->setConduitParameterType($conduit_type); } - $type = id(new PhabricatorCustomFieldEditType()) - ->setCustomField($this->getCustomField()) - ->setConduitParameterType($conduit_type); - return $type; } diff --git a/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldEditEngineExtension.php b/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldEditEngineExtension.php --- a/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldEditEngineExtension.php +++ b/src/infrastructure/customfield/engineextension/PhabricatorCustomFieldEditEngineExtension.php @@ -27,12 +27,6 @@ PhabricatorEditEngine $engine, PhabricatorApplicationTransactionInterface $object) { - // TODO: Remove this hack once Differential modernizes more fully. Today, - // its custom fields are too custom to interact cleanly with EditEngine. - if ($object instanceof DifferentialRevision) { - return array(); - } - $viewer = $this->getViewer(); $field_list = PhabricatorCustomField::getObjectFields(