Page MenuHomePhabricator

Store charts earlier and build them out a little later
ClosedPublic

Authored by epriestley on May 8 2019, 4:35 PM.

Details

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

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.May 8 2019, 4:35 PM
epriestley requested review of this revision.May 8 2019, 4:36 PM

Roughly, old flow was:

  • Turn the parameters ("projects = X") into functions.
  • Turn the functions into a stored chart object.
  • Save the chart.
  • ~ ajax magic ~
  • Load the chart.
  • Render the functions into a chart.

The new flow is:

  • Put the parameters ("projects = X") on a chart.
  • Save the chart.
  • ~ ajax magic ~
  • Load the chart.
  • Turn the stored parameters into functions.
  • Render the functions into a chart.
src/applications/fact/engine/PhabricatorChartEngine.php
71–73

This is the first half of the actual change: we no longer build all the functions here. We just store chart parameters and get a chart key back.

src/applications/fact/engine/PhabricatorChartRenderingEngine.php
118

This is the other half of the actual change: now, we turn the parameters into a real chart after loading them out of the database.

src/applications/project/chart/PhabricatorProjectBurndownChartEngine.php
38–41

These function labels were the main thing I wanted here. Before, we'd save the chart after this, so we would have stored the actual text (like "Tasks Created") in the database.

Now, we save the chart before this, so we'll localize the text properly.

amckinley accepted this revision.May 21 2019, 10:40 PM
This revision is now accepted and ready to land.May 21 2019, 10:40 PM