From ae7f3dfde5b04cb0b468813863da02e4c95461a2 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Mon, 24 Mar 2025 10:18:07 +0000 Subject: [PATCH] MDL-84948 core: require plugin types always define lang strings. This is consistent with subplugin behaviour since 36d842e0. Co-authored-by: Daniel Ziegenberg --- .upgradenotes/MDL-84948-2025032410354799.yml | 8 +++++ public/lib/classes/plugin_manager.php | 24 +++++--------- .../lang/en/fake_fullfeatured.php | 2 ++ public/lib/tests/plugin_manager_test.php | 33 +++++++++++++++---- 4 files changed, 45 insertions(+), 22 deletions(-) create mode 100644 .upgradenotes/MDL-84948-2025032410354799.yml diff --git a/.upgradenotes/MDL-84948-2025032410354799.yml b/.upgradenotes/MDL-84948-2025032410354799.yml new file mode 100644 index 00000000000..f6cafeddfe6 --- /dev/null +++ b/.upgradenotes/MDL-84948-2025032410354799.yml @@ -0,0 +1,8 @@ +issueNumber: MDL-84948 +notes: + core: + - message: >- + The `core_plugin_manager::plugintype_name[_plural]` methods now require + language strings for plugin types always be defined via `type_` + and `type__plural` language strings + type: changed diff --git a/public/lib/classes/plugin_manager.php b/public/lib/classes/plugin_manager.php index 3aac2db8499..b8d406e0a39 100644 --- a/public/lib/classes/plugin_manager.php +++ b/public/lib/classes/plugin_manager.php @@ -629,44 +629,36 @@ class plugin_manager { /** * Returns a localized name of a plugin typed in singular form * - * Most plugin types define their names in core_plugin lang file. In case of subplugins, - * we try to ask the parent plugin for the name. In the worst case, we will return - * the value of the passed $type parameter. + * Plugin types define their names in core_plugin lang file. In the case of subplugins, + * we ask the parent plugin for the name * * @param string $type the type of the plugin, e.g. mod or workshopform * @return string */ public function plugintype_name($type) { - if (get_string_manager()->string_exists('type_' . $type, 'core_plugin')) { - // For most plugin types, their names are defined in core_plugin lang file. - return get_string('type_' . $type, 'core_plugin'); - } else if ($parent = $this->get_parent_of_subplugin($type)) { + if ($parent = $this->get_parent_of_subplugin($type, true)) { // If this is a subplugin, try to ask the parent plugin for the name. return $this->plugin_name($parent) . ' / ' . get_string('subplugintype_' . $type, $parent); } else { - return $type; + return get_string('type_' . $type, 'core_plugin'); } } /** * Returns a localized name of a plugin type in plural form * - * Most plugin types define their names in core_plugin lang file. In case of subplugins, - * we try to ask the parent plugin for the name. In the worst case, we will return - * the value of the passed $type parameter. + * Plugin types define their names in core_plugin lang file. In the case of subplugins, + * we ask the parent plugin for the name * * @param string $type the type of the plugin, e.g. mod or workshopform * @return string */ public function plugintype_name_plural($type) { - if (get_string_manager()->string_exists('type_' . $type . '_plural', 'core_plugin')) { - // For most plugin types, their names are defined in core_plugin lang file. - return get_string('type_' . $type . '_plural', 'core_plugin'); - } else if ($parent = $this->get_parent_of_subplugin($type)) { + if ($parent = $this->get_parent_of_subplugin($type, true)) { // If this is a subplugin, try to ask the parent plugin for the name. return $this->plugin_name($parent) . ' / ' . get_string('subplugintype_' . $type . '_plural', $parent); } else { - return $type; + return get_string('type_' . $type . '_plural', 'core_plugin'); } } diff --git a/public/lib/tests/fixtures/fakeplugins/fake/fullfeatured/lang/en/fake_fullfeatured.php b/public/lib/tests/fixtures/fakeplugins/fake/fullfeatured/lang/en/fake_fullfeatured.php index d51ff19aee2..fa16d9b78d7 100644 --- a/public/lib/tests/fixtures/fakeplugins/fake/fullfeatured/lang/en/fake_fullfeatured.php +++ b/public/lib/tests/fixtures/fakeplugins/fake/fullfeatured/lang/en/fake_fullfeatured.php @@ -26,3 +26,5 @@ defined('MOODLE_INTERNAL') || die; $string['fullfeatured:fakecapability'] = 'Fullfeatured capability description'; $string['pluginname'] = 'Fake full featured plugin'; +$string['subplugintype_fulldeprecatedsubtype'] = 'Full Sub Fake'; +$string['subplugintype_fulldeprecatedsubtype_plural'] = 'Full Sub Fakes'; diff --git a/public/lib/tests/plugin_manager_test.php b/public/lib/tests/plugin_manager_test.php index 55557e3a8a2..f5c5e07fd5e 100644 --- a/public/lib/tests/plugin_manager_test.php +++ b/public/lib/tests/plugin_manager_test.php @@ -847,9 +847,13 @@ final class plugin_manager_test extends \advanced_testcase { $this->assertEquals('fake_fullfeatured', $uninstallurl->param('uninstall')); // Strings are supported for deprecated plugins. + $stringmanager = $this->get_mocked_string_manager(); + $stringmanager->mock_string('type_fake', 'core_plugin', 'Fake'); + $stringmanager->mock_string('type_fake_plural', 'core_plugin', 'Fakes'); + $this->assertEquals('Fake full featured plugin', $pluginman->plugin_name('fake_fullfeatured')); - $this->assertEquals('fake', $pluginman->plugintype_name('fake')); - $this->assertEquals('fake', $pluginman->plugintype_name_plural('fake')); + $this->assertEquals('Fake', $pluginman->plugintype_name('fake')); + $this->assertEquals('Fakes', $pluginman->plugintype_name_plural('fake')); } /** @@ -920,6 +924,13 @@ final class plugin_manager_test extends \advanced_testcase { $uninstallurl = $pluginman->get_uninstall_url('fulldeprecatedsubtype_test'); $this->assertInstanceOf(\moodle_url::class, $uninstallurl); $this->assertEquals('fulldeprecatedsubtype_test', $uninstallurl->param('uninstall')); + + // Strings are supported for deprecated subplugins. + $this->assertEquals('Fake full featured plugin / Full Sub Fake', $pluginman->plugintype_name('fulldeprecatedsubtype')); + $this->assertEquals( + 'Fake full featured plugin / Full Sub Fakes', + $pluginman->plugintype_name_plural('fulldeprecatedsubtype'), + ); } /** @@ -980,8 +991,10 @@ final class plugin_manager_test extends \advanced_testcase { // Without string support, the type name defaults to the plugin type, // while plugin name is set in \core\plugininfo\base::init_is_deprecated(). $this->assertEquals('fullfeatured', $pluginman->plugin_name('fake_fullfeatured')); - $this->assertEquals('fake', $pluginman->plugintype_name('fake')); - $this->assertEquals('fake', $pluginman->plugintype_name_plural('fake')); + $this->assertEquals('[[type_fake]]', $pluginman->plugintype_name('fake')); + $this->assertDebuggingCalled(); + $this->assertEquals('[[type_fake_plural]]', $pluginman->plugintype_name_plural('fake')); + $this->assertDebuggingCalled(); } /** @@ -1056,7 +1069,15 @@ final class plugin_manager_test extends \advanced_testcase { // Without string support, the type name defaults to the plugin type, // while plugin name is set in \core\plugininfo\base::init_is_deprecated(). $this->assertEquals('demo', $pluginman->plugin_name('fulldeletedsubtype_demo')); - $this->assertEquals('fulldeletedsubtype', $pluginman->plugintype_name('fulldeletedsubtype')); - $this->assertEquals('fulldeletedsubtype', $pluginman->plugintype_name_plural('fulldeletedsubtype')); + $this->assertEquals( + 'Fake full featured plugin / [[subplugintype_fulldeletedsubtype]]', + $pluginman->plugintype_name('fulldeletedsubtype'), + ); + $this->assertDebuggingCalled(); + $this->assertEquals( + 'Fake full featured plugin / [[subplugintype_fulldeletedsubtype_plural]]', + $pluginman->plugintype_name_plural('fulldeletedsubtype'), + ); + $this->assertDebuggingCalled(); } } -- 2.43.0