From 5f87d344f11c382dbee3fae8edfc00bb9a5a0265 Mon Sep 17 00:00:00 2001 From: Ryan Wachtl Date: Mon, 13 Jan 2014 14:33:22 -0600 Subject: [PATCH] FIX Overriding of theme templates in project folder Fixes issue of templates not being found when a Page's main/Layout templates are split between the project and theme folders. Adds more expansive testing for template loading. --- core/manifest/TemplateLoader.php | 15 +- core/manifest/TemplateManifest.php | 22 +- tests/core/manifest/TemplateLoaderTest.php | 232 +++++++++++++----- .../myproject/templates/Layout/.gitignore | 0 4 files changed, 197 insertions(+), 72 deletions(-) create mode 100644 tests/core/manifest/fixtures/templatemanifest/myproject/templates/Layout/.gitignore diff --git a/core/manifest/TemplateLoader.php b/core/manifest/TemplateLoader.php index f1e8e6a66..4fd30f25b 100644 --- a/core/manifest/TemplateLoader.php +++ b/core/manifest/TemplateLoader.php @@ -64,27 +64,26 @@ class SS_TemplateLoader { public function findTemplates($templates, $theme = null) { $result = array(); $project = project(); - + foreach ((array) $templates as $template) { $found = false; - + if (strpos($template, '/')) { list($type, $template) = explode('/', $template, 2); } else { $type = null; } - + if ($found = $this->getManifest()->getCandidateTemplate($template, $theme)) { if ($type && isset($found[$type])) { - $found = array( - 'main' => $found[$type] - ); + $found = array( + 'main' => $found[$type] + ); } - $result = array_merge($found, $result); } } - + return $result; } diff --git a/core/manifest/TemplateManifest.php b/core/manifest/TemplateManifest.php index 47de70d56..371cb0371 100644 --- a/core/manifest/TemplateManifest.php +++ b/core/manifest/TemplateManifest.php @@ -110,17 +110,23 @@ class SS_TemplateManifest { * @return array */ public function getCandidateTemplate($name, $theme = null) { + $found = array(); $candidates = $this->getTemplate($name); - - if ($this->project && isset($candidates[$this->project])) { - $found = $candidates[$this->project]; - } else if ($theme && isset($candidates['themes'][$theme])) { + + // theme overrides modules + if ($theme && isset($candidates['themes'][$theme])) { $found = array_merge($candidates, $candidates['themes'][$theme]); - } else { - $found = $candidates; } - if(isset($found['themes'])) unset($found['themes']); - + // project overrides theme + if ($this->project && isset($candidates[$this->project])) { + $found = array_merge($found, $candidates[$this->project]); + } + + $found = ($found) ? $found : $candidates; + + if (isset($found['themes'])) unset($found['themes']); + if (isset($found[$this->project])) unset($found[$this->project]); + return $found; } diff --git a/tests/core/manifest/TemplateLoaderTest.php b/tests/core/manifest/TemplateLoaderTest.php index b4e154ced..878e96c87 100644 --- a/tests/core/manifest/TemplateLoaderTest.php +++ b/tests/core/manifest/TemplateLoaderTest.php @@ -6,64 +6,184 @@ * @subpackage tests */ class TemplateLoaderTest extends SapphireTest { - - public function testFindTemplates() { - $base = dirname(__FILE__) . '/fixtures/templatemanifest'; - $manifest = new SS_TemplateManifest($base, 'myproject', false, true); - $loader = new SS_TemplateLoader(); - - $manifest->regenerate(false); - $loader->pushManifest($manifest); - - $expectPage = array( - 'main' => "$base/module/templates/Page.ss", - 'Layout' => "$base/module/templates/Layout/Page.ss" - ); - $expectPageThemed = array( - 'main' => "$base/themes/theme/templates/Page.ss", - 'Layout' => "$base/themes/theme/templates/Layout/Page.ss" - ); - - $this->assertEquals($expectPage, $loader->findTemplates('Page')); - $this->assertEquals($expectPage, $loader->findTemplates(array('Foo', 'Page'))); - $this->assertEquals($expectPage, $loader->findTemplates('PAGE')); - $this->assertEquals($expectPageThemed, $loader->findTemplates('Page', 'theme')); - - $expectPageLayout = array('main' => "$base/module/templates/Layout/Page.ss"); - $expectPageLayoutThemed = array('main' => "$base/themes/theme/templates/Layout/Page.ss"); - - $this->assertEquals($expectPageLayout, $loader->findTemplates('Layout/Page')); - $this->assertEquals($expectPageLayout, $loader->findTemplates('Layout/PAGE')); - $this->assertEquals($expectPageLayoutThemed, $loader->findTemplates('Layout/Page', 'theme')); - - $expectCustomPage = array( - 'main' => "$base/module/templates/Page.ss", - 'Layout' => "$base/module/templates/Layout/CustomPage.ss" - ); - - $this->assertEquals($expectCustomPage, $loader->findTemplates(array('CustomPage', 'Page'))); - - // 'main' template only exists in theme, and 'Layout' template only exists in module - $expectCustomThemePage = array( - 'main' => "$base/themes/theme/templates/CustomThemePage.ss", - 'Layout' => "$base/module/templates/Layout/CustomThemePage.ss" - ); - $this->assertEquals($expectCustomThemePage, $loader->findTemplates(array('CustomThemePage', 'Page'), 'theme')); + + private $base; + private $manifest; + private $loader; + + /** + * Set up manifest before each test + */ + public function setUp() { + parent::setUp(); + $this->base = dirname(__FILE__) . '/fixtures/templatemanifest'; + $this->manifest = new SS_TemplateManifest($this->base, 'myproject', false, true); + $this->loader = new SS_TemplateLoader(); + $this->refreshLoader(); } - - public function testFindTemplatesApplicationOverridesModule() { - $base = dirname(__FILE__) . '/fixtures/templatemanifest'; - $manifest = new SS_TemplateManifest($base, 'myproject', false, true); - $loader = new SS_TemplateLoader(); - - $manifest->regenerate(false); - $loader->pushManifest($manifest); - - $expectPage = array( - 'main' => "$base/myproject/templates/CustomTemplate.ss" + + /** + * Test that 'main' and 'Layout' templates are loaded from module + */ + public function testFindTemplatesInModule() { + $expect = array( + 'main' => "$this->base/module/templates/Page.ss", + 'Layout' => "$this->base/module/templates/Layout/Page.ss" ); - - $this->assertEquals($expectPage, $loader->findTemplates('CustomTemplate')); + $this->assertEquals($expect, $this->loader->findTemplates('Page')); + $this->assertEquals($expect, $this->loader->findTemplates('PAGE')); + $this->assertEquals($expect, $this->loader->findTemplates(array('Foo', 'Page'))); + } + + /** + * Test that 'main' and 'Layout' templates are loaded from set theme + */ + public function testFindTemplatesInTheme() { + $expect = array( + 'main' => "$this->base/themes/theme/templates/Page.ss", + 'Layout' => "$this->base/themes/theme/templates/Layout/Page.ss" + ); + $this->assertEquals($expect, $this->loader->findTemplates('Page', 'theme')); + $this->assertEquals($expect, $this->loader->findTemplates('PAGE', 'theme')); + $this->assertEquals($expect, $this->loader->findTemplates(array('Foo', 'Page'), 'theme')); + } + + /** + * Test that 'main' and 'Layout' templates are loaded from project without a set theme + */ + public function testFindTemplatesInApplication() { + $templates = array( + $this->base . '/myproject/templates/Page.ss', + $this->base . '/myproject/templates/Layout/Page.ss' + ); + $this->createTestTemplates($templates); + $this->refreshLoader(); + + $expect = array( + 'main' => "$this->base/myproject/templates/Page.ss", + 'Layout' => "$this->base/myproject/templates/Layout/Page.ss" + ); + $this->assertEquals($expect, $this->loader->findTemplates('Page')); + $this->assertEquals($expect, $this->loader->findTemplates('PAGE')); + $this->assertEquals($expect, $this->loader->findTemplates(array('Foo', 'Page'))); + + $this->removeTestTemplates($templates); + } + + /** + * Test that 'Layout' template is loaded from module + */ + public function testFindTemplatesInModuleLayout() { + $expect = array( + 'main' => "$this->base/module/templates/Layout/Page.ss" + ); + $this->assertEquals($expect, $this->loader->findTemplates('Layout/Page')); + } + + /** + * Test that 'Layout' template is loaded from theme + */ + public function testFindTemplatesInThemeLayout() { + $expect = array( + 'main' => "$this->base/themes/theme/templates/Layout/Page.ss" + ); + $this->assertEquals($expect, $this->loader->findTemplates('Layout/Page', 'theme')); + } + + /** + * Test that 'main' template is found in theme and 'Layout' is found in module + */ + public function testFindTemplatesMainThemeLayoutModule() { + $expect = array( + 'main' => "$this->base/themes/theme/templates/CustomThemePage.ss", + 'Layout' => "$this->base/module/templates/Layout/CustomThemePage.ss" + ); + $this->assertEquals($expect, $this->loader->findTemplates(array('CustomThemePage', 'Page'), 'theme')); + } + + /** + * Test that project template overrides module template of same name + */ + public function testFindTemplatesApplicationOverridesModule() { + $expect = array( + 'main' => "$this->base/myproject/templates/CustomTemplate.ss" + ); + $this->assertEquals($expect, $this->loader->findTemplates('CustomTemplate')); + } + + /** + * Test that project templates overrides theme templates + */ + public function testFindTemplatesApplicationOverridesTheme() { + $templates = array( + $this->base . '/myproject/templates/Page.ss', + $this->base . '/myproject/templates/Layout/Page.ss' + ); + $this->createTestTemplates($templates); + $this->refreshLoader(); + + $expect = array( + 'main' => "$this->base/myproject/templates/Page.ss", + 'Layout' => "$this->base/myproject/templates/Layout/Page.ss" + ); + $this->assertEquals($expect, $this->loader->findTemplates('Page'), 'theme'); + + $this->removeTestTemplates($templates); + } + + /** + * Test that project 'Layout' template overrides theme 'Layout' template + */ + public function testFindTemplatesApplicationLayoutOverridesThemeLayout() { + $templates = array( + $this->base . '/myproject/templates/Layout/Page.ss' + ); + $this->createTestTemplates($templates); + $this->refreshLoader(); + + $expect = array( + 'main' => "$this->base/themes/theme/templates/Page.ss", + 'Layout' => "$this->base/myproject/templates/Layout/Page.ss" + ); + $this->assertEquals($expect, $this->loader->findTemplates('Page', 'theme')); + + $this->removeTestTemplates($templates); + } + + /** + * Test that project 'main' template overrides theme 'main' template + */ + public function testFindTemplatesApplicationMainOverridesThemeMain() { + $templates = array( + $this->base . '/myproject/templates/Page.ss' + ); + $this->createTestTemplates($templates); + $this->refreshLoader(); + + $expect = array( + 'main' => "$this->base/myproject/templates/Page.ss", + 'Layout' => "$this->base/themes/theme/templates/Layout/Page.ss" + ); + $this->assertEquals($expect, $this->loader->findTemplates('Page', 'theme')); + + $this->removeTestTemplates($templates); + } + + protected function refreshLoader() { + $this->manifest->regenerate(false); + $this->loader->pushManifest($this->manifest); + } + + protected function createTestTemplates($templates) { + foreach ($templates as $template) { + file_put_contents($template, ''); + } + } + + protected function removeTestTemplates($templates) { + foreach ($templates as $template) { + unlink($template); + } } } diff --git a/tests/core/manifest/fixtures/templatemanifest/myproject/templates/Layout/.gitignore b/tests/core/manifest/fixtures/templatemanifest/myproject/templates/Layout/.gitignore new file mode 100644 index 000000000..e69de29bb