From 1bdf9885562e85982e8ad78612b82139db58934d Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 3 Mar 2016 04:34:47 -0800 Subject: [PATCH] Convert DrydockBlueprints to EditEngine Summary: Ref T10457. Fixes T10024. This primarily just modernizes blueprints to use EditEngine. This also fixes T10024, which was an issue with stored properties not being flagged correctly. Also slightly improves typeaheads for blueprints (more information, disabled state). Test Plan: - Created and edited various types of blueprints. - Set and removed limits. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10024, T10457 Differential Revision: https://secure.phabricator.com/D15390 --- src/__phutil_library_map__.php | 6 +- .../PhabricatorDrydockApplication.php | 4 +- .../controller/DrydockBlueprintController.php | 16 +- .../DrydockBlueprintCreateController.php | 79 -------- .../DrydockBlueprintEditController.php | 191 +++++------------- .../DrydockBlueprintListController.php | 15 +- .../DrydockBlueprintCoreCustomField.php | 1 + .../editor/DrydockBlueprintEditEngine.php | 115 +++++++++++ .../drydock/editor/DrydockBlueprintEditor.php | 32 +++ .../typeahead/DrydockBlueprintDatasource.php | 15 +- .../editfield/PhabricatorStaticEditField.php | 18 ++ ...abricatorStandardCustomFieldCredential.php | 12 ++ ...habricatorStandardCustomFieldTokenizer.php | 12 ++ 13 files changed, 274 insertions(+), 242 deletions(-) delete mode 100644 src/applications/drydock/controller/DrydockBlueprintCreateController.php create mode 100644 src/applications/drydock/editor/DrydockBlueprintEditEngine.php create mode 100644 src/applications/transactions/editfield/PhabricatorStaticEditField.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 5750dee24e..73d24c3d7d 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -867,11 +867,11 @@ phutil_register_library_map(array( 'DrydockBlueprint' => 'applications/drydock/storage/DrydockBlueprint.php', 'DrydockBlueprintController' => 'applications/drydock/controller/DrydockBlueprintController.php', 'DrydockBlueprintCoreCustomField' => 'applications/drydock/customfield/DrydockBlueprintCoreCustomField.php', - 'DrydockBlueprintCreateController' => 'applications/drydock/controller/DrydockBlueprintCreateController.php', 'DrydockBlueprintCustomField' => 'applications/drydock/customfield/DrydockBlueprintCustomField.php', 'DrydockBlueprintDatasource' => 'applications/drydock/typeahead/DrydockBlueprintDatasource.php', 'DrydockBlueprintDisableController' => 'applications/drydock/controller/DrydockBlueprintDisableController.php', 'DrydockBlueprintEditController' => 'applications/drydock/controller/DrydockBlueprintEditController.php', + 'DrydockBlueprintEditEngine' => 'applications/drydock/editor/DrydockBlueprintEditEngine.php', 'DrydockBlueprintEditor' => 'applications/drydock/editor/DrydockBlueprintEditor.php', 'DrydockBlueprintImplementation' => 'applications/drydock/blueprint/DrydockBlueprintImplementation.php', 'DrydockBlueprintImplementationTestCase' => 'applications/drydock/blueprint/__tests__/DrydockBlueprintImplementationTestCase.php', @@ -3287,6 +3287,7 @@ phutil_register_library_map(array( 'PhabricatorStandardCustomFieldUsers' => 'infrastructure/customfield/standard/PhabricatorStandardCustomFieldUsers.php', 'PhabricatorStandardPageView' => 'view/page/PhabricatorStandardPageView.php', 'PhabricatorStandardSelectCustomFieldDatasource' => 'infrastructure/customfield/datasource/PhabricatorStandardSelectCustomFieldDatasource.php', + 'PhabricatorStaticEditField' => 'applications/transactions/editfield/PhabricatorStaticEditField.php', 'PhabricatorStatusController' => 'applications/system/controller/PhabricatorStatusController.php', 'PhabricatorStatusUIExample' => 'applications/uiexample/examples/PhabricatorStatusUIExample.php', 'PhabricatorStorageFixtureScopeGuard' => 'infrastructure/testing/fixture/PhabricatorStorageFixtureScopeGuard.php', @@ -4980,11 +4981,11 @@ phutil_register_library_map(array( 'DrydockBlueprintCustomField', 'PhabricatorStandardCustomFieldInterface', ), - 'DrydockBlueprintCreateController' => 'DrydockBlueprintController', 'DrydockBlueprintCustomField' => 'PhabricatorCustomField', 'DrydockBlueprintDatasource' => 'PhabricatorTypeaheadDatasource', 'DrydockBlueprintDisableController' => 'DrydockBlueprintController', 'DrydockBlueprintEditController' => 'DrydockBlueprintController', + 'DrydockBlueprintEditEngine' => 'PhabricatorEditEngine', 'DrydockBlueprintEditor' => 'PhabricatorApplicationTransactionEditor', 'DrydockBlueprintImplementation' => 'Phobject', 'DrydockBlueprintImplementationTestCase' => 'PhabricatorTestCase', @@ -7834,6 +7835,7 @@ phutil_register_library_map(array( 'AphrontResponseProducerInterface', ), 'PhabricatorStandardSelectCustomFieldDatasource' => 'PhabricatorTypeaheadDatasource', + 'PhabricatorStaticEditField' => 'PhabricatorEditField', 'PhabricatorStatusController' => 'PhabricatorController', 'PhabricatorStatusUIExample' => 'PhabricatorUIExample', 'PhabricatorStorageFixtureScopeGuard' => 'Phobject', diff --git a/src/applications/drydock/application/PhabricatorDrydockApplication.php b/src/applications/drydock/application/PhabricatorDrydockApplication.php index d19e7cb78d..cb229e3410 100644 --- a/src/applications/drydock/application/PhabricatorDrydockApplication.php +++ b/src/applications/drydock/application/PhabricatorDrydockApplication.php @@ -60,8 +60,8 @@ final class PhabricatorDrydockApplication extends PhabricatorApplication { 'authorizations/(?:query/(?P[^/]+)/)?' => 'DrydockAuthorizationListController', ), - 'create/' => 'DrydockBlueprintCreateController', - 'edit/(?:(?P[1-9]\d*)/)?' => 'DrydockBlueprintEditController', + $this->getEditRoutePattern('edit/') + => 'DrydockBlueprintEditController', ), '(?Presource)/' => array( '(?:query/(?P[^/]+)/)?' => 'DrydockResourceListController', diff --git a/src/applications/drydock/controller/DrydockBlueprintController.php b/src/applications/drydock/controller/DrydockBlueprintController.php index b995192eb4..c251871426 100644 --- a/src/applications/drydock/controller/DrydockBlueprintController.php +++ b/src/applications/drydock/controller/DrydockBlueprintController.php @@ -3,24 +3,18 @@ abstract class DrydockBlueprintController extends DrydockController { - public function buildSideNavView() { - $nav = new AphrontSideNavFilterView(); - $nav->setBaseURI(new PhutilURI($this->getApplicationURI())); - - id(new DrydockBlueprintSearchEngine()) - ->setViewer($this->getRequest()->getUser()) - ->addNavigationItems($nav->getMenu()); - - $nav->selectFilter(null); - - return $nav; + public function buildApplicationMenu() { + return $this->newApplicationMenu() + ->setSearchEngine(new DrydockBlueprintSearchEngine()); } protected function buildApplicationCrumbs() { $crumbs = parent::buildApplicationCrumbs(); + $crumbs->addTextCrumb( pht('Blueprints'), $this->getApplicationURI('blueprint/')); + return $crumbs; } diff --git a/src/applications/drydock/controller/DrydockBlueprintCreateController.php b/src/applications/drydock/controller/DrydockBlueprintCreateController.php deleted file mode 100644 index 5398a2327d..0000000000 --- a/src/applications/drydock/controller/DrydockBlueprintCreateController.php +++ /dev/null @@ -1,79 +0,0 @@ -getViewer(); - - $this->requireApplicationCapability( - DrydockCreateBlueprintsCapability::CAPABILITY); - - $implementations = - DrydockBlueprintImplementation::getAllBlueprintImplementations(); - - $errors = array(); - $e_blueprint = null; - - if ($request->isFormPost()) { - $class = $request->getStr('blueprint-type'); - if (!isset($implementations[$class])) { - $e_blueprint = pht('Required'); - $errors[] = pht('You must choose a blueprint type.'); - } - - if (!$errors) { - $edit_uri = $this->getApplicationURI('blueprint/edit/?class='.$class); - return id(new AphrontRedirectResponse())->setURI($edit_uri); - } - } - - $control = id(new AphrontFormRadioButtonControl()) - ->setName('blueprint-type') - ->setLabel(pht('Blueprint Type')) - ->setError($e_blueprint); - - foreach ($implementations as $implementation_name => $implementation) { - $disabled = !$implementation->isEnabled(); - - $control->addButton( - $implementation_name, - $implementation->getBlueprintName(), - array( - pht('Provides: %s', $implementation->getType()), - phutil_tag('br'), - phutil_tag('br'), - $implementation->getDescription(), - ), - $disabled ? 'disabled' : null, - $disabled); - } - - $title = pht('Create New Blueprint'); - $crumbs = $this->buildApplicationCrumbs(); - $crumbs->addTextCrumb(pht('New Blueprint')); - - $form = id(new AphrontFormView()) - ->setUser($viewer) - ->appendChild($control) - ->appendChild( - id(new AphrontFormSubmitControl()) - ->addCancelButton($this->getApplicationURI('blueprint/')) - ->setValue(pht('Continue'))); - - $box = id(new PHUIObjectBoxView()) - ->setFormErrors($errors) - ->setHeaderText($title) - ->setForm($form); - - return $this->buildApplicationPage( - array( - $crumbs, - $box, - ), - array( - 'title' => $title, - )); - } - -} diff --git a/src/applications/drydock/controller/DrydockBlueprintEditController.php b/src/applications/drydock/controller/DrydockBlueprintEditController.php index 2e61ec868c..fbda9d0d29 100644 --- a/src/applications/drydock/controller/DrydockBlueprintEditController.php +++ b/src/applications/drydock/controller/DrydockBlueprintEditController.php @@ -3,168 +3,89 @@ final class DrydockBlueprintEditController extends DrydockBlueprintController { public function handleRequest(AphrontRequest $request) { - $viewer = $request->getViewer(); - $id = $request->getURIData('id'); - - if ($id) { - $blueprint = id(new DrydockBlueprintQuery()) - ->setViewer($viewer) - ->withIDs(array($id)) - ->requireCapabilities( - array( - PhabricatorPolicyCapability::CAN_VIEW, - PhabricatorPolicyCapability::CAN_EDIT, - )) - ->executeOne(); - if (!$blueprint) { - return new Aphront404Response(); - } + $engine = id(new DrydockBlueprintEditEngine()) + ->setController($this); - $impl = $blueprint->getImplementation(); - $cancel_uri = $this->getApplicationURI('blueprint/'.$id.'/'); - } else { + $id = $request->getURIData('id'); + if (!$id) { $this->requireApplicationCapability( DrydockCreateBlueprintsCapability::CAPABILITY); - $class = $request->getStr('class'); + $type = $request->getStr('blueprintType'); - $impl = DrydockBlueprintImplementation::getNamedImplementation($class); + $impl = DrydockBlueprintImplementation::getNamedImplementation($type); if (!$impl || !$impl->isEnabled()) { - return new Aphront400Response(); + return $this->buildTypeSelectionResponse(); } - $blueprint = DrydockBlueprint::initializeNewBlueprint($viewer) - ->setClassName($class) - ->attachImplementation($impl); - - $cancel_uri = $this->getApplicationURI('blueprint/'); + $engine + ->addContextParameter('blueprintType', $type) + ->setBlueprintImplementation($impl); } - $field_list = PhabricatorCustomField::getObjectFields( - $blueprint, - PhabricatorCustomField::ROLE_EDIT); - $field_list - ->setViewer($viewer) - ->readFieldsFromStorage($blueprint); + return $engine->buildResponse(); + } + + private function buildTypeSelectionResponse() { + $request = $this->getRequest(); + $viewer = $this->getViewer(); + + $implementations = + DrydockBlueprintImplementation::getAllBlueprintImplementations(); - $v_name = $blueprint->getBlueprintName(); - $e_name = true; $errors = array(); - $validation_exception = null; + $e_blueprint = null; if ($request->isFormPost()) { - $v_view_policy = $request->getStr('viewPolicy'); - $v_edit_policy = $request->getStr('editPolicy'); - $v_name = $request->getStr('name'); - if (!strlen($v_name)) { - $e_name = pht('Required'); - $errors[] = pht('You must name this blueprint.'); + $class = $request->getStr('blueprintType'); + if (!isset($implementations[$class])) { + $e_blueprint = pht('Required'); + $errors[] = pht('You must choose a blueprint type.'); } + } - if (!$errors) { - $xactions = array(); - - $xactions = $field_list->buildFieldTransactionsFromRequest( - new DrydockBlueprintTransaction(), - $request); - - $xactions[] = id(new DrydockBlueprintTransaction()) - ->setTransactionType(PhabricatorTransactions::TYPE_VIEW_POLICY) - ->setNewValue($v_view_policy); - - $xactions[] = id(new DrydockBlueprintTransaction()) - ->setTransactionType(PhabricatorTransactions::TYPE_EDIT_POLICY) - ->setNewValue($v_edit_policy); - - $xactions[] = id(new DrydockBlueprintTransaction()) - ->setTransactionType(DrydockBlueprintTransaction::TYPE_NAME) - ->setNewValue($v_name); - - $editor = id(new DrydockBlueprintEditor()) - ->setActor($viewer) - ->setContentSourceFromRequest($request) - ->setContinueOnNoEffect(true); - - try { - $editor->applyTransactions($blueprint, $xactions); - - $id = $blueprint->getID(); - $save_uri = $this->getApplicationURI("blueprint/{$id}/"); - - return id(new AphrontRedirectResponse())->setURI($save_uri); - } catch (PhabricatorApplicationTransactionValidationException $ex) { - $validation_exception = $ex; - } - } + $control = id(new AphrontFormRadioButtonControl()) + ->setName('blueprintType') + ->setLabel(pht('Blueprint Type')) + ->setError($e_blueprint); + + foreach ($implementations as $implementation_name => $implementation) { + $disabled = !$implementation->isEnabled(); + + $control->addButton( + $implementation_name, + $implementation->getBlueprintName(), + array( + pht('Provides: %s', $implementation->getType()), + phutil_tag('br'), + phutil_tag('br'), + $implementation->getDescription(), + ), + $disabled ? 'disabled' : null, + $disabled); } - $policies = id(new PhabricatorPolicyQuery()) - ->setViewer($viewer) - ->setObject($blueprint) - ->execute(); + $title = pht('Create New Blueprint'); + $crumbs = $this->buildApplicationCrumbs(); + $crumbs->addTextCrumb(pht('New Blueprint')); $form = id(new AphrontFormView()) ->setUser($viewer) - ->addHiddenInput('class', $request->getStr('class')) - ->appendChild( - id(new AphrontFormTextControl()) - ->setLabel(pht('Name')) - ->setName('name') - ->setValue($v_name) - ->setError($e_name)) - ->appendChild( - id(new AphrontFormStaticControl()) - ->setLabel(pht('Blueprint Type')) - ->setValue($impl->getBlueprintName())) + ->appendChild($control) ->appendChild( - id(new AphrontFormPolicyControl()) - ->setName('viewPolicy') - ->setPolicyObject($blueprint) - ->setCapability(PhabricatorPolicyCapability::CAN_VIEW) - ->setPolicies($policies)) - ->appendChild( - id(new AphrontFormPolicyControl()) - ->setName('editPolicy') - ->setPolicyObject($blueprint) - ->setCapability(PhabricatorPolicyCapability::CAN_EDIT) - ->setPolicies($policies)); - - $field_list->appendFieldsToForm($form); - - $crumbs = $this->buildApplicationCrumbs(); - - if ($blueprint->getID()) { - $title = pht('Edit Blueprint'); - $header = pht('Edit Blueprint %d', $blueprint->getID()); - $crumbs->addTextCrumb(pht('Blueprint %d', $blueprint->getID())); - $crumbs->addTextCrumb(pht('Edit')); - $submit = pht('Save Blueprint'); - } else { - $title = pht('New Blueprint'); - $header = pht('New Blueprint'); - $crumbs->addTextCrumb(pht('New Blueprint')); - $submit = pht('Create Blueprint'); - } - - $form->appendChild( - id(new AphrontFormSubmitControl()) - ->setValue($submit) - ->addCancelButton($cancel_uri)); + id(new AphrontFormSubmitControl()) + ->addCancelButton($this->getApplicationURI('blueprint/')) + ->setValue(pht('Continue'))); $box = id(new PHUIObjectBoxView()) - ->setHeaderText($header) - ->setValidationException($validation_exception) ->setFormErrors($errors) + ->setHeaderText($title) ->setForm($form); - return $this->buildApplicationPage( - array( - $crumbs, - $box, - ), - array( - 'title' => $title, - )); + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($box); } } diff --git a/src/applications/drydock/controller/DrydockBlueprintListController.php b/src/applications/drydock/controller/DrydockBlueprintListController.php index 98757c99b0..91de6e3425 100644 --- a/src/applications/drydock/controller/DrydockBlueprintListController.php +++ b/src/applications/drydock/controller/DrydockBlueprintListController.php @@ -13,17 +13,12 @@ final class DrydockBlueprintListController extends DrydockBlueprintController { } protected function buildApplicationCrumbs() { - $can_create = $this->hasApplicationCapability( - DrydockCreateBlueprintsCapability::CAPABILITY); - $crumbs = parent::buildApplicationCrumbs(); - $crumbs->addAction( - id(new PHUIListItemView()) - ->setName(pht('New Blueprint')) - ->setHref($this->getApplicationURI('/blueprint/create/')) - ->setDisabled(!$can_create) - ->setWorkflow(!$can_create) - ->setIcon('fa-plus-square')); + + id(new DrydockBlueprintEditEngine()) + ->setViewer($this->getViewer()) + ->addActionToCrumbs($crumbs); + return $crumbs; } diff --git a/src/applications/drydock/customfield/DrydockBlueprintCoreCustomField.php b/src/applications/drydock/customfield/DrydockBlueprintCoreCustomField.php index 9af418b4d1..32a4ac1ed8 100644 --- a/src/applications/drydock/customfield/DrydockBlueprintCoreCustomField.php +++ b/src/applications/drydock/customfield/DrydockBlueprintCoreCustomField.php @@ -28,6 +28,7 @@ final class DrydockBlueprintCoreCustomField public function readValueFromObject(PhabricatorCustomFieldInterface $object) { $key = $this->getProxy()->getRawStandardFieldKey(); $this->setValueFromStorage($object->getDetail($key)); + $this->didSetValueFromStorage(); } public function applyApplicationTransactionInternalEffects( diff --git a/src/applications/drydock/editor/DrydockBlueprintEditEngine.php b/src/applications/drydock/editor/DrydockBlueprintEditEngine.php new file mode 100644 index 0000000000..bec86e5d4c --- /dev/null +++ b/src/applications/drydock/editor/DrydockBlueprintEditEngine.php @@ -0,0 +1,115 @@ +blueprintImplementation = $impl; + return $this; + } + + public function getBlueprintImplementation() { + return $this->blueprintImplementation; + } + + protected function newEditableObject() { + $viewer = $this->getViewer(); + $blueprint = DrydockBlueprint::initializeNewBlueprint($viewer); + + $impl = $this->getBlueprintImplementation(); + if ($impl) { + $blueprint + ->setClassName(get_class($impl)) + ->attachImplementation(clone $impl); + } + + return $blueprint; + } + + protected function newObjectQuery() { + return new DrydockBlueprintQuery(); + } + + protected function getObjectCreateTitleText($object) { + return pht('Create Blueprint'); + } + + protected function getObjectCreateButtonText($object) { + return pht('Create Blueprint'); + } + + protected function getObjectEditTitleText($object) { + return pht('Edit Blueprint: %s', $object->getBlueprintName()); + } + + protected function getObjectEditShortText($object) { + return pht('Edit Blueprint'); + } + + protected function getObjectCreateShortText() { + return pht('Create Blueprint'); + } + + protected function getEditorURI() { + return '/drydock/blueprint/edit/'; + } + + protected function getObjectCreateCancelURI($object) { + return '/drydock/blueprint/'; + } + + protected function getObjectViewURI($object) { + $id = $object->getID(); + return "/drydock/blueprint/{$id}/"; + } + + protected function getCreateNewObjectPolicy() { + return $this->getApplication()->getPolicy( + DrydockCreateBlueprintsCapability::CAPABILITY); + } + + protected function buildCustomEditFields($object) { + $impl = $object->getImplementation(); + + return array( + id(new PhabricatorStaticEditField()) + ->setKey('type') + ->setLabel(pht('Blueprint Type')) + ->setDescription(pht('Type of blueprint.')) + ->setValue($impl->getBlueprintName()), + id(new PhabricatorTextEditField()) + ->setKey('name') + ->setLabel(pht('Name')) + ->setDescription(pht('Name of the blueprint.')) + ->setTransactionType(DrydockBlueprintTransaction::TYPE_NAME) + ->setIsRequired(true) + ->setValue($object->getBlueprintName()), + ); + } + +} diff --git a/src/applications/drydock/editor/DrydockBlueprintEditor.php b/src/applications/drydock/editor/DrydockBlueprintEditor.php index 315f91cd5c..125c36811a 100644 --- a/src/applications/drydock/editor/DrydockBlueprintEditor.php +++ b/src/applications/drydock/editor/DrydockBlueprintEditor.php @@ -84,4 +84,36 @@ final class DrydockBlueprintEditor return parent::applyCustomExternalTransaction($object, $xaction); } + + protected function validateTransaction( + PhabricatorLiskDAO $object, + $type, + array $xactions) { + + $errors = parent::validateTransaction($object, $type, $xactions); + + switch ($type) { + case DrydockBlueprintTransaction::TYPE_NAME: + $missing = $this->validateIsEmptyTextField( + $object->getBlueprintName(), + $xactions); + + if ($missing) { + $error = new PhabricatorApplicationTransactionValidationError( + $type, + pht('Required'), + pht('You must choose a name for this blueprint.'), + nonempty(last($xactions), null)); + + $error->setIsMissingFieldError(true); + $errors[] = $error; + continue; + } + + break; + } + + return $errors; + } + } diff --git a/src/applications/drydock/typeahead/DrydockBlueprintDatasource.php b/src/applications/drydock/typeahead/DrydockBlueprintDatasource.php index 8b838cd2b6..79229bfab5 100644 --- a/src/applications/drydock/typeahead/DrydockBlueprintDatasource.php +++ b/src/applications/drydock/typeahead/DrydockBlueprintDatasource.php @@ -26,11 +26,20 @@ final class DrydockBlueprintDatasource ->execute(); $results = array(); - foreach ($handles as $handle) { - $results[] = id(new PhabricatorTypeaheadResult()) - ->setName($handle->getName()) + foreach ($blueprints as $blueprint) { + $handle = $handles[$blueprint->getPHID()]; + + $result = id(new PhabricatorTypeaheadResult()) + ->setName($handle->getFullName()) ->setPHID($handle->getPHID()); + + if ($blueprint->getIsDisabled()) { + $result->setClosed(pht('Disabled')); + } + + $results[] = $result; } + return $results; } } diff --git a/src/applications/transactions/editfield/PhabricatorStaticEditField.php b/src/applications/transactions/editfield/PhabricatorStaticEditField.php new file mode 100644 index 0000000000..62c5a9a888 --- /dev/null +++ b/src/applications/transactions/editfield/PhabricatorStaticEditField.php @@ -0,0 +1,18 @@ +getDatasource(); } + protected function getHTTPParameterType() { + return new AphrontPHIDListHTTPParameterType(); + } + + protected function newConduitSearchParameterType() { + return new ConduitPHIDListParameterType(); + } + + protected function newConduitEditParameterType() { + return new ConduitPHIDListParameterType(); + } + } -- 2.51.2