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.

diff --git a/CRM/Core/BAO/MessageTemplate.php b/CRM/Core/BAO/MessageTemplate.php
index 02a15516c05026be3f9c65a3c0b56ebeecec8326..f0edcfaa3bda1f2b7513bc493e9ca9e0341191d0 100644 (file)
--- 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 32593418bbddce7a6cb06cba5100c1d6da78032c..d164a2f935f21cf08a5b8f70247e7b03c69a4a67 100644 (file)
--- 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 d7ae1bf5b939155a1f1e17c297c797f65538bccb..fa5f13e8481e71f295da496def61375cbd010498 100644 (file)
--- 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...