<imageGraphUrl>
index.php?module=API&method=ImageGraph.get&idSite=1&apiModule=VisitsSummary&apiAction=get&token_auth=x&graphType=evolution&period=range&date=2011-01-01,2012-01-01
</imageGraphUrl>
at line: https://github.com/piwik/piwik/blob/master/plugins/ImageGraph/ImageGraph.php#L68
With the addition of a new graph type in ticket:2704#comment:11 and the refactoring of ImageGraph plugin, I would like to precisely define the following points:
What are the default graph types for each report and each period type (single period and multiple periods)
pretty much all that is currently, except the 2 bugs in this ticket: when period=range
However on trunk, the following debug URL will show all graphs as per imageGraphUrl:
You can see that it loads the Visits summary graph rewritten as: &period=month&date=2009-05-01,2011-10-31
Which seems to look good?
So is there really a bug with period=month?
What are the applicable graph types for each report and each period type. As I've understood, evolution is not applicable to reports with dimension. In the ImageGraph Plugin, this logic is buried way deep in the code at line https://github.com/piwik/piwik/blob/master/plugins/ImageGraph/API.php#L301. I would like to recode and move-up that logic to make it clearer and more maintainable.
The logic seems OK in this function, however this function is bloated and getting complicated. Re-factor sounds good
The default graph type is currently part of the ImageGraphUrl string within reports metadata. Wouldn't it be more suitable to remove it from there and add the 'default graph type' logic in https://github.com/piwik/piwik/blob/master/plugins/ImageGraph/API.php#L74 when the type is not specified as a parameter ? Currently, the logic is broken see : https://github.com/piwik/piwik/blob/master/plugins/ImageGraph/API.php#L105
If it's easier to have the logic in the get method then sure, +1 to remove the parameter from imageGraphUrl and put it to default to false in the API function parameter list
refactoring and simplicity are good ;)
Concerning
when period=range, should be rewritten to period=day in evolution graphs
* Do we add a new parameter to decide which subperiod to plot, or default to "day" (configuration via config file?)
I've currently set period='day' which seems to me a reasonable default.
Shall I add a new entry in the configuration file ?
Shall I add a new entry in the configuration file ?
Yes please it sounds the best approach (graphs_default_period_to_plot_when_period_range ?)
(In [5582]) * fixes #2706, #2828, #2704, refs #1721, #2637, #2711, #2318, #71 : horizontal static graph implemented