From 19aeb8fd64e1f92174ca8623b0c194db0bbda2e0 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Tue, 5 Sep 2017 13:48:28 +1200 Subject: [PATCH] API Add getSubsiteIdWasChanged calculated dynamically --- code/Middleware/InitStateMiddleware.php | 11 +----- code/State/SubsiteState.php | 31 ++++++++------- code/extensions/LeftAndMainSubsites.php | 21 ++++++----- tests/php/State/SubsiteStateTest.php | 50 +++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 32 deletions(-) create mode 100644 tests/php/State/SubsiteStateTest.php diff --git a/code/Middleware/InitStateMiddleware.php b/code/Middleware/InitStateMiddleware.php index 8a79225..ae85cc6 100644 --- a/code/Middleware/InitStateMiddleware.php +++ b/code/Middleware/InitStateMiddleware.php @@ -41,20 +41,13 @@ class InitStateMiddleware implements HTTPMiddleware $subsiteId = $this->detectSubsiteId($request); $state->setSubsiteId($subsiteId); - // Persist to the session if using the CMS - if ($state->getUseSessions()) { - $originalSubsiteId = $request->getSession()->get('SubsiteID'); - } - return $delegate($request); } catch (DatabaseException $ex) { // No-op, database is not ready } finally { + // Persist to the session if using the CMS if ($state->getUseSessions()) { - // Track session changes - $request->getSession()->set('SubsiteID', $subsiteId); - - $state->setSessionWasChanged($subsiteId === $originalSubsiteId); + $request->getSession()->set('SubsiteID', $state->getSubsiteId()); } } } diff --git a/code/State/SubsiteState.php b/code/State/SubsiteState.php index 74e03be..56b88e2 100644 --- a/code/State/SubsiteState.php +++ b/code/State/SubsiteState.php @@ -2,7 +2,6 @@ namespace SilverStripe\Subsites\State; -use SilverStripe\Control\HTTPRequest; use SilverStripe\Core\Injector\Injectable; use SilverStripe\Core\Injector\Injector; @@ -18,15 +17,16 @@ class SubsiteState */ protected $subsiteId; + /** - * @var bool + * @var int|null */ - protected $useSessions; + protected $originalSubsiteId; /** * @var bool */ - protected $sessionWasChanged = false; + protected $useSessions; /** * Get the current subsite ID @@ -39,13 +39,18 @@ class SubsiteState } /** - * Set the current subsite ID + * Set the current subsite ID, and track the first subsite ID set as the "original". This is used to check + * whether the ID has been changed through a request. * * @param int $id * @return $this */ public function setSubsiteId($id) { + if (is_null($this->originalSubsiteId)) { + $this->originalSubsiteId = (int) $id; + } + $this->subsiteId = (int) $id; return $this; @@ -74,16 +79,14 @@ class SubsiteState return $this; } - public function getSessionWasChanged() + /** + * Get whether the subsite ID has been changed during a request, based on the original and current IDs + * + * @return bool + */ + public function getSubsiteIdWasChanged() { - return $this->sessionWasChanged; - } - - public function setSessionWasChanged($changed = true) - { - $this->sessionWasChanged = (bool) $changed; - - return $this; + return $this->originalSubsiteId !== $this->getSubsiteId(); } /** diff --git a/code/extensions/LeftAndMainSubsites.php b/code/extensions/LeftAndMainSubsites.php index 8fa59b0..f9e94e1 100644 --- a/code/extensions/LeftAndMainSubsites.php +++ b/code/extensions/LeftAndMainSubsites.php @@ -6,6 +6,7 @@ use SilverStripe\Admin\AdminRootController; use SilverStripe\Admin\CMSMenu; use SilverStripe\Admin\LeftAndMainExtension; use SilverStripe\CMS\Model\SiteTree; +use SilverStripe\CMS\Controllers\CMSPageEditController; use SilverStripe\Control\Controller; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Convert; @@ -266,22 +267,24 @@ class LeftAndMainSubsites extends LeftAndMainExtension // Catch forced subsite changes that need to cause CMS reloads. if ($request->getVar('SubsiteID') !== null) { // Clear current page when subsite changes (or is set for the first time) - if ($state->getSessionWasChanged()) { + if ($state->getSubsiteIdWasChanged()) { // sessionNamespace() is protected - see for info $override = $this->owner->config()->get('session_namespace'); $sessionNamespace = $override ? $override : get_class($this->owner); $session->clear($sessionNamespace . '.currentPage'); } - // Subsite ID has already been set to the state via InitStateMiddleware - if ($this->owner->canView()) { - // Redirect to clear the current page, retaining the current URL parameters - return $this->owner->redirect( - Controller::join_links($this->owner->Link(), ...array_values($this->owner->getURLParams())) - ); + // Subsite ID has already been set to the state via InitStateMiddleware. If the user cannot view + // the current page, or we're in the context of editing a specific page, redirect to the admin landing + // section to prevent a loop of re-loading the original subsite for the current page. + if (!$this->owner->canView() || Controller::curr() instanceof CMSPageEditController) { + return $this->owner->redirect(AdminRootController::config()->get('url_base') . '/'); } - // Redirect to the default CMS section - return $this->owner->redirect(AdminRootController::config()->get('url_base') . '/'); + + // Redirect to clear the current page, retaining the current URL parameters + return $this->owner->redirect( + Controller::join_links($this->owner->Link(), ...array_values($this->owner->getURLParams())) + ); } // Automatically redirect the session to appropriate subsite when requesting a record. diff --git a/tests/php/State/SubsiteStateTest.php b/tests/php/State/SubsiteStateTest.php new file mode 100644 index 0000000..8255343 --- /dev/null +++ b/tests/php/State/SubsiteStateTest.php @@ -0,0 +1,50 @@ +assertInstanceOf(SubsiteState::class, Injector::inst()->get(SubsiteState::class)); + } + + public function testGetSubsiteIdWasChanged() + { + $state = new SubsiteState; + + // Initial set, doesn't count as being changed + $state->setSubsiteId(123); + $this->assertFalse($state->getSubsiteIdWasChanged()); + + // Subsequent set with the same value, doesn't count as being changed + $state->setSubsiteId(123); + $this->assertFalse($state->getSubsiteIdWasChanged()); + + // Subsequent set with new value, counts as changed + $state->setSubsiteId(234); + $this->assertTrue($state->getSubsiteIdWasChanged()); + } + + public function testWithState() + { + $state = new SubsiteState; + $state->setSubsiteId(123); + + $state->withState(function ($newState) use ($state) { + $this->assertInstanceOf(SubsiteState::class, $newState); + + $this->assertNotSame($newState, $state); + + $newState->setSubsiteId(234); + $this->assertSame(234, $newState->getSubsiteId()); + $this->assertSame(123, $state->getSubsiteId()); + }); + + $this->assertSame(123, $state->getSubsiteId()); + } +}