diff --git a/CRM/Report/Form.php b/CRM/Report/Form.php index 481dd3fd3d136374581a9e47cf004d56e908f51e..945e794f4e82ff943d53fee6cbf4a218ca3f5440 100644 --- a/CRM/Report/Form.php +++ b/CRM/Report/Form.php @@ -3442,6 +3442,7 @@ WHERE cg.extends IN ('" . implode("','", $this->_customGroupExtends) . "') AND 'fullPath' => $csvFullFilename, 'mime_type' => 'text/csv', 'cleanName' => 'CiviReport.csv', + 'charset' => 'utf-8', ]; } if ($this->_outputMode == 'pdf') { diff --git a/CRM/Report/Utils/Report.php b/CRM/Report/Utils/Report.php index e2ca8de2139b9c21c1185296d90ee6884e401184..b3fd9556fa74b4c2e18444ff6ed1523f95775f21 100644 --- a/CRM/Report/Utils/Report.php +++ b/CRM/Report/Utils/Report.php @@ -209,10 +209,6 @@ WHERE inst.report_id = %1"; //Force a download and name the file using the current timestamp. $datetime = date('Ymd-Gi', $_SERVER['REQUEST_TIME']); CRM_Utils_System::setHttpHeader('Content-Disposition', 'attachment; filename=Report_' . $datetime . '.csv'); - // Output UTF BOM so that MS Excel copes with diacritics. This is recommended as - // the Windows variant but is tested with MS Excel for Mac (Office 365 v 16.31) - // and it continues to work on Libre Office, Numbers, Notes etc. - echo "\xEF\xBB\xBF"; echo self::makeCsv($form, $rows); CRM_Utils_System::civiExit(); } @@ -228,7 +224,11 @@ WHERE inst.report_id = %1"; */ public static function makeCsv(&$form, &$rows) { $config = CRM_Core_Config::singleton(); - $csv = ''; + + // Output UTF BOM so that MS Excel copes with diacritics. This is recommended as + // the Windows variant but is tested with MS Excel for Mac (Office 365 v 16.31) + // and it continues to work on Libre Office, Numbers, Notes etc. + $csv = "\xEF\xBB\xBF"; // Add headers if this is the first row. $columnHeaders = array_keys($form->_columnHeaders); diff --git a/CRM/Utils/Mail.php b/CRM/Utils/Mail.php index 2b6e5120a74ac92bb92c7df668bcd9768001b3ec..a49cb27acd77f2a067807232d7def335d8d180ee 100644 --- a/CRM/Utils/Mail.php +++ b/CRM/Utils/Mail.php @@ -261,7 +261,11 @@ class CRM_Utils_Mail { $msg->addAttachment( $attach['fullPath'], $attach['mime_type'], - $attach['cleanName'] + $attach['cleanName'], + TRUE, + 'base64', + 'attachment', + (isset($attach['charset']) ? $attach['charset'] : '') ); } } diff --git a/tests/phpunit/CRM/Report/Form/Contribute/fixtures/DetailPostalCodeTest-ascii.csv b/tests/phpunit/CRM/Report/Form/Contribute/fixtures/DetailPostalCodeTest-ascii.csv index 0a5a5150d77a1b612b573c2b18c33c5be35cda9f..cf9595d864087e4f7dd1f911989d75242a3e557f 100644 --- a/tests/phpunit/CRM/Report/Form/Contribute/fixtures/DetailPostalCodeTest-ascii.csv +++ b/tests/phpunit/CRM/Report/Form/Contribute/fixtures/DetailPostalCodeTest-ascii.csv @@ -1,4 +1,4 @@ -"Donor Name","First Name","Donor Email","Amount","Postal Code" +"Donor Name","First Name","Donor Email","Amount","Postal Code" " Empowerment Association",,,"$ 50.00","B10 G56" "Bachman, Lincoln","Lincoln",,"$ 175.00","B10 G56" "Grant, Megan","Megan","grantm@fishmail.net","$ 500.00","B10 G56" diff --git a/tests/phpunit/CRM/Report/Form/Contribute/fixtures/report-ascii.csv b/tests/phpunit/CRM/Report/Form/Contribute/fixtures/report-ascii.csv index c1990e5d5a632802a8e69055c1582277a064a9bc..82bb1c219113d8869d794aff0cc893f7df3c9d26 100644 --- a/tests/phpunit/CRM/Report/Form/Contribute/fixtures/report-ascii.csv +++ b/tests/phpunit/CRM/Report/Form/Contribute/fixtures/report-ascii.csv @@ -1,4 +1,4 @@ -"Donor Name","First Name","Donor Email","Amount" +"Donor Name","First Name","Donor Email","Amount" " Empowerment Association",,,"$ 50.00" "Bachman, Lincoln","Lincoln",,"$ 175.00" "Blackwell, Sanford","Sanford","st.blackwell3@testmail.co.pl","$ 250.00" diff --git a/tests/phpunit/CRM/Report/Form/Contribute/fixtures/report.csv b/tests/phpunit/CRM/Report/Form/Contribute/fixtures/report.csv index 504d5bcf8f49270aed5be52aef8fa2c5d66092d1..0abd1e3788ff9d8b5986d1bd9776266aa81b83e0 100644 --- a/tests/phpunit/CRM/Report/Form/Contribute/fixtures/report.csv +++ b/tests/phpunit/CRM/Report/Form/Contribute/fixtures/report.csv @@ -1,4 +1,4 @@ -"Donor Name","First Name","Donor Email","Amount" +"Donor Name","First Name","Donor Email","Amount" " Empowerment Association", , ,"$ 50.00" "Bachman, Lincoln","Lincoln", ,"$ 175.00" "Blackwell, Sanford","Sanford","st.blackwell3@testmail.co.pl","$ 250.00" diff --git a/tests/phpunit/CRM/Report/Form/TestCaseTest.php b/tests/phpunit/CRM/Report/Form/TestCaseTest.php index 15f7586e3498741f44b13b0a0186a29d7f82c6b7..124c092b6d4d011610a38e261cac41fe9976c892 100644 --- a/tests/phpunit/CRM/Report/Form/TestCaseTest.php +++ b/tests/phpunit/CRM/Report/Form/TestCaseTest.php @@ -155,6 +155,12 @@ class CRM_Report_Form_TestCaseTest extends CiviReportTestCase { $expectedOutputCsvArray = $this->getArrayFromCsv(dirname(__FILE__) . "/{$expectedOutputCsvFile}"); try { $this->assertCsvArraysEqual($expectedOutputCsvArray, $reportCsvArray); + // @todo But...doesn't this mean this test can't ever notify you of a + // fail? I think what you could do instead is right here in the try + // section throw something that doesn't get caught, since then that means + // the previous line passed and so the arrays ARE equal, which means + // something is wrong. Or just don't use assertCsvArraysEqual and + // explicity compare that they are NOT equal. } catch (PHPUnit\Framework\AssertionFailedError $e) { /* OK */ diff --git a/tests/phpunit/CRM/Report/Utils/ReportTest.php b/tests/phpunit/CRM/Report/Utils/ReportTest.php index 68326f7cdb01ed261db2636c6ff9b823b6d6c5dd..729e3ab7ef15715408c38bc2c51c6a130799464a 100644 --- a/tests/phpunit/CRM/Report/Utils/ReportTest.php +++ b/tests/phpunit/CRM/Report/Utils/ReportTest.php @@ -47,7 +47,7 @@ class CRM_Report_Utils_ReportTest extends CiviUnitTestCase { ENDDETAILS; $expectedOutput = <<<ENDOUTPUT -"Activity Type","Subject","Activity Details"\r +\xEF\xBB\xBF"Activity Type","Subject","Activity Details"\r "Meeting","Meeting with the apostrophe's and that person who does ""air quotes"". Some non-ascii characters: дè","Here's some typical data from an activity details field. дè some non-ascii and html styling and these ̋“weird” quotes’s. diff --git a/tests/phpunit/api/v3/JobTest.php b/tests/phpunit/api/v3/JobTest.php index 9eb857ff85d9706c41e547577c39ed00b288d110..f042fa9585ecd66a4df70d97c8527df671d6409a 100644 --- a/tests/phpunit/api/v3/JobTest.php +++ b/tests/phpunit/api/v3/JobTest.php @@ -38,6 +38,12 @@ class api_v3_JobTest extends CiviUnitTestCase { */ public $membershipTypeID; + /** + * Report instance used in mail_report tests. + * @var array + */ + private $report_instance; + /** * Set up for tests. */ @@ -55,6 +61,7 @@ class api_v3_JobTest extends CiviUnitTestCase { 'parameters' => 'Semi-formal explanation of runtime job parameters', 'is_active' => 1, ]; + $this->report_instance = $this->createReportInstance(); } /** @@ -2253,4 +2260,186 @@ class api_v3_JobTest extends CiviUnitTestCase { return [$membershipTypeID, $groupID, $theChosenOneID, $provider]; } + /** + * Test that the mail_report job sends an email for 'print' format. + * + * We're not testing that the report itself is correct since in 'print' + * format it's a little difficult to parse out, so we're just testing that + * the email was sent and it more or less looks like an email we'd expect. + */ + public function testMailReportForPrint() { + $mut = new CiviMailUtils($this, TRUE); + + // avoid warnings + if (empty($_SERVER['QUERY_STRING'])) { + $_SERVER['QUERY_STRING'] = 'reset=1'; + } + + $this->callAPISuccess('job', 'mail_report', [ + 'instanceId' => $this->report_instance['id'], + 'format' => 'print', + ]); + + $message = $mut->getMostRecentEmail('ezc'); + + $this->assertEquals('This is the email subject', $message->subject); + $this->assertEquals('reportperson@example.com', $message->to[0]->email); + + $parts = $message->fetchParts(NULL, TRUE); + $this->assertCount(1, $parts); + $this->assertStringContainsString('test report', $parts[0]->text); + + $mut->clearMessages(); + $mut->stop(); + } + + /** + * Test that the mail_report job sends an email for 'pdf' format. + * + * We're not testing that the report itself is correct since in 'pdf' + * format it's a little difficult to parse out, so we're just testing that + * the email was sent and it more or less looks like an email we'd expect. + */ + public function testMailReportForPdf() { + $mut = new CiviMailUtils($this, TRUE); + + // avoid warnings + if (empty($_SERVER['QUERY_STRING'])) { + $_SERVER['QUERY_STRING'] = 'reset=1'; + } + + $this->callAPISuccess('job', 'mail_report', [ + 'instanceId' => $this->report_instance['id'], + 'format' => 'pdf', + ]); + + $message = $mut->getMostRecentEmail('ezc'); + + $this->assertEquals('This is the email subject', $message->subject); + $this->assertEquals('reportperson@example.com', $message->to[0]->email); + + $parts = $message->fetchParts(NULL, TRUE); + $this->assertCount(2, $parts); + $this->assertStringContainsString('<title>CiviCRM Report</title>', $parts[0]->text); + $this->assertEquals(ezcMailFilePart::CONTENT_TYPE_APPLICATION, $parts[1]->contentType); + $this->assertEquals('pdf', $parts[1]->mimeType); + $this->assertEquals(ezcMailFilePart::DISPLAY_ATTACHMENT, $parts[1]->dispositionType); + $this->assertGreaterThan(0, filesize($parts[1]->fileName)); + + $mut->clearMessages(); + $mut->stop(); + } + + /** + * Test that the mail_report job sends an email for 'csv' format. + * + * As with the print and pdf we're not super-concerned about report + * functionality itself - we're more concerned with the mailing part, + * but since it's csv we can easily check the output. + */ + public function testMailReportForCsv() { + // Create many contacts, in particular so that the report would be more + // than a one-pager. + for ($i = 0; $i < 110; $i++) { + $this->individualCreate([], $i, TRUE); + } + + $mut = new CiviMailUtils($this, TRUE); + + // avoid warnings + if (empty($_SERVER['QUERY_STRING'])) { + $_SERVER['QUERY_STRING'] = 'reset=1'; + } + + $this->callAPISuccess('job', 'mail_report', [ + 'instanceId' => $this->report_instance['id'], + 'format' => 'csv', + ]); + + $message = $mut->getMostRecentEmail('ezc'); + + $this->assertEquals('This is the email subject', $message->subject); + $this->assertEquals('reportperson@example.com', $message->to[0]->email); + + $parts = $message->fetchParts(NULL, TRUE); + $this->assertCount(2, $parts); + $this->assertStringContainsString('<title>CiviCRM Report</title>', $parts[0]->text); + $this->assertEquals('csv', $parts[1]->subType); + + // Pull all the contacts to get our expected output. + $contacts = $this->callAPISuccess('Contact', 'get', [ + 'return' => 'sort_name', + 'options' => [ + 'limit' => 0, + 'sort' => 'sort_name', + ], + ]); + $rows = []; + foreach ($contacts['values'] as $contact) { + $rows[] = ['civicrm_contact_sort_name' => $contact['sort_name']]; + } + // need this for makeCsv() + $fakeForm = new CRM_Report_Form(); + $fakeForm->_columnHeaders = [ + 'civicrm_contact_sort_name' => [ + 'title' => 'Contact Name', + 'type' => 2, + ], + ]; + + $this->assertEquals( + CRM_Report_Utils_Report::makeCsv($fakeForm, $rows), + $parts[1]->text + ); + + $mut->clearMessages(); + $mut->stop(); + } + + /** + * Helper to create a report instance of the contact summary report. + */ + private function createReportInstance() { + return $this->callAPISuccess('ReportInstance', 'create', [ + 'report_id' => 'contact/summary', + 'title' => 'test report', + 'form_values' => [ + serialize([ + 'fields' => [ + 'sort_name' => '1', + 'street_address' => '1', + 'city' => '1', + 'country_id' => '1', + ], + 'sort_name_op' => 'has', + 'sort_name_value' => '', + 'source_op' => 'has', + 'source_value' => '', + 'id_min' => '', + 'id_max' => '', + 'id_op' => 'lte', + 'id_value' => '', + 'country_id_op' => 'in', + 'country_id_value' => [], + 'state_province_id_op' => 'in', + 'state_province_id_value' => [], + 'gid_op' => 'in', + 'gid_value' => [], + 'tagid_op' => 'in', + 'tagid_value' => [], + 'description' => 'Provides a list of address and telephone information for constituent records in your system.', + 'email_subject' => 'This is the email subject', + 'email_to' => 'reportperson@example.com', + 'email_cc' => '', + 'permission' => 'view all contacts', + 'groups' => '', + 'domain_id' => 1, + ]), + ], + // Email params need to be repeated outside form_values for some reason + 'email_subject' => 'This is the email subject', + 'email_to' => 'reportperson@example.com', + ]); + } + }