From eb6b6a29c936246367ed53dab8c08ab771a62b7d Mon Sep 17 00:00:00 2001 From: Mojmir Fendek Date: Tue, 10 Oct 2023 14:02:27 +1300 Subject: [PATCH] BUG: Incomplete data fix (missing some versioned history records). --- src/ActivityEntry.php | 21 +++++---- src/Snapshot.php | 13 +++--- src/SnapshotItem.php | 3 -- src/SnapshotPublishable.php | 11 +++-- tests/IncompleteDataTest.php | 86 ++++++++++++++++++++++++++++++++++++ tests/IncompleteDataTest.yml | 7 +++ 6 files changed, 120 insertions(+), 21 deletions(-) create mode 100644 tests/IncompleteDataTest.php create mode 100644 tests/IncompleteDataTest.yml diff --git a/src/ActivityEntry.php b/src/ActivityEntry.php index cb0aeac..fef45f3 100644 --- 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 index 7b23fd9..671ef1f 100644 --- a/src/Snapshot.php +++ b/src/Snapshot.php @@ -13,8 +13,6 @@ use SilverStripe\Versioned\Versioned; /** - * Class Snapshot - * * @property string $OriginHash * @property int $OriginID * @property string $OriginClass @@ -22,7 +20,6 @@ * @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 index eb75e9e..586c2fb 100644 --- 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 index 1d83dc1..ef68448 100644 --- 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 new file mode 100644 index 0000000..1b6fbe7 --- /dev/null +++ b/tests/IncompleteDataTest.php @@ -0,0 +1,86 @@ +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 new file mode 100644 index 0000000..749ad0a --- /dev/null +++ b/tests/IncompleteDataTest.yml @@ -0,0 +1,7 @@ +Page: + page1: + Title: 'Page1' + URLSegment: 'page1' + page2: + Title: 'Page2' + URLSegment: 'page2'