API Add getSubsiteIdWasChanged calculated dynamically

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', $state->getSubsiteId());
-                $request->getSession()->set('SubsiteID', $subsiteId);
-
-                $state->setSessionWasChanged($subsiteId === $originalSubsiteId);
             }
         }
     }

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;
+        return $this->originalSubsiteId !== $this->getSubsiteId();
-    }
-
-    public function setSessionWasChanged($changed = true)
-    {
-        $this->sessionWasChanged = (bool) $changed;
-
-        return $this;
     }
 
     /**

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
+            // Subsite ID has already been set to the state via InitStateMiddleware. If the user cannot view
-            if ($this->owner->canView()) {
+            // the current page, or we're in the context of editing a specific page, redirect to the admin landing
-                // Redirect to clear the current page, retaining the current URL parameters
+            // section to prevent a loop of re-loading the original subsite for the current page.
-                return $this->owner->redirect(
+            if (!$this->owner->canView() || Controller::curr() instanceof CMSPageEditController) {
-                    Controller::join_links($this->owner->Link(), ...array_values($this->owner->getURLParams()))
+                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.

tests/php/State/SubsiteStateTest.php

@@ -0,0 +1,50 @@
+<?php
+
+namespace SilverStripe\Subsites\Tests\State;
+
+use SilverStripe\Core\Injector\Injector;
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\Subsites\State\SubsiteState;
+
+class SubsiteStateTest extends SapphireTest
+{
+    public function testStateIsInjectable()
+    {
+        $this->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());
+    }
+}