Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

ImageGraph: export reports as graph #1721

Closed
anonymous-piwik-user opened this Issue · 64 comments

5 participants

@anonymous-piwik-user

See http://piwik.org/docs/analytics-api/metadata/#toc-static-image-graphs

This plugin renders graphs from piwik reports.

Every report with existing columns 'label' and 'nb_visits' is applicable, at this time.
The label values are taken for the abscissa and 'nb_visists' values are drawn as value.

Such reports could be drawn as :
a. basic line
b. basic bar
c. 3d pie

The pChart Framework (http://pchart.sourceforge.net/) is used to render graphs as png.

@anonymous-piwik-user

Attachment: plugin source including pChart framework
ImageGraphPlugin.zip

@anonymous-piwik-user

Attachment: Example of continent report drawn as bar graph
Continent_BasicBar.png

@anonymous-piwik-user

Attachment: Example of continent report drawn as line graph
Continent_BasicLine.png

@anonymous-piwik-user

Attachment: Example of coutry report drawn as 3d pie graph
Country_3dPie.png

@anonymous-piwik-user

Attachment: Example of OS report drawn as 3d pie graph
OS_3dPie.png

@anonymous-piwik-user

Attachment: Example of visitsperlocaltime report drawn as bar graph
VisitsPerLocalTime_BasicBar.png

@robocoder
Collaborator

arni: very nice.

This is a great start to generating graphs on the server side. Other use cases include:

  • inclusion in pdf reports
  • non-Flash charting (#804)
@anonymous-piwik-user

On some php installations the pChart Framework throws an Error, while ErrorReporting is set to "GD".

For first fix, I set the ErrorReporting of pChart to off.

See the new API.php at Line 117

@anonymous-piwik-user

update: You can now set the column of report which is used as ordinate

@anonymous-piwik-user

Attachment: New Source of ImageGraph including pCart-Framwork
ImageGraph.zip

@anonymous-piwik-user

I tried to get ImageGraph to work but failed. Can you provide a few more examples of which reports work? Not just the PNG but the URL you use to generate the image?

I tried clicking on the example URL Piwik generates, got an error "uninitialized string offset 0/1 in pChart/pData.class", see
http://en.qi-hardware.com/piwik/index.php?module=API&method=ImageGraph.get&idSite=1&period=day&date=yesterday&apiModule=UserCountry&apiAction=getCountry&format=Html

Then I tried to get a simple line chart for unique visitors in last 30 days, got another error "undefined index dimension/processedMetrics in API.php", see
http://en.qi-hardware.com/piwik/index.php?module=API&method=ImageGraph.get&idSite=1&period=day&date=last30&apiModule=VisitsSummary&apiAction=get&format=Html

To get the public URLs to work, I commented out Piwik::checkUserIsNotAnonymous() and Piwik::checkUserHasViewAccess($idSite) in API.php - is that a security risk? (my data is all public anyway, I just don't want the server to be hijacked :-))

My goal is the line chart for unique visitors in last 30 days. But I'm a little lost in the various tables, metadata, etc. Any help or more documentation for ImageGraph (example URLs) is very appreciated.
Thanks,
Wolfgang

@anonymous-piwik-user

Replying to wolfgang:

I tried to get ImageGraph to work but failed. Can you provide a few more examples of which reports work? Not just the PNG but the URL you use to generate the image?

The ImageGraph should work with all reports, providing the label column and the dimension column when calling the API.getMetadata.

I tried clicking on the example URL Piwik generates, got an error "uninitialized string offset 0/1 in pChart/pData.class", see
http://en.qi-hardware.com/piwik/index.php?module=API&method=ImageGraph.get&idSite=1&period=day&date=yesterday&apiModule=UserCountry&apiAction=getCountry&format=Html

Im not sure, why this is not working on your copy.
This link works fine in my copy.
Could you please post your php and piwik version?

Then I tried to get a simple line chart for unique visitors in last 30 days, got another error "undefined index dimension/processedMetrics in API.php", see
http://en.qi-hardware.com/piwik/index.php?module=API&method=ImageGraph.get&idSite=1&period=day&date=last30&apiModule=VisitsSummary&apiAction=get&format=Html

This will not work, because the VisitsSummary.get doesnt provide the label column.

To get the public URLs to work, I commented out Piwik::checkUserIsNotAnonymous() and Piwik::checkUserHasViewAccess($idSite) in API.php - is that a security risk? (my data is all public anyway, I just don't want the server to be hijacked :-))

Im not very familiar with piwik, too.
So I dont know if this is a security risk.

My goal is the line chart for unique visitors in last 30 days. But I'm a little lost in the various tables, metadata, etc. Any help or more documentation for ImageGraph (example URLs) is very appreciated.

To get a report for the unique visitor of the last days, that could be drawn by the ImageGraph-Module you will need another plugin, which I will publish in the next few days.

Some links that should work:

index.php?module=API&method=ImageGraph.get&idSite=1&period=day&date=yesterday&apiModule=UserCountry&apiAction=getCountry&format=xml&token_auth=

index.php?module=API&method=ImageGraph.get&idSite=1&period=day&date=yesterday&apiModule=UserCountry&apiAction=getContinent&format=xml&token_auth=

index.php?module=API&method=ImageGraph.get&idSite=1&period=week&date=yesterday&apiModule=UserSettings&apiAction=getOS&format=xml&token_auth=

Thanks,
Wolfgang

@anonymous-piwik-user

(same wolfgang, forgot password and password reminder emails are not being sent out...)

Thanks for your help!
But... :-)

Some links that should work:
index.php?module=API&method=ImageGraph.get&idSite=1&period=day&date=yesterdayapiModule=UserCountry&apiAction=getCountry&format=xml&token_auth=

That is pretty much exactly the link I posted as not working, and if you would have clicked on it you would see the entire (long) error which I didn't want to paste here because you just need to click on it to see it.

Im not sure, why this is not working on your copy. This link works fine in
my copy. Could you please post your php and piwik version?

Piwik 1.0, php 5.3.2
Can you click on my link? Maybe the error message tells you more than it tells me. It seems to be related to the actual ImageGraph plugin, and/or Piwik table data.
Here's the link again
http://en.qi-hardware.com/piwik/index.php?module=API&method=ImageGraph.get&idSite=1&period=day&date=yesterday&apiModule=UserCountry&apiAction=getCountry&format=Html

To get a report for the unique visitor of the last days, that could
be drawn by the ImageGraph-Module you will need another plugin,
which I will publish in the next few days.

Oh great, I will be very much looking forward to it. When I find some time, I will dig deeper into my actual bug, which as I say is still there...

Some links that should work:

No they don't work. All 3 of them don't work. That is the bug I was trying to tell you about. You can click on my link then you see it. I will try to dig deeper if I can.

Thanks a lot for your help, good luck with this plugin and the new one, I think it's really cool!
Wolfgang

@anonymous-piwik-user

Hi Wolfgang.

Ive clicked on your posted links, but I cant reproduce them on my system.
Ive fixed some Code in the pChart-Framework.
Could you please try this new version?

Thanks arni

Replying to wolfgang2:

(same wolfgang, forgot password and password reminder emails are not being sent out...)

Thanks for your help!
But... :-)

Some links that should work:
index.php?module=API&method=ImageGraph.get&idSite=1&period=day&date=yesterdayapiModule=UserCountry&apiAction=getCountry&format=xml&token_auth=

That is pretty much exactly the link I posted as not working, and if you would have clicked on it you would see the entire (long) error which I didn't want to paste here because you just need to click on it to see it.

Im not sure, why this is not working on your copy. This link works fine in
my copy. Could you please post your php and piwik version?

Piwik 1.0, php 5.3.2
Can you click on my link? Maybe the error message tells you more than it tells me. It seems to be related to the actual ImageGraph plugin, and/or Piwik table data.
Here's the link again
http://en.qi-hardware.com/piwik/index.php?module=API&method=ImageGraph.get&idSite=1&period=day&date=yesterday&apiModule=UserCountry&apiAction=getCountry&format=Html

To get a report for the unique visitor of the last days, that could
be drawn by the ImageGraph-Module you will need another plugin,
which I will publish in the next few days.

Oh great, I will be very much looking forward to it. When I find some time, I will dig deeper into my actual bug, which as I say is still there...

Some links that should work:

No they don't work. All 3 of them don't work. That is the bug I was trying to tell you about. You can click on my link then you see it. I will try to dig deeper if I can.

Thanks a lot for your help, good luck with this plugin and the new one, I think it's really cool!
Wolfgang

@anonymous-piwik-user

Attachment: Fixed Code in included pChart - Framework
ImageGraph.2.zip

@anonymous-piwik-user

wow, yes! I looked at what you changed and would not have thought that it makes a difference, but the country chart works now! (you can click on the old link)

Strange though, in pChart.class, you change a $this->Data to $this->data (lower-case), even though everywhere else it's always upper-cased. It looks like you introduce a bug but you fix it... Maybe there are both a lower-case and upper-case data/Data somewhere?

Anyway, you know what you are doing. Country graph is working for me now, and I am eagerly awaiting the one you mentioned that could display unique visitors.
Thanks a lot for your fix! Wolfgang

@anonymous-piwik-user

Hi Wolfgang.

You can check now the ticket #1773 to install the VisitsSummaryEvolution plugin.
After that you can test the link

index.php?module=API&method=ImageGraph.get&idSite=1&period=month&date=yesterday&apiModule=VisitsSummaryEvolution&apiAction=getVisitsSummaryDailyEvolution

You should receive a line chart displaying the visitors of each day for the current month.

arni

Replying to wolfgang2:

wow, yes! I looked at what you changed and would not have thought that it makes a difference, but the country chart works now! (you can click on the old link)

Strange though, in pChart.class, you change a $this->Data to $this->data (lower-case), even though everywhere else it's always upper-cased. It looks like you introduce a bug but you fix it... Maybe there are both a lower-case and upper-case data/Data somewhere?

Anyway, you know what you are doing. Country graph is working for me now, and I am eagerly awaiting the one you mentioned that could display unique visitors.
Thanks a lot for your fix! Wolfgang

@mattab
Owner

arni, are you interested to get this code contributed to core? I would like to do a full code review, if you confirm that you are ready for it, I'll get started :)

It looks like a great start!

@anonymous-piwik-user

Yeah. Of course.

Replying to matt:

arni, are you interested to get this code contributed to core? I would like to do a full code review, if you confirm that you are ready for it, I'll get started :)

It looks like a great start!

@robocoder
Collaborator

The pChart library contains Microsoft fonts (i.e., tahoma.ttf in pChart 1.27d and verdana.ttf in pChart 2) that don't allow for this form of redistribution. The font should be removed from the package. (The user can be instructed to install the missing font, if required.)

@sgiehl
Collaborator

Is anyone still working on this plugin?
It would be nice to use something like that for Piwik Mobile in the future.

@anonymous-piwik-user

I will move all fonts out, except of GeoSansLight. This should be ok. or am I wrong?!

Replying to vipsoft:

The pChart library contains Microsoft fonts (i.e., tahoma.ttf in pChart 1.27d and verdana.ttf in pChart 2) that don't allow for this form of redistribution. The font should be removed from the package. (The user can be instructed to install the missing font, if required.)

@anonymous-piwik-user

Im on. Just a bit busy at this time.
I think, I will upload a new version in the next two weeks.

Replying to SteveG:

Is anyone still working on this plugin?
It would be nice to use something like that for Piwik Mobile in the future.

@robocoder
Collaborator

If labels are translated, what's the coverage wrt Piwik translations? Is there a fallback to English?

@anonymous-piwik-user

Replying to vipsoft:

If labels are translated, what's the coverage wrt Piwik translations? Is there a fallback to English?

I do not really understand your answer.

@mattab
Owner

vipsoft, the ImageGraph plugin is using the Metadata API so it doesn't deal with any translation it is all automatic

arni, I have sent you an email for discussing future of the plugin and how we can integrate it

@robocoder
Collaborator

Sorry, I was posting from my iPhone. I'll try to be clearer.

Since ImageGraph doesn't deal with translations directly, then there's no fallback to English if characters used by a (translated) label can't be rendered using the available font(s).

For example, GeoSansLight appears to be limited to Latin-1 characters (accented characters) and some special symbols (e.g., Euro). It would not be able to render the translated labels for Amharic, Arabic, Belarusian, Bulgarian, Hellenic, Hebrew, Japanese, Georgian, Korean, Russian, Telugu, Thai, Ukranian, or Chinese.

@robocoder
Collaborator

WRT maintenance of third-party libraries:

  • pChart 1.27d is the last of the 1.x series and no longer supported. It is GPLv1 or later.
  • pChart 2 is the current stable dev branch. It is available under either a GPLv3 or commercial license.
  • Unlike tcpdf, both versions of pChart requires the server have the freetype library (e.g., /usr/lib/libfreetype.so) since it calls the gd extension's imagettf* functions.
@mattab
Owner

I have suggested to Arni the following features:

  • the graphs should look as close as possible to the existing flash graph design: same default color, same font size, same vertical grids, etc.
  • possibility to customize by GET Parameter the size (width/height) of the graph
  • possibility to customize the colors, font-size, etc. by GET Parameter as well.
  • default color, font-size etc. would be set as a class const DEFAULT_COLOR = '553344'; etc.
  • We don't need the 3D pie in fact because it is difficult to see the data
  • the supported graph types should be: evolution line chart, 2D pie chart, vertical bar graph.
  • to support Evolution line chart (to plot for example Number of visits over the last 100 days), the graph will be drawn when there is no dimension. This corresponds to the "Simple metrics reports" in http://piwik.org/docs/analytics-api/metadata/#toc-listing-all-the-metadata-api-functions
  • produce a page that will plot a graph of each type, for ALL reports, by simply looping over available metadata reports using the metadata function listing all functions See for example a script a little bit similar to list all widgets in piwik/misc/iframeWidget_localhost.php The page will be used as a visual test that all graphs look good, and we can easily open this page after changing some code, to check for regression :)
  • make sure that the lib used, pCharts, is not modified so we can later upgrade the lib without problem
  • reuse the fonts from the TCPDF directory, because they are already there. Also, there is a bit of code already in TCPDF to choose the best font based on laanguage (so it works in japanese, chinese russian etc.). Don't worry about refactoring TCPDF I can help with this, so we can have a Piwik helper function that will return the font path based on the parameter $language (that will be used in both TCPDF and ImageGraph)

one update

  • the Evolution chart (line chart X/Y over time) is available only for reports without dimensions (ie. to show a metric evolving over time).
  • the Vertical bar graph (and other graphs) are only available for reports with a dimension, to show a given set of dimensions at a given time

Arni confirmed he is interested in finishing the work so this is great news!! then we shall modify PDF reports and of course, Piwik Mobile :)

@mattab
Owner

See also the feature for Metadata improvements, Metadata should return the "best" chart to show by default for a given report: #1491

And maybe the full URL to the graph, so that various clients don't have to have the logic to construct the URL themselves

@anonymous-piwik-user

Attachment: New refactored 3dPie
Resolution_3dPie.png

@anonymous-piwik-user

Attachment: New refactored BasicPie
Resolution_BasicPie.png

@anonymous-piwik-user

Attachment: New refactored BasicBar
VisitsPerLocalTime_BasicBar.2.png

@anonymous-piwik-user

Attachment: New refactored BasicLine
VisitsSummary_BasicLine_Last30.png

@anonymous-piwik-user

Attachment: Bugfix: Throw Error when a report without dimension should be drawn as PieGraph
ImageGraph.4.zip

@anonymous-piwik-user

Replying to matt:

Hi there.

I think and hope, that the first milestone is arrived.
Here the things Ive done:

  • the graphs should look as close as possible to the existing flash graph design: same default color, same font size, same vertical grids, etc.

I think, thats done. Ive found some colors in core/Visualization/... . Was that right? Only the font is not retrieved via tcpdf, yet.

  • possibility to customize by GET Parameter the size (width/height) of the graph

Done. Parameter Names are 'width' and 'height'

  • possibility to customize the colors, font-size, etc. by GET Parameter as well.

Done. Parameter Names are 'fontSize', 'hexColorLineGraph', 'hexColor0BarGraph', 'hexColor1BarGraph', 'hexColor0PieGraph', 'hexColor1PieGraph', 'hexColor2PieGraph', 'hexColor3PieGraph', 'hexColor4PieGraph', 'hexColor5PieGraph', 'hexColor6PieGraph'

  • default color, font-size etc. would be set as a class const DEFAULT_COLOR = '553344'; etc.

Done in the Piwik_ImageGraph_API.

  • We don't need the 3D pie in fact because it is difficult to see the data

I want to keep the 3dPie, because I need it for my PdfExport_Plugin.

Done. The Line Chart could also display reports with given dimension. This is also for my PdfExport_Plugin.

  • produce a page that will plot a graph of each type, for ALL reports, by simply looping over available metadata reports using the metadata function listing all functions See for example a script a little bit similar to list all widgets in piwik/misc/iframeWidget_localhost.php The page will be used as a visual test that all graphs look good, and we can easily open this page after changing some code, to check for regression :)

Done. You reach the page by calling /index.php?module=ImageGraph&action=index&idSite={$idSite}&period={$period}&date={$date}

  • make sure that the lib used, pCharts, is not modified so we can later upgrade the lib without problem

For getting as close as possible to the flash-chart-style, Ive had to extend the pChart-class for overriding some few functions. So updating is not that easy, at this time. Maybe it would be easier to develop a little charting-framework on our own in future.

  • reuse the fonts from the TCPDF directory, because they are already there. Also, there is a bit of code already in TCPDF to choose the best font based on laanguage (so it works in japanese, chinese russian etc.). Don't worry about refactoring TCPDF I can help with this, so we can have a Piwik helper function that will return the font path based on the parameter $language (that will be used in both TCPDF and ImageGraph)

Not done yet.

one update

  • the Evolution chart (line chart X/Y over time) is available only for reports without dimensions (ie. to show a metric evolving over time).

I do not agree. I have a Plugin wrapping the VisitsSummary.get(date = lastX) into a report with date as dimension for my PdfExport_Plugin. So I will need the LineChart for reports providing a dimension, too.

  • the Vertical bar graph (and other graphs) are only available for reports with a dimension, to show a given set of dimensions at a given time

I do not agree. A report without dimension could also be drawn as BarGraph.

Arni confirmed he is interested in finishing the work so this is great news!! then we shall modify PDF reports and of course, Piwik Mobile :)

@mattab
Owner

arni, thanks for the new version, excellent progress!

Sorry for the delay, wanted to get 1.3 out of the way to allow committing this new plugin to SVN. I have some more feedback before it is ready, but it's looking on the right track!

Code review:

  • the graph look great! now that PDF are also using the same colors as the UI (#2320), this will be a really good user experience.
  • it appears labels are encoded in the graph legends (for example & appears as &) because they are encoded in the metadata output I guess. They should be decoded before used in the graphs. You can use Piwik_Common::unsanitizeInputValue() to decode them.
  • when graph has small width, label overlap over each other (small width is Piwik Mobile use case). it is OK to write legends on X axis only every N steps when graph has width < minimum size to show all labels, so it looks good also in small width (see attached file).
  • Naming of public API
    • instead of graphType=1 2 3 or 4 I propose following names, consitent with existing names: pie, verticalBar, evolution
    • rename $ordinateColumn to $column (consistent with other api), $graphOutput to $outputType (consistent with EmailReports api)
  • API: all parameters should default to false, then in the function code you can test if(empty()) for each, and assign default value. This is because of the way the API work. * //If the desired $ordinateCokumn isnot present, -> in this case, I think better to throw the exception instead of returning another data set. Because this is API, it must obey the request because user of API are supposed to request only metrics that exist in the metadata output * really know if the dimension is unlimited (like Browser,Country,etc.) or not (like VisitTime, PagesPerVisit). -> good point, I will do this in #2336 and add a key in metadata output for this information
  • get_class(), better to use if( $table instanceof Piwik_DataTable_Array )
  • for code style we put && and || conditions on a new line for each condition
  • //If there is no dimension present we have to fetch the report directly, what is the reason for doing the direct call? it is important not to do this call, because we can not know if the API is indeed implementing the parameters in this order. It is important that the plugin only uses the Metadata API. When there is no dimension, the graph could draw the metric for the last N periods before the specified date. I can help with this code maybe this is the problem (let me know)
  • good to remove all ?> at end of PHP files because it can cause 'white space' and then "headers already sent" issue
  • I think it is going to be complicated to upgrade and maybe maintain in the future, because the code overriden in ImageGraphObject is very big. Would it be possible to use latest version of pCharts http://www.pchart.net/ or is it a lot of work to upgrade to this version? it would be great for the future of this feature :)
    • in index.tpl, please replace the iframe code with &lt;iframe style="width:1100px;height:400px;" src="index.php? -> bigger width and height so that the graph can fit and is not downsized by browser, and removed /index.php in case piwik is installed in subdirectory

Few related TODOs pending for Piwik core:

  • #2336 Metadata tell fixed sized report or not
  • TCPDF font refactoring
  • #2335 Small bug in label name "Others" after truncate
  • #2334 When flash not enabled in the browser, fallback to show static image
  • Metadata to return best graph type for this report, and maybe full URL to graph (ticket)

and one question: what is missing from the existing PDF report, that you would like to add, so that we can deprecate PdfExport_Plugin ? your feedback is most welcome :) especially now that we will have the HTML reports as well (see #2318) it would be good that the report plugin does all features needed for you and all users.

I will review much faster the next patch.
Cheers

@mattab
Owner

Attachment: Graph for Piwik Mobile labels overlap
piwik mobile graph.png

@mattab
Owner

Per suggestion, new field is now in metadata output 'constantRowsCount' see #2336
only 3 reports are affected, see: http://dev.piwik.org/trac/changeset/4510#file2

@mattab
Owner

One more feedback from thomas, Piwik Mobile developer:

  • Smallest size is about 150x240, on most devices the size will be about 150x300 or about 150x460 / 200x460. If device is in landscape mode, the size of the graph could be up to about 150x800px iPhone: 300x600 and the font size has to be twice as high. iOS scales it then automatically down to 150x300 so that it is really sharp.

it would be great to adjust minimum width/height to these sizes, and check that graphs look good with such small pixels.

  • font size should also be a parameter since iOs scales down
  • Actually, we have also a fullscreen view of the graph. These sizes are about 200x280 (240x320 display: I think less users have this size so it must look not that good) , 300x440 (on a 320x480 display) , 460x760 (480x800px display). On the iPhone4 we again have to render the graph in about 600x880 and display it in 300x440px.
@anonymous-piwik-user

Hi.

Here my comments.

Replying to matt:

arni, thanks for the new version, excellent progress!

Sorry for the delay, wanted to get 1.3 out of the way to allow committing this new plugin to SVN. I have some more feedback before it is ready, but it's looking on the right track!

Code review:

  • the graph look great! now that PDF are also using the same colors as the UI (#2320), this will be a really good user experience.

  • it appears labels are encoded in the graph legends (for example & appears as &) because they are encoded in the metadata output I guess. They should be decoded before used in the graphs. You can use Piwik_Common::unsanitizeInputValue() to decode them.
    Done.

  • when graph has small width, label overlap over each other (small width is Piwik Mobile use case). it is OK to write legends on X axis only every N steps when graph has width < minimum size to show all labels, so it looks good also in small width (see attached file).
    Im computing the 'N' taking width, fontSize, maxLength(Labels) and count(Labels) into account. Im not really sure if this fits all dimensions and fontsizes, but its a better approach than in the last version. Maybe its better, not to compute a 'N', but only to draw a label if the preceding label ends X pixels before the new one starts. This should be tested on mobile version, which solution to choose.

  • Naming of public API

    • instead of graphType=1 2 3 or 4 I propose following names, consitent with existing names: pie, verticalBar, evolution
      Done. Names are 'evolution', 'verticalBar', '3dPie' and 'pie'

    • rename $ordinateColumn to $column (consistent with other api), $graphOutput to $outputType (consistent with EmailReports api)
      Done.

  • API: all parameters should default to false, then in the function code you can test if(empty()) for each, and assign default value. This is because of the way the API work.
    Done.

    *
    //If the desired $ordinateCokumn isnot present,
    -> in this case, I think better to throw the exception instead of returning another data set. Because this is API, it must obey the request because user of API are supposed to request only metrics that exist in the metadata output
    Done.

    *
    really know if the dimension is unlimited (like Browser,Country,etc.) or not (like VisitTime, PagesPerVisit).
    -> good point, I will do this in #2336 and add a key in metadata output for this information
    Implemented check for 'constantRowsCount'. See #2336 .

  • resetting GET is important indeed, in case this is called via [http://piwik.org/docs/analytics-api/calling-techniques/#toc-call-the-piwik-api-in-php PHP internal method] and other functions are called after this one. Best way to reset is to save the full _GET at the start, and then do
    $_GET = $saveGET;
    at the end (safer than restoring each in case you forget one)
    Done.

  • get_class(), better to use if( $table instanceof Piwik_DataTable_Array )
    Done.

  • for code style we put && and || conditions on a new line for each condition
    Done.

  • //If there is no dimension present we have to fetch the report directly,
    what is the reason for doing the direct call? it is important not to do this call, because we can not know if the API is indeed implementing the parameters in this order. It is important that the plugin only uses the Metadata API. When there is no dimension, the graph could draw the metric for the last N periods before the specified date. I can help with this code maybe this is the problem (let me know)
    I need to do this, because I didnt find a way to get a report through the metadataAPI, which has basically no dimension, but which gets a dimension by setting 'date=lastX|previousX'.
    For example, if I call the VisitsSummary.get with 'period=day' and 'date=lastX', I would expect to get X rows, one for each day.
    But the metadataAPI aggregates the X rows to one row, so I will never get a dimension for the evolution chart. Or have I missed something?

  • good to remove all ?> at end of PHP files because it can cause 'white space' and then "headers already sent" issue
    Done.

  • I think it is going to be complicated to upgrade and maybe maintain in the future, because the code overriden in ImageGraphObject is very big.
    Would it be possible to use latest version of pCharts http://www.pchart.net/
    or is it a lot of work to upgrade to this version? it would be great for the future of this feature :)
    I think, this is doable.
    But I would prefer to get this plugin working well with the current pChart, to have a robust basis for migrating to pChart 2.

  • in index.tpl, please replace the iframe code with
    &lt;iframe style="width:1100px;height:400px;" src="index.php?
    -> bigger width and height so that the graph can fit and is not downsized by browser, and removed /index.php in case piwik is installed in subdirectory
    Done.

Few related TODOs pending for Piwik core:

  • #2336 Metadata tell fixed sized report or not
  • TCPDF font refactoring
  • #2335 Small bug in label name "Others" after truncate
  • #2334 When flash not enabled in the browser, fallback to show static image
  • Metadata to return best graph type for this report, and maybe full URL to graph (ticket)

and one question: what is missing from the existing PDF report, that you would like to add, so that we can deprecate PdfExport_Plugin ? your feedback is most welcome :) especially now that we will have the HTML reports as well (see #2318) it would be good that the report plugin does all features needed for you and all users.
My PdfExport_Plugin provides a way to set the order and type(table, ImageGraph) of the reports in the pdf. See http://issues.piwik.org/attachments/1721/AnalyticsReport-Monday_18_April_2011-ADAA_Site.pdf for example.

I will review much faster the next patch.
Cheers

Best arni

@mattab
Owner

good work arni, I have sent feedback by email because few attached files!

@JulienMoumne
Collaborator

(In [4879]) fixes #2414 refs piwik/piwik-mobile-2#2350 refs #5571 - Metadata support for lastN/previousN

@mattab
Owner

Status update: Arnfried is working on the last changes to the plugin.

Other TODOs:

  • matt: build Metadata output to offer the URL to the graph in the metadata, for Piwik mobile and PDF integration. Notes:
    • one preferred type bar > pie.
    • Exclude VisitFrequency::get, VisitsSummary::get from graph.
    • Evolution graphs for "Metadata without dimension" VisitsSummary.get, Goals.get, etc.) and Bar graph (or Pie chart) for "Metadata with dimension" (keywords, countries, etc)
    • When you integrate them in processed metadata report: Please, instead of just display which graph-type shall be used, there should be the url to the graph without formatting options, but with date, period, apiModule, ... Cause otherwise it'll be very difficult to handle/generate the graph URL's in Piwik Mobile when they change in future versions. I'd just add some colors, font size and size parameters in Piwik Mobile.

Then:

  • integrate graph in email and pdf reports
  • Mobile: integrate graphs in Piwik Mobile #2637
@mattab
Owner

(In [5292]) Fixes #1721

Committing work by Arnfried (EXCELLENT!)
I made a few minor modifications, mostly style.

BIG kuddos to arni for his amazing work on the plugin and
integrating all feedback from code reviews and Piwik Mobile integration brainstorms!!

@mattab
Owner

For all improvements suggestions to ImageGraph, please see the new ticket #2704

@mattab
Owner

(In [5296]) Fixes #2670

  • Metadata output now contains "imageGraphUrl"
  • the function used by piwik mobile and others, API.getReportMetadata now accepts period and date parameters
    • for "Evolution graphs" (goals, visitssummary, etc.) we must "plot" multiple days previous to the currently selected date. The metadata APi automatically does the $date parameter rewrite, when period != range, and date is not already a range. Therefore the imageGraphUrl can be used as is.
  • New page available at: index.php?module=ImageGraph&action=index&idSite=1&period=month&date=2011-04-17
    • This page will call metadata APi then display the graph for all existing reports. This is a useful debug/test page to view all graphs as they will be displayed in Piwik Mobile (and PDF reports later refs #2706

Refs #1721

  • Showing less vertical bars in graph by default
  • Displaying empty graph when there is no data
@mattab
Owner

Doc added in Analytics Graphs

@mattab
Owner

For feature requests and other ideas check out: #2704

@JulienMoumne
Collaborator

(In [5415]) fixes #2706

  • refs #2318, #71 : Graphs now supported
  • refs #2670, #898 : Restoring VisitsSummary report metadata before eCommerce
  • refs #1721 : Additional security, filename fixes and new internal outputType
@JulienMoumne
Collaborator

(In [5582]) * fixes #2706, #2828, #2704, refs #1721, #2637, #2711, #2318, #71 : horizontal static graph implemented

  • fixes #2788, refs #2670, #1721, #2637, #2711 : default graph type logic moved to ImageGraph API, improved date/period logic, new parameter graphs_default_period_to_plot_when_period_range
  • fixes #2704, #2804, refs #1721 : pChart updated to 2.1.3, pChart code removed from Piwik code, OOP refactoring, support for unifont.ttf if present in ImageGraph/fonts, testAllSizes now uses report metadata ImageGraphUrl
  • refs #5491 : space between report title and report table reduced to avoid page overflow
  • refs #2829 : TODO display percentages
  • r5544, r5547, r5549 merged
@JulienMoumne
Collaborator

(In [5596]) refs #1721

  • dimension-less graphs with no data now display 'No data for this graph'
  • regression in graph filename
  • narrowing down Piwik_ImageGraph_StaticGraph_Exception requirements to handle early exceptions
  • increasing min space between horizontal values in horizontal graphs
@JulienMoumne
Collaborator

(In [5597]) refs #1721

  • adding PDF/HTML reports image size to testAllSizes
@JulienMoumne
Collaborator

(In [5601]) refs #1721

  • exception now thrown for dimension-less report without multiple periods
  • enhanced logo positioning for horizontal graphs
  • enhanced legend positioning for all graphs
@JulienMoumne
Collaborator

(In [5634]) refs #1721

  • display 'No data for this graph.' when the report does not contain at least one value different than 0
  • Piwik_ImageGraph_StaticGraph_Exception now renders an image with the size specified by the width and height parameters
  • legend positioning now works for all font sizes
  • enhanced label truncation for horizontal graphs
@JulienMoumne
Collaborator

(In [6112]) refs #1721 - wrong package in php doc

@JulienMoumne
Collaborator

(In [6947]) refs #1721 refs #3013

  • show label icons in evolution graph
  • smaller legend font size
  • draw colored shadows in legend
  • vertical legend for evolution graphs
  • enhanced label skipping for daily evolution graph
@JulienMoumne
Collaborator

(In [6951]) refs #1721 refs #3013

  • better legend font sizes
  • legend shadow only when plotting multiple series
  • more shadow padding
@JulienMoumne
Collaborator

(In [7081]) refs #1721

  • new API parameter : $legendFontSize
  • increased legend font size
@JulienMoumne
Collaborator

(In [7139]) refs #1721 increase max width & height for tablet screens

@anonymous-piwik-user anonymous-piwik-user added this to the 1.6 Piwik 1.6 milestone
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.