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.
silverstripe · Jan 14, 2014 · 5f87d34 · 5f87d34
1 parent 9b5a9af
commit 5f87d34
Show file tree

Hide file tree

Showing 4 changed files with 192 additions and 67 deletions.
diff --git a/core/manifest/TemplateLoader.php b/core/manifest/TemplateLoader.php
@@ -64,27 +64,26 @@ public function popManifest() {
 	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(
+						$found = array(
-						'main' => $found[$type]
+							'main' => $found[$type]
-					);
+						);
 					}
-
 					$result = array_merge($found, $result);
 				}
 			}
-
+			
 		return $result;
 	}
 

diff --git a/core/manifest/TemplateManifest.php b/core/manifest/TemplateManifest.php
@@ -110,17 +110,23 @@ public function getTemplate($name) {
 	 * @return array
 	 */
 	public function getCandidateTemplate($name, $theme = null) {
+		$found = array();
 		$candidates = $this->getTemplate($name);
-
+
-		if ($this->project && isset($candidates[$this->project])) {
+		// theme overrides modules
-			$found = $candidates[$this->project];
+		if ($theme && isset($candidates['themes'][$theme])) {
-		} else 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
@@ -6,64 +6,184 @@
  * @subpackage tests
  */
 class TemplateLoaderTest extends SapphireTest {
-
+
-	public function testFindTemplates() {
+	private $base;
-		$base     = dirname(__FILE__) . '/fixtures/templatemanifest';
+	private $manifest;
-		$manifest = new SS_TemplateManifest($base, 'myproject', false, true);
+	private $loader;
-		$loader   = new SS_TemplateLoader();
+
-
+	/**
-		$manifest->regenerate(false);
+	 * Set up manifest before each test
-		$loader->pushManifest($manifest);
+	 */
-
+	public function setUp() {
-		$expectPage = array(
+		parent::setUp();
-			'main'   => "$base/module/templates/Page.ss",
+		$this->base = dirname(__FILE__) . '/fixtures/templatemanifest';
-			'Layout' => "$base/module/templates/Layout/Page.ss"
+		$this->manifest = new SS_TemplateManifest($this->base, 'myproject', false, true);
+		$this->loader = new SS_TemplateLoader();
+		$this->refreshLoader();
+	}
+
+	/**
+	 * 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"
 		);
-		$expectPageThemed = array(
+		$this->assertEquals($expect, $this->loader->findTemplates('Page'));
-			'main'   => "$base/themes/theme/templates/Page.ss",
+		$this->assertEquals($expect, $this->loader->findTemplates('PAGE'));
-			'Layout' => "$base/themes/theme/templates/Layout/Page.ss"
+		$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($expectPage, $loader->findTemplates('Page'));
+		$this->assertEquals($expect, $this->loader->findTemplates('PAGE', 'theme'));
-		$this->assertEquals($expectPage, $loader->findTemplates(array('Foo', 'Page')));
+		$this->assertEquals($expect, $this->loader->findTemplates(array('Foo', 'Page'), 'theme'));
-		$this->assertEquals($expectPage, $loader->findTemplates('PAGE'));
+	}
-		$this->assertEquals($expectPageThemed, $loader->findTemplates('Page', 'theme'));
+
-
+	/**
-		$expectPageLayout       = array('main' => "$base/module/templates/Layout/Page.ss");
+	 * Test that 'main' and 'Layout' templates are loaded from project without a set theme
-		$expectPageLayoutThemed = array('main' => "$base/themes/theme/templates/Layout/Page.ss");
+	 */
-
+	public function testFindTemplatesInApplication() {
-		$this->assertEquals($expectPageLayout, $loader->findTemplates('Layout/Page'));
+		$templates = array(
-		$this->assertEquals($expectPageLayout, $loader->findTemplates('Layout/PAGE'));
+			$this->base . '/myproject/templates/Page.ss',
-		$this->assertEquals($expectPageLayoutThemed, $loader->findTemplates('Layout/Page', 'theme'));
+			$this->base . '/myproject/templates/Layout/Page.ss'
-
-		$expectCustomPage = array(
-			'main'   => "$base/module/templates/Page.ss",
-			'Layout' => "$base/module/templates/Layout/CustomPage.ss"
 		);
-
+		$this->createTestTemplates($templates);
-		$this->assertEquals($expectCustomPage, $loader->findTemplates(array('CustomPage', 'Page')));
+		$this->refreshLoader();
-
+
-		// 'main' template only exists in theme, and 'Layout' template only exists in module
+		$expect = array(
-		$expectCustomThemePage = array(
+			'main'   => "$this->base/myproject/templates/Page.ss",
-			'main'   => "$base/themes/theme/templates/CustomThemePage.ss",
+			'Layout' => "$this->base/myproject/templates/Layout/Page.ss"
-			'Layout' => "$base/module/templates/Layout/CustomThemePage.ss"
 		);
-		$this->assertEquals($expectCustomThemePage, $loader->findTemplates(array('CustomThemePage', 'Page'), 'theme'));
+		$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() {
-		$base     = dirname(__FILE__) . '/fixtures/templatemanifest';
+		$expect = array(
-		$manifest = new SS_TemplateManifest($base, 'myproject', false, true);
+			'main'   => "$this->base/myproject/templates/CustomTemplate.ss"
-		$loader   = new SS_TemplateLoader();
-
-		$manifest->regenerate(false);
-		$loader->pushManifest($manifest);
-
-		$expectPage = array(
-			'main'   => "$base/myproject/templates/CustomTemplate.ss"
 		);
-
+		$this->assertEquals($expect, $this->loader->findTemplates('CustomTemplate'));
-		$this->assertEquals($expectPage, $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