From 5502fca5f43765bad02827dd063d6c684d4bfb92 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 8 Jan 2014 14:12:27 -0800 Subject: [PATCH] Make Drydock blueprint create workflow somewhat more standard Summary: Ref T2015. This workflow is a little weird (runs in a dialog, no edit-before-create step, lots of internal classnames). Make it a little more standard. Test Plan: See screenshots. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T2015 Differential Revision: https://secure.phabricator.com/D7908 --- .../PhabricatorApplicationDrydock.php | 2 +- .../DrydockBlueprintImplementation.php | 5 + ...rydockLocalHostBlueprintImplementation.php | 7 +- ...reallocatedHostBlueprintImplementation.php | 6 +- ...dockWorkingCopyBlueprintImplementation.php | 6 +- .../DrydockBlueprintCreateController.php | 93 +++++++++++-------- .../DrydockBlueprintEditController.php | 69 +++++++------- .../DrydockBlueprintViewController.php | 15 ++- .../HarbormasterStepAddController.php | 2 +- 9 files changed, 126 insertions(+), 79 deletions(-) diff --git a/src/applications/drydock/application/PhabricatorApplicationDrydock.php b/src/applications/drydock/application/PhabricatorApplicationDrydock.php index e5cadd86cc..bc68902cae 100644 --- a/src/applications/drydock/application/PhabricatorApplicationDrydock.php +++ b/src/applications/drydock/application/PhabricatorApplicationDrydock.php @@ -38,7 +38,7 @@ final class PhabricatorApplicationDrydock extends PhabricatorApplication { '(?:query/(?P[^/]+)/)?' => 'DrydockBlueprintListController', '(?P[1-9]\d*)/' => 'DrydockBlueprintViewController', 'create/' => 'DrydockBlueprintCreateController', - 'edit/(?P[1-9]\d*)/' => 'DrydockBlueprintEditController', + 'edit/(?:(?P[1-9]\d*)/)?' => 'DrydockBlueprintEditController', ), 'resource/' => array( '(?:query/(?P[^/]+)/)?' => 'DrydockResourceListController', diff --git a/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php index 43e9535b7c..8c4246aa8a 100644 --- a/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php +++ b/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php @@ -19,6 +19,7 @@ abstract class DrydockBlueprintImplementation { abstract public function isEnabled(); + abstract public function getBlueprintName(); abstract public function getDescription(); public function getBlueprintClass() { @@ -388,6 +389,10 @@ abstract class DrydockBlueprintImplementation { return idx($groups, $type, array()); } + public static function getNamedImplementation($class) { + return idx(self::getAllBlueprintImplementations(), $class); + } + protected function newResourceTemplate($name) { $resource = id(new DrydockResource()) ->setBlueprintPHID($this->getInstance()->getPHID()) diff --git a/src/applications/drydock/blueprint/DrydockLocalHostBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockLocalHostBlueprintImplementation.php index 21a06c6c86..b1bfac2395 100644 --- a/src/applications/drydock/blueprint/DrydockLocalHostBlueprintImplementation.php +++ b/src/applications/drydock/blueprint/DrydockLocalHostBlueprintImplementation.php @@ -7,8 +7,13 @@ final class DrydockLocalHostBlueprintImplementation return false; } + public function getBlueprintName() { + return pht('Local Host'); + } + public function getDescription() { - return pht('Allocates storage on the local host.'); + return pht( + 'Allows Drydock to run on the local host.'); } public function canAllocateMoreResources(array $pool) { diff --git a/src/applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php index 14e147c07f..554b969307 100644 --- a/src/applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php +++ b/src/applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php @@ -7,8 +7,12 @@ final class DrydockPreallocatedHostBlueprintImplementation return true; } + public function getBlueprintName() { + return pht('Remote Host (Preallocated)'); + } + public function getDescription() { - return pht('Leases out preallocated, remote hosts.'); + return pht('Allows Drydock to run on specific remote hosts you configure.'); } public function canAllocateMoreResources(array $pool) { diff --git a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php index 4bfdfa433a..ed92311e65 100644 --- a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php +++ b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php @@ -7,8 +7,12 @@ final class DrydockWorkingCopyBlueprintImplementation return true; } + public function getBlueprintName() { + return pht('Working Copy'); + } + public function getDescription() { - return pht('Allocates out working copies of repositories.'); + return pht('Allows Drydock to check out working copies of repositories.'); } protected function canAllocateLease( diff --git a/src/applications/drydock/controller/DrydockBlueprintCreateController.php b/src/applications/drydock/controller/DrydockBlueprintCreateController.php index 3b0909a90b..0453612169 100644 --- a/src/applications/drydock/controller/DrydockBlueprintCreateController.php +++ b/src/applications/drydock/controller/DrydockBlueprintCreateController.php @@ -10,56 +10,75 @@ final class DrydockBlueprintCreateController $implementations = DrydockBlueprintImplementation::getAllBlueprintImplementations(); + $errors = array(); + $e_blueprint = null; + if ($request->isFormPost()) { $class = $request->getStr('blueprint-type'); if (!isset($implementations[$class])) { - return $this->createDialog($implementations); + $e_blueprint = pht('Required'); + $errors[] = pht('You must choose a blueprint type.'); } - $blueprint = id(new DrydockBlueprint()) - ->setClassName($class) - ->setDetails(array()) - ->setViewPolicy(PhabricatorPolicies::POLICY_ADMIN) - ->setEditPolicy(PhabricatorPolicies::POLICY_ADMIN) - ->save(); - - $edit_uri = $this->getApplicationURI( - "blueprint/edit/".$blueprint->getID()."/"); - - return id(new AphrontRedirectResponse())->setURI($edit_uri); + if (!$errors) { + $edit_uri = $this->getApplicationURI('blueprint/edit/?class='.$class); + return id(new AphrontRedirectResponse())->setURI($edit_uri); + } } - return $this->createDialog($implementations); - } - - function createDialog(array $implementations) { - $request = $this->getRequest(); - $viewer = $request->getUser(); + $error_view = null; + if ($errors) { + $error_view = id(new AphrontErrorView()) + ->setErrors($errors); + } $control = id(new AphrontFormRadioButtonControl()) - ->setName('blueprint-type'); + ->setName('blueprint-type') + ->setLabel(pht('Blueprint Type')) + ->setError($e_blueprint); foreach ($implementations as $implementation_name => $implementation) { - $control - ->addButton( - $implementation_name, - $implementation->getBlueprintClass(), - $implementation->getDescription()); + $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); } - $dialog = new AphrontDialogView(); - $dialog->setTitle(pht('Create New Blueprint')) - ->setUser($viewer) - ->addSubmitButton(pht('Create Blueprint')) - ->addCancelButton($this->getApplicationURI('blueprint/')); - $dialog->appendChild( - phutil_tag( - 'p', - array(), - pht( - 'Select what type of blueprint you want to create: '))); - $dialog->appendChild($control); - return id(new AphrontDialogResponse())->setDialog($dialog); + $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()) + ->setFormError($error_view) + ->setHeaderText($title) + ->setForm($form); + + return $this->buildApplicationPage( + array( + $crumbs, + $box, + ), + array( + 'title' => $title, + 'device' => true, + )); } } diff --git a/src/applications/drydock/controller/DrydockBlueprintEditController.php b/src/applications/drydock/controller/DrydockBlueprintEditController.php index 670de0d5f7..a749c317bd 100644 --- a/src/applications/drydock/controller/DrydockBlueprintEditController.php +++ b/src/applications/drydock/controller/DrydockBlueprintEditController.php @@ -25,10 +25,23 @@ final class DrydockBlueprintEditController extends DrydockBlueprintController { if (!$blueprint) { return new Aphront404Response(); } + + $impl = $blueprint->getImplementation(); + $cancel_uri = $this->getApplicationURI('blueprint/'.$this->id.'/'); } else { + $class = $request->getStr('class'); + + $impl = DrydockBlueprintImplementation::getNamedImplementation($class); + if (!$impl || !$impl->isEnabled()) { + return new Aphront400Response(); + } + $blueprint = new DrydockBlueprint(); + $blueprint->setClassName($class); + $cancel_uri = $this->getApplicationURI('blueprint/'); } + if ($request->isFormPost()) { $v_view_policy = $request->getStr('viewPolicy'); $v_edit_policy = $request->getStr('editPolicy'); @@ -39,8 +52,10 @@ final class DrydockBlueprintEditController extends DrydockBlueprintController { $blueprint->save(); - return id(new AphrontRedirectResponse()) - ->setURI('/drydock/blueprint/'); + $id = $blueprint->getID(); + $save_uri = $this->getApplicationURI("blueprint/{$id}/"); + + return id(new AphrontRedirectResponse())->setURI($save_uri); } $policies = id(new PhabricatorPolicyQuery()) @@ -48,21 +63,13 @@ final class DrydockBlueprintEditController extends DrydockBlueprintController { ->setObject($blueprint) ->execute(); - if ($request->isAjax()) { - $form = id(new PHUIFormLayoutView()) - ->setUser($viewer); - } else { - $form = id(new AphrontFormView()) - ->setUser($viewer); - } - - $form + $form = id(new AphrontFormView()) + ->setUser($viewer) + ->addHiddenInput('class', $request->getStr('class')) ->appendChild( - id(new AphrontFormTextControl()) - ->setName('className') - ->setLabel(pht('Implementation')) - ->setValue($blueprint->getClassName()) - ->setDisabled(true)) + id(new AphrontFormStaticControl()) + ->setLabel(pht('Blueprint Type')) + ->setValue($impl->getBlueprintName())) ->appendChild( id(new AphrontFormPolicyControl()) ->setName('viewPolicy') @@ -78,27 +85,23 @@ final class DrydockBlueprintEditController extends DrydockBlueprintController { $crumbs = $this->buildApplicationCrumbs(); - $title = pht('Edit Blueprint'); - $header = pht('Edit Blueprint %d', $blueprint->getID()); - $crumbs->addTextCrumb(pht('Blueprint %d', $blueprint->getID())); - $crumbs->addTextCrumb(pht('Edit')); - - if ($request->isAjax()) { - $dialog = id(new AphrontDialogView()) - ->setUser($viewer) - ->setWidth(AphrontDialogView::WIDTH_FORM) - ->setTitle($title) - ->appendChild($form) - ->addSubmitButton(pht('Edit Blueprint')) - ->addCancelButton($this->getApplicationURI()); - - return id(new AphrontDialogResponse())->setDialog($dialog); + 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(pht('Save')) - ->addCancelButton($this->getApplicationURI())); + ->setValue($submit) + ->addCancelButton($cancel_uri)); $box = id(new PHUIObjectBoxView()) ->setHeaderText($header) diff --git a/src/applications/drydock/controller/DrydockBlueprintViewController.php b/src/applications/drydock/controller/DrydockBlueprintViewController.php index f73f1d455b..8526f42d9c 100644 --- a/src/applications/drydock/controller/DrydockBlueprintViewController.php +++ b/src/applications/drydock/controller/DrydockBlueprintViewController.php @@ -65,21 +65,28 @@ final class DrydockBlueprintViewController extends DrydockBlueprintController { } private function buildActionListView(DrydockBlueprint $blueprint) { + $viewer = $this->getRequest()->getUser(); + $view = id(new PhabricatorActionListView()) - ->setUser($this->getRequest()->getUser()) + ->setUser($viewer) ->setObjectURI($this->getRequest()->getRequestURI()) ->setObject($blueprint); $uri = '/blueprint/edit/'.$blueprint->getID().'/'; $uri = $this->getApplicationURI($uri); + $can_edit = PhabricatorPolicyFilter::hasCapability( + $viewer, + $blueprint, + PhabricatorPolicyCapability::CAN_EDIT); + $view->addAction( id(new PhabricatorActionView()) ->setHref($uri) - ->setName(pht('Edit Blueprint Policies')) + ->setName(pht('Edit Blueprint')) ->setIcon('edit') - ->setWorkflow(true) - ->setDisabled(false)); + ->setWorkflow(!$can_edit) + ->setDisabled(!$can_edit)); return $view; } diff --git a/src/applications/harbormaster/controller/HarbormasterStepAddController.php b/src/applications/harbormaster/controller/HarbormasterStepAddController.php index c812453048..e09cfa4dab 100644 --- a/src/applications/harbormaster/controller/HarbormasterStepAddController.php +++ b/src/applications/harbormaster/controller/HarbormasterStepAddController.php @@ -33,7 +33,7 @@ final class HarbormasterStepAddController if ($request->isDialogFormPost()) { $class = $request->getStr('step-type'); if (!in_array($class, $implementations)) { - return $this->createDialog($implementations); + return $this->createDialog($implementations, $cancel_uri); } $steps = $plan->loadOrderedBuildSteps();