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

Send email if no tracked data within N days. #13363

Merged
merged 23 commits into from Sep 20, 2018
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
620816a
Remember user who created a site.
diosmosis Aug 29, 2018
7c49031
Send email if no tracked data within N days.
diosmosis Aug 29, 2018
6a9bf34
Add test and get to pass.
diosmosis Aug 30, 2018
b1f2d9d
Fixes after manual tests of emails
diosmosis Aug 30, 2018
12577df
Bump version & change column name to creator_login.
diosmosis Aug 30, 2018
cb56089
Merge branch 'user-site-creation' into tracking-code-reminder
diosmosis Aug 30, 2018
a5510bb
Email tweaks.
diosmosis Aug 30, 2018
185aafc
Merge branch '3.x-dev' into user-site-creation
diosmosis Sep 7, 2018
35837ab
Rename Site::getCreationUserFor
diosmosis Sep 7, 2018
87d8d72
Merge branch 'user-site-creation' into tracking-code-reminder
diosmosis Sep 7, 2018
b2b06ab
Modify Site:: access methiod name
diosmosis Sep 7, 2018
96684fb
Applying PR feedback.
diosmosis Sep 9, 2018
f1cfb1e
Move email HTML content generation logic to separate class in DI.
diosmosis Sep 10, 2018
f5e3c79
tweak translations
diosmosis Sep 10, 2018
f8059cf
Apply PR review feedback.
diosmosis Sep 18, 2018
c1013e0
Couple more tweaks.
diosmosis Sep 18, 2018
ff5a0f4
Make tracking code check a one time task + and save timetable when re…
diosmosis Sep 19, 2018
89f6cbc
Update save call.
diosmosis Sep 20, 2018
e1aca7b
Apply more PR feedback.
diosmosis Sep 19, 2018
916efa9
small performance tweak and put the site name in quotes
tsteur Sep 19, 2018
8ed1f32
Fixing tests.
diosmosis Sep 20, 2018
9fc74bf
Update expected file.
diosmosis Sep 20, 2018
9217388
Merge branch '3.x-dev' into tracking-code-reminder2
diosmosis Sep 20, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions config/global.ini.php
Expand Up @@ -656,8 +656,6 @@
; If set to 0 it also disables the "sent plugin update emails" feature in general and the related setting in the UI.
enable_update_communication = 1



; Comma separated list of plugin names for which console commands should be loaded (applies when Matomo is not installed yet)
always_load_commands_from_plugin=

Expand All @@ -675,6 +673,9 @@
; If set to 0 it will disable advertisements for providers of Professional Support for Matomo.
piwik_professional_support_ads_enabled = 1

; The number of days to wait before sending the JavaScript tracking code email reminder.
num_days_before_tracking_code_reminder = 5

[Tracker]

; Matomo uses "Privacy by default" model. When one of your users visit multiple of your websites tracked in this Matomo,
Expand Down
1 change: 1 addition & 0 deletions core/Db/Schema/Mysql.php
Expand Up @@ -75,6 +75,7 @@ public function getTablesCreateSql()
`group` VARCHAR(250) NOT NULL,
`type` VARCHAR(255) NOT NULL,
keep_url_fragment TINYINT NOT NULL DEFAULT 0,
creator_login VARCHAR(100) NULL,
PRIMARY KEY(idsite)
) ENGINE=$engine DEFAULT CHARSET=utf8
",
Expand Down
32 changes: 32 additions & 0 deletions core/Email/ContentGenerator.php
@@ -0,0 +1,32 @@
<?php
/**
* Piwik - free/libre analytics platform
*
* @link http://piwik.org
* @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
*
*/

namespace Piwik\Email;

use Piwik\View;
use Piwik\View\HtmlReportEmailHeaderView;

class ContentGenerator
{
public function generateHtmlContent(View $body)
{
HtmlReportEmailHeaderView::assignCommonParameters($body);
$bodyHtml = $body->render();

$header = new View("@CoreHome/_htmlEmailHeader.twig");
HtmlReportEmailHeaderView::assignCommonParameters($header);
$headerHtml = $header->render();

$footer = new View("@CoreHome/_htmlEmailFooter.twig");
HtmlReportEmailHeaderView::assignCommonParameters($footer);
$footerHtml = $footer->render();

return $headerHtml . $bodyHtml . $footerHtml;
}
}
44 changes: 44 additions & 0 deletions core/Mail.php
Expand Up @@ -9,8 +9,10 @@
namespace Piwik;

use Piwik\Container\StaticContainer;
use Piwik\Email\ContentGenerator;
use Piwik\Plugins\CoreAdminHome\CustomLogo;
use Piwik\Translation\Translator;
use Piwik\View\HtmlReportEmailHeaderView;
use Zend_Mail;

/**
Expand Down Expand Up @@ -52,6 +54,13 @@ public function setDefaultFromPiwik()
$this->setFrom($fromEmailAddress, $fromEmailName);
}

public function setWrappedHtmlBody(View $body)
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering... should this be maybe in a new class which can be injected etc? Then we could also replace it through DI etc... I'd say the class does otherwise maybe too many things like formatting plus setting mail etc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's here so derived classes can use it.

The idea in this PR is that future emails will create their own class that derived from Mail, then in event handlers plugins can detect exactly which email is about to be sent. The derived class would call this method to set the HTML body. So the methods in this class should done one of two things: configure the email to be sent, or send the email.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was more thinking if it was a class, we could overwrite the wrap globally for all emails. For example on Cloud.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, overwrite just the wrapping logic, gotcha.

$contentGenerator = StaticContainer::get(ContentGenerator::class);
$bodyHtml = $contentGenerator->generateHtmlContent($body);
$this->setBodyHtml($bodyHtml);
}

/**
* Sets the sender.
*
Expand Down Expand Up @@ -123,7 +132,25 @@ private function initSmtpTransport()

public function send($transport = null)
{
if (!$this->shouldSendMail()) {
return $this;
}

$mail = $this;

/**
* This event is posted right before an email is sent. You can use it to customize the email by, for example, replacing
* the subject/body, changing the from address, etc.
* TODO: changelog
* @param Mail $this The Mail instance that is about to be sent.
*/
Piwik::postEvent('Mail.send', [$mail]);

if (defined('PIWIK_TEST_MODE')) { // hack
/**
* @ignore
* @deprecated
*/
Piwik::postTestEvent("Test.Mail.send", array($this));
} else {
return parent::send($transport);
Expand Down Expand Up @@ -183,4 +210,21 @@ function sanitiseString($string)
$string = str_replace($search, $replace, $string);
return $string;
}

private function shouldSendMail()
{
$shouldSendMail = true;

$mail = $this;

/**
* This event is posted before sending an email. You can use it to abort sending a specific email, if you want.
* TODO: changelog
* @param bool &$shouldSendMail Whether to send this email or not. Set to false to skip sending.
* @param Mail $mail The Mail instance that will be sent.
*/
Piwik::postEvent('Mail.shouldSend', [&$shouldSendMail, $mail]);

return $shouldSendMail;
}
}
11 changes: 11 additions & 0 deletions core/Site.php
Expand Up @@ -636,4 +636,15 @@ public static function getExcludedQueryParametersFor($idsite)
{
return self::getFor($idsite, 'excluded_parameters');
}

/**
* Returns the user that created this site.
*
* @param int $idsite The site ID.
* @return string|null If null, the site was created before the creation user was tracked.
*/
public static function getCreatorLoginFor($idsite)
Copy link
Member

@tsteur tsteur Sep 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the public function getCreatorLogin method here on top for consistency?

{
return self::getFor($idsite, 'creator_login');
}
}
43 changes: 43 additions & 0 deletions core/Updates/3.6.1-b2.php
@@ -0,0 +1,43 @@
<?php
/**
* Piwik - free/libre analytics platform
*
* @link http://piwik.org
* @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
*
*/

namespace Piwik\Updates;

use Piwik\Common;
use Piwik\Updater\Migration\Factory as MigrationFactory;
use Piwik\Updates;
use Piwik\Updater;

/**
* Update for version 3.6.1-b2.
*/
class Updates_3_6_1_b2 extends Updates
{
/**
* @var MigrationFactory
*/
private $migration;

public function __construct(MigrationFactory $factory)
{
$this->migration = $factory;
}

public function getMigrations(Updater $updater)
{
return array(
$this->migration->db->addColumn('site', 'creator_login', ' VARCHAR(100) NULL'),
);
}

public function doUpdate(Updater $updater)
{
$updater->executeMigrations(__FILE__, $this->getMigrations($updater));
}
}
2 changes: 1 addition & 1 deletion core/Version.php
Expand Up @@ -20,7 +20,7 @@ final class Version
* The current Matomo version.
* @var string
*/
const VERSION = '3.6.1-b1';
const VERSION = '3.6.1-b2';

public function isStableVersion($version)
{
Expand Down
2 changes: 1 addition & 1 deletion core/View/HtmlEmailFooterView.php
Expand Up @@ -20,6 +20,6 @@ public function __construct()
{
parent::__construct(self::TEMPLATE_FILE);

$this->hasWhiteLabel = \Piwik\Plugin\Manager::getInstance()->isPluginLoaded('WhiteLabel');
HtmlReportEmailHeaderView::assignCommonParameters($this);
}
}
13 changes: 9 additions & 4 deletions core/View/HtmlReportEmailHeaderView.php
Expand Up @@ -47,10 +47,6 @@ public function __construct($reportTitle, $prettyDate, $description, $reportMeta
$this->assign("idSite", $idSite);
$this->assign("period", $period);

$customLogo = new CustomLogo();
$this->assign("isCustomLogo", $customLogo->isEnabled() && CustomLogo::hasUserLogo());
$this->assign("logoHeader", $customLogo->getHeaderLogoUrl($pathOnly = false));

$date = Date::now()->setTimezone(Site::getTimezoneFor($idSite))->toString();
$this->assign("date", $date);

Expand All @@ -72,6 +68,15 @@ public static function assignCommonParameters(View $view)

$view->themeStyles = $themeStyles;
$view->emailStyles = $emailStyles;

$view->fontStyle = 'color:' . $themeStyles->colorText . ';font-family:' . $themeStyles->fontFamilyBase.';';
$view->styleParagraph = 'font-size:15px;line-height:24px;margin:0 0 16px;';

$customLogo = new CustomLogo();
$view->isCustomLogo = $customLogo->isEnabled() && CustomLogo::hasUserLogo();
$view->logoHeader = $customLogo->getHeaderLogoUrl($pathOnly = false);

$view->hasWhiteLabel = \Piwik\Plugin\Manager::getInstance()->isPluginLoaded('WhiteLabel');
}

private static function getPeriodToFrequencyAsAdjective()
Expand Down
92 changes: 92 additions & 0 deletions plugins/CoreAdminHome/Emails/JsTrackingCodeMissingEmail.php
@@ -0,0 +1,92 @@
<?php
/**
* Piwik - free/libre analytics platform
*
* @link http://piwik.org
* @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
*
*/

namespace Piwik\Plugins\CoreAdminHome\Emails;

use Piwik\Mail;
use Piwik\Piwik;
use Piwik\Site;
use Piwik\View;

class JsTrackingCodeMissingEmail extends Mail
{
/**
* @var string
*/
private $login;

/**
* @var string
*/
private $emailAddress;

/**
* @var int
*/
private $idSite;

public function __construct($login, $emailAddress, $idSite)
{
parent::__construct();

$this->login = $login;
$this->emailAddress = $emailAddress;
$this->idSite = $idSite;

$this->setUpEmail();
}

/**
* @return string
*/
public function getLogin()
{
return $this->login;
}

/**
* @return string
*/
public function getEmailAddress()
{
return $this->emailAddress;
}

/**
* @return int
*/
public function getIdSite()
{
return $this->idSite;
}

private function setUpEmail()
{
$this->setDefaultFromPiwik();
$this->addTo($this->emailAddress);
$this->setSubject($this->getDefaultSubject());
$this->setReplyTo($this->getFrom());
$this->setWrappedHtmlBody($this->getDefaultBodyView());
}

private function getDefaultSubject()
{
return Piwik::translate('CoreAdminHome_MissingTrackingCodeEmailSubject', [Site::getMainUrlFor($this->idSite)]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use the name instead of the URL? AFAIK a URL is optional (eg not needed for mobile apps) and also reads better maybe. I was wondering if we can incorporate "analytics" or "Matomo" somehow into the subject but I can see it is tricky.

}

private function getDefaultBodyView()
{
$view = new View('@CoreAdminHome/_jsTrackingCodeMissingEmail.twig');
$view->login = $this->login;
$view->emailAddress = $this->emailAddress;
$view->idSite = $this->idSite;
$view->siteUrl = Site::getMainUrlFor($this->idSite);
return $view;
}
}