From 0c5781ae0298cfca7c1d9690a48eaf66d665d336 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Thu, 7 May 2020 02:21:07 -0700 Subject: [PATCH] MessageTemplate API - Keep the workflow_id and workflow_name sync'd --- CRM/Core/BAO/MessageTemplate.php | 42 ++++++++++++++++- tests/phpunit/api/v3/MessageTemplateTest.php | 47 +++++++++++++++++++ .../phpunit/api/v3/SyntaxConformanceTest.php | 5 +- 3 files changed, 91 insertions(+), 3 deletions(-) diff --git a/CRM/Core/BAO/MessageTemplate.php b/CRM/Core/BAO/MessageTemplate.php index ff9cb3f9c9..3348da19a5 100644 --- a/CRM/Core/BAO/MessageTemplate.php +++ b/CRM/Core/BAO/MessageTemplate.php @@ -79,7 +79,7 @@ class CRM_Core_BAO_MessageTemplate extends CRM_Core_DAO_MessageTemplate { if (!CRM_Core_Permission::check('edit message templates')) { if (!empty($params['id'])) { $details = civicrm_api3('MessageTemplate', 'getSingle', ['id' => $params['id']]); - if (!empty($details['workflow_id'])) { + if (!empty($details['workflow_id']) || !empty($details['workflow_name'])) { if (!CRM_Core_Permission::check('edit system workflow message templates')) { throw new \Civi\API\Exception\UnauthorizedException(ts('%1', [1 => $systemWorkflowPermissionDeniedMessage])); } @@ -89,7 +89,7 @@ class CRM_Core_BAO_MessageTemplate extends CRM_Core_DAO_MessageTemplate { } } else { - if (!empty($params['workflow_id'])) { + if (!empty($params['workflow_id']) || !empty($params['workflow_name'])) { if (!CRM_Core_Permission::check('edit system workflow message templates')) { throw new \Civi\API\Exception\UnauthorizedException(ts('%1', [1 => $systemWorkflowPermissionDeniedMessage])); } @@ -108,6 +108,31 @@ class CRM_Core_BAO_MessageTemplate extends CRM_Core_DAO_MessageTemplate { unset($params['file_id']); } + // 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')) { + case 'id': + $params['workflow_name'] = array_search($params['workflow_id'], self::getWorkflowNameIdMap()); + break; + + case 'name': + $params['workflow_id'] = self::getWorkflowNameIdMap()[$params['workflow_name']] ?? NULL; + break; + + case 'idname': + $map = self::getWorkflowNameIdMap(); + if ($map[$params['workflow_name']] != $params['workflow_id']) { + throw new CRM_Core_Exception("The workflow_id and workflow_name are mismatched. Note: You only need to submit one or the other."); + } + break; + + case '': + // OK, don't care. + break; + + default: + throw new \RuntimeException("Bad code"); + } + $messageTemplates = new CRM_Core_DAO_MessageTemplate(); $messageTemplates->copyValues($params); $messageTemplates->save(); @@ -582,4 +607,17 @@ class CRM_Core_BAO_MessageTemplate extends CRM_Core_DAO_MessageTemplate { return [$sent, $mailContent['subject'], $mailContent['text'], $mailContent['html']]; } + /** + * Create a map between workflow_name and workflow_id. + * + * @return array + * Array(string $workflowName => int $workflowId) + */ + protected static function getWorkflowNameIdMap() { + // There's probably some more clever way to do this, but this seems simple. + return CRM_Core_DAO::executeQuery('SELECT cov.name as name, cov.id as id FROM civicrm_option_group cog INNER JOIN civicrm_option_value cov on cov.option_group_id=cog.id WHERE cog.name LIKE %1', [ + 1 => ['msg_tpl_workflow_%', 'String'], + ])->fetchMap('name', 'id'); + } + } diff --git a/tests/phpunit/api/v3/MessageTemplateTest.php b/tests/phpunit/api/v3/MessageTemplateTest.php index 148b68d736..34e10d86bc 100644 --- a/tests/phpunit/api/v3/MessageTemplateTest.php +++ b/tests/phpunit/api/v3/MessageTemplateTest.php @@ -75,6 +75,52 @@ class api_v3_MessageTemplateTest extends CiviUnitTestCase { $this->assertEquals(0, $checkDeleted['count']); } + /** + * If you give workflow_id, then workflow_name should also be set. + */ + public function testWorkflowIdToName() { + $wfName = 'uf_notify'; + $wfId = CRM_Core_DAO::singleValueQuery('SELECT id FROM civicrm_option_value WHERE name = %1', [ + 1 => [$wfName, 'String'], + ]); + + $created = $this->callAPISuccess('MessageTemplate', 'create', [ + 'msg_title' => __FUNCTION__, + 'msg_subject' => __FUNCTION__, + 'msg_text' => __FUNCTION__, + 'msg_html' => __FUNCTION__, + 'workflow_id' => $wfId, + ]); + $this->assertEquals($wfName, $created['values'][$created['id']]['workflow_name']); + $this->assertEquals($wfId, $created['values'][$created['id']]['workflow_id']); + $get = $this->callAPISuccess('MessageTemplate', 'getsingle', ['id' => $created['id']]); + $this->assertEquals($wfName, $get['workflow_name']); + $this->assertEquals($wfId, $get['workflow_id']); + } + + /** + * If you give workflow_name, then workflow_id should also be set. + */ + public function testWorkflowNameToId() { + $wfName = 'petition_sign'; + $wfId = CRM_Core_DAO::singleValueQuery('SELECT id FROM civicrm_option_value WHERE name = %1', [ + 1 => [$wfName, 'String'], + ]); + + $created = $this->callAPISuccess('MessageTemplate', 'create', [ + 'msg_title' => __FUNCTION__, + 'msg_subject' => __FUNCTION__, + 'msg_text' => __FUNCTION__, + 'msg_html' => __FUNCTION__, + 'workflow_name' => $wfName, + ]); + $this->assertEquals($wfName, $created['values'][$created['id']]['workflow_name']); + $this->assertEquals($wfId, $created['values'][$created['id']]['workflow_id']); + $get = $this->callAPISuccess('MessageTemplate', 'getsingle', ['id' => $created['id']]); + $this->assertEquals($wfName, $get['workflow_name']); + $this->assertEquals($wfId, $get['workflow_id']); + } + public function testPermissionChecks() { $entity = $this->createTestEntity(); CRM_Core_Config::singleton()->userPermissionClass->permissions = ['edit user-driven message templates']; @@ -88,6 +134,7 @@ class api_v3_MessageTemplateTest extends CiviUnitTestCase { unset($testUserEntity['id']); $testUserEntity['msg_subject'] = 'Test user message template'; unset($testUserEntity['workflow_id']); + unset($testUserEntity['workflow_name']); $testuserEntity['check_permissions'] = TRUE; // ensure that it can create user templates; $userEntity = $this->callAPISuccess('MessageTemplate', 'create', $testUserEntity); diff --git a/tests/phpunit/api/v3/SyntaxConformanceTest.php b/tests/phpunit/api/v3/SyntaxConformanceTest.php index 364baf7681..9562535975 100644 --- a/tests/phpunit/api/v3/SyntaxConformanceTest.php +++ b/tests/phpunit/api/v3/SyntaxConformanceTest.php @@ -515,6 +515,8 @@ class api_v3_SyntaxConformanceTest extends CiviUnitTestCase { 'SystemLog', //skip this because it doesn't make sense to update logs, 'Logging', + // Skip message template because workflow_id/workflow_name are sync'd. + 'MessageTemplate', ]; if ($sequential === TRUE) { return $entitiesWithout; @@ -525,7 +527,8 @@ class api_v3_SyntaxConformanceTest extends CiviUnitTestCase { $e, ]; } - return ['pledge']; + // WTF + return ['pledge', 'MessageTemplate']; return $entities; } -- 2.25.1