diff --git a/src/Extensions/CacheKeyExtension.php b/src/Extensions/CacheKeyExtension.php index af6d28e..832265a 100644 --- a/src/Extensions/CacheKeyExtension.php +++ b/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(); + } } diff --git a/src/Extensions/GridFieldOrderableRowsExtension.php b/src/Extensions/GridFieldOrderableRowsExtension.php index f1ca1f2..ffb890e 100644 --- a/src/Extensions/GridFieldOrderableRowsExtension.php +++ b/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 diff --git a/tests/Scenarios/CaresTest.php b/tests/Scenarios/CaresTest.php index fc7cd7d..c91a5b6 100644 --- a/tests/Scenarios/CaresTest.php +++ b/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();