Skip to content

Commit

Permalink
Merge branch '2.1' into 2
Browse files Browse the repository at this point in the history
  • Loading branch information
@github-actions
github-actions[bot] committed Nov 3, 2023
2 parents 79bfcc4 + f589d10 commit 6a15edb
Show file tree
Hide file tree
Showing 8 changed files with 134 additions and 16 deletions.
18 changes: 16 additions & 2 deletions src/ChangeSet.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,23 @@ public function removeObject(DataObject $object)
])->first();

if ($item) {
// TODO: Handle case of implicit added item being removed.

$item->delete();

// Get the implicitly included items for this ChangeSet
$implicit = $this->calculateImplicit();

foreach ($this->Changes()->filter(['Added' => ChangeSetItem::IMPLICITLY]) as $changeSetItem) {
$objectKey = $this->implicitKey($changeSetItem);

// If a ChangeSetItem exists, but isn't in $implicit, it's no longer required, so delete it
if (!array_key_exists($objectKey, $implicit)) {
$changeSetItem->delete();
} else {
// Otherwise it is required, so update ReferencedBy and remove from $implicit
$changeSetItem->ReferencedBy()->setByIDList($implicit[$objectKey]['ReferencedBy']);
unset($implicit[$objectKey]);
}
}
}

$this->sync();
Expand Down
2 changes: 0 additions & 2 deletions src/DataDifferencer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,6 @@ protected function getObjectDisplay($object = null)
}

// Use image tag
// TODO Use CMSThumbnail instead to limit max size, blocked by DataDifferencerTest and GC
// not playing nice with mocked images
if ($object instanceof Image) {
return $object->getTag();
}
Expand Down
1 change: 0 additions & 1 deletion src/RecursivePublishable.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,6 @@ public function findOwners($recursive = true, $list = null)
}

// Build reverse lookup for ownership
// @todo - Cache this more intelligently
$rules = $this->lookupReverseOwners();

// Hand off to recursive method
Expand Down
1 change: 0 additions & 1 deletion src/RecursivePublishableHandler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ class RecursivePublishableHandler extends Extension
{
/**
* Ensure that non-versioned records are published on save.
* @todo Build this action into explicit UX action: https://github.com/silverstripe/silverstripe-versioned/issues/71
* @param DataObject $record
*/
public function onAfterSave(DataObject $record)
Expand Down
3 changes: 0 additions & 3 deletions src/Versioned.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2034,7 +2034,6 @@ public function Versions($filter = "", $sort = "", $limit = "", $join = "", $hav

$list = DataObject::get(DataObject::getSchema()->baseDataClass($owner), $filter, $sort, $join, $limit);
if ($having) {
// @todo - This method doesn't exist on DataList
$list->having($having);
}

Expand Down Expand Up @@ -2669,8 +2668,6 @@ public static function get_latest_version($class, $id)
/**
* Returns whether the current record is the latest one.
*
* @todo Performance - could do this directly via SQL.
*
* @see get_latest_version()
* @see latestPublished
*
Expand Down
118 changes: 113 additions & 5 deletions tests/php/ChangeSetTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ public function testRepeatedSyncIsNOP()
ChangeSetTest\BaseObject::class . '.base' => ChangeSetItem::EXPLICITLY,
ChangeSetTest\MidObject::class . '.mid1' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\MidObject::class . '.mid2' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\MidObject::class . '.mid4' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\EndObject::class . '.end1' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\EndObject::class . '.end2' => ChangeSetItem::IMPLICITLY,
]
Expand All @@ -172,6 +173,7 @@ public function testRepeatedSyncIsNOP()
ChangeSetTest\BaseObject::class . '.base' => ChangeSetItem::EXPLICITLY,
ChangeSetTest\MidObject::class . '.mid1' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\MidObject::class . '.mid2' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\MidObject::class . '.mid4' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\EndObject::class . '.end1' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\EndObject::class . '.end2' => ChangeSetItem::IMPLICITLY,
]
Expand All @@ -196,6 +198,7 @@ public function testSync()
ChangeSetTest\BaseObject::class . '.base' => ChangeSetItem::EXPLICITLY,
ChangeSetTest\MidObject::class . '.mid1' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\MidObject::class . '.mid2' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\MidObject::class . '.mid4' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\EndObject::class . '.end1' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\EndObject::class . '.end2' => ChangeSetItem::IMPLICITLY,
]
Expand All @@ -214,6 +217,7 @@ public function testSync()
ChangeSetTest\BaseObject::class . '.base' => ChangeSetItem::EXPLICITLY,
ChangeSetTest\MidObject::class . '.mid1' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\MidObject::class . '.mid2' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\MidObject::class . '.mid4' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\EndObject::class . '.end1' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\EndObject::class . '.end2' => ChangeSetItem::IMPLICITLY,
]
Expand Down Expand Up @@ -255,6 +259,7 @@ public function testSync()
ChangeSetTest\BaseObject::class . '.base' => ChangeSetItem::EXPLICITLY,
ChangeSetTest\MidObject::class . '.mid1' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\MidObject::class . '.mid2' => ChangeSetItem::EXPLICITLY,
ChangeSetTest\MidObject::class . '.mid4' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\EndObject::class . '.end1' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\EndObject::class . '.end2' => ChangeSetItem::IMPLICITLY,
]
Expand Down Expand Up @@ -301,6 +306,7 @@ public function testIsSynced()
ChangeSetTest\BaseObject::class . '.base' => ChangeSetItem::EXPLICITLY,
ChangeSetTest\MidObject::class . '.mid1' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\MidObject::class . '.mid2' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\MidObject::class . '.mid4' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\EndObject::class . '.end1' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\EndObject::class . '.end2' => ChangeSetItem::IMPLICITLY,
]
Expand All @@ -319,6 +325,7 @@ public function testIsSynced()
[
ChangeSetTest\BaseObject::class . '.base' => ChangeSetItem::EXPLICITLY,
ChangeSetTest\MidObject::class . '.mid2' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\MidObject::class . '.mid4' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\EndObject::class . '.end2' => ChangeSetItem::IMPLICITLY,
]
);
Expand All @@ -334,7 +341,7 @@ public function testCanPublish()
$base = $this->objFromFixture(ChangeSetTest\BaseObject::class, 'base');
$changeSet->addObject($base);
$changeSet->sync();
$this->assertEquals(5, $changeSet->Changes()->count());
$this->assertEquals(6, $changeSet->Changes()->count());

// Test un-authenticated user cannot publish
$this->logOut();
Expand Down Expand Up @@ -378,7 +385,7 @@ public function testCanPublishNested()
$changeSet->addObject($base);
$changeSet->addObject($mid1);
$changeSet->sync();
$this->assertEquals(5, $changeSet->Changes()->count());
$this->assertEquals(6, $changeSet->Changes()->count());

// With model publish permissions only publish is allowed
$this->assertTrue($changeSet->canPublish());
Expand Down Expand Up @@ -434,7 +441,7 @@ public function testCanEdit()
$base = $this->objFromFixture(ChangeSetTest\BaseObject::class, 'base');
$changeSet->addObject($base);
$changeSet->sync();
$this->assertEquals(5, $changeSet->Changes()->count());
$this->assertEquals(6, $changeSet->Changes()->count());

// Check canEdit
$this->logOut();
Expand Down Expand Up @@ -465,7 +472,7 @@ public function testCanDelete()
$base = $this->objFromFixture(ChangeSetTest\BaseObject::class, 'base');
$changeSet->addObject($base);
$changeSet->sync();
$this->assertEquals(5, $changeSet->Changes()->count());
$this->assertEquals(6, $changeSet->Changes()->count());

// Check canDelete
$this->logOut();
Expand All @@ -485,7 +492,7 @@ public function testCanView()
$base = $this->objFromFixture(ChangeSetTest\BaseObject::class, 'base');
$changeSet->addObject($base);
$changeSet->sync();
$this->assertEquals(5, $changeSet->Changes()->count());
$this->assertEquals(6, $changeSet->Changes()->count());

// Check canView
$this->logOut();
Expand Down Expand Up @@ -617,6 +624,7 @@ public function testUnlinkDisassociated()
[
ChangeSetTest\BaseObject::class . '.base' => ChangeSetItem::EXPLICITLY,
ChangeSetTest\MidObject::class . '.mid2' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\MidObject::class . '.mid4' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\EndObject::class . '.end2' => ChangeSetItem::IMPLICITLY,
]
);
Expand Down Expand Up @@ -735,4 +743,104 @@ public function testIsSyncedCanBeSkipped()

$this->assertFalse($changeset->isSyncCalled, 'isSynced is skipped when providing truthy argument to publish');
}

public function testRemoveObject()
{
$this->publishAllFixtures();

$mid1 = $this->objFromFixture(ChangeSetTest\MidObject::class, 'mid1'); // Item mid2 reference Item end2

$changeset = new ChangeSet();
$changeset->write();
$changeset->addObject($mid1);
$changeset->publish();

$this->assertChangeSetLooksLike(
$changeset,
[
ChangeSetTest\MidObject::class . '.mid1' => ChangeSetItem::EXPLICITLY,
ChangeSetTest\EndObject::class . '.end1' => ChangeSetItem::IMPLICITLY,
]
);

$changeset->removeObject($mid1); // Remove mid1

$this->assertChangeSetLooksLike(
$changeset,
[]
);
}

public function testRemoveObjectsWithReferencesToOneItem()
{
$this->publishAllFixtures();

$mid2 = $this->objFromFixture(ChangeSetTest\MidObject::class, 'mid2'); // Item mid2 reference Item end2
$mid3 = $this->objFromFixture(ChangeSetTest\MidObject::class, 'mid4'); // Item mid4 reference Item end2

$changeset = new ChangeSet();
$changeset->write();
$changeset->addObject($mid2);
$changeset->addObject($mid3);
$changeset->publish();

$this->assertChangeSetLooksLike(
$changeset,
[
ChangeSetTest\MidObject::class . '.mid2' => ChangeSetItem::EXPLICITLY,
ChangeSetTest\EndObject::class . '.end2' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\MidObject::class . '.mid4' => ChangeSetItem::EXPLICITLY,
ChangeSetTest\EndObject::class . '.end2' => ChangeSetItem::IMPLICITLY,
]
);

$changeset->removeObject($mid2); // Remove mid2

$this->assertChangeSetLooksLike(
$changeset,
[
ChangeSetTest\MidObject::class . '.mid4' => ChangeSetItem::EXPLICITLY,
ChangeSetTest\EndObject::class . '.end2' => ChangeSetItem::IMPLICITLY, // Item end2 is still in ChangeSet
]
);

$changeset->removeObject($mid3); // Remove mid3

$this->assertChangeSetLooksLike(
$changeset,
[]
);
}

public function testRemoveObjectWithImplicitlyAddedItemOnly()
{
$this->publishAllFixtures();

$mid1 = $this->objFromFixture(ChangeSetTest\MidObject::class, 'mid1'); // Item mid1 reference Item end1
$end1 = $this->objFromFixture(ChangeSetTest\EndObject::class, 'end1'); // Item end1

$changeset = new ChangeSet();
$changeset->write();
$changeset->addObject($mid1);
$changeset->addObject($end1); // Item end1 explicitly added to ChangeSet
$changeset->publish();

$this->assertChangeSetLooksLike(
$changeset,
[
ChangeSetTest\MidObject::class . '.mid1' => ChangeSetItem::EXPLICITLY,
ChangeSetTest\EndObject::class . '.end1' => ChangeSetItem::IMPLICITLY,
ChangeSetTest\EndObject::class . '.end1' => ChangeSetItem::EXPLICITLY
]
);

$changeset->removeObject($mid1); // Remove mid1

$this->assertChangeSetLooksLike(
$changeset,
[
ChangeSetTest\EndObject::class . '.end1' => ChangeSetItem::EXPLICITLY // Item end1 is still in ChangeSet
]
);
}
}
4 changes: 4 additions & 0 deletions tests/php/ChangeSetTest.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ SilverStripe\Versioned\Tests\ChangeSetTest\MidObject:
End: =>SilverStripe\Versioned\Tests\ChangeSetTest\EndObject.end2
mid3:
Base: =>SilverStripe\Versioned\Tests\ChangeSetTest\BaseObject.base2
mid4:
Bar: 3
Base: =>SilverStripe\Versioned\Tests\ChangeSetTest\BaseObject.base
End: =>SilverStripe\Versioned\Tests\ChangeSetTest\EndObject.end2
SilverStripe\Versioned\Tests\ChangeSetTest\UnversionedObject:
unversioned1:
Title: 'object'
Expand Down
3 changes: 1 addition & 2 deletions tests/php/DataDifferencerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,7 @@ public function testArrayValues()
$obj1v2 = Versioned::get_version(DataDifferencerTest\TestObject::class, $obj1->ID, $afterVersion);
$differ = new DataDifferencer($obj1v1, $obj1v2);
$obj1Diff = $differ->diffedData();
// TODO Using getter would split up field again, bug only caused by simulating
// an array-based value in the first place.

$this->assertContainsIgnoreWhitespace('<del>a,b</del><ins>a</ins>', $obj1Diff->getField('Choices'));
}

Expand Down

0 comments on commit 6a15edb

Please sign in to comment.