From d0d1c38b95f7daac9183d97ec012c9949c1a1feb Mon Sep 17 00:00:00 2001 From: demeritcowboy Date: Sun, 12 Jul 2020 21:15:01 -0400 Subject: [PATCH] include BOM in attachment when sending csv civireport via mail_report job --- CRM/Report/Form.php | 1 + CRM/Report/Utils/Report.php | 10 +- CRM/Utils/Mail.php | 6 +- .../fixtures/DetailPostalCodeTest-ascii.csv | 2 +- .../Form/Contribute/fixtures/report-ascii.csv | 2 +- .../Form/Contribute/fixtures/report.csv | 2 +- .../phpunit/CRM/Report/Form/TestCaseTest.php | 6 + tests/phpunit/CRM/Report/Utils/ReportTest.php | 2 +- tests/phpunit/api/v3/JobTest.php | 189 ++++++++++++++++++ 9 files changed, 210 insertions(+), 10 deletions(-) diff --git a/CRM/Report/Form.php b/CRM/Report/Form.php index 31866f648a..4947fa749f 100644 --- a/CRM/Report/Form.php +++ b/CRM/Report/Form.php @@ -3437,6 +3437,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 e2ca8de213..b3fd9556fa 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 2b6e5120a7..a49cb27acd 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 0a5a5150d7..cf9595d864 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 c1990e5d5a..82bb1c2191 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 504d5bcf8f..0abd1e3788 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 15f7586e34..124c092b6d 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 68326f7cdb..729e3ab7ef 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 = << 'Semi-formal explanation of runtime job parameters', 'is_active' => 1, ]; + $this->report_instance = $this->createReportInstance(); } /** @@ -2227,4 +2234,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('CiviCRM Report', $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('CiviCRM Report', $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', + ]); + } + } -- 2.25.1