MDL-50346 core: Allow sub-directories in template names

admin/tool/templatelibrary/amd/src/display.js

@@ -119,7 +119,7 @@ define(['jquery', 'core/ajax', 'core/log', 'core/notification', 'core/templates'
     var loadTemplate = function(templateName) {
         var parts = templateName.split('/');
         var component = parts.shift();
-        var name = parts.shift();
+        var name = parts.join('/');
 
         var promises = ajax.call([{
             methodname: 'core_output_load_template',

admin/tool/templatelibrary/classes/api.php

@@ -99,13 +99,34 @@ class api {
 
         foreach ($templatedirs as $templatecomponent => $dirs) {
             foreach ($dirs as $dir) {
+                if (!is_dir($dir) || !is_readable($dir)) {
+                    continue;
+                }
+                $dir = realpath($dir);
+
                 // List it.
-                $files = glob($dir . '/*.mustache');
+                $directory = new \RecursiveDirectoryIterator($dir);
+                $files = new \RecursiveIteratorIterator($directory);
 
                 foreach ($files as $file) {
-                    $templatename = basename($file, '.mustache');
-                    if ($search == '' || strpos($templatename, $search) !== false) {
-                        $results[$templatecomponent . '/' . $templatename] = 1;
+                    if (!$file->isFile()) {
+                        continue;
+                    }
+                    $filename = substr($file->getRealpath(), strlen($dir) + 1);
+                    if (strpos($templatecomponent, 'theme_') === 0) {
+                        if (strpos($filename, '/') !== false && strpos($filename, 'local/') !== 0) {
+                            // Skip any template in a sub-directory of a theme which is not in a local directory.
+                            // These are theme overrides of core templates.
+                            // Note: There is a rare edge case where a theme may override a template and then have additional
+                            // dependant templates and these will not be shown.
+                            continue;
+                        }
+                    }
+                    $templatename = str_replace('.mustache', '', $filename);
+                    $componenttemplatename = "{$templatecomponent}/{$templatename}";
+
+                    if ($search == '' || strpos($componenttemplatename, $search) !== false) {
+                        $results[$componenttemplatename] = 1;
                     }
                 }
             }
@@ -152,5 +173,4 @@ class api {
         return $templatestr;
     }
 
-
 }

admin/tool/templatelibrary/classes/external.php

@@ -105,7 +105,7 @@ class external extends external_api {
     public static function load_canonical_template_parameters() {
         return new external_function_parameters(
                 array('component' => new external_value(PARAM_COMPONENT, 'component containing the template'),
-                      'template' => new external_value(PARAM_ALPHANUMEXT, 'name of the template'))
+                      'template' => new external_value(PARAM_SAFEPATH, 'name of the template'))
             );
     }
 

admin/tool/templatelibrary/version.php

@@ -21,6 +21,6 @@
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 defined('MOODLE_INTERNAL') || die();
-$plugin->version   = 2019052000; // The current plugin version (Date: YYYYMMDDXX).
+$plugin->version   = 2019052002; // The current plugin version (Date: YYYYMMDDXX).
 $plugin->requires  = 2019051100; // Requires this Moodle version.
 $plugin->component = 'tool_templatelibrary'; // Full name of the plugin (used for diagnostics).

lib/amd/src/templates.js

@@ -304,7 +304,7 @@ define([
         // to be loaded.
         var parts = templateName.split('/');
         var component = parts.shift();
-        var name = parts.shift();
+        var name = parts.join('/');
         var deferred = $.Deferred();
 
         // Add this template to the buffer to be loaded.

lib/classes/output/external.php

@@ -51,7 +51,7 @@ class external extends external_api {
     public static function load_template_parameters() {
         return new external_function_parameters(
                 array('component' => new external_value(PARAM_COMPONENT, 'component containing the template'),
-                      'template' => new external_value(PARAM_ALPHANUMEXT, 'name of the template'),
+                      'template' => new external_value(PARAM_SAFEPATH, 'name of the template'),
                       'themename' => new external_value(PARAM_ALPHANUMEXT, 'The current theme.'),
                       'includecomments' => new external_value(PARAM_BOOL, 'Include comments or not', VALUE_DEFAULT, false)
                          )
@@ -102,7 +102,7 @@ class external extends external_api {
     public static function load_template_with_dependencies_parameters() {
         return new external_function_parameters([
             'component' => new external_value(PARAM_COMPONENT, 'component containing the template'),
-            'template' => new external_value(PARAM_ALPHANUMEXT, 'name of the template'),
+            'template' => new external_value(PARAM_SAFEPATH, 'name of the template'),
             'themename' => new external_value(PARAM_ALPHANUMEXT, 'The current theme.'),
             'includecomments' => new external_value(PARAM_BOOL, 'Include comments or not', VALUE_DEFAULT, false),
             'lang' => new external_value(PARAM_LANG, 'lang', VALUE_DEFAULT, null),

lib/classes/output/mustache_template_finder.php

@@ -107,11 +107,9 @@ class mustache_template_finder {
             throw new coding_exception('Templates names must be specified as "componentname/templatename"' .
                                        ' (' . s($name) . ' requested) ');
         }
+
         list($component, $templatename) = explode('/', $name, 2);
         $component = clean_param($component, PARAM_COMPONENT);
-        if (strpos($templatename, '/') !== false) {
-            throw new coding_exception('Templates cannot be placed in sub directories (' . s($name) . ' requested)');
-        }
 
         $dirs = self::get_template_directories_for_component($component, $themename);
 

lib/tests/mustache_template_finder_test.php

@@ -33,66 +33,168 @@ use core\output\mustache_template_finder;
  */
 class core_output_mustache_template_finder_testcase extends advanced_testcase {
 
-    public function test_get_template_directories_for_component() {
+    /**
+     * Data provider which reutrns a set of valid template directories to be used when testing
+     * get_template_directories_for_component.
+     *
+     * @return array
+     */
+    public function valid_template_directories_provider(): array {
+        return [
+            'plugin: mod_assign' => [
+                'component' => 'mod_assign',
+                'theme' => '',
+                'paths' => [
+                    'theme/boost/templates/mod_assign/',
+                    'mod/assign/templates/'
+                ],
+            ],
+            'plugin: mod_assign with classic' => [
+                'component' => 'mod_assign',
+                'theme' => 'classic',
+                'paths' => [
+                    'theme/classic/templates/mod_assign/',
+                    'theme/boost/templates/mod_assign/',
+                    'mod/assign/templates/'
+                ],
+            ],
+            'subsystem: core_user' => [
+                'component' => 'core_user',
+                'theme' => 'classic',
+                'paths' => [
+                    'theme/classic/templates/core_user/',
+                    'theme/boost/templates/core_user/',
+                    'user/templates/'
+                ],
+            ],
+            'core' => [
+                'component' => 'core',
+                'theme' => 'classic',
+                'paths' => [
+                    'theme/classic/templates/core/',
+                    'theme/boost/templates/core/',
+                    'lib/templates/'
+                ],
+            ],
+        ];
+    }
+
+    /**
+     * Tests for get_template_directories_for_component.
+     *
+     * @dataProvider valid_template_directories_provider
+     * @param   string $component
+     * @param   string $theme
+     * @param   array $paths
+     */
+    public function test_get_template_directories_for_component(string $component, string $theme, array $paths): void {
         global $CFG;
 
         // Test a plugin.
-        $dirs = mustache_template_finder::get_template_directories_for_component('mod_assign', 'classic');
-
-        $correct = array(
-            'theme/classic/templates/mod_assign/',
-            'theme/boost/templates/mod_assign/',
-            'mod/assign/templates/'
-        );
-        foreach ($dirs as $index => $dir) {
-            $this->assertSame($dir, $CFG->dirroot . '/' . $correct[$index]);
-        }
-        // Test a subsystem.
-        $dirs = mustache_template_finder::get_template_directories_for_component('core_user', 'classic');
+        $dirs = mustache_template_finder::get_template_directories_for_component($component, $theme, $paths);
 
-        $correct = array(
-            'theme/classic/templates/core_user/',
-            'theme/boost/templates/core_user/',
-            'user/templates/'
-        );
-        foreach ($dirs as $index => $dir) {
-            $this->assertSame($dir, $CFG->dirroot . '/' . $correct[$index]);
-        }
-        // Test core.
-        $dirs = mustache_template_finder::get_template_directories_for_component('core', 'classic');
+        $correct = array_map(function($path) use ($CFG) {
+            return implode('/', [$CFG->dirroot, $path]);
+        }, $paths);
 
-        $correct = array(
-            'theme/classic/templates/core/',
-            'theme/boost/templates/core/',
-            'lib/templates/'
-        );
-        foreach ($dirs as $index => $dir) {
-            $this->assertSame($dir, $CFG->dirroot . '/' . $correct[$index]);
-        }
-        return;
+        $this->assertEquals($correct, $dirs);
     }
 
     /**
+     * Tests for get_template_directories_for_component when dealing with an invalid component.
+     *
      * @expectedException coding_exception
      */
-    public function test_invalid_get_template_directories_for_component() {
+    public function test_invalid_component_get_template_directories_for_component() {
         // Test something invalid.
-        $dirs = mustache_template_finder::get_template_directories_for_component('octopus', 'classic');
+        mustache_template_finder::get_template_directories_for_component('octopus', 'classic');
     }
 
-    public function test_get_template_filepath() {
+    /**
+     * Data provider which reutrns a set of valid template directories to be used when testing
+     * get_template_directories_for_component.
+     *
+     * @return array
+     */
+    public function valid_template_filepath_provider(): array {
+        return [
+            'Standard core template' => [
+                'template' => 'core/modal',
+                'theme' => '',
+                'location' => 'lib/templates/modal.mustache',
+            ],
+            'Template overridden by theme' => [
+                'template' => 'core_form/element-float-inline',
+                'theme' => '',
+                'location' => 'theme/boost/templates/core_form/element-float-inline.mustache',
+            ],
+            'Template overridden by theme but child theme selected' => [
+                'template' => 'core_form/element-float-inline',
+                'theme' => 'classic',
+                'location' => 'theme/boost/templates/core_form/element-float-inline.mustache',
+            ],
+            'Template overridden by child theme' => [
+                'template' => 'core/full_header',
+                'theme' => 'classic',
+                'location' => 'theme/classic/templates/core/full_header.mustache',
+            ],
+            'Template overridden by child theme but tested against defualt theme' => [
+                'template' => 'core/full_header',
+                'theme' => '',
+                'location' => 'lib/templates/full_header.mustache',
+            ],
+            'Standard plugin template' => [
+                'template' => 'mod_assign/grading_panel',
+                'theme' => '',
+                'location' => 'mod/assign/templates/grading_panel.mustache',
+            ],
+            'Subsystem template' => [
+                'template' => 'core_user/status_details',
+                'theme' => '',
+                'location' => 'user/templates/status_details.mustache',
+            ],
+            'Theme own template' => [
+                'template' => 'theme_classic/columns',
+                'theme' => '',
+                'location' => 'theme/classic/templates/columns.mustache',
+            ],
+            'Theme overridden template against that theme' => [
+                'template' => 'theme_classic/navbar',
+                'theme' => 'classic',
+                'location' => 'theme/classic/templates/navbar.mustache',
+            ],
+            // Note: This one looks strange but is correct. It is legitimate to request theme's component template in
+            // the context of another theme. For example, this is used by child themes making use of parent theme
+            // templates.
+            'Theme overridden template against the default theme' => [
+                'template' => 'theme_classic/navbar',
+                'theme' => '',
+                'location' => 'theme/classic/templates/navbar.mustache',
+            ],
+        ];
+    }
+
+    /**
+     * Tests for get_template_filepath.
+     *
+     * @dataProvider valid_template_filepath_provider
+     * @param   string $template
+     * @param   string $theme
+     * @param   string $location
+     */
+    public function test_get_template_filepath(string $template, string $theme, string $location) {
         global $CFG;
 
-        $filename = mustache_template_finder::get_template_filepath('core/pix_icon', 'classic');
-        $correct = $CFG->dirroot . '/lib/templates/pix_icon.mustache';
-        $this->assertSame($correct, $filename);
+        $filename = mustache_template_finder::get_template_filepath($template, $theme);
+        $this->assertEquals("{$CFG->dirroot}/{$location}", $filename);
     }
 
     /**
+     * Tests for get_template_filepath when dealing with an invalid component.
+     *
      * @expectedException moodle_exception
      */
-    public function test_invalid_get_template_filepath() {
-        // Test something invalid.
-        $dirs = mustache_template_finder::get_template_filepath('core/octopus', 'classic');
+    public function test_invalid_component_get_template_filepath() {
+        mustache_template_finder::get_template_filepath('core/octopus', 'classic');
     }
 }

lib/upgrade.txt

@@ -39,7 +39,17 @@ information provided here is intended especially for developers.
       mod_forum/views/post: mod/forum/amd/src/views/post
 * The 'xxxx_check_password_policy' method now has an extra parameter: $user. It contains the user object to perform password
 validation against and defaults to null (so, no user needed) if not provided.
+* It is now possible to use sub-directories when creating mustache templates.
+  The standard rules for Level 2 namespaces also apply to templates.
+  The sub-directory used must be either an valid component, or placed inside a 'local' directory to ensure that it does not conflict with other components.
+
+    The following are all valid template names and locations in your plugin:
+      mod_forum/forum_post: mod/forum/templates/forum_post.mustache
+      mod_forum/local/post/user: mod/forum/templates/local/post/user.mustache
+      mod_forum/form/checkbox_toggle: mod/forum/templates/form/checkbox_toggle.mustache
 
+    The following are _invalid_ template names and locations:
+      mod_forum/post/user: mod/forum/templates/local/post/user.mustache
 
 === 3.7 ===
 

theme/upgrade.txt

@@ -5,6 +5,21 @@ information provided here is intended especially for theme designer.
 
 * The PHP Less compilier has now been removed from the core library.
   Please consider migrating your theme to use SCSS.
+* It is now possible to use sub-directories when creating mustache templates.
+  The standard rules for Level 2 namespaces also apply to templates.
+  The sub-directory used must be either an valid component, or placed inside a 'local' directory to ensure that it does not conflict with other components.
+
+    The following are all valid template names and locations in your theme:
+      theme_themename/columns2: theme/[themename]/columns2.mustache
+      theme_themename/local/layouts/columns2: theme/[themename]/local/layouts/columns2.mustache
+
+    The following are core templates, locations, and override locations in your theme:
+      core/modal: lib/templates/modal.mustache => theme/[themename]/core/modal.mustache
+      mod_forum/forum_post: mod/forum/templates/forum_post.mustache => theme/[themename]/mod_forum/forum_post.mustache
+      mod_forum/local/post/user: mod/forum/templates/local/post/user.mustache => theme/[themename]/mod_forum/local/post/user.mustache
+
+    The following are _invalid_ template names and locations:
+      theme_themename/layouts/columns2: theme/[themename]/layouts/columns2.mustache
 
 === 3.7 ===
 * The core/form_autocompelte_input template now has a `data-tags` attribute.

version.php

@@ -29,7 +29,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$version  = 2019080100.00;              // YYYYMMDD      = weekly release date of this DEV branch.
+$version  = 2019080700.00;              // YYYYMMDD      = weekly release date of this DEV branch.
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.