API ChangeSet::publish() / canPublish() no longer treats hasChanges()…

… = false as a permission error BUG fix issues with doArchive() in live mode
silverstripe · Dec 21, 2016 · 9be5142 · 9be5142
1 parent 603b64c
commit 9be5142
Show file tree

Hide file tree

Showing 7 changed files with 147 additions and 119 deletions.
diff --git a/admin/code/CampaignAdmin.php b/admin/code/CampaignAdmin.php
@@ -194,7 +194,7 @@ protected function getChangeSetResource(ChangeSet $changeSet)
         try {
             $changeSet->sync();
             $hal['Description'] = $changeSet->getDescription();
-            $hal['canPublish'] = $changeSet->canPublish();
+            $hal['canPublish'] = $changeSet->canPublish() && $changeSet->hasChanges();
 
             foreach ($changeSet->Changes() as $changeSetItem) {
                 if (!$changeSetItem) {

diff --git a/src/ORM/Versioning/ChangeSet.php b/src/ORM/Versioning/ChangeSet.php
@@ -152,6 +152,10 @@ public function addObject(DataObject $object)
             throw new BadMethodCallException("ChangeSet must be saved before adding items");
         }
 
+        if (!$object->isInDB()) {
+            throw new BadMethodCallException("Items must be saved before adding to a changeset");
+        }
+
         $references = [
             'ObjectID'    => $object->ID,
             'ObjectClass' => $object->baseClass(),
@@ -353,17 +357,30 @@ public function canDelete($member = null)
      */
     public function canPublish($member = null)
     {
-        // All changes must be publishable
-        $atLeastOneChange = false;
         foreach ($this->Changes() as $change) {
-            $atLeastOneChange = true;
             /** @var ChangeSetItem $change */
             if (!$change->canPublish($member)) {
                 return false;
             }
         }
+        return true;
+    }
 
-        return $atLeastOneChange;
+    /**
+     * Determine if there are changes to publish
+     *
+     * @return bool
+     */
+    public function hasChanges()
+    {
+        // All changes must be publishable
+        /** @var ChangeSetItem $change */
+        foreach ($this->Changes() as $change) {
+            if ($change->hasChange()) {
+                return true;
+            }
+        }
+        return false;
     }
 
     /**

diff --git a/src/ORM/Versioning/ChangeSetItem.php b/src/ORM/Versioning/ChangeSetItem.php
@@ -360,7 +360,17 @@ public function canPublish($member = null)
             }
         }
 
-        return false;
+        return true;
+    }
+
+    /**
+     * Determine if this item has changes
+     *
+     * @return bool
+     */
+    public function hasChange()
+    {
+        return $this->getChangeType() !== ChangeSetItem::CHANGE_NONE;
     }
 
     /**

diff --git a/src/ORM/Versioning/Versioned.php b/src/ORM/Versioning/Versioned.php
@@ -1612,7 +1612,7 @@ public function doArchive()
 
         $owner->invokeWithExtensions('onBeforeArchive', $this);
         $owner->doUnpublish();
-        $owner->delete();
+        $owner->deleteFromStage(static::DRAFT);
         $owner->invokeWithExtensions('onAfterArchive', $this);
 
         return true;

diff --git a/tests/php/ORM/ChangeSetTest.php b/tests/php/ORM/ChangeSetTest.php
@@ -257,6 +257,31 @@ public function testCanPublish()
         $this->assertTrue($changeSet->canPublish());
     }
 
+    public function testHasChanges()
+    {
+        // Create changeset containing all items (unpublished)
+        Versioned::set_stage(Versioned::DRAFT);
+        $this->logInWithPermission('ADMIN');
+        $changeSet = new ChangeSet();
+        $changeSet->write();
+        $base = new ChangeSetTest\BaseObject();
+        $base->Foo = 1;
+        $base->write();
+        $changeSet->addObject($base);
+
+        // New changeset with changes can be published
+        $this->assertTrue($changeSet->canPublish());
+        $this->assertTrue($changeSet->hasChanges());
+
+        // Writing the record to live dissolves the changes in this changeset
+        $base->publishSingle();
+        $this->assertTrue($changeSet->canPublish());
+        $this->assertFalse($changeSet->hasChanges());
+
+        // Changeset can be safely published without error
+        $changeSet->publish();
+    }
+
     public function testCanRevert()
     {
         $this->markTestSkipped("Requires ChangeSet::revert to be implemented first");