diff --git a/.upgradenotes/MDL-77894-2025093010013740.yml b/.upgradenotes/MDL-77894-2025093010013740.yml new file mode 100644 index 00000000000..e7128c24571 --- /dev/null +++ b/.upgradenotes/MDL-77894-2025093010013740.yml @@ -0,0 +1,5 @@ +issueNumber: MDL-77894 +notes: + core: + - message: Appending an exclamation mark to template names ignores theme overrides + type: improved diff --git a/public/admin/tool/templatelibrary/classes/api.php b/public/admin/tool/templatelibrary/classes/api.php index f0d7a43ce4f..6f422287850 100644 --- a/public/admin/tool/templatelibrary/classes/api.php +++ b/public/admin/tool/templatelibrary/classes/api.php @@ -146,31 +146,18 @@ class api { * @return string the template or false if template doesn't exist. */ public static function load_canonical_template($component, $template) { - // Get the list of possible template directories. - $dirs = mustache_template_finder::get_template_directories_for_component($component); - $filename = false; - $themedir = core_component::get_plugin_types()['theme']; + // Get the list of possible template directories without theme overrides. + $dirs = mustache_template_finder::get_template_directories_for_component($component, themeoverrides: false); foreach ($dirs as $dir) { - // Skip theme dirs - we only want the original plugin/core template. - if (strpos($dir, $themedir) === 0) { - continue; - } - - $candidate = $dir . $template . '.mustache'; - if (file_exists($candidate)) { - $filename = $candidate; - break; + $filename = $dir . $template . '.mustache'; + if (file_exists($filename)) { + return file_get_contents($filename); } } - if ($filename === false) { - // There are occasions where we don't have a core template. - return false; - } - - $templatestr = file_get_contents($filename); - return $templatestr; + // There are occasions where we don't have a core template. + return false; } } diff --git a/public/lib/classes/output/mustache_template_finder.php b/public/lib/classes/output/mustache_template_finder.php index e10f9c6246e..cacf57732cf 100644 --- a/public/lib/classes/output/mustache_template_finder.php +++ b/public/lib/classes/output/mustache_template_finder.php @@ -34,9 +34,10 @@ class mustache_template_finder { * * @param string $component The component to search * @param string $themename The current theme name - * @return string[] List of valid directories for templates for this compoonent. Directories are not checked for existence. + * @param bool $themeoverrides Whether to apply theme overrides. Defaults to true. + * @return string[] List of valid directories for templates for this component. Directories are not checked for existence. */ - public static function get_template_directories_for_component($component, $themename = '') { + public static function get_template_directories_for_component($component, $themename = '', $themeoverrides = true) { global $CFG, $PAGE; // Default the param. @@ -55,26 +56,28 @@ class mustache_template_finder { throw new coding_exception("Component was not valid: " . s($component)); } - // Find the parent themes. - $parents = []; - if ($themename === $PAGE->theme->name) { - $parents = $PAGE->theme->parents; - } else { - $themeconfig = theme_config::load($themename); - $parents = $themeconfig->parents; - } + if ($themeoverrides) { + // Find the parent themes. + $parents = []; + if ($themename === $PAGE->theme->name) { + $parents = $PAGE->theme->parents; + } else { + $themeconfig = theme_config::load($themename); + $parents = $themeconfig->parents; + } - // First check the theme. - $dirs[] = $CFG->dirroot . '/theme/' . $themename . '/templates/' . $component . '/'; - if (isset($CFG->themedir)) { - $dirs[] = $CFG->themedir . '/' . $themename . '/templates/' . $component . '/'; - } - // Now check the parent themes. - // Search each of the parent themes second. - foreach ($parents as $parent) { - $dirs[] = $CFG->dirroot . '/theme/' . $parent . '/templates/' . $component . '/'; + // First check the theme. + $dirs[] = $CFG->dirroot . '/theme/' . $themename . '/templates/' . $component . '/'; if (isset($CFG->themedir)) { - $dirs[] = $CFG->themedir . '/' . $parent . '/templates/' . $component . '/'; + $dirs[] = $CFG->themedir . '/' . $themename . '/templates/' . $component . '/'; + } + // Now check the parent themes. + // Search each of the parent themes second. + foreach ($parents as $parent) { + $dirs[] = $CFG->dirroot . '/theme/' . $parent . '/templates/' . $component . '/'; + if (isset($CFG->themedir)) { + $dirs[] = $CFG->themedir . '/' . $parent . '/templates/' . $component . '/'; + } } } @@ -86,13 +89,17 @@ class mustache_template_finder { /** * Helper function for getting a filename for a template from the template name. * - * @param string $name - This is the componentname/templatename combined. - * @param string $themename - This is the current theme name. + * Theme overrides are automatically applied unless a '!' is appended to `$name`. + * + * Example: Template core/test overridden in theme_foo. + * - `get_template_filepath('core/test', 'foo')` resolves to 'theme/foo/templates/core/test.mustache'. + * - `get_template_filepath('core/test!', 'foo')` resolves to 'lib/templates/test.mustache'. + * + * @param string $name This is the componentname/templatename combined. May end in an exclamation mark. + * @param string $themename This is the current theme name. * @return string */ public static function get_template_filepath($name, $themename = '') { - global $CFG, $PAGE; - if (strpos($name, '/') === false) { throw new coding_exception('Templates names must be specified as "componentname/templatename"' . ' (' . s($name) . ' requested) '); @@ -101,7 +108,15 @@ class mustache_template_finder { [$component, $templatename] = explode('/', $name, 2); $component = clean_param($component, PARAM_COMPONENT); - $dirs = self::get_template_directories_for_component($component, $themename); + // We apply theme overrides if the name does NOT end with an exclamation mark. + $themeoverrides = true; + if (str_ends_with($templatename, '!')) { + $themeoverrides = false; + // Remove exclamation mark. + $templatename = substr($templatename, 0, strlen($templatename) - 1); + } + + $dirs = self::get_template_directories_for_component($component, $themename, $themeoverrides); foreach ($dirs as $dir) { $candidate = $dir . $templatename . '.mustache'; diff --git a/public/lib/tests/output/mustache_template_finder_test.php b/public/lib/tests/output/mustache_template_finder_test.php index 0e5f0dcf610..0aca9a90abf 100644 --- a/public/lib/tests/output/mustache_template_finder_test.php +++ b/public/lib/tests/output/mustache_template_finder_test.php @@ -26,6 +26,7 @@ namespace core\output; * @category test * @copyright 2015 Damyon Wiese * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @coversDefaultClass \core\output\mustache_template_finder */ final class mustache_template_finder_test extends \advanced_testcase { @@ -40,36 +41,90 @@ final class mustache_template_finder_test extends \advanced_testcase { 'plugin: mod_assign' => [ 'component' => 'mod_assign', 'theme' => '', + 'themeoverrides' => true, 'paths' => [ 'theme/boost/templates/mod_assign/', - 'mod/assign/templates/' + 'mod/assign/templates/', ], ], 'plugin: mod_assign with classic' => [ 'component' => 'mod_assign', 'theme' => 'classic', + 'themeoverrides' => true, 'paths' => [ 'theme/classic/templates/mod_assign/', 'theme/boost/templates/mod_assign/', - 'mod/assign/templates/' + 'mod/assign/templates/', ], ], 'subsystem: core_user' => [ 'component' => 'core_user', 'theme' => 'classic', + 'themeoverrides' => true, 'paths' => [ 'theme/classic/templates/core_user/', 'theme/boost/templates/core_user/', - 'user/templates/' + 'user/templates/', + ], + ], + 'theme: theme_boost' => [ + 'component' => 'theme_boost', + 'theme' => 'classic', + 'themeoverrides' => true, + 'paths' => [ + 'theme/classic/templates/theme_boost/', + 'theme/boost/templates/theme_boost/', + 'theme/boost/templates/', ], ], 'core' => [ 'component' => 'core', 'theme' => 'classic', + 'themeoverrides' => true, 'paths' => [ 'theme/classic/templates/core/', 'theme/boost/templates/core/', - 'lib/templates/' + 'lib/templates/', + ], + ], + 'plugin: mod_assign (without overrides)' => [ + 'component' => 'mod_assign', + 'theme' => '', + 'themeoverrides' => false, + 'paths' => [ + 'mod/assign/templates/', + ], + ], + 'plugin: mod_assign with classic (without overrides)' => [ + 'component' => 'mod_assign', + 'theme' => 'classic', + 'themeoverrides' => false, + 'paths' => [ + 'mod/assign/templates/', + ], + ], + 'subsystem: core_user (without overrides)' => [ + 'component' => 'core_user', + 'theme' => 'classic', + 'themeoverrides' => false, + 'paths' => [ + 'user/templates/', + ], + ], + 'theme: theme_boost (without overrides)' => [ + 'component' => 'theme_boost', + 'theme' => 'classic', + 'themeoverrides' => false, + 'paths' => [ + 'theme/boost/templates/', + ], + ], + 'core (without overrides)' => [ + 'component' => 'core', + 'theme' => 'classic', + 'themeoverrides' => false, + 'paths' => [ + 'lib/templates/', ], ], ]; @@ -78,16 +133,23 @@ final class mustache_template_finder_test extends \advanced_testcase { /** * Tests for get_template_directories_for_component. * + * @covers ::get_template_directories_for_component * @dataProvider valid_template_directories_provider - * @param string $component - * @param string $theme - * @param array $paths + * @param string $component + * @param string $theme + * @param bool $themeoverrides + * @param array $paths */ - public function test_get_template_directories_for_component(string $component, string $theme, array $paths): void { + public function test_get_template_directories_for_component( + string $component, + string $theme, + bool $themeoverrides, + array $paths + ): void { global $CFG; // Test a plugin. - $dirs = mustache_template_finder::get_template_directories_for_component($component, $theme, $paths); + $dirs = mustache_template_finder::get_template_directories_for_component($component, $theme, $themeoverrides); $correct = array_map(function($path) use ($CFG) { return implode('/', [$CFG->dirroot, $path]); @@ -98,6 +160,8 @@ final class mustache_template_finder_test extends \advanced_testcase { /** * Tests for get_template_directories_for_component when dealing with an invalid component. + * + * @covers ::get_template_directories_for_component */ public function test_invalid_component_get_template_directories_for_component(): void { // Test something invalid. @@ -133,7 +197,7 @@ final class mustache_template_finder_test extends \advanced_testcase { 'theme' => 'classic', 'location' => 'theme/classic/templates/core/full_header.mustache', ], - 'Template overridden by child theme but tested against defualt theme' => [ + 'Template overridden by child theme but tested against default theme' => [ 'template' => 'core/full_header', 'theme' => '', 'location' => 'lib/templates/full_header.mustache', @@ -166,12 +230,23 @@ final class mustache_template_finder_test extends \advanced_testcase { 'theme' => '', 'location' => 'theme/classic/templates/navbar.mustache', ], + 'Template overridden by theme but original template requested using exclamation mark' => [ + 'template' => 'core/sticky_footer!', + 'theme' => 'boost', + 'location' => 'lib/templates/sticky_footer.mustache', + ], + 'Template overridden by theme which is explicitly specified' => [ + 'template' => 'theme_classic/core/full_header', + 'theme' => '', + 'location' => 'theme/classic/templates/core/full_header.mustache', + ], ]; } /** * Tests for get_template_filepath. * + * @covers ::get_template_filepath * @dataProvider valid_template_filepath_provider * @param string $template * @param string $theme @@ -186,9 +261,35 @@ final class mustache_template_finder_test extends \advanced_testcase { /** * Tests for get_template_filepath when dealing with an invalid component. + * + * @covers ::get_template_filepath */ public function test_invalid_component_get_template_filepath(): void { + $this->expectException(\coding_exception::class); + $this->expectExceptionMessage('Coding error detected, it must be fixed by a programmer: ' . + 'Component was not valid: core_octopus'); + mustache_template_finder::get_template_filepath('core_octopus/octopus', 'classic'); + } + + /** + * Tests for get_template_filepath when dealing with a missing template file. + * + * @covers ::get_template_filepath + */ + public function test_missing_template_get_template_filepath(): void { $this->expectException(\moodle_exception::class); + $this->expectExceptionMessage('Sorry, the requested file could not be found (core/octopus)'); mustache_template_finder::get_template_filepath('core/octopus', 'classic'); } + + /** + * Tests for get_template_filepath when dealing with an invalid template name. + * + * @covers ::get_template_filepath + */ + public function test_invalid_name_get_template_filepath(): void { + $this->expectException(\coding_exception::class); + $this->expectExceptionMessage('Templates names must be specified as "componentname/templatename" (octopus requested)'); + mustache_template_finder::get_template_filepath('octopus', 'classic'); + } }