Add support for non-staged Versioned DataObjects

Author: chrispenny

src/Extensions/CacheKeyExtension.php

@@ -17,7 +17,7 @@
 use Terraformers\KeysForCache\State\StagingState;
 
 /**
- * @property DataObject|$this $owner
+ * @property DataObject|Versioned|$this $owner
  * @method HasManyList|CacheKey CacheKeys()
  */
 class CacheKeyExtension extends DataExtension
@@ -72,16 +72,18 @@ public function getCacheKey(): ?string
     public function onAfterWrite(): void
     {
         // We will want to publish changes to the CacheKey onAfterWrite if the instance triggering this event is *not*
-        // Versioned (the changes should be seen immediately even though the object wasn't Published)
-        $publishUpdates = !$this->owner->hasExtension(Versioned::class);
+        // a Staged Versioned DataObject (the changes should be seen immediately even though the object wasn't
+        // Published)
+        $publishUpdates = !$this->ownerHasStages();
         $this->owner->triggerCacheEvent($publishUpdates);
     }
 
     public function onAfterDelete(): void
     {
         // We will want to publish changes to the CacheKey onAfterWrite if the instance triggering this event is *not*
-        // Versioned (the changes should be seen immediately even though the object wasn't Published)
-        $publishUpdates = !$this->owner->hasExtension(Versioned::class);
+        // a Staged Versioned DataObject (the changes should be seen immediately even though the object wasn't
+        // Published)
+        $publishUpdates = !$this->ownerHasStages();
         // Note: doArchive will call deleteFromStage() which will in turn trigger this extension hook
         $this->owner->triggerCacheEvent($publishUpdates);
         CacheKey::remove($this->owner);
@@ -194,4 +196,19 @@ private function findCacheKeyHash(): ?string
 
         return $cacheKey->KeyHash;
     }
+
+    private function ownerHasStages(): bool
+    {
+        // This DataObject does not have the Versioned extension, so it definitely doesn't have stages (IE: Draft and
+        // Live versions)
+        if (!$this->owner->hasExtension(Versioned::class)) {
+            return false;
+        }
+
+        // The Versioned extensions has two modes. The one that we're (probably) all familiar with, where we have a
+        // Draft and Live version, but there is also a mode that does not have stages, and instead only has _Versions.
+        // If this DataObject does not have stages, then we're going to want to treat this the same as a non-Versioned
+        // DataObject
+        return $this->owner->hasStages();
+    }
 }

src/Extensions/GridFieldOrderableRowsExtension.php

@@ -14,25 +14,27 @@
 use SilverStripe\Versioned\Versioned;
 
 /**
- * You do not need this Extension for Versioned DataObject. They are supported out of the box because
- * GridFieldOrderableRows already performs a write() through the ORM when sorting Versioned DataObjects.
+ * You do not need this Extension if you are using `symbiote/silverstripe-gridfieldextensions` version `3.5.0` or newer.
+ * Full support for Versioned and non-Versioned DataObjects is out of the box.
+ *
+ * You also do not need this Extension for Versioned DataObject. They are supported out of the box because
+ * GridFieldOrderableRows already performs write() through the ORM when sorting Versioned DataObjects.
  *
  * WARNING: We absolutely plan to remove this extension once GridFieldOrderableRows supports sorting on non-Versioned
- * DataObjects. If you need it, probably best to copy/paste it to your project, and we empower you to own it from that
+ * DataObjects. If you need it, probably best to copy/paste this to your project, and we empower you to own it from that
  * point forward.
  *
  * This Extension is *not* automatically applied because I think you should seriously consider Versioning your
  * DataObject. If you are adding this DataObject to (something like) an Element, which *is* Versioned, then (imo) it is
- * best that all of the related DataObjects (like its "Items") are also Versioned. This gives a consistent author
+ * best that all the related DataObjects (like its "Items") are also Versioned. This gives a consistent author
  * experience - where they can have draft/live versions of things. You can then also rely on the existing support from
  * GridFieldOrderableRows.
  *
- * There is an open ticket on the GridFieldExtensions module to try and get GridFieldOrderableRows to use the ORM for
- * both Versioned and non-Versioned DataObjects:
+ * There is a closed ticket on the GridFieldExtensions module that explains the issue (now fixed in v3.5.0):
  * https://github.com/symbiote/silverstripe-gridfieldextensions/issues/335
  *
- * In the meantime though, this Extension provides you a way to support the clearing of CacheKeys on non-Versioned
- * DataObjects when you are using the GridFieldOrderableRows component.
+ * For folks on a version lower than 3.5.0, this Extension provides you a way to support the clearing of CacheKeys on
+ * non-Versioned DataObjects when you are using the GridFieldOrderableRows component.
  *
  * This Extension also doesn't have any test coverage (because of everything we mentioned above). It has only gone
  * through manual testing. Use at your own risk and be prepared to submit tickets if you find any issues or use cases

tests/Scenarios/CaresTest.php

@@ -157,32 +157,45 @@ public function testCaresHasOneNonVersioned(string $readingMode): void
         });
     }
 
-    public function testCaresHasOneVersionedNonStaged(): void
+    /**
+     * @dataProvider readingModes
+     */
+    public function testCaresHasOneVersionedNonStaged(string $readingMode): void
     {
-        // Updates are processed as part of scaffold, so we need to flush before we kick off
-        ProcessedUpdatesService::singleton()->flush();
-
         $page = $this->objFromFixture(CaresPage::class, 'page1');
         $model = $this->objFromFixture(CaredHasOneVersionedNonStaged::class, 'model1');
 
+        // Make sure our page is published (the model is not Versioned)
+        $page->publishRecursive();
+
         // Check that we're set up correctly
         $this->assertEquals(CaredHasOneVersionedNonStaged::class, $model->ClassName);
         $this->assertEquals($page->CaredHasOneVersionedNonStagedID, $model->ID);
 
-        $originalKey = $page->getCacheKey();
+        Versioned::withVersionedMode(function () use ($page, $model, $readingMode): void {
+            // We perform our save method and read in the same reading mode
+            Versioned::set_stage($readingMode);
 
-        $this->assertNotNull($originalKey);
-        $this->assertNotEmpty($originalKey);
+            // Specifically fetching this way to make sure it's us fetching without any generation of KeyHash
+            $originalKey = CacheKey::findInStage($page);
 
-        // Begin changes
-        $model->forceChange();
-        $model->write();
+            $this->assertNotNull($originalKey);
+            $this->assertNotEmpty($originalKey);
+
+            // Flush updates so that new changes generate new CacheKey hashes
+            ProcessedUpdatesService::singleton()->flush();
 
-        $newKey = $page->getCacheKey();
+            // Begin changes
+            $model->forceChange();
+            $model->write();
 
-        $this->assertNotNull($newKey);
-        $this->assertNotEmpty($newKey);
-        $this->assertNotEquals($originalKey, $newKey);
+            // Specifically fetching this way to make sure it's us fetching without any generation of KeyHash
+            $newKey = CacheKey::findInStage($page);
+
+            $this->assertNotNull($newKey);
+            $this->assertNotEmpty($newKey);
+            $this->assertNotEquals($originalKey, $newKey);
+        });
     }
 
     /**
@@ -327,6 +340,14 @@ function () use ($page, $model, $readingMode, $saveMethod, $expectKeyChange): vo
         );
     }
 
+    public function readingModes(): array
+    {
+        return [
+            [Versioned::DRAFT],
+            [Versioned::LIVE],
+        ];
+    }
+
     public function readingModesWithSaveMethods(): array
     {
         return [
@@ -345,14 +366,6 @@ public function readingModesWithSaveMethods(): array
         ];
     }
 
-    public function readingModes(): array
-    {
-        return [
-            [Versioned::DRAFT],
-            [Versioned::LIVE],
-        ];
-    }
-
     protected function tearDown(): void
     {
         Injector::inst()->get(Graph::CACHE_KEY)->clear();