From 09d86c2d2031f171968395d7f91c34fb428781a1 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 23 Sep 2019 08:32:21 -0700 Subject: [PATCH] Unprototype "Facts" to clear Maniphest/chart fatals Summary: Ref T13279. Facts is still fairly rough, but not broken/policy-violating, so it can be unprototyped to fix the issue where Maniphest reports (which are now driven by Facts) don't work if prototypes are disabled. Test Plan: Viewed Maniphest reports and Project reports with prototypes on/off and Fact installed/uninstalled. Subscribers: yelirekim Maniphest Tasks: T13279 Differential Revision: https://secure.phabricator.com/D20822 --- .../PhabricatorFactApplication.php | 5 -- .../PhabricatorFactChartController.php | 41 +++++++------- .../PhabricatorFactHomeController.php | 53 +++---------------- .../engine/PhabricatorDemoChartEngine.php | 2 + .../controller/ManiphestReportController.php | 8 ++- ...abricatorProjectReportsProfileMenuItem.php | 5 ++ 6 files changed, 39 insertions(+), 75 deletions(-) diff --git a/src/applications/fact/application/PhabricatorFactApplication.php b/src/applications/fact/application/PhabricatorFactApplication.php index b3e0417754..f32d6839e2 100644 --- a/src/applications/fact/application/PhabricatorFactApplication.php +++ b/src/applications/fact/application/PhabricatorFactApplication.php @@ -22,15 +22,10 @@ final class PhabricatorFactApplication extends PhabricatorApplication { return self::GROUP_UTILITIES; } - public function isPrototype() { - return true; - } - public function getRoutes() { return array( '/fact/' => array( '' => 'PhabricatorFactHomeController', - 'chart/' => 'PhabricatorFactChartController', 'chart/(?P[^/]+)/(?:(?Pdraw)/)?' => 'PhabricatorFactChartController', 'object/(?[^/]+)/' => 'PhabricatorFactObjectController', diff --git a/src/applications/fact/controller/PhabricatorFactChartController.php b/src/applications/fact/controller/PhabricatorFactChartController.php index dc8c436dfe..4004f6f27c 100644 --- a/src/applications/fact/controller/PhabricatorFactChartController.php +++ b/src/applications/fact/controller/PhabricatorFactChartController.php @@ -1,13 +1,14 @@ getViewer(); $chart_key = $request->getURIData('chartKey'); - if ($chart_key === null) { - return $this->newDemoChart(); + if (!$chart_key) { + return new Aphront404Response(); } $engine = id(new PhabricatorChartRenderingEngine()) @@ -24,21 +25,28 @@ final class PhabricatorFactChartController extends PhabricatorFactController { $mode = $request->getURIData('mode'); $is_draw_mode = ($mode === 'draw'); - // TODO: For now, always pull the data. We'll throw it away if we're just - // drawing the frame, but this makes errors easier to debug. - $chart_data = $engine->newChartData(); + $want_data = $is_draw_mode; - if ($is_draw_mode) { - return id(new AphrontAjaxResponse())->setContent($chart_data); + // In developer mode, always pull the data in the main request. We'll + // throw it away if we're just drawing the chart frame, but this currently + // makes errors quite a bit easier to debug. + if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) { + $want_data = true; + } + + if ($want_data) { + $chart_data = $engine->newChartData(); + if ($is_draw_mode) { + return id(new AphrontAjaxResponse())->setContent($chart_data); + } } $chart_view = $engine->newChartView(); - $tabular_view = $engine->newTabularView(); - return $this->newChartResponse($chart_view, $tabular_view); + return $this->newChartResponse($chart_view); } - private function newChartResponse($chart_view, $tabular_view) { + private function newChartResponse($chart_view) { $box = id(new PHUIObjectBoxView()) ->setHeaderText(pht('Chart')) ->appendChild($chart_view); @@ -55,18 +63,7 @@ final class PhabricatorFactChartController extends PhabricatorFactController { ->appendChild( array( $box, - $tabular_view, )); } - private function newDemoChart() { - $viewer = $this->getViewer(); - - $chart = id(new PhabricatorDemoChartEngine()) - ->setViewer($viewer) - ->newStoredChart(); - - return id(new AphrontRedirectResponse())->setURI($chart->getURI()); - } - } diff --git a/src/applications/fact/controller/PhabricatorFactHomeController.php b/src/applications/fact/controller/PhabricatorFactHomeController.php index 56ffe3930b..666835db13 100644 --- a/src/applications/fact/controller/PhabricatorFactHomeController.php +++ b/src/applications/fact/controller/PhabricatorFactHomeController.php @@ -1,59 +1,20 @@ getViewer(); + $viewer = $this->getViewer(); - if ($request->isFormPost()) { - $uri = new PhutilURI('/fact/chart/'); - $uri->replaceQueryParam('y1', $request->getStr('y1')); - return id(new AphrontRedirectResponse())->setURI($uri); - } + $chart = id(new PhabricatorDemoChartEngine()) + ->setViewer($viewer) + ->newStoredChart(); - $chart_form = $this->buildChartForm(); - - $crumbs = $this->buildApplicationCrumbs(); - $crumbs->addTextCrumb(pht('Home')); - - $title = pht('Facts'); - - return $this->newPage() - ->setTitle($title) - ->setCrumbs($crumbs) - ->appendChild( - array( - $chart_form, - )); - } - - private function buildChartForm() { - $request = $this->getRequest(); - $viewer = $request->getUser(); - - $specs = PhabricatorFact::getAllFacts(); - $options = mpull($specs, 'getName', 'getKey'); - - $form = id(new AphrontFormView()) - ->setUser($viewer) - ->appendChild( - id(new AphrontFormSelectControl()) - ->setLabel(pht('Y-Axis')) - ->setName('y1') - ->setOptions($options)) - ->appendChild( - id(new AphrontFormSubmitControl()) - ->setValue(pht('Plot Chart'))); - - $panel = new PHUIObjectBoxView(); - $panel->setForm($form); - $panel->setHeaderText(pht('Plot Chart')); - - return $panel; + return id(new AphrontRedirectResponse())->setURI($chart->getURI()); } } diff --git a/src/applications/fact/engine/PhabricatorDemoChartEngine.php b/src/applications/fact/engine/PhabricatorDemoChartEngine.php index 71fec03309..218d30473d 100644 --- a/src/applications/fact/engine/PhabricatorDemoChartEngine.php +++ b/src/applications/fact/engine/PhabricatorDemoChartEngine.php @@ -17,6 +17,7 @@ final class PhabricatorDemoChartEngine array('shift', 256)); $function->getFunctionLabel() + ->setKey('cos-x') ->setName(pht('cos(x)')) ->setColor('rgba(0, 200, 0, 1)') ->setFillColor('rgba(0, 200, 0, 0.15)'); @@ -27,6 +28,7 @@ final class PhabricatorDemoChartEngine array('constant', 345)); $function->getFunctionLabel() + ->setKey('constant-345') ->setName(pht('constant(345)')) ->setColor('rgba(0, 0, 200, 1)') ->setFillColor('rgba(0, 0, 200, 0.15)'); diff --git a/src/applications/maniphest/controller/ManiphestReportController.php b/src/applications/maniphest/controller/ManiphestReportController.php index a5141e6122..f1940dfd80 100644 --- a/src/applications/maniphest/controller/ManiphestReportController.php +++ b/src/applications/maniphest/controller/ManiphestReportController.php @@ -35,8 +35,12 @@ final class ManiphestReportController extends ManiphestController { $nav->addLabel(pht('Open Tasks')); $nav->addFilter('user', pht('By User')); $nav->addFilter('project', pht('By Project')); - $nav->addLabel(pht('Burnup')); - $nav->addFilter('burn', pht('Burnup Rate')); + + $class = 'PhabricatorFactApplication'; + if (PhabricatorApplication::isClassInstalledForViewer($class, $viewer)) { + $nav->addLabel(pht('Burnup')); + $nav->addFilter('burn', pht('Burnup Rate')); + } $this->view = $nav->selectFilter($this->view, 'user'); diff --git a/src/applications/project/menuitem/PhabricatorProjectReportsProfileMenuItem.php b/src/applications/project/menuitem/PhabricatorProjectReportsProfileMenuItem.php index d1350238f5..8639b6d72c 100644 --- a/src/applications/project/menuitem/PhabricatorProjectReportsProfileMenuItem.php +++ b/src/applications/project/menuitem/PhabricatorProjectReportsProfileMenuItem.php @@ -34,6 +34,11 @@ final class PhabricatorProjectReportsProfileMenuItem return false; } + $class = 'PhabricatorFactApplication'; + if (!PhabricatorApplication::isClassInstalledForViewer($class, $viewer)) { + return false; + } + return true; }