From e2a02e8d4de61022f9d75d10fdabaec23ed15d53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Moumn=C3=A9?= Date: Tue, 23 Apr 2013 09:12:44 +0200 Subject: [PATCH] fix #3896 --- core/ReportRenderer.php | 4 +- core/ReportRenderer/Html.php | 5 +- core/ReportRenderer/Pdf.php | 8 +-- plugins/MobileMessaging/MobileMessaging.php | 10 ++-- .../ReportRenderer/Exception.php | 2 +- .../MobileMessaging/ReportRenderer/Sms.php | 2 +- plugins/PDFReports/API.php | 42 ++++++++++------ plugins/PDFReports/PDFReports.php | 8 +-- tests/PHPUnit/Plugins/MobileMessagingTest.php | 49 +++++++++++++++++++ tests/PHPUnit/Plugins/PDFReportsTest.php | 29 +++++++++++ 10 files changed, 125 insertions(+), 34 deletions(-) diff --git a/core/ReportRenderer.php b/core/ReportRenderer.php index 9b26b7f2364..0c0bc73243a 100644 --- a/core/ReportRenderer.php +++ b/core/ReportRenderer.php @@ -100,12 +100,12 @@ abstract public function getRenderedReport(); /** * Generate the first page. * - * @param string $websiteName + * @param string $reportTitle * @param string $prettyDate formatted date * @param string $description * @param array $reportMetadata metadata for all reports */ - abstract public function renderFrontPage($websiteName, $prettyDate, $description, $reportMetadata); + abstract public function renderFrontPage($reportTitle, $prettyDate, $description, $reportMetadata); /** * Render the provided report. diff --git a/core/ReportRenderer/Html.php b/core/ReportRenderer/Html.php index 45c14bffa45..24339d2d57a 100644 --- a/core/ReportRenderer/Html.php +++ b/core/ReportRenderer/Html.php @@ -82,12 +82,13 @@ private function epilogue() $this->rendering .= $smarty->fetch(self::prefixTemplatePath("html_report_footer.tpl")); } - public function renderFrontPage($websiteName, $prettyDate, $description, $reportMetadata) + public function renderFrontPage($reportTitle, $prettyDate, $description, $reportMetadata) { $smarty = new Piwik_Smarty(); $this->assignCommonParameters($smarty); - $smarty->assign("websiteName", $websiteName); + // todo rename 'websiteName' to 'reportTitle' once branch twig is merged + $smarty->assign("websiteName", $reportTitle); $smarty->assign("prettyDate", $prettyDate); $smarty->assign("description", $description); $smarty->assign("reportMetadata", $reportMetadata); diff --git a/core/ReportRenderer/Pdf.php b/core/ReportRenderer/Pdf.php index 23d24ab18f0..d14b7365079 100644 --- a/core/ReportRenderer/Pdf.php +++ b/core/ReportRenderer/Pdf.php @@ -142,14 +142,14 @@ public function getRenderedReport() return $this->TCPDF->Output(null, 'S'); } - public function renderFrontPage($websiteName, $prettyDate, $description, $reportMetadata) + public function renderFrontPage($reportTitle, $prettyDate, $description, $reportMetadata) { - $websiteTitle = $this->formatText($websiteName); + $reportTitle = $this->formatText($reportTitle); $dateRange = $this->formatText(Piwik_Translate('General_DateRange') . " " . $prettyDate); //Setup Footer font and data $this->TCPDF->SetFooterFont(array($this->reportFont, $this->reportFontStyle, $this->reportSimpleFontSize)); - $this->TCPDF->SetFooterContent($websiteTitle . " | " . $dateRange . " | "); + $this->TCPDF->SetFooterContent($reportTitle . " | " . $dateRange . " | "); $this->TCPDF->setPrintHeader(false); // $this->SetMargins($left = , $top, $right=-1, $keepmargins=true) @@ -163,7 +163,7 @@ public function renderFrontPage($websiteName, $prettyDate, $description, $report $this->TCPDF->SetFont($this->reportFont, '', $this->reportHeaderFontSize + 5); $this->TCPDF->SetTextColor($this->headerTextColor[0], $this->headerTextColor[1], $this->headerTextColor[2]); - $this->TCPDF->Cell(40, 210, $websiteTitle); + $this->TCPDF->Cell(40, 210, $reportTitle); $this->TCPDF->Ln(8 * 4); $this->TCPDF->SetFont($this->reportFont, '', $this->reportHeaderFontSize); diff --git a/plugins/MobileMessaging/MobileMessaging.php b/plugins/MobileMessaging/MobileMessaging.php index 62028ba3e46..426798f1834 100644 --- a/plugins/MobileMessaging/MobileMessaging.php +++ b/plugins/MobileMessaging/MobileMessaging.php @@ -242,14 +242,14 @@ function sendReport($notification) $notificationInfo = $notification->getNotificationInfo(); $report = $notificationInfo[Piwik_PDFReports_API::REPORT_KEY]; $contents = $notificationInfo[Piwik_PDFReports_API::REPORT_CONTENT_KEY]; + $reportSubject = $notificationInfo[Piwik_PDFReports_API::REPORT_SUBJECT_KEY]; $parameters = $report['parameters']; $phoneNumbers = $parameters[self::PHONE_NUMBERS_PARAMETER]; - if (in_array('MultiSites_getAll', $report['reports'])) { - $from = Piwik_Translate('General_Reports'); - } else { - $from = $notificationInfo[Piwik_PDFReports_API::WEBSITE_NAME_KEY]; + // 'All Websites' is one character above the limit, use 'Reports' instead + if ($reportSubject == Piwik_Translate('General_MultiSitesSummary')) { + $reportSubject = Piwik_Translate('General_Reports'); } $mobileMessagingAPI = Piwik_MobileMessaging_API::getInstance(); @@ -257,7 +257,7 @@ function sendReport($notification) $mobileMessagingAPI->sendSMS( $contents, $phoneNumber, - $from + $reportSubject ); } } diff --git a/plugins/MobileMessaging/ReportRenderer/Exception.php b/plugins/MobileMessaging/ReportRenderer/Exception.php index a1eac80d8f6..5fa57968950 100644 --- a/plugins/MobileMessaging/ReportRenderer/Exception.php +++ b/plugins/MobileMessaging/ReportRenderer/Exception.php @@ -59,7 +59,7 @@ public function getRenderedReport() return $this->rendering; } - public function renderFrontPage($websiteName, $prettyDate, $description, $reportMetadata) + public function renderFrontPage($reportTitle, $prettyDate, $description, $reportMetadata) { // nothing to do } diff --git a/plugins/MobileMessaging/ReportRenderer/Sms.php b/plugins/MobileMessaging/ReportRenderer/Sms.php index 96f2999d9fe..d492dc1281b 100644 --- a/plugins/MobileMessaging/ReportRenderer/Sms.php +++ b/plugins/MobileMessaging/ReportRenderer/Sms.php @@ -47,7 +47,7 @@ public function getRenderedReport() return $this->rendering; } - public function renderFrontPage($websiteName, $prettyDate, $description, $reportMetadata) + public function renderFrontPage($reportTitle, $prettyDate, $description, $reportMetadata) { // nothing to do } diff --git a/plugins/PDFReports/API.php b/plugins/PDFReports/API.php index b57c05d5dce..aacba73e00e 100644 --- a/plugins/PDFReports/API.php +++ b/plugins/PDFReports/API.php @@ -46,7 +46,8 @@ class Piwik_PDFReports_API const REPORT_CONTENT_KEY = 'contents'; const FILENAME_KEY = 'filename'; const PRETTY_DATE_KEY = 'prettyDate'; - const WEBSITE_NAME_KEY = 'websiteName'; + const REPORT_SUBJECT_KEY = 'reportSubject'; + const REPORT_TITLE_KEY = 'reportTitle'; const ADDITIONAL_FILES_KEY = 'additionalFiles'; const REPORT_TRUNCATE = 23; @@ -405,16 +406,10 @@ public function generateReport($idReport, $date, $language = false, $outputType // render report $description = str_replace(array("\r", "\n"), ' ', $report['description']); - // if the only report is "All websites", we don't display the site name - $websiteName = Piwik_Translate('General_Website') . " " . Piwik_Site::getNameFor($idSite); - if (count($report['reports']) == 1 - && $report['reports'][0] == 'MultiSites_getAll' - ) { - $websiteName = Piwik_Translate('General_MultiSitesSummary'); - } - $reportTitle = "$websiteName - $prettyDate - $description"; + list($reportSubject, $reportTitle) = self::getReportSubjectAndReportTitle(Piwik_Site::getNameFor($idSite), $report['reports']); + $filename = "$reportTitle - $prettyDate - $description"; - $reportRenderer->renderFrontPage($websiteName, $prettyDate, $description, $reportMetadata); + $reportRenderer->renderFrontPage($reportTitle, $prettyDate, $description, $reportMetadata); array_walk($processedReports, array($reportRenderer, 'renderReport')); switch ($outputType) { @@ -447,14 +442,15 @@ public function generateReport($idReport, $date, $language = false, $outputType return array( $outputFilename, $prettyDate, - $websiteName, + $reportSubject, + $reportTitle, $additionalFiles, ); break; case self::OUTPUT_INLINE: - $reportRenderer->sendToBrowserInline($reportTitle); + $reportRenderer->sendToBrowserInline($filename); break; case self::OUTPUT_RETURN: @@ -464,7 +460,7 @@ public function generateReport($idReport, $date, $language = false, $outputType default: case self::OUTPUT_DOWNLOAD: - $reportRenderer->sendToBrowserDownload($reportTitle); + $reportRenderer->sendToBrowserDownload($filename); break; } } @@ -491,7 +487,7 @@ public function sendReport($idReport, $period = false, $date = false) $language = Piwik_LanguagesManager_API::getInstance()->getLanguageForUser($report['login']); // generate report - list($outputFilename, $prettyDate, $websiteName, $additionalFiles) = + list($outputFilename, $prettyDate, $reportSubject, $reportTitle, $additionalFiles) = $this->generateReport( $idReport, $date, @@ -519,7 +515,8 @@ public function sendReport($idReport, $period = false, $date = false) self::REPORT_CONTENT_KEY => $contents, self::FILENAME_KEY => $filename, self::PRETTY_DATE_KEY => $prettyDate, - self::WEBSITE_NAME_KEY => $websiteName, + self::REPORT_SUBJECT_KEY => $reportSubject, + self::REPORT_TITLE_KEY => $reportTitle, self::ADDITIONAL_FILES_KEY => $additionalFiles, ) ); @@ -536,6 +533,21 @@ public function sendReport($idReport, $period = false, $date = false) } } + private static function getReportSubjectAndReportTitle($websiteName, $reports) + { + // if the only report is "All websites", we don't display the site name + $reportTitle = Piwik_Translate('General_Website') . " " . $websiteName; + $reportSubject = $websiteName; + if (count($reports) == 1 + && $reports[0] == 'MultiSites_getAll' + ) { + $reportSubject = Piwik_Translate('General_MultiSitesSummary'); + $reportTitle = $reportSubject; + } + + return array($reportSubject, $reportTitle); + } + private static function validateReportParameters($reportType, $parameters) { // get list of valid parameters diff --git a/plugins/PDFReports/PDFReports.php b/plugins/PDFReports/PDFReports.php index 6a0ead94698..1533a70192d 100644 --- a/plugins/PDFReports/PDFReports.php +++ b/plugins/PDFReports/PDFReports.php @@ -304,7 +304,7 @@ function sendReport($notification) if (self::manageEvent($notification)) { $notificationInfo = $notification->getNotificationInfo(); $report = $notificationInfo[Piwik_PDFReports_API::REPORT_KEY]; - $websiteName = $notificationInfo[Piwik_PDFReports_API::WEBSITE_NAME_KEY]; + $reportTitle = $notificationInfo[Piwik_PDFReports_API::REPORT_TITLE_KEY]; $prettyDate = $notificationInfo[Piwik_PDFReports_API::PRETTY_DATE_KEY]; $contents = $notificationInfo[Piwik_PDFReports_API::REPORT_CONTENT_KEY]; $filename = $notificationInfo[Piwik_PDFReports_API::FILENAME_KEY]; @@ -312,7 +312,7 @@ function sendReport($notification) $periods = self::getPeriodToFrequency(); $message = Piwik_Translate('PDFReports_EmailHello'); - $subject = Piwik_Translate('General_Report') . ' ' . $websiteName . " - " . $prettyDate; + $subject = Piwik_Translate('General_Report') . ' ' . $reportTitle . " - " . $prettyDate; $mail = new Piwik_Mail(); $mail->setSubject($subject); @@ -328,13 +328,13 @@ function sendReport($notification) // Needed when using images as attachment with cid $mail->setType(Zend_Mime::MULTIPART_RELATED); - $message .= "
" . Piwik_Translate('PDFReports_PleaseFindBelow', array($periods[$report['period']], $websiteName)); + $message .= "
" . Piwik_Translate('PDFReports_PleaseFindBelow', array($periods[$report['period']], $reportTitle)); $mail->setBodyHtml($message . "

" . $contents); break; default: case 'pdf': - $message .= "\n" . Piwik_Translate('PDFReports_PleaseFindAttachedFile', array($periods[$report['period']], $websiteName)); + $message .= "\n" . Piwik_Translate('PDFReports_PleaseFindAttachedFile', array($periods[$report['period']], $reportTitle)); $mail->setBodyText($message); $mail->createAttachment( $contents, diff --git a/tests/PHPUnit/Plugins/MobileMessagingTest.php b/tests/PHPUnit/Plugins/MobileMessagingTest.php index 94796c5da34..04689274c06 100644 --- a/tests/PHPUnit/Plugins/MobileMessagingTest.php +++ b/tests/PHPUnit/Plugins/MobileMessagingTest.php @@ -211,4 +211,53 @@ public function testPhoneNumberIsSanitized() $mobileMessagingAPI->addPhoneNumber(' 6 76 93 26 47'); $this->assertEquals('676932647', key($mobileMessagingAPI->getPhoneNumbers())); } + + /** + * Dataprovider for testSendReport + */ + public function getSendReportTestCases() + { + return array( + array('reportContent', '0101010101', 'Piwik.org', 'reportContent', '0101010101', 'Piwik.org'), + array('reportContent', '0101010101', 'General_Reports', 'reportContent', '0101010101', 'General_MultiSitesSummary'), + ); + } + + /** + * @group Plugins + * @group MobileMessaging + * @dataProvider getSendReportTestCases + */ + public function testSendReport($expectedReportContent, $expectedPhoneNumber, $expectedFrom, $reportContent, $phoneNumber, $reportSubject) + { + $eventNotification = new Piwik_Event_Notification( + $this, + null, + array( + Piwik_PDFReports_API::REPORT_CONTENT_KEY => $reportContent, + Piwik_PDFReports_API::REPORT_SUBJECT_KEY => $reportSubject, + Piwik_PDFReports_API::REPORT_TYPE_INFO_KEY => Piwik_MobileMessaging::MOBILE_TYPE, + Piwik_PDFReports_API::REPORT_KEY => array( + 'parameters' => array(Piwik_MobileMessaging::PHONE_NUMBERS_PARAMETER => array($phoneNumber)), + ), + ) + ); + + $stubbedMobileMessagingAPI = $this->getMock('Piwik_MobileMessaging_API'); + $stubbedMobileMessagingAPI->expects($this->once())->method('sendSMS')->with( + $this->equalTo($expectedReportContent, 0), + $this->equalTo($expectedPhoneNumber, 1), + $this->equalTo($expectedFrom, 2) + ); + + $stubbedMobileMessagingAPIClass = new ReflectionProperty('Piwik_MobileMessaging_API', 'instance'); + $stubbedMobileMessagingAPIClass->setAccessible(true); + $stubbedMobileMessagingAPIClass->setValue($stubbedMobileMessagingAPI); + + $mobileMessaging = new Piwik_MobileMessaging(); + $mobileMessaging->sendReport($eventNotification); + + // restore Piwik_MobileMessaging_API + $stubbedMobileMessagingAPIClass->setValue(null); + } } diff --git a/tests/PHPUnit/Plugins/PDFReportsTest.php b/tests/PHPUnit/Plugins/PDFReportsTest.php index 31320e36e28..23777914db4 100644 --- a/tests/PHPUnit/Plugins/PDFReportsTest.php +++ b/tests/PHPUnit/Plugins/PDFReportsTest.php @@ -388,6 +388,35 @@ public function testGetScheduledTasks() $stubbedPDFReportsAPIClass->setValue(null); } + /** + * Dataprovider for testGetReportSubjectAndReportTitle + */ + public function getGetReportSubjectAndReportTitleTestCases() + { + return array( + array('Piwik.org', 'General_Website Piwik.org', 'Piwik.org', array('UserSettings_getBrowserType')), + array('Piwik.org', 'General_Website Piwik.org', 'Piwik.org', array('MultiSites_getAll', 'UserSettings_getBrowserType')), + array('General_MultiSitesSummary', 'General_MultiSitesSummary', 'Piwik.org', array('MultiSites_getAll')), + ); + } + + /** + * @group Plugins + * @group PDFReports + * @dataProvider getGetReportSubjectAndReportTitleTestCases + */ + public function testGetReportSubjectAndReportTitle($expectedReportSubject, $expectedReportTitle, $websiteName, $reports) + { + $getReportSubjectAndReportTitle = new ReflectionMethod( + 'Piwik_PDFReports_API', 'getReportSubjectAndReportTitle' + ); + $getReportSubjectAndReportTitle->setAccessible(true); + + list($reportSubject, $reportTitle) = $getReportSubjectAndReportTitle->invoke(new Piwik_PDFReports_API(), $websiteName, $reports); + $this->assertEquals($expectedReportSubject, $reportSubject); + $this->assertEquals($expectedReportTitle, $reportTitle); + } + private function assertReportsEqual($report, $data) { foreach ($data as $key => $value) {