Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide possibility to add header to PDF reports #8234

Closed
wants to merge 4 commits into from

Conversation

Fensterbank
Copy link
Contributor

At the moment PDF reports are very basic and a little boring.
To get a more professional look I would like to have the possibility to add a custom header to PDF pages.
Especially when scheduled email reports are a service provided for non-technical users, a headline with e.g. the company logo would be nice.

I made some changes to the PDF creation and the user interface:
headline1
part of a created pdf page with header

headline2
pdf header settings can be set when report format PDF is selected

Is there some interest to implement this feature?
The UI and PDF creation are ready, but we have to implement storing the data and give it to the constructor of the PDF creation.
At the moment I have no idea how and were to store the settings in the database and how I would get piwik to upgrade the database, also I have no idea of ZEND.

Maybe in the future we could add more PDF customization settings (other image in headline, changes on footer, watermark, more business like front page, etc.) so I think all PDF specific settings including my current implementation of header settings should not be in the general scheduled reports table.

What do you think of my changes?
Does somebody wants to help to get this thing complete?

@Fensterbank
Copy link
Contributor Author

I read more of the code and saw that several report settings are stored in the column parameters, so I added logic to read and write pdf customization settings using the parameters.
Seems to be the fastest way with lowest impact on existing reports and installations and doesn't need some database changes.

At the current state the headline implementation is finished and working.
Settings are persisted and all pdf reports can be customized with a custom headline.

@Fensterbank
Copy link
Contributor Author

I would be very happy if someone would advance an opinion to this PR... :-)
If there are further questions or if some changes have to be done, feel free to tell me.

@mattab
Copy link
Member

mattab commented Jul 14, 2015

Hi @Fensterbank

Thanks for your pull request!

Is there some interest to implement this feature?

That's a good question... I don't think that we need this feature. Ideally, it would be done in a plugin published on the Marketplace. Some users will love this feature, while most users don't need it. We prefer not to add a feature in the UI when only few users would benefit from it. In general, we even try hard to simplify the UIs :-)

One challenge I can think of is that maybe it's not completely possible to do this in a plugin yet... maybe this is related to our vision in #6951 where we would have consistent "Manage entity" screen, and these screens would be extendable by plugins via a documented API. cc @tsteur

@tsteur
Copy link
Member

tsteur commented Jul 14, 2015

I'm not sure how this would be related but I also haven't read everything? "Manage entitiy" screen wouldn't affect PDF reports but they could maybe allow to add such settings.

Refs #7893 as we will introduce "Measurable Settings" there. Maybe we need at some point even "Manage Entity settings" if "Manage Measurables / Manage Websites" was built with a "#6951 Manage Entities component" but not really related.

@mattab
Copy link
Member

mattab commented Jul 15, 2015

I'm not sure how this would be related but I also haven't read everything? "Manage entitiy" screen wouldn't affect PDF reports but they could maybe allow to add such settings.

I thought that when we build a generic "Manage entity" screen, it would include the "Listing" view as well as the "Edit" view. The "Edit" view would be pluggable by plugins. This would make automatically all listing + edit templates editable by plugins. But maybe I'm wrong and it wouldn't be so magically powerful ;-)

@Fensterbank
Copy link
Contributor Author

Hi all,

thanks for your replies.
I also thought of create a plugin for this, but ScheduledReport themselve is already a plugin.
So I modified the plugin by adding the settings to the template and by modifying some lines of ReportRenderer/pdf.php.

I don't think a plugin can extend a menu template or the pdf rendering logic of an existing plugin, so I would have to make a bareley new plugin which replaces the ScheduledReports plugin, copy the whole code and add my changes. Then I would have to put ongoing effort in it to keep my plugin in sync with all potentially new features of ScheduledReports.

This is a huge effort for such a small change. Or am I wrong and the current PDF render methods and the UI could easily be extended by a plugin?
At the moment I also don't understand what "Managing Entity" would bring or how it would change things... :-)

But is it the right way, expressed with some exaggeration, to say "Let the pdf reports simple and ugly like it had ever been since Piwik 0.1 and bring new features only with a plugin"? ;-)
In my eyes a headline setting for pdf could be a default feature as enabling or disable page numbering or select another front page image.

@tsteur
Copy link
Member

tsteur commented Jul 15, 2015

Adding options via a "Manage Entity component" would be possible but think the main problem is to modify the look of Scheduled Reports (PDF / HTML / ...). That's why I was wondering how it is related. That component would be most likely extensible yes.

@mattab
Copy link
Member

mattab commented Jul 16, 2015

@Fensterbank Thanks for suggestion, What would be really perfect is when we can do this via a plugin rather than in core. I prefer not to merge the pull request for now in core. I hope you understand and hope to see more PR from you in the future still :-) in the future, maybe you could first ask in an issue, whether we would likely merge a given feature/PR. Cheers

@mattab mattab closed this Jul 16, 2015
@mattab mattab added the wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it. label Jul 16, 2015
@Fensterbank
Copy link
Contributor Author

@mattab Okay. I think I will take a look to the plugin system.

The way I made it (to define pdf customizing settings in the edit scheduled report form) I don't think it is possible as a plugin, because I need to extend the plugin ScheduledReports by a plugin.

But maybe I find an alternative way to provide this user friendly in a plugin.

Cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants