From 438fe02116365569893f3370d08c4b1f20f0b19e Mon Sep 17 00:00:00 2001 From: Will Morgan Date: Tue, 8 Apr 2014 12:55:57 +0100 Subject: [PATCH 1/2] FIX change action variable source to getViewer Previously we were using the request to get the action parameter. However for custom URL structures and nested controllers, this won't work and causes template selection to break. --- control/Controller.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/control/Controller.php b/control/Controller.php index 4c25024a5..481c8bd4e 100644 --- a/control/Controller.php +++ b/control/Controller.php @@ -154,7 +154,7 @@ class Controller extends RequestHandler implements TemplateGlobalProvider { Debug::message("Request handler $body->class object to $this->class controller;" . "rendering with template returned by $body->class::getViewer()"); } - $body = $body->getViewer($request->latestParam('Action'))->process($body); + $body = $body->getViewer($this->getAction())->process($body); } $this->response->setBody($body); From c6797f52ea5527ae55748edf16272006ccb9d3d9 Mon Sep 17 00:00:00 2001 From: Will Morgan Date: Wed, 16 Apr 2014 13:34:09 +0100 Subject: [PATCH 2/2] Test nested controller actions and Controller->getViewer Adds tests and supporting classes for testing that the correct action is passed to Controller->getViewer inside Controller->handleRequest. --- tests/control/ControllerTest.php | 70 +++++++++++++++++++++++++++++++- 1 file changed, 69 insertions(+), 1 deletion(-) diff --git a/tests/control/ControllerTest.php b/tests/control/ControllerTest.php index a50be5af7..fc0ca3e0b 100644 --- a/tests/control/ControllerTest.php +++ b/tests/control/ControllerTest.php @@ -373,6 +373,27 @@ class ControllerTest extends FunctionalTest { "Doesn't redirect on external URLs" ); } + + public function testSubActions() { + /* If a controller action returns another controller, ensure that the $action variable is correctly forwarded */ + $response = $this->get("ControllerTest_ContainerController/subcontroller/subaction"); + $this->assertEquals('subaction', $response->getBody()); + + $request = new SS_HTTPRequest( + 'GET', + 'ControllerTest_ContainerController/subcontroller/substring/subvieweraction' + ); + /* Shift to emulate the director selecting the controller */ + $request->shift(); + /* Handle the request to create conditions where improperly passing the action to the viewer might fail */ + $controller = new ControllerTest_ContainerController(); + try { + $controller->handleRequest($request, DataModel::inst()); + } + catch(ControllerTest_SubController_Exception $e) { + $this->fail($e->getMessage()); + } + } } /** @@ -498,5 +519,52 @@ class ControllerTest_HasAction extends Controller { class ControllerTest_HasAction_Unsecured extends ControllerTest_HasAction implements TestOnly { public function defined_action() { } - + +} + +class ControllerTest_ContainerController extends Controller implements TestOnly { + + private static $allowed_actions = array( + 'subcontroller', + ); + + public function subcontroller() { + return new ControllerTest_SubController(); + } + +} + +class ControllerTest_SubController extends Controller implements TestOnly { + + private static $allowed_actions = array( + 'subaction', + 'subvieweraction', + ); + + private static $url_handlers = array( + 'substring/subvieweraction' => 'subvieweraction', + ); + + public function subaction() { + return $this->getAction(); + } + + /* This is messy, but Controller->handleRequest is a hard to test method which warrants such measures... */ + public function getViewer($action) { + if(empty($action)) { + throw new ControllerTest_SubController_Exception("Null action passed, getViewer will break"); + } + return parent::getViewer($action); + } + + public function subvieweraction() { + return $this->customise(array( + 'Thoughts' => 'Hope this works', + )); + } + +} + +class ControllerTest_SubController_Exception extends Exception { + }