Restoring a deleted page causes the version number to increment twice

[mikenz]

With the latest Versioned changes when a deleted page in CMS is restored there is a gap in the version numbering.

[cms]$ git diff -w tests/php/Model/SiteTreeTest.php
diff --git a/tests/php/Model/SiteTreeTest.php b/tests/php/Model/SiteTreeTest.php
index e69d937c..86a9f3ee 100644
--- a/tests/php/Model/SiteTreeTest.php
+++ b/tests/php/Model/SiteTreeTest.php
@@ -343,13 +343,18 @@ class SiteTreeTest extends SapphireTest
     {
         $page = $this->objFromFixture('Page', 'about');
         $pageID = $page->ID;
+        $this->assertEquals(1, Versioned::get_versionnumber_by_stage(Page::class, Versioned::DRAFT, $pageID));
         $page->delete();
         $this->assertTrue(!DataObject::get_by_id("Page", $pageID));

         $deletedPage = Versioned::get_latest_version(SiteTree::class, $pageID);
+        $this->assertEquals(1, $deletedPage->Version);
+
         $resultPage = $deletedPage->doRestoreToStage();
+        $this->assertEquals(2, $resultPage->Version);

         $requeriedPage = DataObject::get_by_id("Page", $pageID);
+        $this->assertEquals(2, $requeriedPage->Version);

         $this->assertEquals($pageID, $resultPage->ID);
         $this->assertEquals($pageID, $requeriedPage->ID);

fails with

1) SilverStripe\CMS\Tests\Model\SiteTreeTest::testRestoreToStage
Failed asserting that '3' matches expected 2.

Both resultPage and requeriedPage Version is 3, not the expected 2. There is no version 2

[tractorcow]

Yes this is intentional. Version history tracking now tracks not only changed pages, but also changed states, including being published / on draft. Restoring a page from history now results in a new state, and thus a new version number being assigned.

You'll find that version 2 is tracked as the "deleted" state, which is unqueryable, but exists in the _Versioned table as a placeholder audit log for "this is when we deleted this record from draft".

Is that particular row present in your _Versions table?

[tractorcow]

I'm closing this for now; Happy to discuss this further if you want to understand the updated model a bit more. :)

[mikenz]

Hmm... I would have expected this as a major change then, not a point release.

[tractorcow]

We'll do major versions for breakages in api, but we don't strictly treat behaviour itself as an API.