From 72c096abe12495677d4a89484ff1bec85b52c1fe Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Sat, 3 Dec 2022 15:38:39 +1300 Subject: [PATCH] Make flexmailer mandatory --- .../Incremental/php/FiveFiftySeven.php | 1 + api/v3/examples/Setting/GetFields.ex.php | 25 ----------------- ext/flexmailer/info.xml | 3 ++ .../settings/flexmailer.setting.php | 28 ------------------- ext/flexmailer/src/Listener/Abdicator.php | 13 +-------- .../ClickTracker/HtmlClickTrackerTest.php | 1 - .../ClickTracker/TextClickTrackerTest.php | 1 - .../FlexMailer/ConcurrentDeliveryTest.php | 13 --------- .../Civi/FlexMailer/FlexMailerSystemTest.php | 23 --------------- .../Civi/FlexMailer/MailingPreviewTest.php | 2 -- .../phpunit/Civi/FlexMailer/ValidatorTest.php | 1 - ext/flexmailer/xml/Menu/flexmailer.xml | 11 -------- .../CRM/Mailing/BaseMailingSystemTest.php | 13 ++++----- .../phpunit/CRM/Mailing/MailingSystemTest.php | 19 +++++-------- 14 files changed, 18 insertions(+), 136 deletions(-) delete mode 100644 ext/flexmailer/settings/flexmailer.setting.php delete mode 100644 ext/flexmailer/xml/Menu/flexmailer.xml diff --git a/CRM/Upgrade/Incremental/php/FiveFiftySeven.php b/CRM/Upgrade/Incremental/php/FiveFiftySeven.php index 33afb62b0e..32a0260432 100644 --- a/CRM/Upgrade/Incremental/php/FiveFiftySeven.php +++ b/CRM/Upgrade/Incremental/php/FiveFiftySeven.php @@ -30,6 +30,7 @@ class CRM_Upgrade_Incremental_php_FiveFiftySeven extends CRM_Upgrade_Incremental public function upgrade_5_57_alpha1($rev): void { $this->addTask(ts('Upgrade DB to %1: SQL', [1 => $rev]), 'runSql', $rev); $this->addExtensionTask('Enable SearchKit extension', ['org.civicrm.search_kit'], 1100); + $this->addExtensionTask('Enable Flexmailer extension', ['org.civicrm.flexmailer']); } } diff --git a/api/v3/examples/Setting/GetFields.ex.php b/api/v3/examples/Setting/GetFields.ex.php index ee1e78de0e..fae8b85756 100644 --- a/api/v3/examples/Setting/GetFields.ex.php +++ b/api/v3/examples/Setting/GetFields.ex.php @@ -3256,31 +3256,6 @@ function setting_getfields_expectedresult() { '0' => 'financialacls_toggle', ], ], - 'flexmailer_traditional' => [ - 'group_name' => 'Flexmailer Preferences', - 'group' => 'flexmailer', - 'name' => 'flexmailer_traditional', - 'type' => 'String', - 'html_type' => 'select', - 'html_attributes' => [ - 'class' => 'crm-select2', - ], - 'pseudoconstant' => [ - 'callback' => '_flexmailer_traditional_options', - ], - 'default' => 'auto', - 'add' => '5.13', - 'title' => 'Traditional Mailing Handler', - 'is_domain' => 1, - 'is_contact' => 0, - 'description' => 'For greater backward-compatibility, process \"traditional\" mailings with the CiviMail\'s hard-coded BAO.
For greater forward-compatibility, process \"traditional\" mailings with Flexmailer\'s extensible pipeline.', - 'help_text' => '', - 'settings_pages' => [ - 'flexmailer' => [ - 'weight' => 5, - ], - ], - ], 'recaptchaPublicKey' => [ 'group_name' => 'CiviCRM Preferences', 'group' => 'core', diff --git a/ext/flexmailer/info.xml b/ext/flexmailer/info.xml index c725f129e8..11c28e7511 100644 --- a/ext/flexmailer/info.xml +++ b/ext/flexmailer/info.xml @@ -25,6 +25,9 @@ 5.57 + + mgmt:required + diff --git a/ext/flexmailer/settings/flexmailer.setting.php b/ext/flexmailer/settings/flexmailer.setting.php deleted file mode 100644 index ea227d9ef0..0000000000 --- a/ext/flexmailer/settings/flexmailer.setting.php +++ /dev/null @@ -1,28 +0,0 @@ - [ - 'group_name' => 'Flexmailer Preferences', - 'group' => 'flexmailer', - 'name' => 'flexmailer_traditional', - 'type' => 'String', - 'html_type' => 'select', - 'html_attributes' => ['class' => 'crm-select2'], - 'pseudoconstant' => ['callback' => '_flexmailer_traditional_options'], - 'default' => 'auto', - 'add' => '5.13', - 'title' => E::ts('Traditional Mailing Handler'), - 'is_domain' => 1, - 'is_contact' => 0, - 'description' => E::ts('For greater backward-compatibility, process "traditional" mailings with the CiviMail\'s hard-coded BAO.') . '
' - . E::ts('For greater forward-compatibility, process "traditional" mailings with Flexmailer\'s extensible pipeline.'), - 'help_text' => NULL, - 'settings_pages' => [ - 'flexmailer' => [ - 'weight' => 5, - ], - ], - ], -]; diff --git a/ext/flexmailer/src/Listener/Abdicator.php b/ext/flexmailer/src/Listener/Abdicator.php index b237d5899e..dd42953b3d 100644 --- a/ext/flexmailer/src/Listener/Abdicator.php +++ b/ext/flexmailer/src/Listener/Abdicator.php @@ -38,18 +38,7 @@ class Abdicator { if ($mailing->template_type && $mailing->template_type !== 'traditional') { return TRUE; } - - switch (\Civi::settings()->get('flexmailer_traditional')) { - case 'bao': - return FALSE; - - case 'auto': - case 'flexmailer': - return TRUE; - - default: - throw new \RuntimeException("Unrecognized value for setting 'flexmailer_traditional'"); - } + return TRUE; } /** diff --git a/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ClickTracker/HtmlClickTrackerTest.php b/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ClickTracker/HtmlClickTrackerTest.php index 082e2c0f76..4533fdaecd 100644 --- a/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ClickTracker/HtmlClickTrackerTest.php +++ b/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ClickTracker/HtmlClickTrackerTest.php @@ -27,7 +27,6 @@ class HtmlClickTrackerTest extends \CiviUnitTestCase { } parent::setUp(); - \Civi::settings()->set('flexmailer_traditional', 'flexmailer'); } public function getHrefExamples() { diff --git a/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ClickTracker/TextClickTrackerTest.php b/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ClickTracker/TextClickTrackerTest.php index 64f8b7f8c0..db96537485 100644 --- a/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ClickTracker/TextClickTrackerTest.php +++ b/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ClickTracker/TextClickTrackerTest.php @@ -27,7 +27,6 @@ class TextClickTrackerTest extends \CiviUnitTestCase { } parent::setUp(); - \Civi::settings()->set('flexmailer_traditional', 'flexmailer'); } public function getHrefExamples() { diff --git a/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ConcurrentDeliveryTest.php b/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ConcurrentDeliveryTest.php index 69a496fa6a..f9f6061b3f 100644 --- a/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ConcurrentDeliveryTest.php +++ b/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ConcurrentDeliveryTest.php @@ -41,19 +41,6 @@ class ConcurrentDeliveryTest extends \api_v3_JobProcessMailingTest { } parent::setUp(); - - \Civi::settings()->set('flexmailer_traditional', 'flexmailer'); - } - - public function tearDown(): void { - // We're building on someone else's test and don't fully trust them to - // protect our settings. Make sure they did. - $ok = ('flexmailer' == \Civi::settings()->get('flexmailer_traditional')) - && ('s:10:"flexmailer";' === \CRM_Core_DAO::singleValueQuery('SELECT value FROM civicrm_setting WHERE name ="flexmailer_traditional"')); - - parent::tearDown(); - - $this->assertTrue($ok, 'FlexMailer remained active during testing'); } // ---- Boilerplate ---- diff --git a/ext/flexmailer/tests/phpunit/Civi/FlexMailer/FlexMailerSystemTest.php b/ext/flexmailer/tests/phpunit/Civi/FlexMailer/FlexMailerSystemTest.php index a104bf12d2..dbba1d7ab5 100644 --- a/ext/flexmailer/tests/phpunit/Civi/FlexMailer/FlexMailerSystemTest.php +++ b/ext/flexmailer/tests/phpunit/Civi/FlexMailer/FlexMailerSystemTest.php @@ -47,7 +47,6 @@ class FlexMailerSystemTest extends \CRM_Mailing_BaseMailingSystemTest { } parent::setUp(); - \Civi::settings()->set('flexmailer_traditional', 'flexmailer'); $dispatcher = \Civi::service('dispatcher'); foreach (FlexMailer::getEventTypes() as $event => $class) { @@ -110,28 +109,6 @@ class FlexMailerSystemTest extends \CRM_Mailing_BaseMailingSystemTest { parent::testUrlTracking($inputHtml, $htmlUrlRegex, $textUrlRegex, $params); } - /** - * - * This takes CiviMail's own ones, but removes one that tested for a - * non-feature (i.e. that tokenised links are not handled). - * - * @return array - */ - public function urlTrackingExamples() { - $cases = parent::urlTrackingExamples(); - - // When it comes to URLs with embedded tokens, support diverges - Flexmailer - // can track them, but BAO mailer cannot. - $cases[6] = [ - '

Foo

', - ';

Foo

;', - ';\\[1\\] .*(extern/url.php|civicrm/mailing/url)[\?&]u=\d+.*&id=\d+.*;', - ['url_tracking' => 1], - ]; - - return $cases; - } - public function testBasicHeaders(): void { parent::testBasicHeaders(); } diff --git a/ext/flexmailer/tests/phpunit/Civi/FlexMailer/MailingPreviewTest.php b/ext/flexmailer/tests/phpunit/Civi/FlexMailer/MailingPreviewTest.php index a12ddad7c5..f15d8dffeb 100644 --- a/ext/flexmailer/tests/phpunit/Civi/FlexMailer/MailingPreviewTest.php +++ b/ext/flexmailer/tests/phpunit/Civi/FlexMailer/MailingPreviewTest.php @@ -30,8 +30,6 @@ class MailingPreviewTest extends \CiviUnitTestCase { parent::setUp(); - \Civi::settings()->set('flexmailer_traditional', 'flexmailer'); - $this->useTransaction(); // DGW \CRM_Mailing_BAO_MailingJob::$mailsProcessed = 0; diff --git a/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ValidatorTest.php b/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ValidatorTest.php index 642d4527b2..58e8108309 100644 --- a/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ValidatorTest.php +++ b/ext/flexmailer/tests/phpunit/Civi/FlexMailer/ValidatorTest.php @@ -25,7 +25,6 @@ class ValidatorTest extends \CiviUnitTestCase { } parent::setUp(); - \Civi::settings()->set('flexmailer_traditional', 'flexmailer'); } public function getExamples() { diff --git a/ext/flexmailer/xml/Menu/flexmailer.xml b/ext/flexmailer/xml/Menu/flexmailer.xml deleted file mode 100644 index 605cfa1228..0000000000 --- a/ext/flexmailer/xml/Menu/flexmailer.xml +++ /dev/null @@ -1,11 +0,0 @@ - - - - civicrm/admin/setting/flexmailer - CRM_Admin_Form_Generic - Flexmailer Settings - CiviMail - admin/small/Profile.png - administer CiviCRM - - diff --git a/tests/phpunit/CRM/Mailing/BaseMailingSystemTest.php b/tests/phpunit/CRM/Mailing/BaseMailingSystemTest.php index 141c1c208e..7d8d83d5aa 100644 --- a/tests/phpunit/CRM/Mailing/BaseMailingSystemTest.php +++ b/tests/phpunit/CRM/Mailing/BaseMailingSystemTest.php @@ -286,14 +286,13 @@ abstract class CRM_Mailing_BaseMailingSystemTest extends CiviUnitTestCase { ';\\[1\\] .*(extern/url.php|civicrm/mailing/url)[\?&]u=\d+.*;', ['url_tracking' => 1], ]; - $cases[6] = [ - // FIXME: CiviMail URL tracking doesn't track tokenized links. + $cases['url_trackin_enabled'] = [ '

Foo

', - // FIXME: Legacy tracker adds extra quote after URL - ';

Foo

;', - ';\\[1\\] http://example\.net/\?id=\d+;', + ';

Foo

;', + ';\\[1\\] .*(extern/url.php|civicrm/mailing/url)[\?&]u=\d+.*&id=\d+.*;', ['url_tracking' => 1], ]; + $cases[7] = [ // It would be redundant/slow to track the action URLs? '

Foo

', @@ -391,13 +390,13 @@ abstract class CRM_Mailing_BaseMailingSystemTest extends CiviUnitTestCase { */ protected function runMailingSuccess($params) { $mailingParams = array_merge($this->defaultParams, $params); - $this->callAPISuccess('mailing', 'create', $mailingParams); + $this->callAPISuccess('Mailing', 'create', $mailingParams); $this->_mut->assertRecipients([]); $this->callAPISuccess('job', 'process_mailing', ['runInNonProductionEnvironment' => TRUE]); $allMessages = $this->_mut->getAllMessages('ezc'); // There are exactly two contacts produced by setUp(). - $this->assertEquals(2, count($allMessages)); + $this->assertCount(2, $allMessages); return $allMessages; } diff --git a/tests/phpunit/CRM/Mailing/MailingSystemTest.php b/tests/phpunit/CRM/Mailing/MailingSystemTest.php index 0929dbb629..264ff4f674 100644 --- a/tests/phpunit/CRM/Mailing/MailingSystemTest.php +++ b/tests/phpunit/CRM/Mailing/MailingSystemTest.php @@ -48,24 +48,16 @@ class CRM_Mailing_MailingSystemTest extends CRM_Mailing_BaseMailingSystemTest { */ public function setUp(): void { parent::setUp(); - // If we happen to execute with flexmailer active, use BAO mode. - // There is a parallel FlexMailerSystemTest which runs in flexmailer mode. - Civi::settings()->add(['flexmailer_traditional' => 'bao']); - $hooks = \CRM_Utils_Hook::singleton(); $hooks->setHook('civicrm_alterMailParams', [$this, 'hook_alterMailParams']); - error_reporting(E_ALL && !E_USER_DEPRECATED); } /** * @see CRM_Utils_Hook::alterMailParams */ - public function hook_alterMailParams(&$params, $context = NULL): void { + public function hook_alterMailParams(): void { $this->counts['hook_alterMailParams'] = 1; - if ($this->checkMailParamsContext) { - $this->assertEquals('civimail', $context); - } } /** @@ -90,6 +82,7 @@ class CRM_Mailing_MailingSystemTest extends CRM_Mailing_BaseMailingSystemTest { $this->assertNotEmpty($displayName); $params = $this->_params; + /** @noinspection HttpUrlsUsage */ $params['body_html'] = 'Forward this email written in ckeditor'; $params['api.Mailing.preview'] = [ 'id' => '$value.id', @@ -113,13 +106,15 @@ class CRM_Mailing_MailingSystemTest extends CRM_Mailing_BaseMailingSystemTest { * Generate a fully-formatted mailing (with body_html content). * * @dataProvider urlTrackingExamples + * + * @throws \CRM_Core_Exception */ public function testUrlTracking( $inputHtml, $htmlUrlRegex, $textUrlRegex, $params - ) { + ): void { parent::testUrlTracking($inputHtml, $htmlUrlRegex, $textUrlRegex, $params); } @@ -175,7 +170,7 @@ class CRM_Mailing_MailingSystemTest extends CRM_Mailing_BaseMailingSystemTest { $replyComponent = $this->callAPISuccess('MailingComponent', 'get', ['id' => CRM_Mailing_PseudoConstant::defaultComponent('Reply', ''), 'sequential' => 1])['values'][0]; $replyComponent['body_html'] .= ' {domain.address} '; - $replyComponent['body_txt'] .= ' {domain.address} '; + $replyComponent['body_txt'] = $replyComponent['body_txt'] ?? '' . ' {domain.address} '; $this->callAPISuccess('MailingComponent', 'create', $replyComponent); // Create initial mailing to the group. @@ -258,7 +253,7 @@ class CRM_Mailing_MailingSystemTest extends CRM_Mailing_BaseMailingSystemTest { * @dataProvider multiLingual * */ - public function testGitLabIssue1108($isMultiLingual) { + public function testGitLabIssue1108($isMultiLingual): void { // We need to make sure the mailing IDs are higher than the groupIDs. // We do this by adding mailings until the mailing.id value is at least 10 -- 2.25.1