From 2fe8b92040a60602fde21225bc487c329f1163f5 Mon Sep 17 00:00:00 2001 From: eileen Date: Mon, 15 May 2017 15:17:34 +1200 Subject: [PATCH] Towards CRM-20577 preliminary form tidy up --- CRM/Contact/Form/Task/PDFLetterCommon.php | 22 ++++++- CRM/Contribute/Form/Task/PDFLetterCommon.php | 60 ++++++++++++++----- .../Form/Task/PDFLetterCommonTest.php | 13 ++++ 3 files changed, 78 insertions(+), 17 deletions(-) diff --git a/CRM/Contact/Form/Task/PDFLetterCommon.php b/CRM/Contact/Form/Task/PDFLetterCommon.php index 6f1003913f..21614edf97 100644 --- a/CRM/Contact/Form/Task/PDFLetterCommon.php +++ b/CRM/Contact/Form/Task/PDFLetterCommon.php @@ -36,6 +36,8 @@ */ class CRM_Contact_Form_Task_PDFLetterCommon { + protected static $tokenCategories; + /** * @return array * Array(string $machineName => string $label). @@ -328,9 +330,7 @@ class CRM_Contact_Form_Task_PDFLetterCommon { $bao->savePdfFormat($formValues, $formValues['format_id']); } - $tokens = array(); - CRM_Utils_Hook::tokens($tokens); - $categories = array_keys($tokens); + $categories = self::getTokenCategories(); //time being hack to strip ' ' //from particular letter line, CRM-6798 @@ -352,6 +352,8 @@ class CRM_Contact_Form_Task_PDFLetterCommon { * Process the form after the input has been submitted and validated. * * @param CRM_Core_Form $form + * + * @throws \CRM_Core_Exception */ public static function postProcess(&$form) { $formValues = $form->controller->exportValues($form->getName()); @@ -604,4 +606,18 @@ class CRM_Contact_Form_Task_PDFLetterCommon { } } + /** + * Get the categories required for rendering tokens. + * + * @return array + */ + protected static function getTokenCategories() { + if (self::$tokenCategories === NULL) { + $tokens = array(); + CRM_Utils_Hook::tokens($tokens); + self::$tokenCategories = array_keys($tokens); + } + return self::$tokenCategories; + } + } diff --git a/CRM/Contribute/Form/Task/PDFLetterCommon.php b/CRM/Contribute/Form/Task/PDFLetterCommon.php index c26b46cd63..681c07590c 100644 --- a/CRM/Contribute/Form/Task/PDFLetterCommon.php +++ b/CRM/Contribute/Form/Task/PDFLetterCommon.php @@ -53,8 +53,6 @@ class CRM_Contribute_Form_Task_PDFLetterCommon extends CRM_Contact_Form_Task_PDF } } $separator = '****~~~~';// a placeholder in case the separator is common in the string - e.g ', ' - $validated = FALSE; - $groupBy = $formValues['group_by']; // skip some contacts ? @@ -67,35 +65,30 @@ class CRM_Contribute_Form_Task_PDFLetterCommon extends CRM_Contact_Form_Task_PDF } list($contributions, $contacts) = self::buildContributionArray($groupBy, $contributionIDs, $returnProperties, $skipOnHold, $skipDeceased, $messageToken, $task, $separator, $form->_includesSoftCredits); $html = array(); + $contactHtml = array(); foreach ($contributions as $contributionId => $contribution) { $contact = &$contacts[$contribution['contact_id']]; - $grouped = $groupByID = 0; + $grouped = FALSE; + $groupByID = 0; if ($groupBy) { $groupByID = empty($contribution[$groupBy]) ? 0 : $contribution[$groupBy]; $contribution = $contact['combined'][$groupBy][$groupByID]; $grouped = TRUE; } - self::assignCombinedContributionValues($contact, $contributions, $groupBy, $groupByID); - if (empty($groupBy) || empty($contact['is_sent'][$groupBy][$groupByID])) { - if (!$validated && in_array($realSeparator, $tableSeparators) && !self::isValidHTMLWithTableSeparator($messageToken, $html_message)) { - $realSeparator = ', '; - CRM_Core_Session::setStatus(ts('You have selected the table cell separator, but one or more token fields are not placed inside a table cell. This would result in invalid HTML, so comma separators have been used instead.')); - } - $validated = TRUE; - $html[$contributionId] = str_replace($separator, $realSeparator, self::resolveTokens($html_message, $contact, $contribution, $messageToken, $categories, $grouped, $separator)); - $contact['is_sent'][$groupBy][$groupByID] = TRUE; + $html[$contributionId] = self::generateHtml($contact, $contribution, $groupBy, $contributions, $realSeparator, $tableSeparators, $messageToken, $html_message, $separator, $grouped, $groupByID); + $contactHtml[$contact['contact_id']][] = $html[$contributionId]; if (!empty($formValues['email_options'])) { if (self::emailLetter($contact, $html[$contributionId], $isPDF, $formValues, $emailParams)) { $emailed++; if (!stristr($formValues['email_options'], 'both')) { - unset($html[$contributionId]); + $emailedHtml[$contributionId] = TRUE; } } } + $contact['is_sent'][$groupBy][$groupByID] = TRUE; } - // update dates (do it for each contribution including grouped recurring contribution) //@todo - the 2 calls below bypass all hooks. Using the api would possibly be slower than one call but not than 2 if ($receipt_update) { @@ -112,14 +105,17 @@ class CRM_Contribute_Form_Task_PDFLetterCommon extends CRM_Contact_Form_Task_PDF } } + if (!empty($formValues['is_unit_test'])) { return $html; } + //createActivities requires both $form->_contactIds and $contacts - //@todo - figure out why $form->_contactIds = array_keys($contacts); self::createActivities($form, $html_message, $form->_contactIds); + $html = array_diff_key($html, $emailedHtml); //CRM-19761 if (!empty($html)) { $type = $formValues['document_type']; @@ -394,4 +390,40 @@ class CRM_Contribute_Form_Task_PDFLetterCommon extends CRM_Contact_Form_Task_PDF } } + /** + * @param $contact + * @param $formValues + * @param $contribution + * @param $groupBy + * @param $contributions + * @param $realSeparator + * @param $tableSeparators + * @param $messageToken + * @param $html_message + * @param $separator + * @param $categories + * @param bool $grouped + * @param int $groupByID + * + * @return string + */ + protected static function generateHtml(&$contact, $contribution, $groupBy, $contributions, $realSeparator, $tableSeparators, $messageToken, $html_message, $separator, $grouped, $groupByID) { + static $validated = FALSE; + $html = NULL; + + $categories = self::getTokenCategories(); + self::assignCombinedContributionValues($contact, $contributions, $groupBy, $groupByID); + + if (empty($groupBy) || empty($contact['is_sent'][$groupBy][$groupByID])) { + if (!$validated && in_array($realSeparator, $tableSeparators) && !self::isValidHTMLWithTableSeparator($messageToken, $html_message)) { + $realSeparator = ', '; + CRM_Core_Session::setStatus(ts('You have selected the table cell separator, but one or more token fields are not placed inside a table cell. This would result in invalid HTML, so comma separators have been used instead.')); + } + $validated = TRUE; + $html = str_replace($separator, $realSeparator, self::resolveTokens($html_message, $contact, $contribution, $messageToken, $categories, $grouped, $separator)); + } + + return $html; + } + } diff --git a/tests/phpunit/CRM/Contribute/Form/Task/PDFLetterCommonTest.php b/tests/phpunit/CRM/Contribute/Form/Task/PDFLetterCommonTest.php index ca0104d0e8..110e93af5c 100644 --- a/tests/phpunit/CRM/Contribute/Form/Task/PDFLetterCommonTest.php +++ b/tests/phpunit/CRM/Contribute/Form/Task/PDFLetterCommonTest.php @@ -43,6 +43,15 @@ class CRM_Contribute_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase { protected $_contactIds = NULL; + /** + * Count how many times the hookTokens is called. + * + * This only needs to be called once, check refactoring doesn't change this. + * + * @var int + */ + protected $hookTokensCalled = 0; + protected function setUp() { parent::setUp(); $this->_individualId = $this->individualCreate(array('first_name' => 'Anthony', 'last_name' => 'Collins')); @@ -281,6 +290,9 @@ class CRM_Contribute_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase { ", $html[2]); + // Checking it is not called multiple times. + // once for each contact create + once for the activities. + $this->assertEquals(3, $this->hookTokensCalled); } @@ -288,6 +300,7 @@ class CRM_Contribute_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase { * Implements civicrm_tokens(). */ function hook_tokens(&$tokens) { + $this->hookTokensCalled++; $tokens['aggregate'] = array('rendered_token' => 'rendered_token'); } -- 2.25.1