Store charts earlier and build them out a little later

Summary:
Ref T13279. Currently, we store a fairly low-level description of functions and datasets in a chart. This will create problems with (for example) translating function labels.

If you view a chart someone links you, it should say "El Charto" if you speak Spanish, not "The Chart" if the original viewer speaks English.

To support this, store a slightly higher level version of the chart: the chart engine key, plus configuration parameters. This is very similar to how SearchEngine works.

For example, the burndown chart now stores a list of project PHIDs, instead of a list of `[accumulate [sum [fact task.open <project-phid>]]]` functions.

(This leaves some serialization code with no callsites, but we may eventually have a "CustomChartEngine" which stores raw functions, so I'm leaving it for now.)

As a result, function labels provided by the chart engine are now translatable.

(Note that the actual chart is meaningless since the underlying facts can't be stacked like they're being stacked, as some are negative in some areas of their accumulation.)

Test Plan: {F6439121}

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: yelirekim

Maniphest Tasks: T13279

Differential Revision: https://secure.phabricator.com/D20504

src/applications/fact/chart/PhabricatorChartStackedAreaDataset.php

@@ -148,9 +148,6 @@ final class PhabricatorChartStackedAreaDataset
     $wire_labels = array();
     foreach ($functions as $function_key => $function) {
       $label = $function->getFunctionLabel();
-
-      $label->setName(pht('Important Data %s', $function_key));
-
       $wire_labels[] = $label->toWireFormat();
     }
 

src/applications/fact/engine/PhabricatorChartEngine.php

@@ -4,6 +4,10 @@ abstract class PhabricatorChartEngine
   extends Phobject {
 
   private $viewer;
+  private $engineParameters = array();
+
+  const KEY_ENGINE = 'engineKey';
+  const KEY_PARAMETERS = 'engineParameters';
 
   final public function setViewer(PhabricatorUser $viewer) {
     $this->viewer = $viewer;
@@ -14,26 +18,65 @@ abstract class PhabricatorChartEngine
     return $this->viewer;
   }
 
+  final protected function setEngineParameter($key, $value) {
+    $this->engineParameters[$key] = $value;
+    return $this;
+  }
+
+  final protected function getEngineParameter($key, $default = null) {
+    return idx($this->engineParameters, $key, $default);
+  }
+
+  final protected function getEngineParameters() {
+    return $this->engineParameters;
+  }
+
+  final public static function newFromChart(PhabricatorFactChart $chart) {
+    $engine_key = $chart->getChartParameter(self::KEY_ENGINE);
+
+    $engine_map = self::getAllChartEngines();
+    if (!isset($engine_map[$engine_key])) {
+      throw new Exception(
+        pht(
+          'Chart uses unknown engine key ("%s") and can not be rendered.',
+          $engine_key));
+    }
+
+    return clone id($engine_map[$engine_key]);
+  }
+
+  final public static function getAllChartEngines() {
+    return id(new PhutilClassMapQuery())
+      ->setAncestorClass(__CLASS__)
+      ->setUniqueMethod('getChartEngineKey')
+      ->execute();
+  }
+
   final public function getChartEngineKey() {
     return $this->getPhobjectClassConstant('CHARTENGINEKEY', 32);
   }
 
-  abstract protected function newChart();
+  final public function buildChart(PhabricatorFactChart $chart) {
+    $map = $chart->getChartParameter(self::KEY_PARAMETERS, array());
+    return $this->newChart($chart, $map);
+  }
 
-  final public function buildChart() {
+  abstract protected function newChart(PhabricatorFactChart $chart, array $map);
+
+  final public function buildChartPanel() {
     $viewer = $this->getViewer();
 
-    $chart = $this->newChart();
+    $parameters = $this->getEngineParameters();
+
+    $chart = id(new PhabricatorFactChart())
+      ->setChartParameter(self::KEY_ENGINE, $this->getChartEngineKey())
+      ->setChartParameter(self::KEY_PARAMETERS, $this->getEngineParameters());
 
     $rendering_engine = id(new PhabricatorChartRenderingEngine())
       ->setViewer($viewer)
       ->setChart($chart);
 
-    return $rendering_engine->getStoredChart();
-  }
-
-  final public function buildChartPanel() {
-    $chart = $this->buildChart();
+    $chart = $rendering_engine->getStoredChart();
 
     $panel_type = id(new PhabricatorDashboardChartPanelType())
       ->getPanelTypeKey();
@@ -45,4 +88,10 @@ abstract class PhabricatorChartEngine
     return $chart_panel;
   }
 
+  final protected function newFunction($name /* , ... */) {
+    $argv = func_get_args();
+    return id(new PhabricatorComposeChartFunction())
+      ->setArguments(array($argv));
+  }
+
 }

src/applications/fact/engine/PhabricatorChartRenderingEngine.php

@@ -113,6 +113,10 @@ final class PhabricatorChartRenderingEngine
     $chart = $this->getStoredChart();
     $chart_key = $chart->getChartKey();
 
+    $chart_engine = PhabricatorChartEngine::newFromChart($chart)
+      ->setViewer($this->getViewer());
+    $chart_engine->buildChart($chart);
+
     $datasets = $chart->getDatasets();
 
     $functions = array();

src/applications/fact/storage/PhabricatorFactChart.php

@@ -7,7 +7,7 @@ final class PhabricatorFactChart
   protected $chartKey;
   protected $chartParameters = array();
 
-  private $datasets;
+  private $datasets = self::ATTACHABLE;
 
   protected function getConfiguration() {
     return array(
@@ -54,35 +54,14 @@ final class PhabricatorFactChart
     return parent::save();
   }
 
-  public function setDatasets(array $datasets) {
+  public function attachDatasets(array $datasets) {
     assert_instances_of($datasets, 'PhabricatorChartDataset');
-
-    $dataset_list = array();
-    foreach ($datasets as $dataset) {
-      $dataset_list[] = $dataset->toDictionary();
-    }
-
-    $this->setChartParameter('datasets', $dataset_list);
-    $this->datasets = null;
-
+    $this->datasets = $datasets;
     return $this;
   }
 
   public function getDatasets() {
-    if ($this->datasets === null) {
-      $this->datasets = $this->newDatasets();
-    }
-    return $this->datasets;
-  }
-
-  private function newDatasets() {
-    $datasets = $this->getChartParameter('datasets', array());
-
-    foreach ($datasets as $key => $dataset) {
-      $datasets[$key] = PhabricatorChartDataset::newFromDictionary($dataset);
-    }
-
-    return $datasets;
+    return $this->assertAttached($this->datasets);
   }
 
   public function getURI() {

src/applications/project/chart/PhabricatorProjectBurndownChartEngine.php

@@ -5,52 +5,89 @@ final class PhabricatorProjectBurndownChartEngine
 
   const CHARTENGINEKEY = 'project.burndown';
 
-  private $projects;
-
   public function setProjects(array $projects) {
     assert_instances_of($projects, 'PhabricatorProject');
-
-    $this->projects = $projects;
-
-    return $this;
+    $project_phids = mpull($projects, 'getPHID');
+    return $this->setEngineParameter('projectPHIDs', $project_phids);
   }
 
-  public function getProjects() {
-    return $this->projects;
-  }
+  protected function newChart(PhabricatorFactChart $chart, array $map) {
+    $viewer = $this->getViewer();
 
-  protected function newChart() {
-    if ($this->projects !== null) {
-      $project_phids = mpull($this->projects, 'getPHID');
+    $map = $map + array(
+      'projectPHIDs' => array(),
+    );
+
+    if ($map['projectPHIDs']) {
+      $projects = id(new PhabricatorProjectQuery())
+        ->setViewer($viewer)
+        ->withPHIDs($map['projectPHIDs'])
+        ->execute();
+      $project_phids = mpull($projects, 'getPHID');
     } else {
-      $project_phids = null;
-    }
-
-    $argvs = array();
-    if ($project_phids) {
-      foreach ($project_phids as $project_phid) {
-        $argvs[] = array(
-          'accumulate',
-          array('fact', 'tasks.open-count.create.project', $project_phid),
-        );
-        $argvs[] = array(
-          'accumulate',
-          array('fact', 'tasks.open-count.status.project', $project_phid),
-        );
-        $argvs[] = array(
-          'accumulate',
-          array('fact', 'tasks.open-count.assign.project', $project_phid),
-        );
-      }
-    } else {
-      $argvs[] = array('accumulate', array('fact', 'tasks.open-count.create'));
-      $argvs[] = array('accumulate', array('fact', 'tasks.open-count.status'));
+      $project_phids = array();
     }
 
     $functions = array();
-    foreach ($argvs as $argv) {
-      $functions[] = id(new PhabricatorComposeChartFunction())
-        ->setArguments(array($argv));
+    if ($project_phids) {
+      foreach ($project_phids as $project_phid) {
+        $function = $this->newFunction(
+          'accumulate',
+          array('fact', 'tasks.open-count.create.project', $project_phid));
+
+        $function->getFunctionLabel()
+          ->setName(pht('Tasks Created'))
+          ->setColor('rgba(0, 0, 200, 1)')
+          ->setFillColor('rgba(0, 0, 200, 0.15)');
+
+        $functions[] = $function;
+
+
+        $function = $this->newFunction(
+          'accumulate',
+          array('fact', 'tasks.open-count.status.project', $project_phid));
+
+        $function->getFunctionLabel()
+          ->setName(pht('Tasks Closed / Reopened'))
+          ->setColor('rgba(200, 0, 200, 1)')
+          ->setFillColor('rgba(200, 0, 200, 0.15)');
+
+        $functions[] = $function;
+
+
+        $function = $this->newFunction(
+          'accumulate',
+          array('fact', 'tasks.open-count.assign.project', $project_phid));
+
+        $function->getFunctionLabel()
+          ->setName(pht('Tasks Rescoped'))
+          ->setColor('rgba(0, 200, 200, 1)')
+          ->setFillColor('rgba(0, 200, 200, 0.15)');
+
+        $functions[] = $function;
+      }
+    } else {
+      $function = $this->newFunction(
+        'accumulate',
+        array('fact', 'tasks.open-count.create'));
+
+      $function->getFunctionLabel()
+        ->setName(pht('Tasks Created'))
+        ->setColor('rgba(0, 200, 200, 1)')
+        ->setFillColor('rgba(0, 200, 200, 0.15)');
+
+      $functions[] = $function;
+
+      $function = $this->newFunction(
+        'accumulate',
+        array('fact', 'tasks.open-count.status'));
+
+      $function->getFunctionLabel()
+        ->setName(pht('Tasks Closed / Reopened'))
+        ->setColor('rgba(200, 0, 200, 1)')
+        ->setFillColor('rgba(200, 0, 200, 0.15)');
+
+      $functions[] = $function;
     }
 
     $datasets = array();
@@ -58,10 +95,7 @@ final class PhabricatorProjectBurndownChartEngine
     $datasets[] = id(new PhabricatorChartStackedAreaDataset())
       ->setFunctions($functions);
 
-    $chart = id(new PhabricatorFactChart())
-      ->setDatasets($datasets);
-
-    return $chart;
+    $chart->attachDatasets($datasets);
   }
 
 }