API Don’t unpublish owners when unpublishing children

API Introduce new hasPublishedOwners() method to toggle warnings prior to unpublish
Fixes #25

src/Versioned.php

@@ -2,31 +2,31 @@
 
 namespace SilverStripe\Versioned;
 
+use InvalidArgumentException;
+use LogicException;
 use SilverStripe\Control\Controller;
-use SilverStripe\Control\HTTPRequest;
-use SilverStripe\Control\Session;
-use SilverStripe\Control\Director;
 use SilverStripe\Control\Cookie;
-use SilverStripe\Core\Config\Config;
+use SilverStripe\Control\Director;
+use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Core\ClassInfo;
+use SilverStripe\Core\Config\Config;
+use SilverStripe\Core\Extension;
 use SilverStripe\Core\Resettable;
 use SilverStripe\Dev\Deprecation;
 use SilverStripe\Forms\FieldList;
-use SilverStripe\ORM\DataQuery;
-use SilverStripe\ORM\DataObject;
-use SilverStripe\ORM\DB;
 use SilverStripe\ORM\ArrayList;
-use SilverStripe\ORM\DataList;
 use SilverStripe\ORM\DataExtension;
+use SilverStripe\ORM\DataList;
+use SilverStripe\ORM\DataObject;
+use SilverStripe\ORM\DataQuery;
+use SilverStripe\ORM\DB;
 use SilverStripe\ORM\FieldType\DBDatetime;
 use SilverStripe\ORM\Queries\SQLSelect;
 use SilverStripe\ORM\Queries\SQLUpdate;
 use SilverStripe\Security\Member;
 use SilverStripe\Security\Permission;
 use SilverStripe\Security\Security;
 use SilverStripe\View\TemplateGlobalProvider;
-use InvalidArgumentException;
-use LogicException;
 
 /**
  * The Versioned extension allows your DataObjects to have several versions,
@@ -1102,7 +1102,7 @@ protected function lookupReverseOwners()
     /**
      * Find objects in the given relationships, merging them into the given list
      *
-     * @param array $source Config property to extract relationships from
+     * @param string $source Config property to extract relationships from
      * @param bool $recursive True if recursive
      * @param ArrayList $list Optional list to add items to
      * @return ArrayList The list
@@ -1461,6 +1461,7 @@ public function extendWithSuffix($table)
         if (count($versionableExtensions)) {
             foreach ($versionableExtensions as $versionableExtension => $suffixes) {
                 if ($owner->hasExtension($versionableExtension)) {
+                    /** @var VersionableExtension|Extension $ext */
                     $ext = $owner->getExtensionInstance($versionableExtension);
                     $ext->setOwner($owner);
                     $table = $ext->extendWithSuffix($table);
@@ -1635,6 +1636,11 @@ public function doArchive()
         return true;
     }
 
+    /**
+     * Remove this item from any changesets
+     *
+     * @return bool
+     */
     public function deleteFromChangeSets()
     {
         $owner = $this->owner;
@@ -1650,6 +1656,7 @@ public function deleteFromChangeSets()
         ChangeSetItem::get()
             ->filter(['ObjectID' => $ids])
             ->removeAll();
+        return true;
     }
 
     /**
@@ -1690,20 +1697,25 @@ public function doUnpublish()
     }
 
     /**
-     * Trigger unpublish of owning objects
+     * Determine if this object is published, and has any published owners.
+     * If this is true, a warning should be shown before this is published.
+     *
+     * Note: This method returns false if the object itself is unpublished,
+     * since owners are only considered on the same stage as the record itself.
+     *
+     * @return bool
      */
-    public function onAfterUnpublish()
+    public function hasPublishedOwners()
     {
-        $owner = $this->owner;
-
-        // Any objects which owned (and thus relied on the unpublished object) are now unpublished automatically.
-        foreach ($owner->findOwners(false) as $object) {
-            /** @var Versioned|DataObject $object */
-            $object->doUnpublish();
+        if (!$this->isPublished()) {
+            return false;
         }
+        // Count live owners
+        /** @var Versioned|DataObject $liveRecord */
+        $liveRecord = static::get_by_stage(get_class($this->owner), Versioned::LIVE)->byID($this->owner->ID);
+        return $liveRecord->findOwners(false)->count() > 0;
     }
 
-
     /**
      * Revert the draft changes: replace the draft content with the content on live
      *
@@ -2631,7 +2643,6 @@ public function hasStages()
      * @param ArrayList $list Global list. Object will not be added if already added to this list.
      * @param ArrayList $added Additional list to insert into
      * @param DataObject $item Item to add
-     * @return mixed
      */
     protected function mergeRelatedObject($list, $added, $item)
     {

tests/php/VersionedOwnershipTest.php

@@ -354,7 +354,7 @@ public function testRecursivePublish()
     }
 
     /**
-     * Test that owning objects get unpublished as needed
+     * Test that owning objects don't get unpublished when object is unpublished
      */
     public function testRecursiveUnpublish()
     {
@@ -367,47 +367,48 @@ public function testRecursiveUnpublish()
         $banner3Unpublished = $this->objFromFixture(VersionedOwnershipTest\RelatedMany::class, 'relatedmany3');
         $this->assertFalse($banner3Unpublished->doUnpublish());
 
-        // First test: mid-level unpublish; We expect that owners should be unpublished, but not
-        // owned objects, nor other siblings shared by the same owner.
+        // First test: mid-level unpublish; no other objects are unpublished
+        /** @var VersionedOwnershipTest\Related $related2 */
         $related2 = $this->objFromFixture(VersionedOwnershipTest\Related::class, 'related2_published');
         /** @var VersionedOwnershipTest\Attachment $attachment3 */
         $attachment3 = $this->objFromFixture(VersionedOwnershipTest\Attachment::class, 'attachment3_published');
         /** @var VersionedOwnershipTest\RelatedMany $relatedMany4 */
         $relatedMany4 = $this->objFromFixture(VersionedOwnershipTest\RelatedMany::class, 'relatedmany4_published');
+
+        // Ensure that this object, and it's owned objects, are aware of published parents
+        $this->assertTrue($attachment3->hasPublishedOwners());
+        $this->assertTrue($related2->hasPublishedOwners());
+
         /** @var VersionedOwnershipTest\Related $related2 */
         $this->assertTrue($related2->doUnpublish());
+        /** @var VersionedOwnershipTest\Subclass $subclass2 */
         $subclass2 = $this->objFromFixture(VersionedOwnershipTest\Subclass::class, 'subclass2_published');
 
+        // After unpublish this should change
+        $this->assertFalse($attachment3->hasPublishedOwners()); // Because owner is unpublished
+        $this->assertFalse($related2->hasPublishedOwners()); // Because self is unpublished
+        $this->assertFalse($subclass2->hasPublishedOwners()); // Because no owners
+
         /** @var VersionedOwnershipTest\Subclass $subclass2 */
-        $this->assertFalse($subclass2->isPublished()); // Owner IS unpublished
+        $this->assertTrue($subclass2->isPublished()); // Owner is not unpublished
         $this->assertTrue($attachment3->isPublished()); // Owned object is NOT unpublished
         $this->assertTrue($relatedMany4->isPublished()); // Owned object by owner is NOT unpublished
 
-        // Second test: multi-level unpublish should recursively cascade down all owning objects
-        // Publish related2 again
+        // Second test: Re-publishing the owner should re-publish this item
         $subclass2->publishRecursive();
         $this->assertTrue($subclass2->isPublished());
         $this->assertTrue($related2->isPublished());
         $this->assertTrue($attachment3->isPublished());
-
-        // Unpublish leaf node
-        $this->assertTrue($attachment3->doUnpublish());
-
-        // Now all owning objects (only) are unpublished
-        $this->assertFalse($attachment3->isPublished()); // Unpublished because we just unpublished it
-        $this->assertFalse($related2->isPublished()); // Unpublished because it owns attachment3
-        $this->assertFalse($subclass2->isPublished()); // Unpublished ecause it owns related2
-        $this->assertTrue($relatedMany4->isPublished()); // Stays live because recursion only affects owners not owned.
     }
 
     public function testRecursiveArchive()
     {
-        // When archiving an object, any published owners should be unpublished at the same time
-        // but NOT achived
+        // When archiving an object owners are not affected
 
         /** @var VersionedOwnershipTest\Attachment $attachment3 */
         $attachment3 = $this->objFromFixture(VersionedOwnershipTest\Attachment::class, 'attachment3_published');
         $attachment3ID = $attachment3->ID;
+        $this->assertTrue($attachment3->hasPublishedOwners()); // Warning should be shown
         $this->assertTrue($attachment3->doArchive());
 
         // This object is on neither stage nor live
@@ -418,17 +419,17 @@ public function testRecursiveArchive()
         $this->assertEmpty($stageAttachment);
         $this->assertEmpty($liveAttachment);
 
-        // Owning object is unpublished only
+        // Owning object is not unpublished or archived
         /** @var VersionedOwnershipTest\Related $stageOwner */
         $stageOwner = $this->objFromFixture(VersionedOwnershipTest\Related::class, 'related2_published');
         $this->assertTrue($stageOwner->isOnDraft());
-        $this->assertFalse($stageOwner->isPublished());
+        $this->assertTrue($stageOwner->isPublished());
 
-        // Bottom level owning object is also unpublished
+        // Bottom level owning object is also unaffected
         /** @var VersionedOwnershipTest\Subclass $stageTopOwner */
         $stageTopOwner = $this->objFromFixture(VersionedOwnershipTest\Subclass::class, 'subclass2_published');
         $this->assertTrue($stageTopOwner->isOnDraft());
-        $this->assertFalse($stageTopOwner->isPublished());
+        $this->assertTrue($stageTopOwner->isPublished());
     }
 
     public function testRecursiveRevertToLive()

tests/php/VersionedOwnershipTest/Subclass.php

@@ -3,7 +3,6 @@
 namespace SilverStripe\Versioned\Tests\VersionedOwnershipTest;
 
 use SilverStripe\Dev\TestOnly;
-use SilverStripe\Versioned\Tests\VersionedOwnershipTest;
 
 /**
  * Object which: