Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#2414 closed New feature (fixed)

Metadata support for lastX & previousX

Reported by: JulienM Owned by: JulienM
Priority: normal Milestone: 1.5 - Piwik 1.5
Component: Core Keywords:
Cc: Sensitive: no

Description

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

Attachments (1)

multi_period_metadata.patch (98.0 KB) - added by JulienM 3 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 3 years ago by matt (mattab)

  • Summary changed from lastX & previousX throws exception to Metadata support for lastX & previousX
  • Type changed from Bug to New feature

comment:2 Changed 3 years ago by matt (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).

comment:3 Changed 3 years ago by JulienM (JulienMoumne)

and a requirement for #151

comment:4 Changed 3 years ago by JulienM (JulienMoumne)

  • Owner set to JulienM

comment:5 Changed 3 years ago by JulienM (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>

comment:6 Changed 3 years ago by JulienM (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 ?

comment:7 Changed 3 years ago by JulienM (JulienMoumne)

  • Status changed from new to assigned

comment:8 Changed 3 years ago by matt (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

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!)?

comment:9 Changed 3 years ago by JulienM (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.

comment:10 Changed 3 years ago by matt (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. :)

comment:11 Changed 3 years ago by JulienM (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.

comment:12 Changed 3 years ago by matt (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

comment:13 Changed 3 years ago by JulienM (JulienMoumne)

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [4879]) fixes #2414 refs #2350 refs #151

comment:14 Changed 3 years ago by JulienM (JulienMoumne)

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

Note: See TracTickets for help on using tickets.