From 12d8880731a3b57eff850396bab406538657254c Mon Sep 17 00:00:00 2001 From: eileen Date: Wed, 28 Apr 2021 11:23:29 +1200 Subject: [PATCH] [REF] Complete decommissioning on CRM/Contribute/Form/Task/PDFLetterCommon.php This removes the last function fromCRM/Contribute/Form/Task/PDFLetterCommon.php The function was calling a protected function on the parent which had to be unravelled. I used the method we used in https://github.com/civicrm/civicrm-core/blob/5e67eb7ff3f1f202d5112d15ad5c6a5f23f35795/CRM/SMS/Form/Upload.php#L341 to replace the code with the part that we have tested as being done in via render - ie replacing hook, contact, domain tokens & smarty parsing. However, there are quite a few tests on this code & one checked that we don't over-call the token hook. I added some caching for the results of this hook so we can start to eliminate these but I also allowed the calls to increment by 1 because it is not 1 per row I made the results of this hook optional to replaceHookTokens & in general I think that is the only place it needs to be called - perhaps once it used to pass more parameters & the results were dynamic but now they really aren't --- CRM/Contribute/Form/Task/PDFLetter.php | 28 ++++++++++- CRM/Contribute/Form/Task/PDFLetterCommon.php | 46 ------------------- CRM/Utils/Token.php | 19 +++++++- Civi/Token/TokenCompatSubscriber.php | 13 ++---- .../Form/Task/PDFLetterCommonTest.php | 8 ++-- 5 files changed, 55 insertions(+), 59 deletions(-) delete mode 100644 CRM/Contribute/Form/Task/PDFLetterCommon.php diff --git a/CRM/Contribute/Form/Task/PDFLetter.php b/CRM/Contribute/Form/Task/PDFLetter.php index 246990ecaa..d5046e8046 100644 --- a/CRM/Contribute/Form/Task/PDFLetter.php +++ b/CRM/Contribute/Form/Task/PDFLetter.php @@ -435,7 +435,7 @@ class CRM_Contribute_Form_Task_PDFLetter extends CRM_Contribute_Form_Task { 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, CRM_Contribute_Form_Task_PDFLetterCommon::resolveTokens($html_message, $contact, $contribution, $messageToken, $grouped, $separator, $groupedContributions)); + $html = str_replace($separator, $realSeparator, $this->resolveTokens($html_message, $contact, $contribution, $messageToken, $grouped, $separator, $groupedContributions)); } return $html; @@ -539,4 +539,30 @@ class CRM_Contribute_Form_Task_PDFLetter extends CRM_Contribute_Form_Task { return TRUE; } + /** + * + * @param string $html_message + * @param array $contact + * @param array $contribution + * @param array $messageToken + * @param bool $grouped + * Does this letter represent more than one contribution. + * @param string $separator + * What is the preferred letter separator. + * @param array $contributions + * + * @return string + */ + protected function resolveTokens(string $html_message, $contact, $contribution, $messageToken, $grouped, $separator, $contributions): string { + if ($grouped) { + $tokenHtml = CRM_Utils_Token::replaceMultipleContributionTokens($separator, $html_message, $contributions, $messageToken); + } + else { + // no change to normal behaviour to avoid risk of breakage + $tokenHtml = CRM_Utils_Token::replaceContributionTokens($html_message, $contribution, TRUE, $messageToken); + } + $useSmarty = (defined('CIVICRM_MAIL_SMARTY') && CIVICRM_MAIL_SMARTY); + return CRM_Core_BAO_MessageTemplate::renderMessageTemplate(['text' => '', 'html' => $tokenHtml, 'subject' => ''], !$useSmarty, $contact['contact_id'], ['contact' => $contact])['html']; + } + } diff --git a/CRM/Contribute/Form/Task/PDFLetterCommon.php b/CRM/Contribute/Form/Task/PDFLetterCommon.php deleted file mode 100644 index e2a00b3542..0000000000 --- a/CRM/Contribute/Form/Task/PDFLetterCommon.php +++ /dev/null @@ -1,46 +0,0 @@ -assign_by_ref('contact', $contact); - $tokenHtml = $smarty->fetch("string:$tokenHtml"); - } - return $tokenHtml; - } - -} diff --git a/CRM/Utils/Token.php b/CRM/Utils/Token.php index 6d17edcfc3..2dd76bf4f8 100644 --- a/CRM/Utils/Token.php +++ b/CRM/Utils/Token.php @@ -781,10 +781,13 @@ class CRM_Utils_Token { public static function &replaceHookTokens( $str, &$contact, - &$categories, + $categories = NULL, $html = FALSE, $escapeSmarty = FALSE ) { + if (!$categories) { + $categories = self::getTokenCategories(); + } foreach ($categories as $key) { $str = preg_replace_callback( self::tokenRegex($key), @@ -797,6 +800,20 @@ class CRM_Utils_Token { return $str; } + /** + * Get the categories required for rendering tokens. + * + * @return array + */ + public static function getTokenCategories(): array { + if (!isset(\Civi::$statics[__CLASS__]['token_categories'])) { + $tokens = []; + \CRM_Utils_Hook::tokens($tokens); + \Civi::$statics[__CLASS__]['token_categories'] = array_keys($tokens); + } + return \Civi::$statics[__CLASS__]['token_categories']; + } + /** * Parse html through Smarty resolving any smarty functions. * @param string $tokenHtml diff --git a/Civi/Token/TokenCompatSubscriber.php b/Civi/Token/TokenCompatSubscriber.php index bf291635c6..d957c3cb9d 100644 --- a/Civi/Token/TokenCompatSubscriber.php +++ b/Civi/Token/TokenCompatSubscriber.php @@ -37,14 +37,11 @@ class TokenCompatSubscriber implements EventSubscriberInterface { * @throws TokenException */ public function onEvaluate(TokenValueEvent $e) { - // For reasons unknown, replaceHookTokens requires a pre-computed list of - // hook *categories* (aka entities aka namespaces). We'll cache - // this in the TokenProcessor's context. - - $hookTokens = []; - \CRM_Utils_Hook::tokens($hookTokens); - $categories = array_keys($hookTokens); - $e->getTokenProcessor()->context['hookTokenCategories'] = $categories; + // For reasons unknown, replaceHookTokens used to require a pre-computed list of + // hook *categories* (aka entities aka namespaces). We cache + // this in the TokenProcessor's context but can likely remove it now. + + $e->getTokenProcessor()->context['hookTokenCategories'] = \CRM_Utils_Token::getTokenCategories(); $messageTokens = $e->getTokenProcessor()->getMessageTokens(); $returnProperties = array_fill_keys($messageTokens['contact'] ?? [], 1); diff --git a/tests/phpunit/CRM/Contribute/Form/Task/PDFLetterCommonTest.php b/tests/phpunit/CRM/Contribute/Form/Task/PDFLetterCommonTest.php index 4acfc3c6c3..cd7495b0ec 100644 --- a/tests/phpunit/CRM/Contribute/Form/Task/PDFLetterCommonTest.php +++ b/tests/phpunit/CRM/Contribute/Form/Task/PDFLetterCommonTest.php @@ -304,7 +304,7 @@ class CRM_Contribute_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase { + --> Total $ 100.00 @@ -322,7 +322,7 @@ class CRM_Contribute_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase { Source + --> 25 December 2016 $ 10.00 @@ -354,7 +354,9 @@ class CRM_Contribute_Form_Task_PDFLetterCommonTest extends CiviUnitTestCase { $this->assertEquals($html[2], $activities['values'][1]['details']); // Checking it is not called multiple times. // once for each contact create + once for the activities. - $this->assertEquals(3, $this->hookTokensCalled); + // & once for the token processor, for now. + // By calling the cached function we can get this down to 1 + $this->assertEquals(4, $this->hookTokensCalled); $this->mut->checkAllMailLog($html); } -- 2.25.1