Merge pull request #14449 from jdalsem/elgg5-minor

chore(views): removed the need to clamp the viewtype on boot
Elgg · Aug 7, 2023 · 7f3b385 · 7f3b385
2 parents 1121693 + 4158550
commit 7f3b385
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 80 deletions.
diff --git a/engine/classes/Elgg/Application/BootHandler.php b/engine/classes/Elgg/Application/BootHandler.php
@@ -133,7 +133,6 @@ public function bootApplication(): void {
 
 		$events = $this->app->internal_services->events;
 
-		$this->app->internal_services->views->clampViewtypeToPopulatedViews();
 		$this->app->allowPathRewrite();
 
 		// Complete the boot process for both engine and plugins

diff --git a/engine/classes/Elgg/ViewsService.php b/engine/classes/Elgg/ViewsService.php
@@ -130,18 +130,6 @@ public function getViewtype(): string {
 		return $this->viewtype;
 	}
 
-	/**
-	 * If the current viewtype has no views, reset it to "default"
-	 *
-	 * @return void
-	 */
-	public function clampViewtypeToPopulatedViews(): void {
-		$viewtype = $this->getViewtype();
-		if (empty($this->locations[$viewtype])) {
-			$this->viewtype = 'default';
-		}
-	}
-
 	/**
 	 * Resolve the initial viewtype
 	 *
@@ -150,13 +138,13 @@ public function clampViewtypeToPopulatedViews(): void {
 	private function resolveViewtype(): string {
 		if ($this->request) {
 			$view = $this->request->getParam('view', '', false);
-			if ($this->isValidViewtype($view)) {
+			if ($this->isValidViewtype($view) && !empty($this->locations[$view])) {
 				return $view;
 			}
 		}
 
 		$view = (string) elgg_get_config('view');
-		if ($this->isValidViewtype($view)) {
+		if ($this->isValidViewtype($view) && !empty($this->locations[$view])) {
 			return $view;
 		}
 

diff --git a/engine/tests/phpunit/unit/Elgg/ViewsServiceUnitTest.php b/engine/tests/phpunit/unit/Elgg/ViewsServiceUnitTest.php
@@ -25,7 +25,14 @@ public function up() {
 
 		$this->views = new ViewsService($this->events, _elgg_services()->request);
 		$this->views->setLogger($logger);
-		$this->views->autoregisterViews('', "$this->viewsDir/default", 'default');
+		$this->views->autoregisterViews('', "{$this->viewsDir}/default", 'default');
+		$this->views->autoregisterViews('', "{$this->viewsDir}/json", 'json');
+		$this->views->setViewtype('');
+	}
+
+	public function down() {
+		set_input('view', '');
+		elgg_set_config('view', null);
 	}
 
 	public function testCanExtendViews() {
@@ -75,9 +82,7 @@ public function testRegistersStaticFilesAsViews() {
 	}
 
 	public function testUsesPhpToRenderNonStaticViews() {
-		$this->assertEquals("// PHPin", $this->views->renderView('js/interpreted.js', array(
-					'in' => 'in',
-		)));
+		$this->assertEquals("// PHPin", $this->views->renderView('js/interpreted.js', ['in' => 'in']));
 	}
 
 	public function testDoesNotUsePhpToRenderStaticViews() {
@@ -103,9 +108,7 @@ public function testCanSetViewPathsViaSpec() {
 		$expected = file_get_contents("$this->viewsDir/default/js/static.js");
 		$this->assertEquals($expected, $this->views->renderView('hello.js'));
 
-		$this->assertEquals("// PHPin", $this->views->renderView('hello/world.js', array(
-					'in' => 'in',
-		)));
+		$this->assertEquals("// PHPin", $this->views->renderView('hello/world.js', ['in' => 'in']));
 	}
 
 	public function testCanSetViewsDirs() {
@@ -201,7 +204,7 @@ public function testCanGetViewRenderingList() {
 		$list = $this->views->getViewList('foo');
 		$this->assertEquals([
 			500 => 'foo',
-				], $list);
+		], $list);
 
 		$this->views->extendView('foo', 'bar');
 		$this->views->extendView('foo', 'bing', 499);
@@ -211,7 +214,7 @@ public function testCanGetViewRenderingList() {
 			499 => 'bing',
 			500 => 'foo',
 			501 => 'bar',
-				], $list);
+		], $list);
 	}
 
 	public function testPreventExtensionOnSelf() {
@@ -253,4 +256,49 @@ public function getExampleNormalizedViews() {
 			['view.jpg', 'css/view.jpg'],
 		];
 	}
+
+	public function testSetViewtype() {
+		$this->assertTrue($this->views->setViewtype('test'));
+		$this->assertEquals('test', $this->views->getViewtype());
+	}
+
+	public function testDefaultViewtype() {
+		$this->assertEquals('default', $this->views->getViewtype());
+	}
+
+	public function testInputSetsInitialViewtype() {
+		set_input('view', 'json');
+		$this->assertEquals('json', $this->views->getViewtype());
+	}
+
+	public function testConfigSetsInitialViewtype() {
+		elgg_set_config('view', 'json');
+
+		$this->assertEquals('json', $this->views->getViewtype());
+	}
+
+	public function testSettingInputDoesNotChangeViewtype() {
+		$this->assertEquals('default', $this->views->getViewtype());
+
+		set_input('view', 'json');
+		$this->assertEquals('default', $this->views->getViewtype());
+	}
+
+	public function testSettingConfigDoesNotChangeViewtype() {
+		$this->assertEquals('default', $this->views->getViewtype());
+
+		elgg_set_config('view', 'json');
+		$this->assertEquals('default', $this->views->getViewtype());
+	}
+
+	public function testIsValidViewtype() {
+		$this->assertTrue($this->views->isValidViewtype('valid'));
+		$this->assertTrue($this->views->isValidViewtype('valid_viewtype'));
+		$this->assertTrue($this->views->isValidViewtype('0'));
+		$this->assertTrue($this->views->isValidViewtype(123)); // will be autocasted to string
+
+		$this->assertFalse($this->views->isValidViewtype('a;b'));
+		$this->assertFalse($this->views->isValidViewtype('invalid-viewtype'));
+		$this->assertFalse($this->views->isValidViewtype(''));
+	}
 }
diff --git a/engine/tests/phpunit/unit/ElggCoreViewtypeUnitTest.php b/engine/tests/phpunit/unit/ElggCoreViewtypeUnitTest.php