Fix versioning

Author: emteknetnz

README.md

@@ -240,7 +240,7 @@ Example:
 
 ### DELETE<a name="readme-http-delete"></a>
 
-The `DELETE` HTTP request is used to delete an existing record where ID is specified in the URL.
+The `DELETE` HTTP request is used to delete an existing record where ID is specified in the URL. If the `Versioned` extension from the `silverstripe/versioned` module has been applied to the DataObject then the `doArchive()` method is called on the DataObject which deletes it from other the draft and live versions of the site. If the `Versioned` extension has not bee applied then the `delete()` method will be called on DataObject instead.
 
 Example:
 - `curl -X DELETE https://mysite.test/api/pages/123`
@@ -258,9 +258,9 @@ Actions can only be run on existing records. The action parameter is added to th
 The following actions are available:
 | Action | Description |
 | - | - |
-| `publish` | Publish a record. Requires `silverstripe/versioned` to be installed. |
-| `unpublish` | Unpublish a record. Requires `silverstripe/versioned` to be installed. |
-| `archive` | Archive a record. Requires `silverstripe/versioned` to be installed. |
+| `publish` | Publish a record. Requires the `Versioned` extension from `silverstripe/versioned` module to be applied to the `DataObject`. |
+| `unpublish` | Unpublish a record. Requires the `Versioned` extension from `silverstripe/versioned` module to be applied to the `DataObject`. |
+| `archive` | Archive a record. Requires the `Versioned` extension from `silverstripe/versioned` module to be applied to the `DataObject`. Note this similar to using the `DELETE` HTTP method. |
 
 Example:
 - `curl -X PUT https://mysite.test/api/pages/123/publish`

src/Controllers/RestApiEndpoint.php

@@ -564,7 +564,11 @@ private function apiDelete(): HTTPResponse
         }
         $obj = $this->dataObjectFromRequest($dataClass);
         $this->invokeWithExtensions('onDeleteBeforeDelete', $obj);
-        $obj->delete();
+        if ($obj->hasExtension(Versioned::class)) {
+            $obj->doArchive();
+        } else {
+            $obj->delete();
+        }
         $this->invokeWithExtensions('onDeleteAfterDelete', $obj);
         // Will default to a 200 status code
         // A 204 status code is not used because there is stil JSON content returned in the body
@@ -597,11 +601,10 @@ private function apiAction(): HTTPResponse
      */
     private function apiVersioning(DataObject $obj, string $action): HTTPResponse
     {
-        $dataClass = $this->endpointConfig(self::DATA_CLASS, false);
         $callCanMethods = $this->endpointConfig(self::CALL_CAN_METHODS, true);
         $doCheck = $this->configContains($callCanMethods, self::ACTION);
-        if (!$dataClass::singleton()->hasExtension(Versioned::class)) {
-            $message = !Director::isDev() ? '' : "$dataClass does not support Versioning";
+        if (!$obj->hasExtension(Versioned::class)) {
+            $message = !Director::isDev() ? '' : get_class($obj) . ' does not support Versioning';
             throw new RestApiEndpointException($message, 404);
         }
         if ($action === 'publish') {

tests/Controllers/RestApiEndpointTest.php

@@ -17,6 +17,7 @@
 use emteknetnz\RestApi\Tests\Controllers\RestApiTest\TestCanMethodStatic;
 use emteknetnz\RestApi\Exceptions\RestApiEndpointConfigException;
 use SilverStripe\Security\SecurityToken;
+use SilverStripe\Versioned\Versioned;
 
 # vendor/bin/phpunit app/tests/Controllers/RestApiTest.php flush=1
 
@@ -186,6 +187,10 @@ protected function setUp(): void
             'AllowedOperationsCreate' => 'TestTask 04 AllowedOperationsCreate',
             'AllowedOperationsEdit' => 'TestTask 04 AllowedOperationsEdit',
         ])->write();
+        // TestTask has the Versioned extension on it to support versioning tests
+        foreach (TestTask::get() as $testTask) {
+            $testTask->publishRecursive();
+        }
     }
 
     /**
@@ -2268,8 +2273,10 @@ public function testETag()
 
     public function testExtensionHooks(): void
     {
+        $task = TestTask::get()->first();
+        $taskID = $task->ID;
         // ViewOne
-        $this->req('GET', TestTask::get()->first()->ID);
+        $this->req('GET', $taskID);
         $this->assertTrue(TestApiEndpoint::$hooksCalled['onViewOne']);
         // ViewMany
         $this->req('GET');
@@ -2279,17 +2286,17 @@ public function testExtensionHooks(): void
         $this->assertTrue(TestApiEndpoint::$hooksCalled['onCreateBeforeWrite']);
         $this->assertTrue(TestApiEndpoint::$hooksCalled['onCreateAfterWrite']);
         // Edit
-        $this->req('PATCH', TestTask::get()->first()->ID, null, null, ['title' => 'test']);
+        $this->req('PATCH', $taskID, null, null, ['title' => 'test']);
         $this->assertTrue(TestApiEndpoint::$hooksCalled['onEditBeforeWrite']);
         $this->assertTrue(TestApiEndpoint::$hooksCalled['onEditAfterWrite']);
-        // Delete
-        $this->req('DELETE', TestTask::get()->first()->ID);
-        $this->assertTrue(TestApiEndpoint::$hooksCalled['onDeleteBeforeDelete']);
-        $this->assertTrue(TestApiEndpoint::$hooksCalled['onDeleteAfterDelete']);
         // Action
-        $this->req('ACTION', TestTask::get()->first()->ID, 'publish');
+        $this->req('PUT', $taskID, 'publish');
         $this->assertTrue(TestApiEndpoint::$hooksCalled['onBeforeAction']);
         $this->assertTrue(TestApiEndpoint::$hooksCalled['onAfterAction']);
+        // Delete - need to call this after all the others
+        $this->req('DELETE', $taskID);
+        $this->assertTrue(TestApiEndpoint::$hooksCalled['onDeleteBeforeDelete']);
+        $this->assertTrue(TestApiEndpoint::$hooksCalled['onDeleteAfterDelete']);
         // BeforeSendResponse
         $this->assertTrue(TestApiEndpoint::$hooksCalled['onBeforeSendResponse']);
     }

tests/Controllers/RestApiEndpointTest/TestApiEndpoint.php

@@ -118,6 +118,8 @@ class TestApiEndpoint extends RestApiEndpoint implements PermissionProvider, Tes
         'onEditAfterWrite' => false,
         'onDeleteBeforeDelete' => false,
         'onDeleteAfterDelete' => false,
+        'onBeforeAction' => false,
+        'onAfterAction' => false,
         'onBeforeSendResponse' => false,
     ];
 

tests/Controllers/RestApiEndpointTest/TestMilestone.php

@@ -6,6 +6,7 @@
 use emteknetnz\RestApi\Tests\Controllers\RestApiTest\TestProject;
 use emteknetnz\RestApi\Tests\Controllers\RestApiTest\TestCanMethodsTrait;
 use SilverStripe\ORM\DataObject;
+use SilverStripe\Versioned\Versioned;
 
 class TestMilestone extends DataObject implements TestOnly
 {
@@ -20,4 +21,9 @@ class TestMilestone extends DataObject implements TestOnly
     ];
 
     private static $table_name = 'TestMilestone';
+
+    private static $extensions = [
+        Versioned::class,
+        TestVersionedExtension::class,
+    ];
 }

tests/Controllers/RestApiEndpointTest/TestProject.php

@@ -8,6 +8,7 @@
 use emteknetnz\RestApi\Tests\Controllers\RestApiTest\TestMilestone;
 use emteknetnz\RestApi\Tests\Controllers\RestApiTest\TestCanMethodsTrait;
 use SilverStripe\ORM\DataObject;
+use SilverStripe\Versioned\Versioned;
 
 class TestProject extends DataObject implements TestOnly
 {
@@ -27,4 +28,9 @@ class TestProject extends DataObject implements TestOnly
     ];
 
     private static $table_name = 'TestProject';
+
+    private static $extensions = [
+        Versioned::class,
+        TestVersionedExtension::class,
+    ];
 }

tests/Controllers/RestApiEndpointTest/TestReporter.php

@@ -5,6 +5,7 @@
 use SilverStripe\Dev\TestOnly;
 use emteknetnz\RestApi\Tests\Controllers\RestApiTest\TestCanMethodsTrait;
 use SilverStripe\ORM\DataObject;
+use SilverStripe\Versioned\Versioned;
 
 class TestReporter extends DataObject implements TestOnly
 {
@@ -15,4 +16,9 @@ class TestReporter extends DataObject implements TestOnly
     ];
 
     private static $table_name = 'TestReporter';
+
+    private static $extensions = [
+        Versioned::class,
+        TestVersionedExtension::class,
+    ];
 }

tests/Controllers/RestApiEndpointTest/TestTask.php

@@ -8,6 +8,8 @@
 use emteknetnz\RestApi\Tests\Controllers\RestApiTest\TestCanMethodsTrait;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\ValidationResult;
+use emteknetnz\RestApi\Tests\Controllers\RestApiTest\TestVersionedExtension;
+use SilverStripe\Versioned\Versioned;
 
 class TestTask extends DataObject implements TestOnly
 {
@@ -33,6 +35,11 @@ class TestTask extends DataObject implements TestOnly
         'TestReporter' => TestReporter::class,
     ];
 
+    private static $extensions = [
+        Versioned::class,
+        TestVersionedExtension::class,
+    ];
+
     private static $table_name = 'TestTask';
 
     public function validate()

tests/Controllers/RestApiEndpointTest/TestTeam.php

@@ -6,6 +6,7 @@
 use emteknetnz\RestApi\Tests\Controllers\RestApiTest\TestProject;
 use emteknetnz\RestApi\Tests\Controllers\RestApiTest\TestCanMethodsTrait;
 use SilverStripe\ORM\DataObject;
+use SilverStripe\Versioned\Versioned;
 
 class TestTeam extends DataObject implements TestOnly
 {
@@ -48,4 +49,9 @@ class TestTeam extends DataObject implements TestOnly
     ];
 
     private static $table_name = 'TestTeam';
+
+    private static $extensions = [
+        Versioned::class,
+        TestVersionedExtension::class,
+    ];
 }

tests/Controllers/RestApiEndpointTest/TestVersionedExtension.php

@@ -0,0 +1,18 @@
+<?php
+
+namespace emteknetnz\RestApi\Tests\Controllers\RestApiTest;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\DataExtension;
+use SilverStripe\Versioned\Versioned;
+
+/**
+ * Automatically publish a DataObject when it is written - this is to make asserting things easier
+ */
+class TestVersionedExtension extends DataExtension implements TestOnly
+{
+    public function onAfterWrite()
+    {
+        $this->owner->publishRecursive();
+    }
+}