[BUGFIX] Skip swapping/publishing of deleted records 77/50877/4
authorOliver Hader <oliver@typo3.org>
Sat, 3 Dec 2016 15:44:24 +0000 (16:44 +0100)
committerSusanne Moog <susanne.moog@typo3.org>
Sat, 3 Dec 2016 17:49:42 +0000 (18:49 +0100)
In case a content element and the accordant page have been deleted in
separate actions and get published together, the workspace process will
trigger an error message since the removed content element cannot be
published anymore (since it has been processed already with the page).

To avoid this behavior deleted records are collected and checked in the
workspace swapping/publishing process.

Change-Id: If04a198abf81efdc88e75da79da0c01cfaa361ff
Resolves: #47384
Releases: master, 7.6
Reviewed-on: https://review.typo3.org/50877
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Susanne Moog <susanne.moog@typo3.org>
Tested-by: Susanne Moog <susanne.moog@typo3.org>
typo3/sysext/core/Classes/DataHandling/DataHandler.php
typo3/sysext/version/Classes/Hook/DataHandlerHook.php
typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/AbstractActionTestCase.php
typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Modify/ActionTest.php
typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Modify/DataSet/deleteContentAndPage.csv [new file with mode: 0644]
typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Publish/ActionTest.php
typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Publish/DataSet/deleteContentAndPage.csv [new file with mode: 0644]
typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/PublishAll/ActionTest.php
typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/PublishAll/DataSet/deleteContentAndPage.csv [new file with mode: 0644]

index 084595f..f872bb7 100644 (file)
@@ -305,6 +305,13 @@ class DataHandler
     public $copyMappingArray_merged = [];
 
     /**
+     * Per-table array with UIDs that have been deleted.
+     *
+     * @var array
+     */
+    protected $deletedRecords = [];
+
+    /**
      * A map between input file name and final destination for files being attached to records.
      *
      * @var array
@@ -5021,6 +5028,7 @@ class DataHandler
             $this->databaseConnection->exec_UPDATEquery($table, 'uid=' . (int)$uid, $updateFields);
             // Delete all l10n records as well, impossible during undelete because it might bring too many records back to life
             if (!$undeleteRecord) {
+                $this->deletedRecords[$table][] = (int)$uid;
                 $this->deleteL10nOverlayRecords($table, $uid);
             }
         } else {
@@ -5052,6 +5060,7 @@ class DataHandler
             }
             // Delete the hard way...:
             $this->databaseConnection->exec_DELETEquery($table, 'uid=' . (int)$uid);
+            $this->deletedRecords[$table][] = (int)$uid;
             $this->deleteL10nOverlayRecords($table, $uid);
         }
         if ($this->enableLogging) {
@@ -8215,6 +8224,22 @@ class DataHandler
     }
 
     /**
+     * Determines whether a particular record has been deleted
+     * using DataHandler::deleteRecord() in this instance.
+     *
+     * @param string $tableName
+     * @param string $uid
+     * @return bool
+     */
+    public function hasDeletedRecord($tableName, $uid)
+    {
+        return
+            !empty($this->deletedRecords[$tableName])
+            && in_array($uid, $this->deletedRecords[$tableName])
+        ;
+    }
+
+    /**
      * Gets the automatically versionized id of a record.
      *
      * @param string $table Name of the table
index b9652db..fb21903 100644 (file)
@@ -788,6 +788,11 @@ class DataHandlerHook
 
         // Check prerequisites before start swapping
 
+        // Skip records that have been deleted during the current execution
+        if ($tcemainObj->hasDeletedRecord($table, $id)) {
+            return;
+        }
+
         // First, check if we may actually edit the online record
         if (!$tcemainObj->checkRecordUpdateAccess($table, $id)) {
             $tcemainObj->newlog('Error: You cannot swap versions for a record you do not have access to edit!', 1);
index 801d98e..2ecbe79 100644 (file)
@@ -231,6 +231,15 @@ abstract class AbstractActionTestCase extends \TYPO3\CMS\Core\Tests\Functional\D
     }
 
     /**
+     * @see DataSet/Assertion/deleteContentAndPage.csv
+     */
+    public function deleteContentAndPage()
+    {
+        $this->actionService->deleteRecord(self::TABLE_Content, self::VALUE_ContentIdSecond);
+        $this->actionService->deleteRecord(self::TABLE_Page, self::VALUE_PageId);
+    }
+
+    /**
      * @see DataSet/Assertion/copyPageRecord.csv
      */
     public function copyPage()
index c9ed478..6b3102a 100644 (file)
@@ -281,6 +281,19 @@ class ActionTest extends \TYPO3\CMS\Workspaces\Tests\Functional\DataHandling\Reg
 
     /**
      * @test
+     * @see DataSet/Assertion/deleteContentAndPage.csv
+     */
+    public function deleteContentAndPage()
+    {
+        parent::deleteContentAndPage();
+        $this->assertAssertionDataSet('deleteContentAndPage');
+
+        $response = $this->getFrontendResponse(self::VALUE_PageId, 0, self::VALUE_BackendUserId, self::VALUE_WorkspaceId, false);
+        $this->assertContains('RuntimeException', $response->getError());
+    }
+
+    /**
+     * @test
      * @see DataSet/Assertion/copyPageRecord.csv
      */
     public function copyPage()
diff --git a/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Modify/DataSet/deleteContentAndPage.csv b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Modify/DataSet/deleteContentAndPage.csv
new file mode 100644 (file)
index 0000000..cfaf435
--- /dev/null
@@ -0,0 +1,16 @@
+"pages",,,,,,,,,,,,,
+,"uid","pid","sorting","deleted","t3_origuid","t3ver_wsid","t3ver_state","t3ver_stage","t3ver_oid","t3ver_move_id","title",,
+,1,0,256,0,0,0,0,0,0,0,"FunctionalTest",,
+,88,1,256,0,0,0,0,0,0,0,"DataHandlerTest",,
+,89,88,256,0,0,0,0,0,0,0,"Relations",,
+,90,88,512,0,0,0,0,0,0,0,"Target",,
+,91,-1,256,0,89,1,2,0,89,0,"Relations",,
+"tt_content",,,,,,,,,,,,,
+,"uid","pid","sorting","deleted","sys_language_uid","l18n_parent","t3_origuid","t3ver_wsid","t3ver_state","t3ver_stage","t3ver_oid","t3ver_move_id","header"
+,296,88,256,0,0,0,0,0,0,0,0,0,"Regular Element #0"
+,297,89,256,0,0,0,0,0,0,0,0,0,"Regular Element #1"
+,298,89,512,0,0,0,0,0,0,0,0,0,"Regular Element #2"
+,299,89,768,0,0,0,0,0,0,0,0,0,"Regular Element #3"
+,300,89,1024,0,1,299,299,0,0,0,0,0,"[Translate to Dansk:] Regular Element #3"
+,301,-1,512,0,0,0,298,2,2,0,298,0,"Regular Element #2"
+,302,-1,512,0,0,0,298,1,2,0,298,0,"Regular Element #2"
index ba656ec..ba609c0 100644 (file)
@@ -280,6 +280,20 @@ class ActionTest extends \TYPO3\CMS\Workspaces\Tests\Functional\DataHandling\Reg
 
     /**
      * @test
+     * @see DataSet/Assertion/deleteContentAndPage.csv
+     */
+    public function deleteContentAndPage()
+    {
+        parent::deleteContentAndPage();
+        $this->actionService->publishRecord(self::TABLE_Page, self::VALUE_PageId);
+        $this->assertAssertionDataSet('deleteContentAndPage');
+
+        $response = $this->getFrontendResponse(self::VALUE_PageId, 0, 0, 0, false);
+        $this->assertContains('PageNotFoundException', $response->getError());
+    }
+
+    /**
+     * @test
      * @see DataSet/Assertion/copyPageRecord.csv
      */
     public function copyPage()
diff --git a/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Publish/DataSet/deleteContentAndPage.csv b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/Publish/DataSet/deleteContentAndPage.csv
new file mode 100644 (file)
index 0000000..58d6118
--- /dev/null
@@ -0,0 +1,17 @@
+"pages",,,,,,,,,,,,,
+,"uid","pid","sorting","deleted","t3_origuid","t3ver_wsid","t3ver_state","t3ver_stage","t3ver_oid","t3ver_move_id","title",,
+,1,0,256,0,0,0,0,0,0,0,"FunctionalTest",,
+,88,1,256,0,0,0,0,0,0,0,"DataHandlerTest",,
+,89,88,1000000000,1,89,0,0,0,0,0,"Relations",,
+,90,88,512,0,0,0,0,0,0,0,"Target",,
+,91,-1,1000000000,1,0,0,0,0,89,0,"Relations",,
+"tt_content",,,,,,,,,,,,,
+,"uid","pid","sorting","deleted","sys_language_uid","l18n_parent","t3_origuid","t3ver_wsid","t3ver_state","t3ver_stage","t3ver_oid","t3ver_move_id","header"
+,296,88,256,0,0,0,0,0,0,0,0,0,"Regular Element #0"
+,297,89,1000000000,1,0,0,0,0,0,0,0,0,"Regular Element #1"
+,298,89,1000000000,1,0,0,0,0,0,0,0,0,"Regular Element #2"
+,299,89,1000000000,1,0,0,0,0,0,0,0,0,"Regular Element #3"
+,300,89,1000000000,1,1,299,299,0,0,0,0,0,"[Translate to Dansk:] Regular Element #3"
+,301,-1,512,0,0,0,298,2,2,0,298,0,"Regular Element #2"
+,302,-1,1000000000,1,0,0,298,1,2,0,298,0,"Regular Element #2"
+,303,-1,1000000000,1,1,299,300,1,2,0,300,0,"[Translate to Dansk:] Regular Element #3"
index 6a4b0c1..813439d 100644 (file)
@@ -265,6 +265,20 @@ class ActionTest extends \TYPO3\CMS\Workspaces\Tests\Functional\DataHandling\Reg
 
     /**
      * @test
+     * @see DataSet/Assertion/deleteContentAndPage.csv
+     */
+    public function deleteContentAndPage()
+    {
+        parent::deleteContentAndPage();
+        $this->actionService->publishWorkspace(self::VALUE_WorkspaceId);
+        $this->assertAssertionDataSet('deleteContentAndPage');
+
+        $response = $this->getFrontendResponse(self::VALUE_PageId, 0, 0, 0, false);
+        $this->assertContains('PageNotFoundException', $response->getError());
+    }
+
+    /**
+     * @test
      * @see DataSet/Assertion/copyPageRecord.csv
      */
     public function copyPage()
@@ -372,10 +386,6 @@ class ActionTest extends \TYPO3\CMS\Workspaces\Tests\Functional\DataHandling\Reg
      */
     public function createPlaceholdersAndDeleteDraftParentPage()
     {
-        // @todo These two log entries could be avoided in DataHandlerHook, but are expected
-        // "[newlog()] Error: You cannot swap versions for a record you do not have access to edit!"
-        $this->expectedErrorLogEntries = 2;
-
         parent::createPlaceholdersAndDeleteDraftParentPage();
         $this->actionService->publishWorkspace(self::VALUE_WorkspaceId);
         $this->assertAssertionDataSet('createPlaceholdersAndDeleteDraftParentPage');
diff --git a/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/PublishAll/DataSet/deleteContentAndPage.csv b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/PublishAll/DataSet/deleteContentAndPage.csv
new file mode 100644 (file)
index 0000000..58d6118
--- /dev/null
@@ -0,0 +1,17 @@
+"pages",,,,,,,,,,,,,
+,"uid","pid","sorting","deleted","t3_origuid","t3ver_wsid","t3ver_state","t3ver_stage","t3ver_oid","t3ver_move_id","title",,
+,1,0,256,0,0,0,0,0,0,0,"FunctionalTest",,
+,88,1,256,0,0,0,0,0,0,0,"DataHandlerTest",,
+,89,88,1000000000,1,89,0,0,0,0,0,"Relations",,
+,90,88,512,0,0,0,0,0,0,0,"Target",,
+,91,-1,1000000000,1,0,0,0,0,89,0,"Relations",,
+"tt_content",,,,,,,,,,,,,
+,"uid","pid","sorting","deleted","sys_language_uid","l18n_parent","t3_origuid","t3ver_wsid","t3ver_state","t3ver_stage","t3ver_oid","t3ver_move_id","header"
+,296,88,256,0,0,0,0,0,0,0,0,0,"Regular Element #0"
+,297,89,1000000000,1,0,0,0,0,0,0,0,0,"Regular Element #1"
+,298,89,1000000000,1,0,0,0,0,0,0,0,0,"Regular Element #2"
+,299,89,1000000000,1,0,0,0,0,0,0,0,0,"Regular Element #3"
+,300,89,1000000000,1,1,299,299,0,0,0,0,0,"[Translate to Dansk:] Regular Element #3"
+,301,-1,512,0,0,0,298,2,2,0,298,0,"Regular Element #2"
+,302,-1,1000000000,1,0,0,298,1,2,0,298,0,"Regular Element #2"
+,303,-1,1000000000,1,1,299,300,1,2,0,300,0,"[Translate to Dansk:] Regular Element #3"