From 41ce741963694c114410a2b78c62995d0b3ae86c Mon Sep 17 00:00:00 2001 From: colemanw Date: Thu, 12 Oct 2023 19:44:23 -0400 Subject: [PATCH] Angular - Noisily deprecate 'settings' in favor of 'settingsFactory' --- CRM/Utils/Check/Component/Env.php | 21 ++++++++++++++++++ Civi/Angular/Manager.php | 3 +++ tests/phpunit/Civi/Angular/LoaderTest.php | 25 +++++----------------- tests/phpunit/Civi/Angular/ManagerTest.php | 9 +------- 4 files changed, 30 insertions(+), 28 deletions(-) diff --git a/CRM/Utils/Check/Component/Env.php b/CRM/Utils/Check/Component/Env.php index 2ac6246f6f..f0a88a7162 100644 --- a/CRM/Utils/Check/Component/Env.php +++ b/CRM/Utils/Check/Component/Env.php @@ -1106,4 +1106,25 @@ class CRM_Utils_Check_Component_Env extends CRM_Utils_Check_Component { return $messages; } + public function checkAngularModuleSettings() { + $messages = []; + $modules = Civi::container()->get('angular')->getModules(); + foreach ($modules as $name => $module) { + if (!empty($module['settings'])) { + $messages[] = new CRM_Utils_Check_Message( + __FUNCTION__ . $name, + ts('The Angular file "%1" from extension "%2" must be updated to use "settingsFactory" instead of "settings". Developer info...', [ + 1 => $name, + 2 => $module['ext'], + 3 => 'target="_blank" href="https://github.com/civicrm/civicrm-core/pull/19052"', + ]), + ts('Unsupported Angular Setting'), + \Psr\Log\LogLevel::WARNING, + 'fa-code' + ); + } + } + return $messages; + } + } diff --git a/Civi/Angular/Manager.php b/Civi/Angular/Manager.php index 9daf7ca622..1a382743cb 100644 --- a/Civi/Angular/Manager.php +++ b/Civi/Angular/Manager.php @@ -135,6 +135,9 @@ class Manager { foreach ($angularModules as $module => $info) { // Merge in defaults $angularModules[$module] += ['basePages' => ['civicrm/a']]; + if (!empty($info['settings'])) { + \CRM_Core_Error::deprecatedWarning('Angular "settings" is not supported. See https://github.com/civicrm/civicrm-core/pull/19052'); + } // Validate settingsFactory callables if (isset($info['settingsFactory'])) { // To keep the cache small, we want `settingsFactory` to contain the string names of class & function, not an object diff --git a/tests/phpunit/Civi/Angular/LoaderTest.php b/tests/phpunit/Civi/Angular/LoaderTest.php index daccaa74bb..0136ffde87 100644 --- a/tests/phpunit/Civi/Angular/LoaderTest.php +++ b/tests/phpunit/Civi/Angular/LoaderTest.php @@ -16,22 +16,20 @@ namespace Civi\Angular; */ class LoaderTest extends \CiviUnitTestCase { - public static $dummy_setting_count = 0; public static $dummy_callback_count = 0; public function setUp(): void { parent::setUp(); $this->hookClass->setHook('civicrm_angularModules', [$this, 'hook_angularModules']); - self::$dummy_setting_count = 0; self::$dummy_callback_count = 0; $this->createLoggedInUser(); } public function factoryScenarios() { return [ - ['dummy1', 2, 1, ['access CiviCRM', 'administer CiviCRM']], - ['dummy2', 2, 0, []], - ['dummy3', 2, 2, ['access CiviCRM', 'administer CiviCRM', 'view debug output']], + ['dummy1', 1, ['access CiviCRM', 'administer CiviCRM']], + ['dummy2', 0, []], + ['dummy3', 2, ['access CiviCRM', 'administer CiviCRM', 'view debug output']], ]; } @@ -41,11 +39,10 @@ class LoaderTest extends \CiviUnitTestCase { * * @dataProvider factoryScenarios * @param $module - * @param $expectedSettingCount * @param $expectedCallbackCount * @param $expectedPermissions */ - public function testSettingFactory($module, $expectedSettingCount, $expectedCallbackCount, $expectedPermissions) { + public function testSettingFactory($module, $expectedCallbackCount, $expectedPermissions) { $loader = \Civi::service('angularjs.loader'); $loader->addModules([$module]); $loader->useApp(); @@ -65,29 +62,21 @@ class LoaderTest extends \CiviUnitTestCase { // Dummy3 module's factory setting should be set if it is loaded directly $this->assertTrue(($expectedCallbackCount > 1) === isset($actual['dummy3']['dummy_setting_factory'])); - // Dummy1 module's regular setting should be set if it is loaded directly or required by dummy3 - $this->assertTrue(($module !== 'dummy2') === isset($actual['dummy1']['dummy_setting'])); - // Dummy2 module's regular setting should be set if loaded - $this->assertTrue(($module === 'dummy2') === isset($actual['dummy2']['dummy_setting'])); - // Assert appropriate permissions have been added $this->assertEquals($expectedPermissions, array_keys($actual['permissions'])); - // Assert the callback functions ran the expected number of times - $this->assertEquals($expectedSettingCount, self::$dummy_setting_count); + // Assert the callback function ran the expected number of times $this->assertEquals($expectedCallbackCount, self::$dummy_callback_count); } public function hook_angularModules(&$modules) { $modules['dummy1'] = [ 'ext' => 'civicrm', - 'settings' => $this->getDummySetting(), 'permissions' => ['access CiviCRM', 'administer CiviCRM'], 'settingsFactory' => [self::class, 'getDummySettingFactory'], ]; $modules['dummy2'] = [ 'ext' => 'civicrm', - 'settings' => $this->getDummySetting(), ]; $modules['dummy3'] = [ 'ext' => 'civicrm', @@ -99,10 +88,6 @@ class LoaderTest extends \CiviUnitTestCase { ]; } - public function getDummySetting() { - return ['dummy_setting' => self::$dummy_setting_count++]; - } - public static function getDummySettingFactory() { return ['dummy_setting_factory' => self::$dummy_callback_count++]; } diff --git a/tests/phpunit/Civi/Angular/ManagerTest.php b/tests/phpunit/Civi/Angular/ManagerTest.php index a7c69be7dd..39d5233502 100644 --- a/tests/phpunit/Civi/Angular/ManagerTest.php +++ b/tests/phpunit/Civi/Angular/ManagerTest.php @@ -47,7 +47,6 @@ class ManagerTest extends \CiviUnitTestCase { 'js' => 0, 'css' => 0, 'partials' => 0, - 'settings' => 0, 'settingsFactory' => 0, ]; @@ -75,12 +74,7 @@ class ManagerTest extends \CiviUnitTestCase { $counts['partials']++; } } - if (isset($module['settings'])) { - $this->assertTrue(is_array($module['settings'])); - foreach ($module['settings'] as $name => $value) { - $counts['settings']++; - } - } + $this->assertArrayNotHasKey('settings', $module); if (isset($module['settingsFactory'])) { $this->assertTrue(is_callable($module['settingsFactory'])); $counts['settingsFactory']++; @@ -91,7 +85,6 @@ class ManagerTest extends \CiviUnitTestCase { $this->assertTrue($counts['css'] > 0, 'Expect to find at least one CSS file'); $this->assertTrue($counts['partials'] > 0, 'Expect to find at least one partial HTML file'); $this->assertTrue($counts['settingsFactory'] > 0, 'Expect to find at least one settingsFactory'); - $this->assertEquals(0, $counts['settings'], 'Angular settings are deprecated in favor of settingsFactory'); } /** -- 2.25.1