From 6b2bb39dae8a3d6d969cc269dd20999eea8309dc Mon Sep 17 00:00:00 2001 From: eileen Date: Mon, 1 Feb 2021 16:01:35 +1300 Subject: [PATCH] [REF] Prelminary tidy up to support fix for dev/core#2344 This is all the formatting, doc block & test support fixes to support my fix for https://lab.civicrm.org/dev/core/-/issues/2344 It should all be NFC - the key change is better test support for when pdfs or docs are generated - we now throw an exception that can be handled in the test. --- CRM/Contribute/Form/AdditionalInfo.php | 3 +- CRM/Contribute/Form/Task/PDFLetterCommon.php | 12 ++--- CRM/Utils/PDF/Document.php | 3 ++ CRM/Utils/PDF/Utils.php | 3 ++ CRM/Utils/Token.php | 9 ++-- .../CRM/Contribute/Form/ContributionTest.php | 4 +- .../Form/Task/PDFLetterCommonTest.php | 45 ++++++++++++++----- tests/phpunit/CiviTest/CiviUnitTestCase.php | 2 +- 8 files changed, 53 insertions(+), 28 deletions(-) diff --git a/CRM/Contribute/Form/AdditionalInfo.php b/CRM/Contribute/Form/AdditionalInfo.php index 29403fad6488..f16b26c0a791 100644 --- a/CRM/Contribute/Form/AdditionalInfo.php +++ b/CRM/Contribute/Form/AdditionalInfo.php @@ -426,7 +426,8 @@ public static function emailReceipt(&$form, &$params, $ccContribution = FALSE) { $eventTaxAmt = $template->get_template_vars('totalTaxAmount'); $prefixValue = Civi::settings()->get('contribution_invoice_settings'); $invoicing = $prefixValue['invoicing'] ?? NULL; - if ((!empty($taxAmt) || isset($eventTaxAmt)) && (isset($invoicing) && isset($prefixValue['is_email_pdf']))) { + // @todo - align with https://lab.civicrm.org/dev/financial/-/issues/162 + if ((!empty($taxAmt) || isset($eventTaxAmt)) && (isset($invoicing) && !empty($prefixValue['is_email_pdf']))) { $isEmailPdf = TRUE; } else { diff --git a/CRM/Contribute/Form/Task/PDFLetterCommon.php b/CRM/Contribute/Form/Task/PDFLetterCommon.php index be8422ac9acd..e9a4a4c70786 100644 --- a/CRM/Contribute/Form/Task/PDFLetterCommon.php +++ b/CRM/Contribute/Form/Task/PDFLetterCommon.php @@ -30,7 +30,7 @@ public static function postProcess(&$form, $formValues = NULL) { if (empty($formValues)) { $formValues = $form->controller->exportValues($form->getName()); } - list($formValues, $categories, $html_message, $messageToken, $returnProperties) = self::processMessageTemplate($formValues); + [$formValues, $categories, $html_message, $messageToken, $returnProperties] = self::processMessageTemplate($formValues); $isPDF = FALSE; $emailParams = []; if (!empty($formValues['email_options'])) { @@ -82,7 +82,7 @@ public static function postProcess(&$form, $formValues = NULL) { //@todo - comment on what is stored there $contributionIDs = $form->getVar('_contributionContactIds'); } - list($contributions, $contacts) = self::buildContributionArray($groupBy, $contributionIDs, $returnProperties, $skipOnHold, $skipDeceased, $messageToken, $task, $separator, $form->_includesSoftCredits); + [$contributions, $contacts] = self::buildContributionArray($groupBy, $contributionIDs, $returnProperties, $skipOnHold, $skipDeceased, $messageToken, $task, $separator, $form->_includesSoftCredits); $html = []; $contactHtml = $emailedHtml = []; foreach ($contributions as $contributionId => $contribution) { @@ -127,15 +127,11 @@ public static function postProcess(&$form, $formValues = NULL) { self::createActivities($form, $html_message, $contactIds, CRM_Utils_Array::value('subject', $formValues, ts('Thank you letter')), CRM_Utils_Array::value('campaign_id', $formValues), $contactHtml); $html = array_diff_key($html, $emailedHtml); - if (!empty($formValues['is_unit_test'])) { - return $html; - } - //CRM-19761 if (!empty($html)) { $type = $formValues['document_type']; - if ($type == 'pdf') { + if ($type === 'pdf') { CRM_Utils_PDF_Utils::html2pdf($html, "CiviLetter.pdf", FALSE, $formValues); } else { @@ -265,7 +261,7 @@ public static function buildContributionArray($groupBy, $contributionIDs, $retur if ($isIncludeSoftCredits) { //@todo find out why this happens & add comments - list($contactID) = explode('-', $item); + [$contactID] = explode('-', $item); $contactID = (int) $contactID; } else { diff --git a/CRM/Utils/PDF/Document.php b/CRM/Utils/PDF/Document.php index e087a9228aac..16c614325cea 100644 --- a/CRM/Utils/PDF/Document.php +++ b/CRM/Utils/PDF/Document.php @@ -48,6 +48,9 @@ class CRM_Utils_PDF_Document { * @param array|int $format */ public static function html2doc($pages, $fileName, $format = []) { + if (CIVICRM_UF === 'UnitTests') { + throw new CRM_Core_Exception_PrematureExitException('html2doc called', ['pages' => $pages]); + } if (is_array($format)) { // PDF Page Format parameters passed in - merge with defaults $format += CRM_Core_BAO_PdfFormat::getDefaultValues(); diff --git a/CRM/Utils/PDF/Utils.php b/CRM/Utils/PDF/Utils.php index 730bbc3d4448..b21fe464115a 100644 --- a/CRM/Utils/PDF/Utils.php +++ b/CRM/Utils/PDF/Utils.php @@ -34,6 +34,9 @@ class CRM_Utils_PDF_Utils { * @return string|void */ public static function html2pdf($text, $fileName = 'civicrm.pdf', $output = FALSE, $pdfFormat = NULL) { + if (CIVICRM_UF === 'UnitTests') { + throw new CRM_Core_Exception_PrematureExitException('html2pdf called', ['text' => $text]); + } if (is_array($text)) { $pages = &$text; } diff --git a/CRM/Utils/Token.php b/CRM/Utils/Token.php index 923635f8ec56..4e7633cb32d4 100644 --- a/CRM/Utils/Token.php +++ b/CRM/Utils/Token.php @@ -1106,7 +1106,7 @@ public static function getTokens($string) { if ($matches[1]) { foreach ($matches[1] as $token) { - list($type, $name) = preg_split('/\./', $token, 2); + [$type, $name] = preg_split('/\./', $token, 2); if ($name && $type) { if (!isset($tokens[$type])) { $tokens[$type] = []; @@ -1135,7 +1135,7 @@ public static function getReturnProperties(&$string) { ); if ($matches[1]) { foreach ($matches[1] as $token) { - list($type, $name) = preg_split('/\./', $token, 2); + [$type, $name] = preg_split('/\./', $token, 2); if ($name) { $returnProperties["{$name}"] = 1; } @@ -1407,7 +1407,7 @@ public static function replaceGreetingTokens(&$tokenString, $contactDetails = NU ); // Prepare variables for calling replaceHookTokens $categories = array_keys($greetingTokens); - list($contact) = $greetingDetails; + [$contact] = $greetingDetails; // Replace tokens defined in Hooks. $tokenString = CRM_Utils_Token::replaceHookTokens($tokenString, $contact[$contactId], $categories); } @@ -1532,7 +1532,7 @@ function ($matches) use ($escapeSmarty) { public static function getUserTokenReplacement($token, $escapeSmarty = FALSE) { $value = ''; - list($objectName, $objectValue) = explode('-', $token, 2); + [$objectName, $objectValue] = explode('-', $token, 2); switch ($objectName) { case 'permission': @@ -1792,6 +1792,7 @@ public static function getMembershipTokenReplacement($entity, $token, $membershi * @param bool $escapeSmarty * * @return mixed|string + * @throws \CRM_Core_Exception */ public static function getContributionTokenReplacement($token, &$contribution, $html = FALSE, $escapeSmarty = FALSE) { self::_buildContributionTokens(); diff --git a/tests/phpunit/CRM/Contribute/Form/ContributionTest.php b/tests/phpunit/CRM/Contribute/Form/ContributionTest.php index 37ba9509563c..3bdf2aa96811 100644 --- a/tests/phpunit/CRM/Contribute/Form/ContributionTest.php +++ b/tests/phpunit/CRM/Contribute/Form/ContributionTest.php @@ -1161,7 +1161,7 @@ public function testSubmitWithOutSaleTax() { * * @throws \Exception */ - public function testReSubmitSaleTax($thousandSeparator) { + public function testReSubmitSaleTax($thousandSeparator): void { $this->setCurrencySeparators($thousandSeparator); $this->enableTaxAndInvoicing(); $this->addTaxAccountToFinancialType($this->_financialTypeId); @@ -1205,7 +1205,7 @@ public function testReSubmitSaleTax($thousandSeparator) { $mut->checkMailLog($strings); $this->callAPISuccessGetCount('FinancialTrxn', [], 3); $items = $this->callAPISuccess('FinancialItem', 'get', ['sequential' => 1])['values']; - $this->assertEquals(2, count($items)); + $this->assertCount(2, $items); $this->assertEquals('Contribution Amount', $items[0]['description']); $this->assertEquals('Sales Tax', $items[1]['description']); diff --git a/tests/phpunit/CRM/Contribute/Form/Task/PDFLetterCommonTest.php b/tests/phpunit/CRM/Contribute/Form/Task/PDFLetterCommonTest.php index 5d606d08103a..6a29fbe5f5e4 100644 --- a/tests/phpunit/CRM/Contribute/Form/Task/PDFLetterCommonTest.php +++ b/tests/phpunit/CRM/Contribute/Form/Task/PDFLetterCommonTest.php @@ -22,7 +22,7 @@ class CRM_Contribute_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase { protected $_docTypes = NULL; - protected $_contactIds = NULL; + protected $_contactIds; /** * Count how many times the hookTokens is called. @@ -41,6 +41,8 @@ protected function setUp() { /** * Clean up after each test. + * + * @throws \CRM_Core_Exception */ public function tearDown() { $this->quickCleanUpFinancialEntities(); @@ -50,8 +52,10 @@ public function tearDown() { /** * Test the buildContributionArray function. + * + * @throws \CRM_Core_Exception */ - public function testBuildContributionArray() { + public function testBuildContributionArray(): void { $this->_individualId = $this->individualCreate(); $customGroup = $this->callAPISuccess('CustomGroup', 'create', [ @@ -69,7 +73,7 @@ public function testBuildContributionArray() { ]; $customField = $this->callAPISuccess('CustomField', 'create', $params); $customFieldKey = 'custom_' . $customField['id']; - $campaignTitle = 'Test Campaign ' . substr(sha1(rand()), 0, 7); + $campaignTitle = 'Test Campaign '; $params = [ 'contact_id' => $this->_individualId, @@ -94,7 +98,7 @@ public function testBuildContributionArray() { ], ]; - list($contributions, $contacts) = CRM_Contribute_Form_Task_PDFLetterCommon::buildContributionArray('contact_id', $contributionIDs, $returnProperties, TRUE, TRUE, $messageToken, 'test', '**', FALSE); + [$contributions, $contacts] = CRM_Contribute_Form_Task_PDFLetterCommon::buildContributionArray('contact_id', $contributionIDs, $returnProperties, TRUE, TRUE, $messageToken, 'test', '**', FALSE); $this->assertEquals('Anthony', $contacts[$this->_individualId]['first_name']); $this->assertEquals('emo', $contacts[$this->_individualId]['favourite_emoticon']); @@ -126,13 +130,15 @@ public function hookTokenValues(&$details, $contactIDs, $jobID, $tokens, $classN /** * Test contribution token replacement in * html returned by postProcess function. + * + * @throws \CiviCRM_API3_Exception + * @throws \CRM_Core_Exception */ - public function testPostProcess() { + public function testPostProcess(): void { $this->createLoggedInUser(); $this->_individualId = $this->individualCreate(); foreach (['docx', 'odt'] as $docType) { $formValues = [ - 'is_unit_test' => TRUE, 'group_by' => NULL, 'document_file' => [ 'name' => __DIR__ . "/sample_documents/Template.$docType", @@ -153,7 +159,12 @@ public function testPostProcess() { $date = CRM_Utils_Date::getToday(); $displayDate = CRM_Utils_Date::customFormat($date, $format); - $html = CRM_Contribute_Form_Task_PDFLetterCommon::postProcess($form, $formValues); + try { + CRM_Contribute_Form_Task_PDFLetterCommon::postProcess($form, $formValues); + } + catch (CRM_Core_Exception_PrematureExitException $e) { + $html = $e->errorData['pages']; + } $expectedValues = [ 'Hello Anthony', '$ 100.00', @@ -171,10 +182,14 @@ public function testPostProcess() { /** * Test assignment of variables when using the group by function. * - * We are looking to see that the contribution aggregate and contributions arrays reflect the most - * recent contact rather than a total aggregate, since we are using group by. + * We are looking to see that the contribution aggregate and contributions + * arrays reflect the most recent contact rather than a total aggregate, + * since we are using group by. + * + * @throws \CiviCRM_API3_Exception + * @throws \CRM_Core_Exception */ - public function testPostProcessGroupByContact() { + public function testPostProcessGroupByContact(): void { $this->createLoggedInUser(); $this->hookClass->setHook('civicrm_tokenValues', [$this, 'hook_aggregateTokenValues']); $this->hookClass->setHook('civicrm_tokens', [$this, 'hook_tokens']); @@ -183,12 +198,12 @@ public function testPostProcessGroupByContact() { $this->_individualId2 = $this->individualCreate(); $htmlMessage = "{aggregate.rendered_token}"; $formValues = [ - 'is_unit_test' => TRUE, 'group_by' => 'contact_id', 'html_message' => $htmlMessage, 'email_options' => 'both', 'subject' => 'Testy test test', 'from' => 'info@example.com', + 'document_type' => 'doc', ]; $contributionIDs = []; @@ -218,7 +233,13 @@ public function testPostProcessGroupByContact() { $form = new CRM_Contribute_Form_Task_PDFLetter(); $form->setContributionIds($contributionIDs); - $html = CRM_Contribute_Form_Task_PDFLetterCommon::postProcess($form, $formValues); + try { + CRM_Contribute_Form_Task_PDFLetterCommon::postProcess($form, $formValues); + } + catch (CRM_Core_Exception_PrematureExitException $e) { + $html = $e->errorData['pages']; + } + $this->assertEquals(" diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index 54fb3746d968..b0ac63886fb5 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -3065,7 +3065,7 @@ protected function enableTaxAndInvoicing($params = []) { 'due_date' => 10, 'due_date_period' => 'days', 'notes' => '', - 'is_email_pdf' => 1, + 'is_email_pdf' => FALSE, 'tax_term' => 'Sales Tax', 'tax_display_settings' => 'Inclusive', ]