From b7ade3e09d84952b3c26e7a3a7bafaeb7e8239e4 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Tue, 28 Sep 2021 15:01:11 -0700 Subject: [PATCH] MessageTemplate - Allow 'workflow'/'valueName' as aliases. Consolidate alias bits. The params given to `MessageTemplate::sendTemplate()` are evolving. But they are also exposed via hook. Blerg. This technically does two things: 1. It takes a couple of bits which are responsible for aliasing/transitioning specific params, and it moves them to a common `$sync()` function. 2. It deprecates `valueName` in favor of `workflow`, and it updates the hook test accordingly. --- CRM/Core/BAO/MessageTemplate.php | 23 +++++++++++++++---- Civi/WorkflowMessage/WorkflowMessage.php | 6 ----- .../hook_civicrm_alterMailParams.evch.php | 11 ++++++++- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/CRM/Core/BAO/MessageTemplate.php b/CRM/Core/BAO/MessageTemplate.php index 02a15516c0..f0edcfaa3b 100644 --- a/CRM/Core/BAO/MessageTemplate.php +++ b/CRM/Core/BAO/MessageTemplate.php @@ -324,22 +324,35 @@ class CRM_Core_BAO_MessageTemplate extends CRM_Core_DAO_MessageTemplate { 'PDFFilename' => NULL, ]; + // Some params have been deprecated/renamed. Synchronize old<=>new params. We periodically resync after exchanging data with other parties. + $sync = function () use (&$params, $modelDefaults, $viewDefaults) { + CRM_Utils_Array::pathSync($params, ['workflow'], ['valueName']); + CRM_Utils_Array::pathSync($params, ['tokenContext', 'contactId'], ['contactId']); + CRM_Utils_Array::pathSync($params, ['tokenContext', 'smarty'], ['disableSmarty'], function ($v, bool $isCanon) { + return !$v; + }); + + // Core#644 - handle Email ID passed as "From". + if (isset($params['from'])) { + $params['from'] = \CRM_Utils_Mail::formatFromAddress($params['from']); + } + }; + $sync(); + // Allow WorkflowMessage to run any filters/mappings/cleanups. - $model = $params['model'] ?? WorkflowMessage::create($params['valueName'] ?? 'UNKNOWN'); + $model = $params['model'] ?? WorkflowMessage::create($params['workflow'] ?? 'UNKNOWN'); $params = WorkflowMessage::exportAll(WorkflowMessage::importAll($model, $params)); unset($params['model']); // Subsequent hooks use $params. Retaining the $params['model'] might be nice - but don't do it unless you figure out how to ensure data-consistency (eg $params['tplParams'] <=> $params['model']). // If you want to expose the model via hook, consider interjecting a new Hook::alterWorkflowMessage($model) between `importAll()` and `exportAll()`. + $sync(); $params = array_merge($modelDefaults, $viewDefaults, $envelopeDefaults, $params); CRM_Utils_Hook::alterMailParams($params, 'messageTemplate'); $mailContent = self::loadTemplate((string) $params['valueName'], $params['isTest'], $params['messageTemplateID'] ?? NULL, $params['groupName'] ?? '', $params['messageTemplate'], $params['subject'] ?? NULL); - $params['tokenContext'] = array_merge([ - 'smarty' => (bool) !$params['disableSmarty'], - 'contactId' => $params['contactId'], - ], $params['tokenContext']); + $sync(); $rendered = CRM_Core_TokenSmarty::render(CRM_Utils_Array::subset($mailContent, ['text', 'html', 'subject']), $params['tokenContext'], $params['tplParams']); if (isset($rendered['subject'])) { $rendered['subject'] = trim(preg_replace('/[\r\n]+/', ' ', $rendered['subject'])); diff --git a/Civi/WorkflowMessage/WorkflowMessage.php b/Civi/WorkflowMessage/WorkflowMessage.php index 32593418bb..d164a2f935 100644 --- a/Civi/WorkflowMessage/WorkflowMessage.php +++ b/Civi/WorkflowMessage/WorkflowMessage.php @@ -96,12 +96,6 @@ class WorkflowMessage { unset($params['model']); } - \CRM_Utils_Array::pathMove($params, ['contactId'], ['tokenContext', 'contactId']); - - // Core#644 - handle Email ID passed as "From". - if (isset($params['from'])) { - $params['from'] = \CRM_Utils_Mail::formatFromAddress($params['from']); - } if (isset($params['tplParams'])) { $model->import('tplParams', $params['tplParams']); diff --git a/tests/events/hook_civicrm_alterMailParams.evch.php b/tests/events/hook_civicrm_alterMailParams.evch.php index d7ae1bf5b9..fa5f13e848 100644 --- a/tests/events/hook_civicrm_alterMailParams.evch.php +++ b/tests/events/hook_civicrm_alterMailParams.evch.php @@ -46,6 +46,11 @@ return new class() extends \Civi\Test\EventCheck implements \Civi\Test\HookInter 'tokenContext' => ['type' => 'array', 'for' => 'messageTemplate'], 'tplParams' => ['type' => 'array', 'for' => 'messageTemplate'], 'contactId' => ['type' => 'int|NULL', 'for' => 'messageTemplate' /* deprecated in favor of tokenContext[contactId] */], + 'workflow' => [ + 'regex' => '/^([a-zA-Z_]+)$/', + 'type' => 'string', + 'for' => 'messageTemplate', + ], 'valueName' => [ 'regex' => '/^([a-zA-Z_]+)$/', 'type' => 'string', @@ -124,7 +129,11 @@ return new class() extends \Civi\Test\EventCheck implements \Civi\Test\HookInter } if ($context === 'messageTemplate') { - $this->assertTrue(!empty($params['valueName']), "$msg: Message templates must always specify the name of the workflow step\n$dump"); + $this->assertTrue(!empty($params['workflow']), "$msg: Message templates must always specify a symbolic name of the step/task\n$dump"); + if (isset($params['valueName'])) { + // This doesn't require that valueName be supplied - but if it is supplied, it must match the workflow name. + $this->assertEquals($params['workflow'], $params['valueName'], "$msg: If given, workflow and valueName must match\n$dump"); + } $this->assertEquals($params['contactId'] ?? NULL, $params['tokenContext']['contactId'] ?? NULL, "$msg: contactId moved to tokenContext, but legacy value should be equivalent\n$dump"); // This assertion is surprising -- yet true. We should perhaps check if it was true in past releases... -- 2.25.1