Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Display Graphs in scheduled Email reports (PDF / HTML) #2706

Closed
mattab opened this Issue · 31 comments

3 participants

@mattab
Owner

Now that ImageGraph plugin is included in core (see #2670), we could add graphs in the Email reports.

Proposal graphs feature in Email reports:

  • By default, we could display evolution graphs for all "Simple metrics report" ie. all the "Goals", "Visits summary", where the "Evolution" graph actually adds analytics value. Explanation:
    • a bar graph showing "Top browsers" or "Top keywords" is a duplicate of the dataset presented in the table: it does not add anything useful except a new visualization.
      • however a line graph showing "Ecommerce conversions" over the last 30 days (or months), or a line graph showing "Visits" for the last 30 weeks is very interesting and useful to have by default in all PDF. Because the report only shows by default the "Current month" visits/ecommerce orders...
  • Also, some users like looking at graphs. So, for them, we could add an option in the report User interface: "Also include a Graph next to each dataset. Note: by default, some reports display an historical line graph (Goals, Ecommerce, Visits), but when you enable this feature all the other reports will display a vertical bar graph."

Ideally the graph and data set would both fit on 1 page but I'm not sure how we can make it look nice.

Thoughts?

@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
@JulienMoumne
Collaborator

Concerning

add an option in the report User interface: "Also include a Graph next to each dataset. Note: by default, some reports display an historical line graph (Goals, Ecommerce, Visits), but when you enable this feature all the other reports will display a vertical bar graph."
and
Ideally the graph and data set would both fit on 1 page but I'm not sure how we can make it look nice.

For non-historical reports, could we simply only display graphs instead of tables ?

Option text would be :

For non-historical reports, display a graph instead of a table

In that case I understand we need to come up with better wording.

We could even go further and keep an option to insert both tables and graphs.

It all boils down to users' needs.

@mattab
Owner

Interesting idea. In this case I think we could offer these options:

  • For non historical reports:
    • Display Reports tables only (default)
    • Display Graphs only
    • Display Graphs and tables

I agree the wording could be better but I can't think of a better one now?!

@JulienMoumne
Collaborator

Does it impair user understandability to only allow those settings for a specific subset of reports (ie. non-historical reports) ?

@mattab
Owner

I think it's OK to "force" always display evolution graph above "VisitsSummary" "Goals summary", etc. since they are very useful (and I really see it as a missing feature that they are currently not displayed). Does it sound good?

@JulienMoumne
Collaborator

Concerning the three options

* For non historical reports:
    o Display Reports tables only (default)
    o Display Graphs only
    o Display Graphs and tables

1) What is the default ? (For new reports and reports created before this evolution)

2) Shall I add a new column in the _pdf table ?

Wording

How about replacing "non-historical reports" by "aggregate reports" or "break down reports" or something similar ?

@mattab
Owner

1) What is the default ? (For new reports and reports created before this evolution)

Default is to NOT show graphs for non historical.

But, previous reports and newly created reports will ALWAYS show the graphs for VisitsSummary, Goals, etc.

2) Shall I add a new column in the _pdf table ?

Sounds good

Wording

How about replacing "non-historical reports" by "aggregate reports" or "break down reports" or something similar ?

Aggregated reports sounds good :)

@JulienMoumne
Collaborator

Shall we add/edit a column in the report list to display which setting is currently applied to a report ? Shall we use icons ? text ?

In the edit form, should we use a drop down list with the 3 options ?

@mattab
Owner

I think it's ok NOT to display it in the list. Mostly, people will leave the default which is only display graphs for historical report. It's OK to just display the option when editing/creating a report (less noise!)

@JulienMoumne
Collaborator

Concerning HTML reports, at first I was thinking that we could simply include an IMG markup with a SRC attribute linking to the Piwik instance (same mechanism for Piwik and country logo).

However, when anonymous read access is not activated, it means the token auth is sent in the email content.

Is this ok?

@mattab
Owner

Good point. Sending token_auth in clear text is not a good thing to do (no existing Piwik core mechanism leak the token_auth). I think it's safer if the IMG is fetched during report generation and then included in the HTML/PDF directly.

@JulienMoumne
Collaborator

For PDF reports, no problem.

For HTML reports, we're in a pickle.

As I understand, there are two ways to include the binary content of images in an e-mail. One uses the src="data:BASE64_ENCODED_IMAGE" and one uses e-mail attachment. According to http://www.campaignmonitor.com/blog/post/1761/embedding-images-in-email/ and http://www.campaignmonitor.com/blog/post/1759/embedding-images-revisited/ it seems those methods are not universally supported by mail clients.

@mattab
Owner

Good point. We can do the "Image as attachment" solution which works in Desktop clients, better than nothing. Otherwise, for mail clients it will not work for HTML report. This is an acceptable limtation. Better than disclosing the token_auth for sure!!

An alternative solution would involve only enabling graphs to be rendered for a given token, added in the email for this report only, and The Report plugin would act as a proxy to the ImageGraph API and give the proper token_auth if the second token was provided correctly... But too complicated for the gain here I think?

Since it works as attacment in desktop clients + in PDF reports, it sounds good...

@JulienMoumne
Collaborator

Images as attachment using a content id appear correctly and in-place on gmail and yahoo.

@JulienMoumne
Collaborator

Attachment:
2706.patch

@JulienMoumne
Collaborator

Attachment:
Graphs and Tables.pdf

@JulienMoumne
Collaborator

Attachment:
Graphs Only.pdf

@JulienMoumne
Collaborator

Patch submitted for review. @Review are questions left to reviewers.

  • HTML Reports:

  • when downloaded, raw image data is included using

src="data:image/png;base64,DATA_STRING"
  • when sent by mail, images are sent as attachments using cid (tested with thunderbird, gmail & yahoo)
src="cid:{$reportId}"
  • PDF Reports:

  • Display 3 graphs per page when 'Aggregate Report Format' = 'Display Graphs Only'.

  • '''We need to come-up with a better page-fitting algorithm for 'Display Tables Only' to avoid as much blank space as possible
    '''

  • Metadata API:

  • 'Goals_Ecommerce' category appears first when calling getReportMetadata. 'sort' has been updated to restore 'VisitsSummary' first

  • ImageGraph Plugin

  • additional security

  • new outputType to retrieve the generated image as a PHP return variable
  • filename fixes

  • DB Update:

  • 1.7.php contains the update code for 'piwik_pdf'

''Can't upload my 'Tables Only' PDF : Submission rejected as potential spam (Content contained these blacklisted patterns: 'rx')
Nor the HTML report : Submission rejected as potential spam (Content contained these blacklisted patterns: 'DJ', 'rx')
''

@mattab
Owner

julien, excellent patch!! Quite neat changes done :)

  • Have you had chance to check Outlook? I know it sucks, but so many people still use it.

Should we add an "alt" attribute ?
Yes, empty probably

@Review 'isAggregateReport' report should ideally be in the metadata ? or somewhere in core ?
Actually I propose to remove the helper function, and simply have a local variable every time it's used: isAggregateReport = !empty($report['dimension']);
There is no need to add overhead in the metadata in this case since it is simply a boolean check.

// @Review : Is this the right place ?
Idem, simply remove function

For the spam, you can upload a zip with all files, then it doesn't check for spam ;) tricky but this trac is a beast to modify and we run a 4 yo update haha...

Looks good otherwise!

@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

Commit r5415 includes all remarks from comment:18

Tested on outlook 2010

Includes a better page-fitting algorithm for table only reports (mentioned in comment:17)

@mattab
Owner

Great patch and code :)!

Feedback

  • PDFReports test is failing
     [exec] <span class="fail">Exception</span>: /home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/plugins/PDFReports/tests/PDFReports.test.php -&gt; Test_Piwik_PDFReports -&gt; test_addReport_getReports -&gt; <strong>Unexpected exception of type [Exception] with message [General_ExceptionInvalidAggregateReportsFormat] in [/home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/plugins/PDFReports/API.php line 666]</strong><br />
     [exec] <span class="fail">Exception</span>: /home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/plugins/PDFReports/tests/PDFReports.test.php -&gt; Test_Piwik_PDFReports -&gt; test_getReports_invalidPermission -&gt; <strong>Unexpected exception of type [Exception] with message [General_ExceptionInvalidAggregateReportsFormat] in [/home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/plugins/PDFReports/API.php line 666]</strong><br />
     [exec] <span class="fail">Exception</span>: /home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/plugins/PDFReports/tests/PDFReports.test.php -&gt; Test_Piwik_PDFReports -&gt; test_updateReport -&gt; <strong>Unexpected exception of type [Exception] with message [General_ExceptionInvalidAggregateReportsFormat] in [/home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/plugins/PDFReports/API.php line 666]</strong><br />
     [exec] <span class="fail">Exception</span>: /home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/plugins/PDFReports/tests/PDFReports.test.php -&gt; Test_Piwik_PDFReports -&gt; test_deleteReport -&gt; <strong>Unexpected exception of type [Exception] with message [General_ExceptionInvalidAggregateReportsFormat] in [/home/www/data/root/jenkins.private/jobs/Piwik/workspace/build/plugins/PDFReports/API.php line 666]</strong><br />

  • Not directly related but would be nice to refactor all strings in the lang file that look like:

    'General_ExceptionMethodNotFound' => 'The method \'%s\' does not exist or is not available in the module \'%s\'.',
    'General_ExceptionInvalidRendererFormat' => 'Renderer format \'%s\' not valid. Try any of the following instead: %s.',
    'General_ExceptionInvalidReportRendererFormat' => 'Report format \'%s\' not valid. Try any of the following instead: %s.',
+   'General_ExceptionInvalidAggregateReportsFormat' => 'Aggregate reports format \'%s\' not valid. Try any of the following instead: %s.',
    'General_ExceptionInvalidPeriod' => 'The period \'%s\' is not supported. Try any of the following instead: %s',
@mattab
Owner

It would be nice to fix the Graphs legending because most graphs are currently useless because X axis does not show labels.... see #2704

I love the new fitting algorithm and 3 graphs in the same page, looks very nice.

@mattab
Owner

(In [5417]) Refs #2706

  • Moved to version 1.7-b1 and rename so that update is picked up
  • Simplify UI and text to clarify the feature (hopefully ;)
  • Fix 1 integration test (1 other unit test is failing)
@JulienMoumne
Collaborator

(In [5423]) refs #2706 - fixing unit tests

@JulienMoumne
Collaborator

(In [5424]) refs #2706 removing magic numbers

@JulienMoumne
Collaborator

(In [5436]) refs #2706, #1454 API category reports excluded from PDFReports Plugin forms

@JulienMoumne
Collaborator

(In [5437]) refs #2706, r5436 - API category reports also need to be excluded in the test report ($idReport == 0)

@anonymous-piwik-user

Hello Everyone I am new in piwik code I have a bug in the implementation of patch graphs Support Email reports in 2706 and gives me error as: Fatal error: Call to undefined method Piwik_Common: json_encode () in ... \ plugins \ PDFReports \ Controller.php on line 47

@mattab
Owner

(In [5544]) Refs #2706

  • Scheduled reports should generate when GD is disabled (but without graph)
  • refactored gd check
@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
@mattab mattab added this to the 1.7 Piwik 1.7 milestone
@JulienMoumne JulienMoumne was assigned by mattab
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.