Allow to customize Subject and Text for Email Reports

[pokoli]

This PR enables three new Parameters in Email Reports:

• Enable Custom Email Text Activates whether custom text is Used or Not. Also enables/disables the other two parameters on the UI.
• Custom Email Subject The text to be used as the subject of the Email
• Custom Email Text The text to be used as the introduced text of the Email. "You can'd find the atached report text is preserverd".

For me is quiete usable now. What do you think?

[julienmoumne]

Could you elaborate why the 'customEmailText' parameter is persisted? Couldn't you simply check for non null 'emailSubject' & 'emailContent'?

Is there no sanitizing to perform on those new parameters?

[mattab]

I'm not sure if we want to do this... ?

if we do, at least these option should be "hidden" by default in the form, and only acessible on "click" on "advanced settings" or similar, since very few people would use that feature.

[faceleg]

Likely that feature would be useful for those using "white labelled" installations.

[pokoli]

Al 04/02/13 23:40, En/na Matthieu Aubry ha escrit:

I'm not sure if we want to do this... ?

if we do, at least these option should be "hidden" by default in the
form, and only acessible on "click" on "advanced settings" or similar,
since very few people would use that feature.

As it's implemented it's "hidden" by default, you have to check the
customize email texts to enable this feature. Is this sufficient?

Could you elaborate why very few people would use this feature?

[pokoli]

@julienmoumne:

Could you elaborate why the 'customEmailText' parameter is persisted? Couldn't you simply check for non null '>emailSubject' & 'emailContent'?

If you persist the customEmailText you can disable the custom text and don't lose your emailSubject & emailContent texts. I don't know if that is a suficient reason to store and adition parameter to the database.

I can remove this parametre and simple check for null if you prefer.

Is there no sanitizing to perform on those new parameters?

Of course, but i don't know the best way to do it in this case, as the variable is only printed to an HTML email. Should htmlspecialchars[1] be suficient for this use case?

[1] http://www.php.net/manual/en/function.htmlspecialchars.php

[halfdan]

Close/Reopen to trigger Travis-CI build.

[mattab]

Thank you for proposing this, I really appreciate, but I prefer not to go ahead and add complexity in the emais at this stage. Later we will add some features in emails and we can revisit. Thanks for your understanding!

[pokoli]

IMHO it doesn't add any complexity as you mustn't define this settings if you don't want, you can use the default settings if desired.

If you want to review it later i could understand. Could you please leave this issue open so it will be easier to find to anybody interesed?

Matthieu Aubry notifications@github.com ha escrit:

Thank you for proposing this, I really appreciate, but I prefer not to
go ahead and add complexity in the emais at this stage. Later we will
add some features in emails and we can revisit. Thanks for your
understanding!

---

Reply to this email directly or view it on GitHub:
#23 (comment)

[mattab]

I'm not sure best practise of github, but I rather leave it closed so it doesnt clutter (and take some mental effort to remember we already decided not to include it). You are very welcome to create a ticket at: dev.piwik.org with the patch there so anyone could apply it if they wish (in "Feature requests") . Thanks!

[robocoder]

I agree with Matt. If a PR is being rejected, then the PR should be closed. Trac can be used to track the feature request.

[robocoder]

http://dev.piwik.org/trac/ticket/3723