From d53da69a685c03a48128cd9eb439caf1ce027870 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Fri, 1 Oct 2021 14:56:26 -0700 Subject: [PATCH] MessageTemplate API - Fix saving of templates with workflow_name sans workflow_id Overview -------- This fixes a bug when saving (updating) a `MessageTemplate` record that involves a `workflow_name` and no `workflow_id`. It is an alternative to #21674. Steps to reproduce ------------------- Suppose you read a MessagTemplate with values: ```php $values = ['workflow_name' => 'foo', 'workflow_id' => NULL, ...] ``` Then you save it back: ```php civicrm_api4('MessageTemplate', 'update', [ 'values' => $values ]); ``` Before ------ The `update` raises an exception. After ----- The `update` works. --- CRM/Core/BAO/MessageTemplate.php | 5 +- .../api/v4/Entity/MessageTemplateTest.php | 119 ++++++++++++++++++ 2 files changed, 123 insertions(+), 1 deletion(-) create mode 100644 tests/phpunit/api/v4/Entity/MessageTemplateTest.php diff --git a/CRM/Core/BAO/MessageTemplate.php b/CRM/Core/BAO/MessageTemplate.php index c8d6693a87..7d7d239f13 100644 --- a/CRM/Core/BAO/MessageTemplate.php +++ b/CRM/Core/BAO/MessageTemplate.php @@ -111,7 +111,10 @@ class CRM_Core_BAO_MessageTemplate extends CRM_Core_DAO_MessageTemplate { } // The workflow_id and workflow_name should be sync'd. But what mix of inputs do we have to work with? - switch ((empty($params['workflow_id']) ? '' : 'id') . (empty($params['workflow_name']) ? '' : 'name')) { + $empty = function ($key) use (&$params) { + return empty($params[$key]) || $params[$key] === 'null'; + }; + switch (($empty('workflow_id') ? '' : 'id') . ($empty('workflow_name') ? '' : 'name')) { case 'id': $params['workflow_name'] = array_search($params['workflow_id'], self::getWorkflowNameIdMap()); break; diff --git a/tests/phpunit/api/v4/Entity/MessageTemplateTest.php b/tests/phpunit/api/v4/Entity/MessageTemplateTest.php new file mode 100644 index 0000000000..485865b2a4 --- /dev/null +++ b/tests/phpunit/api/v4/Entity/MessageTemplateTest.php @@ -0,0 +1,119 @@ + 'My Template', + 'msg_subject' => 'My Subject', + 'msg_text' => 'My body as text', + 'msg_html' => '

My body as HTML

', + 'is_reserved' => TRUE, + 'is_default' => FALSE, + ]; + + /** + * Create/update a MessageTemplate with workflow_name and no corresponding workflow_id. + */ + public function testWorkflowName_clean() { + $create = civicrm_api4('MessageTemplate', 'create', [ + 'values' => $this->baseTpl + ['workflow_name' => 'first', 'workflow_id' => NULL], + ])->single(); + $this->assertDBQuery('first', 'SELECT workflow_name FROM civicrm_msg_template WHERE id = %1', [1 => [$create['id'], 'Int']]); + $this->assertDBQuery(NULL, 'SELECT workflow_id FROM civicrm_msg_template WHERE id = %1', [1 => [$create['id'], 'Int']]); + + civicrm_api4('MessageTemplate', 'update', [ + 'where' => [['id', '=', $create['id']]], + 'values' => ['workflow_name' => 'second', 'workflow_id' => NULL], + ])->single(); + $this->assertDBQuery('second', 'SELECT workflow_name FROM civicrm_msg_template WHERE id = %1', [1 => [$create['id'], 'Int']]); + $this->assertDBQuery(NULL, 'SELECT workflow_id FROM civicrm_msg_template WHERE id = %1', [1 => [$create['id'], 'Int']]); + } + + /** + * Create/update a MessageTemplate with workflow_name - a name which happens to have an older/corresponding workflow_id. + */ + public function testWorkflowName_legacyMatch() { + [$firstId, $secondId] = $this->createFirstSecond(); + + $create = civicrm_api4('MessageTemplate', 'create', [ + 'values' => $this->baseTpl + ['workflow_name' => 'first', 'workflow_id' => NULL], + ])->single(); + $this->assertDBQuery('first', 'SELECT workflow_name FROM civicrm_msg_template WHERE id = %1', [1 => [$create['id'], 'Int']]); + $this->assertDBQuery($firstId, 'SELECT workflow_id FROM civicrm_msg_template WHERE id = %1', [1 => [$create['id'], 'Int']]); + + civicrm_api4('MessageTemplate', 'update', [ + 'where' => [['id', '=', $create['id']]], + 'values' => ['workflow_name' => 'second', 'workflow_id' => NULL], + ])->single(); + $this->assertDBQuery('second', 'SELECT workflow_name FROM civicrm_msg_template WHERE id = %1', [1 => [$create['id'], 'Int']]); + $this->assertDBQuery($secondId, 'SELECT workflow_id FROM civicrm_msg_template WHERE id = %1', [1 => [$create['id'], 'Int']]); + } + + /** + * Create/update a MessageTempalte with workflow_id. Ensure the newer workflow_name is set. + */ + public function testWorkflowId_legacyMatch() { + [$firstId, $secondId] = $this->createFirstSecond(); + + $create = civicrm_api4('MessageTemplate', 'create', [ + 'values' => $this->baseTpl + ['workflow_id' => $firstId, 'workflow_name' => NULL], + ])->single(); + $this->assertDBQuery('first', 'SELECT workflow_name FROM civicrm_msg_template WHERE id = %1', [1 => [$create['id'], 'Int']]); + $this->assertDBQuery($firstId, 'SELECT workflow_id FROM civicrm_msg_template WHERE id = %1', [1 => [$create['id'], 'Int']]); + + civicrm_api4('MessageTemplate', 'update', [ + 'where' => [['id', '=', $create['id']]], + 'values' => ['workflow_id' => $secondId, 'workflow_name' => NULL], + ])->single(); + $this->assertDBQuery('second', 'SELECT workflow_name FROM civicrm_msg_template WHERE id = %1', [1 => [$create['id'], 'Int']]); + $this->assertDBQuery($secondId, 'SELECT workflow_id FROM civicrm_msg_template WHERE id = %1', [1 => [$create['id'], 'Int']]); + } + + protected function createFirstSecond() { + $first = civicrm_api4('OptionValue', 'create', [ + 'values' => [ + 'option_group_id:name' => 'msg_tpl_workflow_meta', + 'label' => 'First', + 'name' => 'first', + ], + ]); + $second = civicrm_api4('OptionValue', 'create', [ + 'values' => [ + 'option_group_id:name' => 'msg_tpl_workflow_meta', + 'label' => 'Second', + 'name' => 'second', + ], + ]); + return [$first->single()['id'], $second->single()['id']]; + } + +} -- 2.25.1