BUG: Incomplete data fix (missing some versioned history records).

silverstripe · Oct 10, 2023 · 2337a0e · 2337a0e
1 parent a56953c
commit 2337a0e
Show file tree

Hide file tree

Showing 6 changed files with 120 additions and 21 deletions.
diff --git a/src/ActivityEntry.php b/src/ActivityEntry.php
@@ -4,11 +4,15 @@
 
 use Exception;
 use SilverStripe\ORM\DataObject;
+use SilverStripe\ORM\FieldType\DBDatetime;
 use SilverStripe\Versioned\Versioned;
 use SilverStripe\View\ArrayData;
 
 /**
- * @property DataObject $Subject
+ * @property DataObject|null $Subject
+ * @property string|null $Action
+ * @property string|null $Owner
+ * @property string $Date
  */
 class ActivityEntry extends ArrayData
 {
@@ -26,7 +30,7 @@ class ActivityEntry extends ArrayData
 
     const UNPUBLISHED = 'UNPUBLISHED';
 
-    public static function createFromSnapshotItem(SnapshotItem $item): self
+    public function createFromSnapshotItem(SnapshotItem $item): ?self
     {
         /** @var DataObject|Versioned $itemObj */
         $itemObj = $item->getItem();
@@ -75,18 +79,19 @@ public static function createFromSnapshotItem(SnapshotItem $item): self
         }
 
         if (!$itemObj) {
-            throw new Exception(sprintf(
-                'Could not resolve SnapshotItem %s to a previous %s version',
-                $item->ID,
-                $item->ObjectClass
-            ));
+            // We couldn't compose the activity entry from available versioned history records
+            // This is a valid case as some projects opt to delete some of their older versioned records
+            return null;
         }
 
+        /** @var DBDatetime $createdField */
+        $createdField = $item->obj('Created');
+
         return static::create([
             'Subject' => $itemObj,
             'Action' => $flag,
             'Owner' => null,
-            'Date' => $item->obj('Created')->Nice(),
+            'Date' => $createdField->Nice(),
         ]);
     }
 

diff --git a/src/Snapshot.php b/src/Snapshot.php
@@ -13,16 +13,13 @@
 use SilverStripe\Versioned\Versioned;
 
 /**
- * Class Snapshot
- *
  * @property string $OriginHash
  * @property int $OriginID
  * @property string $OriginClass
  * @property int AuthorID
  * @method DataObject Origin()
  * @method Member Author()
  * @method HasManyList|SnapshotItem[] Items()
- * @package SilverStripe\Snapshots
  */
 class Snapshot extends DataObject
 {
@@ -177,9 +174,10 @@ public function getActivityDescription(): string
     {
         /** @var SnapshotItem $item */
         $item = $this->getOriginItem();
+        $entry = ActivityEntry::singleton()->createFromSnapshotItem($item);
 
-        return $item
-            ? ActivityEntry::createFromSnapshotItem($item)->getDescription()
+        return $entry
+            ? $entry->getDescription()
             : _t(self::class . 'ACTIVITY_NONE', 'none');
     }
 
@@ -227,9 +225,10 @@ public function getActivityType(): ?string
     {
         /** @var SnapshotItem $item */
         $item = $this->getOriginItem();
+        $entry = ActivityEntry::singleton()->createFromSnapshotItem($item);
 
-        return $item
-            ? ActivityEntry::createFromSnapshotItem($item)->Action
+        return $entry
+            ? $entry->Action
             : null;
     }
 

diff --git a/src/SnapshotItem.php b/src/SnapshotItem.php
@@ -12,8 +12,6 @@
 use SilverStripe\Versioned\Versioned;
 
 /**
- * Class SnapshotItem
- *
  * @property int $ObjectVersion
  * @property int $WasPublished
  * @property int $WasUnpublished
@@ -29,7 +27,6 @@
  * @method DataObject Object()
  * @method SnapshotItem Parent()
  * @method HasManyList|SnapshotItem[] Children()
- * @package SilverStripe\Snapshots
  */
 class SnapshotItem extends DataObject
 {

diff --git a/src/SnapshotPublishable.php b/src/SnapshotPublishable.php
@@ -17,8 +17,6 @@
 use SilverStripe\Versioned\Versioned;
 
 /**
- * Class SnapshotPublishable
- *
  * @property DataObject|SnapshotPublishable|Versioned $owner
  */
 class SnapshotPublishable extends RecursivePublishable
@@ -754,7 +752,14 @@ public function getActivityFeed(?int $minVersion = null, ?int $maxVersion = null
         $list = ArrayList::create();
 
         foreach ($items as $item) {
-            $list->push(ActivityEntry::createFromSnapshotItem($item));
+            $entry = ActivityEntry::singleton()->createFromSnapshotItem($item);
+
+            if (!$entry) {
+                // We couldn't compose an activity entry from the item, so we will skip this entry
+                continue;
+            }
+
+            $list->push($entry);
         }
 
         return $list;

diff --git a/tests/IncompleteDataTest.php b/tests/IncompleteDataTest.php
@@ -0,0 +1,86 @@
+<?php
+
+namespace SilverStripe\Snapshots\Tests;
+
+use Exception;
+use Page;
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\ORM\FieldType\DBDatetime;
+use SilverStripe\ORM\Queries\SQLDelete;
+use SilverStripe\ORM\ValidationException;
+use SilverStripe\Snapshots\Snapshot;
+use SilverStripe\Snapshots\SnapshotPublishable;
+
+class IncompleteDataTest extends SapphireTest
+{
+    /**
+     * @var string
+     */
+    protected static $fixture_file = 'IncompleteDataTest.yml';
+
+    /**
+     * @return void
+     * @throws Exception
+     */
+    protected function setUp(): void
+    {
+        DBDatetime::set_mock_now('2020-01-01 00:00:00');
+
+        parent::setUp();
+    }
+
+    /**
+     * @return void
+     * @throws ValidationException
+     * @throws Exception
+     */
+    public function testGetActivityFeed(): void
+    {
+        /** @var Page|SnapshotPublishable $page1 */
+        $page1 = $this->objFromFixture(Page::class, 'page1');
+
+        /** @var Page|SnapshotPublishable $page2 */
+        $page2 = $this->objFromFixture(Page::class, 'page2');
+
+        $actions = [
+            '2020-01-02 00:00:00' => [
+                $page1,
+                [
+                    $page2,
+                ],
+            ],
+            '2020-01-03 00:00:00' => [
+                $page2,
+                [
+                    $page1,
+                ],
+            ],
+            '2020-01-04 00:00:00' => [
+                $page1,
+                [
+                    $page2,
+                ],
+            ],
+        ];
+
+        // Create some mock actions
+        foreach ($actions as $time => $data) {
+            [$origin, $models] = $data;
+            DBDatetime::set_mock_now($time);
+            $snapshot = Snapshot::singleton()->createSnapshot($origin, $models);
+            $snapshot->write();
+        }
+
+        // Remove all traces of page 2 to create an incomplete data set
+        $page2->doArchive();
+        $query = SQLDelete::create(
+            '"SiteTree_Versions"',
+            [
+                '"RecordID"' => $page2->ID,
+            ]
+        );
+        $query->execute();
+
+        $this->assertCount(2, $page1->getActivityFeed(), 'We expect only entries which have complete data');
+    }
+}
diff --git a/tests/IncompleteDataTest.yml b/tests/IncompleteDataTest.yml
@@ -0,0 +1,7 @@
+Page:
+  page1:
+    Title: 'Page1'
+    URLSegment: 'page1'
+  page2:
+    Title: 'Page2'
+    URLSegment: 'page2'