Metadata support for lastX & previousX

[julienmoumne]

If you try to set date=last10&period=day as in /docs/analytics-api/reference/ you get

Date format must be: YYYY-MM-DD, or 'today' or 'yesterday' or any keyword supported by the strtotime function (see http://php.net/strtotime for more information)

Reproducible on the demo http://demo.piwik.org/index.php?module=API&method=API.getProcessedReport&idSite=7&period=day&date=last10&apiModule=UserCountry&apiAction=getCountry&format=xml&token_auth=anonymous

[mattab]

Metadata lack of support of previous|last is a known limitation but we would like to fix it, because it would be useful for the ImageGraph plugin #1721 (to plot evolution graph).

[julienmoumne]

and a requirement for #151

[julienmoumne]

Concerning metadata <prettyDate/> please confirm output for the following cases:

1.
&period=day
$date=last10

<prettyDate>Last 10 days</prettyDate>

2.
&period=month
$date=previous10

<prettyDate>Previous 10 months</prettyDate>

3.
&period=month
$date=2011-01-01,2011-04-01

<prettyDate>Months included in 1 Jan 11 - 1 Apr 11</prettyDate>

[julienmoumne]

Concerning <reportData/>, what is the best option

1.
<reportData>
    <row period="2011-05-16_to_2011-05-22">
        <row><row/>
        <row><row/>
        [...]
    <row/>
    [...]
<reportData/>

2.
<reportData>
    <row>
        <row><row/>
        <row><row/>
        [...]
    <row/>
    [...]
<reportData/>

+

<reportMetadata>
    <row>
        <period>2011-05-16_to_2011-05-22</period>
        <row>
           <idsubdatatable>1739</idsubdatatable>
        </row>
        <row>
           <idsubdatatable>1739</idsubdatatable>
        </row>
        [...]
    </row>
    [...]

Something completely different or a variant of above ideas ?

[mattab]

• For <prettyDate>, you can use the same output as Period=range. For example, last10 will be: 20 May 11 - 29 May 11

  For last3&period=month, it will be: March 2011 - May 2011

  For period=week: 9 May 11 - 29 May 11

• for <reportData> and <reportMetadata>, I think solution 1) is best. But, maybe we can stay consistent with existing output for last10

  Day: <result date="2011-04-30">

  week: <result date="2010-11-01 to 2010-11-07">

The code writing <result date=""> is in renderDataTableArray() in Xml renderer. It works on a Piwik_DataTable_Array. Maybe the metadata for last10&period=day for example, would build a Piwik_DataTable_Array? Or maybe you have a different plan (which might be OK!)?

[julienmoumne]

I like the idea of staying consistent with the general API.

Concerning <prettyDate>, it means we have the same output for two quite different calls:

1.
&period=day
&date=last10

<prettyDate>20 May 11 - 29 May 11</prettyDate>

2.
&period=range
&date=2011-05-20,2011-05-29

<prettyDate>20 May 11 - 29 May 11</prettyDate>

In the first case we give no indication there are multiple periods in the output.

[mattab]

I think it's fine having same string for these 2 outputs, because the consumers (RSS & Mobile App) will not I think use this prettyDate? Also, better not introduce new dates until we are sure we need them. :)

[julienmoumne]

Attachment:
multi_period_metadata.patch

[julienmoumne]

Here is the patch for review.

Info:

1. There is a change of output for dimension-less report both for JSON & XML formats. (from http://pastebin.com/tD4JY6V2 to http://pastebin.com/2QzDnqpq see 'reportData'). This has been done to unify the general API and the metadata API,

2. The metadata getProcessedReport API returning values have changed. Instead of returning PHP arrays for reportData and reportMetadata, it now uses Piwik_DataTable (for a single period) and Piwik_DataTable_Array (for multiple periods). This has been done to leverage Piwik_DataTable_Renderer (see changes in Piwik_API_ResponseBuilder),

3. Because of second point, both PDF and HTML report renderers have been updated,

4. Also a side effect of second point, some existing integration test XML files (concerning reports with dimension) have been updated because Piwik_DataTable_Renderer_Xml does not append an additional EOL caracter before </row>,

5. I reused an integration test scenario to test multiple period metadata reports. It only tests for period="day". If need be, I am willing to add more scenarios,

6. @review annotations are questions left for the reviewers.

[mattab]

Great stuff!! :)

• I think tests are covering all use cases? so it's great

  @review is this the correct place for this factoring ?
  Looks good

@review, should renderers provide a way to directly access the rendered body ?
Maybe if it makes the code easier... but not required, when there is only 1 use case
@review does $key need a particular treatment ?
Don't think so.. technically it should be escaped, but we will only set safe values so its fine
// @review does $value need a particular treatment ?
I'd be tempted to json_encode the value, but maybe it doesn't work as expected? this would be safer here

[julienmoumne]

(In [4879]) fixes #2414 refs matomo-org/matomo-mobile-2#2350 refs #5571

[julienmoumne]

(In [4900]) refs #2414 #2350 #151 - fix for bug introduced on json arrays and objects + adding first layer of unit tests