Plugin to provide PDF export of data

[matt]

Users need to share Piwik reports with colleagues, customers or externals. PDF is the standard way of exchanging rich data, with graphs, colors, consistent design.

Proposed features set of Piwik PDF export

For the first release of PDF export in Piwik, I think the following features would be important to have:

• Create the pdf: the PDF would not be customizable and would include all Piwik reports. The tables would be "all columns" tables, ie. they contain Visits, unique visitors, bounce rate, goal conversion rate, etc.
• Send & receive the PDF: the PDF would be downloadable via a click in the "User Settings" page. Also, a click on a link would send an email with the PDF attached to the given email addresses (easier for users to forward the emails to others rather than create a new email, attaching the PDF, etc.).
• Look and feel: the PDF reports would contain tables, that look similar to the tables in Piwik reports. For example, search engine icons, countries flags would be displayed. Each page would contain the title of the report, and as many rows as can fit on the page (maybe 20 rows per table?).
• Internationalization. The PDF reports would be localized in any of the 35+ languages.

Other features probably for later releases:

• Customize PDF: each user would be able to customize which reports make it in the PDF. Possibility to create several PDF reports, per website, or for a given website.
• Receive the PDF by email: using a new hook, see #1184, the PDF reports plugin could automatically generate and send PDF reports by email to each user that requested delivery. Users could customize the frequency (daily, weekly, monthly), and recipients (themselves and others) to send the PDF reports to.
• PDF contains sparklines
• Each Piwik widget/report can be exported in PDF (add a new export link below the table, next to CSV, XML, etc.).

Features currently not considered

• PDF contains graphs: bar, pie, evolution charts. Graphs export in PDF would be a major maintenance / time intensive task, as we would have to support a second graphing library, as OFC is flash based and can't generate PNGs.

Thoughts about implementation

Here are some thoughts about the implementation. Please discuss before implementing, there might be better solutions.

API to render PDF

Rendering a PDF could then be done by using following call:

module=API&
method=PDF.getReport&
report=$REPORT_ID&
idSite=$WEBSITE_ID&
token_auth=$TOKEN&
date=...&period=...

In V1, when customReports are not implemented, we would have a function PDF.getFullWebsiteReport which would return the PDF containing all reports for a given website.

There could also be a PDF.getWebsitesDashboard which would return a simple PDF containing main metrics for all websites visible to the user.

Flow of a request to generate a PDF containing all reports

When called, the PDF API function would then:

• create and setup as default the PDF view.
• init the PDF: set colors, margins, titles, properties, generated time, etc.
• loop over all controllers, and methods that we wish to render.
• call the methods in each plugin's controller which add data to the PDF. All methods would render the "all columns" data table (that shows visits, pages, time per visit, etc.) rather than just the simple table with only the visits column.
• render the final PDF and output it to the browser

Basic skeleton for the API method PDF.getReport

function getReport($report, $idSite, $date, $period)
{

 // set the default View to PDF Piwik_View::setDefaultView(new Piwik_View_PDF());
 // define an array of Controllers/methods to call to render in this PDF, 
 // eg. array( array( 'Goals', 'index'), array('UserSettings', 'getBrowser') )
 // loop over all the array of methods to render
           // here we could add a new page, a pdf page title for the next report, etc.
            // for each controller method, call it, for example
            $content = Piwik_FrontController::getInstance()->fetchDispatch( 'Goals', 'index');
            // this will call the index() on Goals/Controller.php
            // which will, at the end, render the view eg. $view->getView()->render()
           // this will result in Piwik_View_PDF->render() call which adds the right data to the PDF file being built.

 // once all reports have been added to PDF, return PDF to the user
}

A new View type

Currently in plugins Controllers, views are created with:

$view = new Piwik_View('Goals/templates/single_goal.tpl');

The view object encapsulates the Smarty implementation. Variables are set to the view $view->name = $goalDefinition['name']; and the view is rendered at the end of the controller method: echo $view->render();

In order to add PDF export with minimal effort, PDF could be a new View class, implementing the iView interface.

Currently there is core/View.php which should probably be moved to core/View/Smarty.php

there is some generic code in render() which could probably be left in the new core/View.php parent object.

We could have Piwik_View_Smarty extends Piwik_View then we would build Piwik_View_PDF extends Piwik_View

When building a PDF report, each controller's method would be called, and the data would be added to the PDF report hold in this object. The report builder API would then output this aggregated PDF to the user.

Adding renderer

Piwik_View_PDF could look at the template name passed in the view constructor, and based on this find out if it knows how to render this template. For example, calling the Goals.goalReport method (in plugins/Goals/Controller.php), the View is using the template Goals/templates/single_goal.tpl - if the view does know how to render this (given the variables are set properly like on the smarty template), then it can render it. If not, an error can be output. The PDF view would have logic that says: "Goals/templates/single_goal" must be rendered this way. Most templates are generic (datatable, etc.) so most of the work to render the reports would be done only once and automatically work for most reports.

Rendering text, tables in the PDF

The library might support automatic conversion of HTML into the PDF, but most likely this doesn't work very well. We will most likely have to write a basic PDF renderer for the datatable (equivalent of datatable.tpl).

Custom reports

By default, we can include all tables in the PDF. But reports are useful only if they contain the subset of information a given user is interested in. We could have a simple UI, listing all available reports, with a click and add to report feature. This would be internally linked to calls to controller methods. The layout could be saved as JSON in the reports table.

UI integration

PDF reports do not apply on per report basis, like Bar graphs or Pie charts apply. There could be a link to generate a PDF report for the current website in the header.

Contributions

Interested by this feature? Let us know in the comments.

[cornbread]

I'd love to see this happen

[mbirth]

Why not render the graphs in PDF? The TCPDF PHP library already has a PieSector (  http://www.tecnick.com/pagefiles/tcpdf/doc/com-tecnick-tcpdf/TCPDF.html#methodPieSector ) function to render a "Pie Piece". This would generate a vector graph which only need a few bytes in the PDF and can be freely scaled without getting blurry. I'm sure there's also some bargraph code somewhere ... if not, it's easy to create one.

[ChristianSchneider]

Started to work on this plugin. Will be have a beta version for testing soon. Today I want to tell you what we implemented so far. Also I'll screenshots.

First we add in the admin interface a new tab: PDF Export Settings Here you can set some text blocks which will be used as comments in the PDF export. It takes all data from the widget preview view.

In the beta release we also want to have a dropdown where you can choose a template for the PDF report (own logo, etc.)

Also we gave you the possibility to save the reports on you server in a specified directory.

In the front we add a new tab like "goals" or "actions". The rest is similar to the goal plugin.

You can add a new report and also see which reports exists for this idSite. This feature is usefull if you want to generate different reports for different user groups (e.g. management report or design/graphical report).

Here you can choose start and end date and also if you want to have diagrams in the report.

At the moment you can only download the PDF by clicking on a button.

For PDF generation we use fpdf library, because we use this library before (over the API) and it was easily to install in the plugin.

If you have some comments on the screens please let me know. More screens will follow until we release the beta version.

[ChristianSchneider]

Admin screen (not complete yet)

[ChristianSchneider]

Position of plugin entry for users

[matt]

Christian, thanks for updating us.

• Could you keep all PDF related actions in the administration area? Adding a new tab in the main menu is not possible as this menu is only for web analytics reports. Also, it makes sense to have all PDF related actions (add, edit, delete, customize with comments) in the same place. We can give access to "view" users to these screens if necessary (just like they can, in 0.6, access the User Settings tab in the admin area)
• can you explain what you mean by "dropdown where you can choose a template for the PDF report"
• when you say "It takes all data from the widget preview view.", how do you actually fetch this data? Also, how do you fetch the list of available reports (is it the list of widgets)?

It sounds like you are making great progress. Let us know if you have questions, missing hooks in the code, etc. that would block your development. We're really looking forward to seeing this PDF export :)

[mmimlav]

Hello,

This plugin seems to be very interresting. I must developp a similary plugin which enables people to export data in PDF format. I would like to know the project progress and if I can help you on your work.

Could you please contact me by mail at: jeremy.lavaux@… Thanks

[matt]

V 0.1

[matt]

Feedback on V0.1 submitted by Christian.

First of all, very good start :) Here is a review of the screens/UI and the code.

UI/product feedback

• by default, select Piwik logo (see example here)
• display the currently selected logo in the page. Do not shrink/expand the logo, leave it with same resolution.
• remove the feature "Save export on server". This can be a security issue as the URL is easy to discover. Instead,
  • PDF should triger the 'download' browser window
  • Possibility to send by email (see below for more explanation)
• the tables in the PDF should contain all columns by default (bounce rates, unique visitors, time on site, etc. when applicable)
• Icons are missing in the tables (countries, browsers, search engines, etc.)

Code feedback

• never read $_REQUEST or $_GET directly. the code for example currently has a SQL injection (try id=1' in the URL. Always use Piwik_Common::getRequestVar (see the full  security check up page, must read for piwik devs)
• security: also always use bind parameters in the SQL
• to include in piwik core, change plugin author/website to 'Piwik'
• the Controller should be in its own Controller.php file
• it would be easier to understand/maintain if the fpdf specific code was in a different class than the Piwik API calls. For example in a class 'PDFRenderer' that would take all data to display as input, and not know anything about Piwik specifics.
• creating reports with accents and quotes ' doesn't work (see for example 'screen resolution' in French, only the first word display, and the accent doesn't display right in the PDF)
• the code that creates PDF custom reports should call the PDFReport plugin API. In Piwik, all the logic (SQL and other) must be in the APIs, and the UI / Controllers / etc. must call the API to add/edit/delete/generate reports. In this particular case, the PdfReports/API.php should allow to
  • create a custom PDF report
  • edit/delete a custom PDF report (see example of the Goals plugin)
  • generate a given PDF report (the API will return the PDF directly)
  • send a given PDF report by email, to a given user email, and/or a list of specified email addresses

Clarification on 'generating PDF' screen

• user should not be able to select a date or a period. Instead, the PDF will generate for the currently selected date and period in the Piwik calendar.
• Later, we will add possibility to generate PDF automatically and send them by email every day / every week / every month. If the user selects 'every day' then he will receive the daily stats in his email. If he selects 'every week' he will receive weekly stats. Therefore, at this stage, there is no need for date/period in the UI.
• Same applies for the language: the PDF is generated in the currently selected language.
• The 'generate' PDF link should call the API directly.
• Is it possible to edit PDF templates, or simply delete them?
• There should be a new column called 'Email PDF Report' with the following fields
  • Send PDF by email to [ $here_current_user_email_is_printed$ ].. The user can also add new emails to the INPUT field by adding a comma and another email.
  • Note: you can get current user email by doing

    $user = Piwik_UsersManager_API::getInstance()->getUser(Piwik::getCurrentUserLogin());
    $userEmail = $user['email']; 
    

  • He can then click 'Send' to send the email with the PDF attached. Note: we use Zend_Mail to send emails.

I would recommend if possible you work on these features first without adding new ones, and we can try to have a beta version that we can integrate in core. After this we could work on other new features (sending reports by email, adding graphs, etc.). Thanks!

[matt]

(In [2312]) Refs #71

• Fixing GIF images that were corrupted. While displaying fine in the browser, they wouldn't load in photoshop and would throw an exception in FPDF

[ChristianSchneider]

Attached the beta version for PDF export with the comments by Matt. Waiting for a new code review. Thank to Jeremy and Alex for the work done on this plugin to the beta status.

[matt]

In the submitted version, I wasn't able to generate a PDF with reports (list of reports didn't display). I'll still do a code review:

• remove postLoad() from code
• change Piwik_Menu::add to Piwik_AddMenu and move to addMenu() method
• in trunk, Piwik::prefixTable doesn't exist anymore, it was deprecated for Piwik_Common::prefixTable (I updated code to pass the install)
• logo.png in root is still the old logo, but logo was displaying fine in the PDF first page (only page that was generated)
• PDFRenderer.php, file_exists('./plugins/Pdfex should use PIWIK_INCLUDE_PATH instead of relative path. Also, filename should be in a variable and not copied twice. Same for pdf->Output('./plugins/Pdfexpor in API.php and all other places
• in /libs/, are all files necessary to build the PDF (other files can be removed, like date.js?)? Other css js files used can be moved to templates/
• Also, I assume that external libs files are unmodified? we can move then to piwik/libs/ when we integrate.
• API.php. Comments are not always useful ("Get Full PDF report", @access public is not necessary as the functions are public, etc.)
• in the API.php files, you should not use Piwik_Common::getRequestVar. Instead, all parameters are in the function methods and are automatically passed to the function by the Piwik front controller. Check other API.php files for examples.
• getReport function is the same as getFullWebsiteReport, so getFullWebsiteReport should be renamed in getReport
• idem for createTemplate and savetemplate, deteteTemplate and deleteReportTemplate (btw delete is spelled wrong)
• some code indendation is different in API.php
• I don't really understand the $useForApi parameter
• because the methods are a bit long, I would recommend putting all SQL requests in small methods. Also uppercase all SQL keywords (AND, WHERE, SELECT etc.)
• days should not be hardcoded in english. Instead you can use translations from the translation files. Use Piwik_Date->getLocalized('%longDay%')
• do not do Date operations when it is possible to use Piwik_ helpers. To get the day-7, you can use Piwik_Date::subDay(7)
• in savetemplate(), the function does a HTTP referer. The API are functions that can be called from HTTP, json, etc. They can 'do stuff' and return data, but they can't set cookies or other http headers. Instead the calling function should do the redirect.
• The code starting line 399 to 430 should be refactored in Piwik_Mail class, in the constructor or when send() is called. You can create these methods in Piwik_Mail and submit a patch to the file in the same .zip as the plugin.
• there are 'echo', var_dump etc. in various files that should be removed (throw exception instead when it makes sense) For example when I tried to generate the pdf the message Please contact your administrator to configurate the SMTP server and port in 'PDF export Settings' was echoed to the screen.
• your API class extends the controller class...? but it should not extend anything. so you couldn't use $this
• Controller.php
• you write period=yesterday but yesterday is a 'date' value.
• date=today. but is it this used? I think date and period should always be set so need for default maybe?
• if/else and other code should be consistent with piwik code (if else on different lines as the brackets). Check out  coding standards
• as said in other review, you can't write "Piwik_Query("SET NAMES 'utf8'"); ". Other plugins like SiteManager record UTF data as site name, so this can work without the set names (piwik might support other DB soon, on top of mysql)
• in various methods, code is not refactored and copied pasted. same code should never appear twice, eg.

  	$data[$k]['filename']    = "./plugins/Pdfexport/reports/".$template['date_from']."-".$template['date_period']."_".$template['site_id'].".pdf";
  	        	
  	        	if (file_exists('./plugins/Pdfexport/reports/'.$template['date_from'].'-'.$template['date_period'].'_'.$template['site_id'].".pdf"))
  	        
  

• the configurate() method is very long. You can split it in smaller methods (one for email configure, other for reports)
• is it expected that you write TRUNCATE TABLE in the controller?
• why do you manually write stuff in the config files? the mail configuration will be in config/global.ini.php so you can submit a patch to this file, and directly reuse Piwik_Config methods to update the config file..
• you can use the SitesManager/API.php file to see a good example of an API class

• UI
• for ajax requests, showing errors and showing success messages, you can reuse existing code. See example on how to do ajax request in the Goal plugin, when you create edit/delete goals. see plugins/Goals/templates/add_edit_goal.tpl
• for non ajax requests ( If the form is submitted and refreshes the page) you can see an example in the CoreAdminHome controller (simple clean example). See also the templates.
• for the table, can you reuse styles of the Goal table? see plugins/Goals/templates/list_goal_edit.tpl
• all CSS should be set as css classes and you can create a css file for all UI screens
• in the UI, either you talk in Ajax to the API directly. Or you must submit the form to the controller actions that will then, in the controller code, call the API. You can't however submit a form to the API directly.
• remove page.tpl
• HTML in PdfConfig.tpl has caps letter but we write HTML without caps
• idem the css should be refactored
• you create one html TABLE per line of settings... instead do simply like the General Settings screen: plugins/CoreAdminHome/templates/generalSettings.tpl it will be shorter code and be consistent with other screens
• none of this template code was displaying in my browser though: I just see the SMTP settings form

Let me know when you need a new code review, good progress, but some cleaning work left before we can integrate. Thanks!

[mmimlav]

Plugin + patch

[matt]

Code review

Core patch

• be careful, when you merged, you didn't keep all new changes from trunk. This results that the patch is reverting some other commits. For example config.ini.php after patch doesn't have the SEO plugin enabled, or the Adapter controller is reverted to 2 parameters which breaks Piwik install. It is very important during merge of your code and trunk, that you keep all existing code from trunk
• in the code $adapter->query('SET NAMES '.$dbInfos['charset']); I think this is already done by Zend, check out:  http://dev.piwik.org/trac/browser/trunk/libs/Zend/Db/Adapter/Pdo/Mysql.php#L79 Therefore it should not be necessary to add this query there.
• in the config file, you created useSmtp but transport is not used. Use transport instead (when transport, by default, is empty, then use normal non smtp)
• the function addPeriod() ?
• to access the mail section of config file, simply do Zend_Registry::get('config') instead of using Piwik_Config_Ini....
• to follow coding standards, the multiple OR statements must be on new lines
• otherwise patch looks good, I'll do a complete review once merging is ok

PDF plugin feedback

User Interface

• the plugin doesn't install properly. two errors (PiwiK_Menu not found and then tables already install). for tables already installed, change by 1050 in PdfReport.php (which makes it work when tables are already installed)
• Image upload form, should show currently uploaded Logo. Also should display some help text explaining supported format, maximum logo size, recommended size, and explain where the logo will be used (First report page?). Of course, simply refactor the list of allowed types as a private members of the controller. Also refactor the upload function in a private helper method. Maybe we could move this to Piwik:: helper actually.

I'll do a quick code review as I couldn't merge the patch therefore not see the plugin.

code review

• Coding standards: Indentation is not like other files in Piwik. please read other files and you can then indent the same (one tab instead of two, no empty line after function declarations, etc.)
• PdfRenderer: function drawTable is very long and does lots of things. can you split it in private smaller functions?
• can you put all colors used in the PDF as variables of the class? this will make it easier later to have a custom class changing colors. For example, private $color_CellBackground = array(1,2,3); (Also it would be good to have cell widths, fonts, font sizes as members)
• same for the font: Arial is written many times. Instead $font = 'Arial' should be private member of class. All sizes, colors, etc. should not be copied paste but refactored
• deletedUnusedColumns() instead, use an array with all fields to be deleted, loop over them and unset()
• Instead of writing code like

  	            	if (isset($standardColumnNameToTranslation[$key]))
  	            		$elementsArray[$key]   = Piwik_Translate($standardColumnNameToTranslation[$key]);
  	            	else
  	            		$elementsArray[$key] = $key;
  

you can write

	            		$elementsArray[$key] = $key;
	            	if (isset($standardColumnNameToTranslation[$key]))
	            		$elementsArray[$key]   = Piwik_Translate($standardColumnNameToTranslation[$key]);

• Controller.php as said in previous reviews, NEVER EVER read $_POST directly!!
• in the controller submit handler, you must check for the token_auth (and submit it in the POST). Check example of how to call $this->checkTokenInUrl(); in other Controller.php
• remove unused boolToString()
• API.php
• some functions take parameters in different orders. The order should always be the default: $idSite, $period, $date
• you can use Piwik::getCurrentUserTokenAuth() instead of $authCookie->get('token_auth'); and Piwik::getcurrentUserLogin() instead of Zend_Registry::get('config')->General->login_cookie_name
• you request json and then use json_encode. Instead you can request 'original' to get the original object back (php)
• there is an empty if (isset($contentresult?) && $contentresult?=="error")
• function generatePdfPageForCall() is used once only, but has a parameter
• instead of doing

            if ($this->date == 'today')
        		$this->date = date('Y-m-d');
        	if ($this->date == 'yesterday')

use Piwik_Date::factory($this->date)->get('Y-m-d') which will handle all cases

• some parameters $template_name, $date_from - Piwik coding standards use camelCase notation
• you write

          $url = "method=".'Pdfexport.getReport';
          $url .= "&idSite=".$idSite."&period=".$period."&date=".$date_from."&id=".$insertResult->getAdapter()->lastInsertId();
          $url .= "&format=php";
          $request = new Piwik_API_Request($url);
          $request->process();
  

Instead you can just do $this->getReport($idSite, .... ) which is the same

• some code is not refactored like

   if (file_exists($this->reportFilePathGenerate($template)))
          	unlink($this->reportFilePathGenerate($template));
  

Please check all the code and make all is refactored (very important for maintenance)

• Some API functions issue redirects (which I pointed in last review)

• the code

			switch ($templateToSend[0]['date_period'])
			{
				case "day":
					$date_To = $piwik_date->addPeriod(1,'day')->toString();
					break;
				case "week":
					$date_To = $piwik_date->addPeriod(1,'week')->toString();
					break;
				case "month":
					$date_To = $piwik_date->addPeriod(1,'month')->toString();
					break;
				case "year":
					$date_To = $piwik_date->addPeriod(1,'year')->toString();
					break;
				default:
					break;
			}

should simply be

$period = $templateToSend[0]['date_period'];
$dateTo = $piwik_date->addPeriod(1,$period)->toString();

• refactor code to send email in private method
• instead of doing

  	switch ($piwik_date->getLocalized('%longDay%'))
  			{
  				case Piwik_Translate('General_LongDay_1'):
  					$date_from = $piwik_date->toString();
  					break;
  				case Piwik_Translate('General_LongDay_2'):
  					$date_from = $piwik_date->subDay(1)->toString();
  					break;
  				case Piwik_Translate('General_LongDay_3'):
  					$date_from = $piwik_date->subDay(2)->toString();
  					break;
  				case Piwik_Translate('General_LongDay_4'):
  					$date_from = $piwik_date->subDay(3)->toString();
  					break;	
  				case Piwik_Translate('General_LongDay_5'):
  					$date_from = $piwik_date->subDay(4)->toString();
  					break;
  				case Piwik_Translate('General_LongDay_6'):
  					$date_from = $piwik_date->subDay(5)->toString();
  					break;
  				case Piwik_Translate('General_LongDay_7'):
  					$date_from = $piwik_date->subDay(6)->toString();
  					break;
  				default:
  					
  					break;
  			}
  

you could write something like

$dateFrom = $date->subDay($date->get('N'))->toString(); (see http://fr.php.net/manual/en/function.date.php)

• replace strcmp($period,"month")==0) by $period == "month"

as said, I haven't tested functionnality yet as I'm waiting for patch. Good luck :) let me know if you need help

[matt]

new files:  http://dl.free.fr/d98xMeThE

Good work on the review, the code is now getting much better!! I hope we can reach a good working version soon. Generating pdf was not working for me (see attached pdf) but I still reviewed code and UIs

Could you please attach a PDF with all reports that are possible to generate, and some data, so I can see how it looks? thanks

Core Code review

• Patch now applies :) cool
• uploadFile: parameters naming doesnt respect coding standards, eg allowed_filetypes should be allowedFiletypes (see previous code reviews).
• the exception message should use the max size variable in the message instead of harcoding
• parameter $existing_filename looks like it doesn't belong to this upload function
• $newFileName shoudln't have default value
• MAX_SIZE should be a parameter defaulting to 100000 instead of const
• instead of

  			if($_FILES['userfile']['size'] < MAX_SIZE)
  			{
  				[.....] very long
  			}
  			else
  			{
  				throw new Exception('Size error: a maximum of 100ko is allowed');
  			}
  

write the more simple

			if($_FILES['userfile']['size'] > MAX_SIZE)
			{
				throw new Exception('Size error: a maximum of 100ko is allowed');
			}
                        [....] very long

makes code much more readable. i saw it in other codes like sendTemplate() and sendEmail() etc.

• patch to en.php has regressions (re-adds deleted strings)
• (said in previous review) remove useSmtp from config file, and use transport==smtp to test if smtp should be used

UI

• in PDF logo should be aligned on the left, instead of centered
• logo upload, when there is a problem, just fails with a message on white page... it should instead be displayed with the standard error message on the screen (catch the exception in controller and set error message)
• add PNG to list of supported files (and of course, refactor list of allowed filetypes to not have different list in help message and in the code...)
• if I upload test.jpg, it is renamed as logo.png... of course same file extension should be kept  http://thedailywtf.com/Articles/Pipe-Up.aspx ;)
• Smtp: can you preselect "no" by default (after removing useSmtp=no)
• Generate should open PDF in a new window
• After creating a new template, window redirects to open the PDF. Instead page should redirect to the list of templates, and user can decide to view it, or send by emails, etc.
• Form add new template: please reuse same style as Add a new goal (style used already for listing templates). For example, items are in a table, the Add a new PDF Report link hides when clicked and displays the form, etc.
• Can you add a simple "PDF" menu in the top bar instead of main menu? This can now be done with hooks (see other plugins top menu)
• you introduce a new Form checking mechanism.... why?? please please reuse what is already available: other forms are submitted as ajax, then the API checks the parameters, and if one is empty (eg. the name) it throws an exception with the message being displayed in red at the top. Please look at Goal plugin for an example. The form should work this way and reuse the code (which will make your life easier, and very important will be consistent across Piwik)
• Clicking on the label report name doesn't check the checkbox
• in report list, don't display group names when groups are empty
• I tried several times to generate a PDF, but either only had the first page, or it was showing empty columns and rows (see attachment)
• instead of using this mechanism &action=callPdfApiAction&call=getReport, instead just call the correct method directly from the UI. These proxy mechanisms are generally not a good idea when they can be easily avoided. in this case, as said above, because your code will call api directly, youll have to remove it anyway.
• the Send PDF to field is empty for super user, you can read email from config file
• Send PDF by email: also here the request should be submitted by Ajax, the Loading... displayed, so that all errors and feedback messages are automatically displayed on screen. For example in my case the mail server doesn't work, when I click Send I get a white page error with only back button. By using ajax request and the piwikHelper js functions, this problem is solved. Note that the UI can call directly an APi method (rather than calling the controller), for example, API->sendReportByEmail($idSite, $reportId, for example.).
• Creating report named "Test '" test" it doesnt display properly in the table. the variable in smarty probably needs a {$var|escape}
• List of reports contains items that can't be generated in the PDF, like 'Last visits graph'. Such reports should not be displayed in the list, as the API function doesn't exist for them. Can you maybe add an array of API action that you know doesn't exist.

• There are a few SQL requests in the code, please refactor into small private methods. Makes maintenance and readability much easier

Code review

• replace if $user eq 'admin' in templates by {if $isSuperUser}
• remove loop foreach($existing_filename as $file) in controller, simply set the default path to the piwik logo inside the plugin
• several variables $mail_day_reports = ; are not used

Renderer

• why does the renderer look for a logo? instead, this logic can be put into a function used by the UI to display the current logo, and used by the renderer to get the logo path (refactoring)
• in function drawReportTableHeader(), you do if(count==0). Instead it is better practise to put this init code before the for loop (makes much better code)

API

• generatePdfPageForCall: errors are not displayed currently. Errors are very important, for example right now pdf is empty and I don't know why. Instead, I suggest that you keep all Errors and exception messages caught during the PDF generation, and display them, at the end of the PDF, in a page "Errors while generating this PDF:" that would not display if there is no error (most cases I guess?)
• why does deleteTemplate() need period and date parameters? it shouldnt
• same for createTemplate()
• remove updateTemplate not used, or if time allows, makes the Template listing have a Edit button, like Goals, and reopen the Add template form pre-selected with the current template to edit... The edit should only update the name, and list of reports to display, just like the Add template screen.
• $idSite not used in sendTemplate call.. (please check parameters and variables are used)
• instead of having sendTemplate throw an exception, just let exceptions flow through, users need to know the error that happened.
• switchPeriod code is unecessary: the API can take any $date inside the period, you will get same result with period=month&date=2010-07-01 as with period=month&date=2010-07-22
• please dont do your own email validation... ( if (preg_match("#^{[a-zA-Z0-9._-]+@[a-zA-Z0-9._-]{2,}\\.[a-z]{2,4}$#", $mail_single))}

) but reuse existing one: Piwik::isValidEmailString

• setFrom should be set to noreply@ domain, like password recovery email. you can use following code

  			$piwikHost = @$_SERVER['HTTP_HOST'];
  			if(strlen($piwikHost) == 0)
  			{
  				$piwikHost = 'piwik.org';
  			}
  
  

Schemas

To meet requirements from spec, here are some changes to make to schema. Some other templates/php changes will follow in some cases...

• pdf_templates should not have a date_time column. Instead, PDF reports are always generated for the selected date.
• also remove date_from field
• add a 'ts_created' column, as in piwik_site table (with datetime of creation date), useful for debug purposes only
• many variables and parameters have _ but should be camelCased->likeOthersInPiwik
• column user_id is never set apparently... rename to 'author' and set the login of user who created the alert. This can be used for debugging purposes, but not used in the code
• consistency with all other piwik tables, rename site_id to idsite
• change pdf_templates.id to pdf_templates.idtemplate and pdf_config.id to pdf_config.idconfig for consistency

Finally, can you please remove all TCPDF files that are not directly used by Piwik? TCPDF is a great library but is very heavy. Can you please remove fonts, images, docs, etc. that are not used? (you can leave README and LICENSE please). We are trying hard not to increase the piwik package size and are careful about adding only useful files :)

Looking forward to next release and hopefully I can generate a PDF next time :) we're getting closed to commit to trunk for sure, good stuff!

[matt]

empty report with empty columns, and with a large logo (logo can be aligned on left and resized to fit on page)

[matt]

Actually, the PDF now load, but I had to install a Japanese font. Why? without this font (of course I didn't take a screenshot of the popup...) I couldn't display in acrobat reader, strange..

[matt]

OK I was able to generate the PDF

Looks good :) few suggestions

• Maybe using font = helvetica would solve the "need Japanese font" error?
• when table has only 2 columns, it would look better if the right column had fixed width. Maybe all columns, except the first one, could have fixed width, and the label use the rest
• logo seem resized, they look pixellised and are higher than standard size. If not resized, they maybe would fit in the row height which would also look better
• I haven't yet fully tested to generate a very long report with lots of data and all reports possible to generate. that would be great if you could submit such PDF :) you can use visitgenerator plugin to generate thousands of visits and then have lots of data to display.

Thanks!

[mmimlav]

Hi, You can find above our last version.

Things which are not in the V0.4:

* when table has only 2 columns, it would look better if the right column had fixed width. Maybe all columns, except the first one, could have fixed width, and the label use the rest

* logo seem resized, they look pixellised and are higher than standard size. If not resized, they maybe would fit in the row height which would also look better

Our choices:

logo upload, when there is a problem, just fails with a message on white page... it should instead be displayed with the standard error message on the screen (catch the exception in controller and set error message)

==> For the moment it's quite difficult to manage the problem in Ajax so the error will be displayed in a white page.

Remove updateTemplate not used, or if time allows, makes the Template listing have a Edit button, like Goals, and reopen the Add template form pre-selected with the current template to edit... The edit should only update the name, and list of reports to display, just like the Add template screen.

==> We made a remove of updateTemplate. A later version could take care of this modification.

Known issues:

Due to Ajax request, sometimes the page does not refresh and block on "Loading Data". Just Click again on "Pdf export" in the top bar to refresh.

[matt]

(In [2662]) Fixes #1509 - Thanks Jeremy for initial code! Refs #71

Note: I haven't tested SMTP myself. will require beta tests

[matt]

(In [2663]) Refs #1485

• Adding a magic method to request a given report but with all the metadata returned (report name, column list, translations, etc.) as well as the data, already filtered, ready to be displayed. I think this could be incredibly useful to Mobile Piwik Client!! Check out: getProcessedReport($idSite, $date, $period, $apiModule, $apiAction, $apiParameters = false)
• This is used in the future PDF export plugin refs #71
• Minor updates to UI text

[peterb]

Great plugin so far!

My Ideas for discussion: - There should be some kind of meta-data in the footer of every page. So when the report is printed, everybody can easily associate a single page with a vertain given report.

Footer-MetaData imaginable: - Page x of xx (mandatory) - reporting date range: 2010-07-01 - 2010-07-31 (date must be i18n) (mandatory) - if a certain section includes several pages, the title is not on every following page but only on the first page of the section (e.g. country). Shouldn't we then write in the footer to which section the current site belongs? (e.g. Website-report / keyword-report, etc) - Would it make sense to additionally assign a unique number to every report that is repeated in every site's footer so that a given site can be associated with a certain report even faster? (e.g. Report-ID or something)

the usecase would be that someone compares two or more reports in printed versions. Without these meta-data you would be lost in loose sites.

Are seo-metrics already included in the reports? e.g. google indexed sites, backlinks, pagerank, etc.?

[matt]

(In [2697]) Refs #71

• Adding PDF plugin, based on the submission from jeremy lavaux and Lyzun Oleksandr.
• I rewrote nearly all code to comply with Piwik coding styles/guidelines/ etc. and also because it had to use the Metadata which wasn't yet created when the PDF code was initially written
• Features customizable PDF reports (based on metadata reports), with description + scheduling (daily/weekly/monthly) and send to current user as well as additional emails listed
• Added helper function Piwik::getCurrentUserEmail()
• Refactored window modal popover into a helper method piwikHelper.windowModal (used to ask confirmation when deleting stuff) Refs #1490
• Refactored the Goals CSS into generic CSS which can be reused and is reused for PDF UI

Refs #1184

• The callback must pass $this instead of the class name as it

TODO

• test scheduled tasks send email properly (email looks good + attachment works)
• Add comment header in PDFReports files
• Can we remove some files in TCPDF which adds a lot of space in the release (eg. some fonts in libs/fonts ?)
• Test PDF with UTF8 characters

[matt]

(In [2700]) Refs #71 Fixing small ui issues

[vipsoft]

(In [2706]) refs #71

[matt]

(In [2737]) Refs #71

• Scheduled PDF reports by email work as expected
  • fixed issue with current week used instead of last finished period,
  • fixed issue that all recipients were listed in the same TO: field, now sending one email per address.
  • Super user API methods will return all PDF reports by default, but UI now only displays PDF created by Super User.
• Refs #1184 Better logging of what task was ran and how long it took
• The API call to run scheduled tasks must also be ported to Powershell refs #1411

[matt]

(In [2747]) Refs #71 Anonymous users can't create/view/schedule PDF.

[matt]

(In [2749]) Refs #71 Removing unused fonts from libs trying to keep release size at minimum

[Beatgarantie]

I created a new report containing the info I need. Afterwards I press "download" in order to see the result an get 3 big messages:

1: Warning: imagepng() [<a href=' http://de.php.net/function.imagepng'>function.imagepng</a>]: open_basedir restriction in effect. File(/tmp/jpg_3S5g7f) is not within the allowed path(s): (/users/jmp/temp:/users/jmp/www) in /users/jmp/www/piwik/libs/tcpdf/tcpdf.php on line 6701

2: Warning: imagepng() [<a href=' http://de.php.net/function.imagepng'>function.imagepng</a>]: Invalid filename in /users/jmp/www/piwik/libs/tcpdf/tcpdf.php on line 6701

3: Warning: fopen() [<a href=' http://de.php.net/function.fopen'>function.fopen</a>]: open_basedir restriction in effect. File(/tmp/jpg_3S5g7f) is not within the allowed path(s): (/users/jmp/temp:/users/jmp/www) in /users/jmp/www/piwik/libs/tcpdf/tcpdf.php on line 6765

4: Warning: fopen(/tmp/jpg_3S5g7f) [<a href=' http://de.php.net/function.fopen'>function.fopen</a>]: failed to open stream: Operation not permitted in /users/jmp/www/piwik/libs/tcpdf/tcpdf.php on line 6765

TCPDF ERROR: Can't open image file: /tmp/jpg_3S5g7f

Each message has its own backtrace

Is it a failure or is it related to my shared webspace settings?

[Beatgarantie]

@matt: I tried your patch but there was no difference. Seems also that this randomly generated jpg_xxx-file does not exist. I think that the issue is related to my settings which cannot be changed. Thanks.

[Beatgarantie]

please add those 2 folders:

libs/tcpdf/cache libs/tcpdf/images

Afterwards pdf-files can be generated. Thanks!

[machoyer]

PDFReport Cache Folders are not in Piwik 1.0 installation package

libs/tcpdf/cache libs/tcpdf/images

please put those folders in package.

greetings

[vipsoft]

Re: last two comments. See #1656

This ticket is closed.

[JulienM]

(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

[JulienM]

(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 #71 : space between report title and report table reduced to avoid page overflow
• refs #2829 : TODO display percentages
• r5544, r5547, r5549 merged