Changeset View
Standalone View
src/applications/dashboard/controller/panel/PhabricatorDashboardPanelTabsController.php
| <?php | <?php | ||||
| final class PhabricatorDashboardPanelTabsController | final class PhabricatorDashboardPanelTabsController | ||||
| extends PhabricatorDashboardController { | extends PhabricatorDashboardController { | ||||
| private $contextObject; | |||||
| private function setContextObject($context_object) { | |||||
| $this->contextObject = $context_object; | |||||
| return $this; | |||||
| } | |||||
| private function getContextObject() { | |||||
| return $this->contextObject; | |||||
| } | |||||
| public function handleRequest(AphrontRequest $request) { | public function handleRequest(AphrontRequest $request) { | ||||
| $viewer = $this->getViewer(); | $viewer = $this->getViewer(); | ||||
| $panel = id(new PhabricatorDashboardPanelQuery()) | $panel = id(new PhabricatorDashboardPanelQuery()) | ||||
| ->setViewer($viewer) | ->setViewer($viewer) | ||||
| ->withIDs(array($request->getURIData('id'))) | ->withIDs(array($request->getURIData('id'))) | ||||
| ->requireCapabilities( | ->requireCapabilities( | ||||
| array( | array( | ||||
| ▲ Show 20 Lines • Show All 67 Lines • ▼ Show 20 Lines | if ($target !== null) { | ||||
| pht( | pht( | ||||
| 'Target tab ("%s") was not found on this panel. It may have '. | 'Target tab ("%s") was not found on this panel. It may have '. | ||||
| 'been removed.', | 'been removed.', | ||||
| $target)) | $target)) | ||||
| ->addCancelButton($cancel_uri); | ->addCancelButton($cancel_uri); | ||||
| } | } | ||||
| } | } | ||||
| // Tab panels may be edited from the panel page, or from the context of | |||||
| // a dashboard. If we're editing from a dashboard, we want to redirect | |||||
| // back to the dashboard after making changes. | |||||
| $context_phid = $request->getStr('contextPHID'); | |||||
| $context = null; | |||||
| if (strlen($context_phid)) { | |||||
| $context = id(new PhabricatorObjectQuery()) | |||||
| ->setViewer($viewer) | |||||
| ->withPHIDs(array($context_phid)) | |||||
| ->executeOne(); | |||||
| if (!$context) { | |||||
| return new Aphront404Response(); | |||||
| } | |||||
| switch (phid_get_type($context_phid)) { | |||||
| case PhabricatorDashboardDashboardPHIDType::TYPECONST: | |||||
| $cancel_uri = $context->getURI(); | |||||
| break; | |||||
| case PhabricatorDashboardPanelPHIDType::TYPECONST: | |||||
| $cancel_uri = $context->getURI(); | |||||
| break; | |||||
| default: | |||||
| return $this->newDialog() | |||||
| ->setTitle(pht('Context Object Unsupported')) | |||||
| ->appendParagraph( | |||||
| pht( | |||||
| 'Context object ("%s") has unsupported type. Panels should '. | |||||
| 'be rendered from the context of a dashboard or another '. | |||||
| 'panel.', | |||||
amckinley: Maybe `throw` this in `setContextObject()` to make the resulting stack trace more clear? (And… | |||||
Done Inline ActionsIf we throw, we either have to catch the exception and turn it back into a dialog, or sacrifice the UX somewhat (produce a response which says "Exception!" and doesn't have a title), since we don't currently have a general-purpose way to throw an exception which has "here's how to render a nice dialog out of this" information. It's easier to just throw a little deeper, but I'm trying to make this a little nicer if users fiddle with contextPHID for some reason, since it's technically user-editable. Possibly we should have a user-facing sort of exception (and PhortuneDisplayException is a small step toward this), but then the signature needs to be setContextObject($context, $cancel_uri), which seems unintuitive. Maybe this should be: try {
$this->setContextObject();
} catch (PhabricatorUserFacingException $ex) {
$ex->setCancelURI($cancel_uri);
throw $ex;
}...with: $developer_message = pht('contextPHID points at an invalid object');
$user_message = pht("oopsie woopsie OwO that's not a valid context ;)");
throw new id(new PhabricatorUserFacingException($developer_message))
->setTitle(pht('Bad Context Object'))
->setMessage($user_message)...but that sure feels like a lot of work and very easy to get wrong. epriestley: If we throw, we either have to catch the exception and turn it back into a dialog, or sacrifice… | |||||
| $context_phid)) | |||||
| ->addCancelButton($cancel_uri); | |||||
| } | |||||
| $this->setContextObject($context); | |||||
| } | |||||
| switch ($op) { | switch ($op) { | ||||
| case 'add': | case 'add': | ||||
| return $this->handleAddOperation($panel, $after, $cancel_uri); | return $this->handleAddOperation($panel, $after, $cancel_uri); | ||||
| case 'remove': | case 'remove': | ||||
| return $this->handleRemoveOperation($panel, $target, $cancel_uri); | return $this->handleRemoveOperation($panel, $target, $cancel_uri); | ||||
| case 'move': | case 'move': | ||||
| break; | break; | ||||
| case 'rename': | case 'rename': | ||||
| ▲ Show 20 Lines • Show All 74 Lines • ▼ Show 20 Lines | $form = id(new AphrontFormView()) | ||||
| ->appendControl( | ->appendControl( | ||||
| id(new AphrontFormTokenizerControl()) | id(new AphrontFormTokenizerControl()) | ||||
| ->setDatasource(new PhabricatorDashboardPanelDatasource()) | ->setDatasource(new PhabricatorDashboardPanelDatasource()) | ||||
| ->setLimit(1) | ->setLimit(1) | ||||
| ->setName('panelPHID') | ->setName('panelPHID') | ||||
| ->setLabel(pht('Panel')) | ->setLabel(pht('Panel')) | ||||
| ->setValue($v_panel)); | ->setValue($v_panel)); | ||||
| return $this->newDialog() | return $this->newEditDialog() | ||||
| ->setTitle(pht('Choose Dashboard Panel')) | ->setTitle(pht('Choose Dashboard Panel')) | ||||
| ->setErrors($errors) | ->setErrors($errors) | ||||
| ->setWidth(AphrontDialogView::WIDTH_FORM) | |||||
| ->addHiddenInput('after', $after) | ->addHiddenInput('after', $after) | ||||
| ->appendForm($form) | ->appendForm($form) | ||||
| ->addCancelButton($cancel_uri) | ->addCancelButton($cancel_uri) | ||||
| ->addSubmitButton(pht('Add Panel')); | ->addSubmitButton(pht('Add Panel')); | ||||
| } | } | ||||
| private function handleRemoveOperation( | private function handleRemoveOperation( | ||||
| PhabricatorDashboardPanel $panel, | PhabricatorDashboardPanel $panel, | ||||
| Show All 9 Lines | if ($request->isFormPost()) { | ||||
| $old_config = $impl->getPanelConfiguration($panel); | $old_config = $impl->getPanelConfiguration($panel); | ||||
| $new_config = $this->removePanel($old_config, $target); | $new_config = $this->removePanel($old_config, $target); | ||||
| $this->writePanelConfig($panel, $new_config); | $this->writePanelConfig($panel, $new_config); | ||||
| return id(new AphrontRedirectResponse())->setURI($cancel_uri); | return id(new AphrontRedirectResponse())->setURI($cancel_uri); | ||||
| } | } | ||||
| return $this->newDialog() | return $this->newEditDialog() | ||||
| ->setTitle(pht('Remove tab?')) | ->setTitle(pht('Remove tab?')) | ||||
| ->addHiddenInput('target', $target) | ->addHiddenInput('target', $target) | ||||
| ->appendParagraph(pht('Really remove this tab?')) | ->appendParagraph(pht('Really remove this tab?')) | ||||
| ->addCancelButton($cancel_uri) | ->addCancelButton($cancel_uri) | ||||
| ->addSubmitButton(pht('Remove Tab')); | ->addSubmitButton(pht('Remove Tab')); | ||||
| } | } | ||||
| private function handleRenameOperation( | private function handleRenameOperation( | ||||
| Show All 21 Lines | private function handleRenameOperation( | ||||
| $form = id(new AphrontFormView()) | $form = id(new AphrontFormView()) | ||||
| ->setViewer($viewer) | ->setViewer($viewer) | ||||
| ->appendControl( | ->appendControl( | ||||
| id(new AphrontFormTextControl()) | id(new AphrontFormTextControl()) | ||||
| ->setValue($name) | ->setValue($name) | ||||
| ->setName('name') | ->setName('name') | ||||
| ->setLabel(pht('Tab Name'))); | ->setLabel(pht('Tab Name'))); | ||||
| return $this->newDialog() | return $this->newEditDialog() | ||||
| ->setTitle(pht('Rename Panel')) | ->setTitle(pht('Rename Panel')) | ||||
| ->addHiddenInput('target', $target) | ->addHiddenInput('target', $target) | ||||
| ->appendForm($form) | ->appendForm($form) | ||||
| ->addCancelButton($cancel_uri) | ->addCancelButton($cancel_uri) | ||||
| ->addSubmitButton(pht('Rename Tab')); | ->addSubmitButton(pht('Rename Tab')); | ||||
| } | } | ||||
| Show All 32 Lines | private function removePanel(array $config, $target) { | ||||
| return $result; | return $result; | ||||
| } | } | ||||
| private function renamePanel(array $config, $target, $name) { | private function renamePanel(array $config, $target, $name) { | ||||
| $config[$target]['name'] = $name; | $config[$target]['name'] = $name; | ||||
| return $config; | return $config; | ||||
| } | } | ||||
| protected function newEditDialog() { | |||||
| $dialog = $this->newDialog() | |||||
| ->setWidth(AphrontDialogView::WIDTH_FORM); | |||||
| $context = $this->getContextObject(); | |||||
| if ($context) { | |||||
| $dialog->addHiddenInput('contextPHID', $context->getPHID()); | |||||
| } | |||||
| return $dialog; | |||||
| } | |||||
| } | } | ||||
Maybe throw this in setContextObject() to make the resulting stack trace more clear? (And this should probably include the phid_get_type of the phid to skip one extra step).