diff --git a/src/applications/harbormaster/controller/HarbormasterController.php b/src/applications/harbormaster/controller/HarbormasterController.php --- a/src/applications/harbormaster/controller/HarbormasterController.php +++ b/src/applications/harbormaster/controller/HarbormasterController.php @@ -2,21 +2,4 @@ abstract class HarbormasterController extends PhabricatorController { - protected function buildApplicationCrumbs() { - $crumbs = parent::buildApplicationCrumbs(); - - $can_create = $this->hasApplicationCapability( - HarbormasterManagePlansCapability::CAPABILITY); - - $crumbs->addAction( - id(new PHUIListItemView()) - ->setName(pht('New Build Plan')) - ->setHref($this->getApplicationURI('plan/edit/')) - ->setDisabled(!$can_create) - ->setWorkflow(!$can_create) - ->setIcon('fa-plus-square')); - - return $crumbs; - } - } diff --git a/src/applications/harbormaster/controller/HarbormasterPlanDisableController.php b/src/applications/harbormaster/controller/HarbormasterPlanDisableController.php --- a/src/applications/harbormaster/controller/HarbormasterPlanDisableController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanDisableController.php @@ -3,22 +3,20 @@ final class HarbormasterPlanDisableController extends HarbormasterPlanController { - private $id; - - public function willProcessRequest(array $data) { - $this->id = $data['id']; - } - - public function processRequest() { - $request = $this->getRequest(); - $viewer = $request->getUser(); + public function handleRequest(AphrontRequest $request) { + $viewer = $this->getViewer(); $this->requireApplicationCapability( HarbormasterManagePlansCapability::CAPABILITY); $plan = id(new HarbormasterBuildPlanQuery()) ->setViewer($viewer) - ->withIDs(array($this->id)) + ->withIDs(array($request->getURIData('id'))) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) ->executeOne(); if (!$plan) { return new Aphront404Response(); @@ -63,14 +61,11 @@ $button = pht('Disable Plan'); } - $dialog = id(new AphrontDialogView()) - ->setUser($viewer) + return $this->newDialog() ->setTitle($title) ->appendChild($body) ->addSubmitButton($button) ->addCancelButton($plan_uri); - - return id(new AphrontDialogResponse())->setDialog($dialog); } } diff --git a/src/applications/harbormaster/controller/HarbormasterPlanEditController.php b/src/applications/harbormaster/controller/HarbormasterPlanEditController.php --- a/src/applications/harbormaster/controller/HarbormasterPlanEditController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanEditController.php @@ -2,23 +2,22 @@ final class HarbormasterPlanEditController extends HarbormasterPlanController { - private $id; - - public function willProcessRequest(array $data) { - $this->id = idx($data, 'id'); - } - - public function processRequest() { - $request = $this->getRequest(); - $viewer = $request->getUser(); + public function handleRequest(AphrontRequest $request) { + $viewer = $this->getViewer(); $this->requireApplicationCapability( HarbormasterManagePlansCapability::CAPABILITY); - if ($this->id) { + $id = $request->getURIData('id'); + if ($id) { $plan = id(new HarbormasterBuildPlanQuery()) ->setViewer($viewer) - ->withIDs(array($this->id)) + ->withIDs(array($id)) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) ->executeOne(); if (!$plan) { return new Aphront404Response(); @@ -43,6 +42,7 @@ $editor = id(new HarbormasterBuildPlanEditor()) ->setActor($viewer) + ->setContinueOnNoEffect(true) ->setContentSourceFromRequest($request); try { diff --git a/src/applications/harbormaster/controller/HarbormasterPlanListController.php b/src/applications/harbormaster/controller/HarbormasterPlanListController.php --- a/src/applications/harbormaster/controller/HarbormasterPlanListController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanListController.php @@ -2,19 +2,13 @@ final class HarbormasterPlanListController extends HarbormasterPlanController { - private $queryKey; - public function shouldAllowPublic() { return true; } - public function willProcessRequest(array $data) { - $this->queryKey = idx($data, 'queryKey'); - } - - public function processRequest() { + public function handleRequest(AphrontRequest $request) { $controller = id(new PhabricatorApplicationSearchController()) - ->setQueryKey($this->queryKey) + ->setQueryKey($request->getURIData('queryKey')) ->setSearchEngine(new HarbormasterBuildPlanSearchEngine()) ->setNavigation($this->buildSideNavView()); @@ -44,4 +38,22 @@ return $this->buildSideNavView(true)->getMenu(); } + protected function buildApplicationCrumbs() { + $crumbs = parent::buildApplicationCrumbs(); + + $can_create = $this->hasApplicationCapability( + HarbormasterManagePlansCapability::CAPABILITY); + + $crumbs->addAction( + id(new PHUIListItemView()) + ->setName(pht('New Build Plan')) + ->setHref($this->getApplicationURI('plan/edit/')) + ->setDisabled(!$can_create) + ->setWorkflow(!$can_create) + ->setIcon('fa-plus-square')); + + return $crumbs; + } + + } diff --git a/src/applications/harbormaster/controller/HarbormasterPlanRunController.php b/src/applications/harbormaster/controller/HarbormasterPlanRunController.php --- a/src/applications/harbormaster/controller/HarbormasterPlanRunController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanRunController.php @@ -2,20 +2,18 @@ final class HarbormasterPlanRunController extends HarbormasterController { - private $id; - - public function willProcessRequest(array $data) { - $this->id = $data['id']; - } - - public function processRequest() { - $request = $this->getRequest(); - $viewer = $request->getUser(); + public function handleRequest(AphrontRequest $request) { + $viewer = $this->getViewer(); $this->requireApplicationCapability( HarbormasterManagePlansCapability::CAPABILITY); - $plan_id = $this->id; + $plan_id = $request->getURIData('id'); + + // NOTE: At least for now, this only requires the "Can Manage Plans" + // capability, not the "Can Edit" capability. Possibly it should have + // a more stringent requirement, though. + $plan = id(new HarbormasterBuildPlanQuery()) ->setViewer($viewer) ->withIDs(array($plan_id)) @@ -87,15 +85,12 @@ ->setError($e_name) ->setValue($v_name)); - $dialog = id(new AphrontDialogView()) + return $this->newDialog() ->setWidth(AphrontDialogView::WIDTH_FULL) - ->setUser($viewer) ->setTitle($title) ->appendChild($form) ->addCancelButton($cancel_uri) ->addSubmitButton($save_button); - - return id(new AphrontDialogResponse())->setDialog($dialog); } } diff --git a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php --- a/src/applications/harbormaster/controller/HarbormasterPlanViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterPlanViewController.php @@ -2,17 +2,10 @@ final class HarbormasterPlanViewController extends HarbormasterPlanController { - private $id; + public function handleRequest(AphrontRequest $request) { + $viewer = $this->getviewer(); - public function willProcessRequest(array $data) { - $this->id = $data['id']; - } - - public function processRequest() { - $request = $this->getRequest(); - $viewer = $request->getUser(); - - $id = $this->id; + $id = $request->getURIData('id'); $plan = id(new HarbormasterBuildPlanQuery()) ->setViewer($viewer) @@ -79,11 +72,9 @@ } private function buildStepList(HarbormasterBuildPlan $plan) { - $request = $this->getRequest(); - $viewer = $request->getUser(); + $viewer = $this->getViewer(); - $run_order = - HarbormasterBuildGraph::determineDependencyExecution($plan); + $run_order = HarbormasterBuildGraph::determineDependencyExecution($plan); $steps = id(new HarbormasterBuildStepQuery()) ->setViewer($viewer) @@ -91,9 +82,16 @@ ->execute(); $steps = mpull($steps, null, 'getPHID'); - $can_edit = $this->hasApplicationCapability( + $has_manage = $this->hasApplicationCapability( HarbormasterManagePlansCapability::CAPABILITY); + $has_edit = PhabricatorPolicyFilter::hasCapability( + $viewer, + $plan, + PhabricatorPolicyCapability::CAN_EDIT); + + $can_edit = ($has_manage && $has_edit); + $step_list = id(new PHUIObjectItemListView()) ->setUser($viewer) ->setNoDataString( @@ -222,12 +220,32 @@ $step_list->addItem($item); } - return array($step_list, $has_any_conflicts, $is_deadlocking); + $step_list->setFlush(true); + + $plan_id = $plan->getID(); + + $header = id(new PHUIHeaderView()) + ->setHeader(pht('Build Steps')) + ->addActionLink( + id(new PHUIButtonView()) + ->setText(pht('Add Build Step')) + ->setHref($this->getApplicationURI("step/add/{$plan_id}/")) + ->setTag('a') + ->setIcon( + id(new PHUIIconView()) + ->setIconFont('fa-plus')) + ->setDisabled(!$can_edit) + ->setWorkflow(true)); + + $step_box = id(new PHUIObjectBoxView()) + ->setHeader($header) + ->appendChild($step_list); + + return array($step_box, $has_any_conflicts, $is_deadlocking); } private function buildActionList(HarbormasterBuildPlan $plan) { - $request = $this->getRequest(); - $viewer = $request->getUser(); + $viewer = $this->getViewer(); $id = $plan->getID(); $list = id(new PhabricatorActionListView()) @@ -235,9 +253,16 @@ ->setObject($plan) ->setObjectURI($this->getApplicationURI("plan/{$id}/")); - $can_edit = $this->hasApplicationCapability( + $has_manage = $this->hasApplicationCapability( HarbormasterManagePlansCapability::CAPABILITY); + $has_edit = PhabricatorPolicyFilter::hasCapability( + $viewer, + $plan, + PhabricatorPolicyCapability::CAN_EDIT); + + $can_edit = ($has_manage && $has_edit); + $list->addAction( id(new PhabricatorActionView()) ->setName(pht('Edit Plan')) @@ -266,18 +291,10 @@ $list->addAction( id(new PhabricatorActionView()) - ->setName(pht('Add Build Step')) - ->setHref($this->getApplicationURI("step/add/{$id}/")) - ->setWorkflow(true) - ->setDisabled(!$can_edit) - ->setIcon('fa-plus')); - - $list->addAction( - id(new PhabricatorActionView()) ->setName(pht('Run Plan Manually')) ->setHref($this->getApplicationURI("plan/run/{$id}/")) ->setWorkflow(true) - ->setDisabled(!$can_edit) + ->setDisabled(!$has_manage) ->setIcon('fa-play-circle')); return $list; diff --git a/src/applications/harbormaster/controller/HarbormasterStepAddController.php b/src/applications/harbormaster/controller/HarbormasterStepAddController.php --- a/src/applications/harbormaster/controller/HarbormasterStepAddController.php +++ b/src/applications/harbormaster/controller/HarbormasterStepAddController.php @@ -2,22 +2,20 @@ final class HarbormasterStepAddController extends HarbormasterController { - private $id; - - public function willProcessRequest(array $data) { - $this->id = $data['id']; - } - - public function processRequest() { - $request = $this->getRequest(); - $viewer = $request->getUser(); + public function handleRequest(AphrontRequest $request) { + $viewer = $this->getViewer(); $this->requireApplicationCapability( HarbormasterManagePlansCapability::CAPABILITY); $plan = id(new HarbormasterBuildPlanQuery()) ->setViewer($viewer) - ->withIDs(array($this->id)) + ->withIDs(array($request->getURIData('id'))) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) ->executeOne(); if (!$plan) { return new Aphront404Response(); diff --git a/src/applications/harbormaster/controller/HarbormasterStepDeleteController.php b/src/applications/harbormaster/controller/HarbormasterStepDeleteController.php --- a/src/applications/harbormaster/controller/HarbormasterStepDeleteController.php +++ b/src/applications/harbormaster/controller/HarbormasterStepDeleteController.php @@ -2,27 +2,25 @@ final class HarbormasterStepDeleteController extends HarbormasterController { - private $id; - - public function willProcessRequest(array $data) { - $this->id = $data['id']; - } - - public function processRequest() { - $request = $this->getRequest(); - $viewer = $request->getUser(); + public function handleRequest(AphrontRequest $request) { + $viewer = $this->getViewer(); $this->requireApplicationCapability( HarbormasterManagePlansCapability::CAPABILITY); - $id = $this->id; + $id = $request->getURIData('id'); $step = id(new HarbormasterBuildStepQuery()) ->setViewer($viewer) ->withIDs(array($id)) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) ->executeOne(); - if ($step === null) { - throw new Exception(pht('Build step not found!')); + if (!$step) { + return new Aphront404Response(); } $plan_id = $step->getBuildPlan()->getID(); @@ -33,19 +31,14 @@ return id(new AphrontRedirectResponse())->setURI($done_uri); } - $dialog = new AphrontDialogView(); - $dialog->setTitle(pht('Really Delete Step?')) - ->setUser($viewer) - ->addSubmitButton(pht('Delete Build Step')) - ->addCancelButton($done_uri); - $dialog->appendChild( - phutil_tag( - 'p', - array(), + return $this->newDialog() + ->setTitle(pht('Really Delete Step?')) + ->appendParagraph( pht( "Are you sure you want to delete this step? ". - "This can't be undone!"))); - return id(new AphrontDialogResponse())->setDialog($dialog); + "This can't be undone!")) + ->addCancelButton($done_uri) + ->addSubmitButton(pht('Delete Build Step')); } } diff --git a/src/applications/harbormaster/controller/HarbormasterStepEditController.php b/src/applications/harbormaster/controller/HarbormasterStepEditController.php --- a/src/applications/harbormaster/controller/HarbormasterStepEditController.php +++ b/src/applications/harbormaster/controller/HarbormasterStepEditController.php @@ -2,27 +2,22 @@ final class HarbormasterStepEditController extends HarbormasterController { - private $id; - private $planID; - private $className; - - public function willProcessRequest(array $data) { - $this->id = idx($data, 'id'); - $this->planID = idx($data, 'plan'); - $this->className = idx($data, 'class'); - } - - public function processRequest() { - $request = $this->getRequest(); - $viewer = $request->getUser(); + public function handleRequest(AphrontRequest $request) { + $viewer = $this->getViewer(); $this->requireApplicationCapability( HarbormasterManagePlansCapability::CAPABILITY); - if ($this->id) { + $id = $request->getURIData('id'); + if ($id) { $step = id(new HarbormasterBuildStepQuery()) ->setViewer($viewer) - ->withIDs(array($this->id)) + ->withIDs(array($id)) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) ->executeOne(); if (!$step) { return new Aphront404Response(); @@ -31,23 +26,30 @@ $is_new = false; } else { + $plan_id = $request->getURIData('plan'); + $class = $request->getURIData('class'); + $plan = id(new HarbormasterBuildPlanQuery()) - ->setViewer($viewer) - ->withIDs(array($this->planID)) - ->executeOne(); + ->setViewer($viewer) + ->withIDs(array($plan_id)) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->executeOne(); if (!$plan) { return new Aphront404Response(); } - $impl = HarbormasterBuildStepImplementation::getImplementation( - $this->className); + $impl = HarbormasterBuildStepImplementation::getImplementation($class); if (!$impl) { return new Aphront404Response(); } $step = HarbormasterBuildStep::initializeNewStep($viewer) ->setBuildPlanPHID($plan->getPHID()) - ->setClassName($this->className); + ->setClassName($class); $is_new = true; } diff --git a/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php b/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php --- a/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php +++ b/src/applications/harbormaster/storage/configuration/HarbormasterBuildPlan.php @@ -102,6 +102,7 @@ public function getCapabilities() { return array( PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, ); } @@ -109,6 +110,10 @@ switch ($capability) { case PhabricatorPolicyCapability::CAN_VIEW: return PhabricatorPolicies::getMostOpenPolicy(); + case PhabricatorPolicyCapability::CAN_EDIT: + // NOTE: In practice, this policy is always limited by the "Mangage + // Build Plans" policy. + return PhabricatorPolicies::getMostOpenPolicy(); } } @@ -119,4 +124,5 @@ public function describeAutomaticCapability($capability) { return null; } + } diff --git a/src/applications/harbormaster/storage/configuration/HarbormasterBuildStep.php b/src/applications/harbormaster/storage/configuration/HarbormasterBuildStep.php --- a/src/applications/harbormaster/storage/configuration/HarbormasterBuildStep.php +++ b/src/applications/harbormaster/storage/configuration/HarbormasterBuildStep.php @@ -118,6 +118,7 @@ public function getCapabilities() { return array( PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, ); }