From aa5804ddb098c9bbfa967d75000b0e3f06bff408 Mon Sep 17 00:00:00 2001 From: Oliver Hader <oliver@typo3.org> Date: Wed, 19 Feb 2014 19:30:02 +0100 Subject: [PATCH] [BUGFIX] moveContentRecordToDifferentPageAndChangeSorting fails In a workspace, an existing content record is moved to an existing page. Another existing record is moved after the previously moved record on the target page. The Functional Tests show, that the content records are faulty after the processing and the first content record disappeared. A similar behaviour has been discovered for pages which finally lead to the regression causing this bug in issue #33104. Back then a hook has been introduced for moving page records and post-processing the database values. However, this hook has been called for all move operations for any table and was wrong in terms of the expected specific problem to be solved. The hook gets reverted, since it's sufficient to resolve move placeholders if a record shall be created after an existing one. Resolves: #55573 Releases: 6.2 Change-Id: Ie5cbc95daf4d46f4204cf18e80e17ff4fa37f496 Reviewed-on: https://review.typo3.org/27733 Reviewed-by: Wouter Wolters Tested-by: Wouter Wolters Reviewed-by: Oliver Hader Tested-by: Oliver Hader --- .../core/Classes/DataHandling/DataHandler.php | 4 +++ .../Fixtures/Frontend/JsonRenderer.ts | 11 ++++++- .../version/Classes/Hook/DataHandlerHook.php | 26 ---------------- .../Regular/AbstractActionTestCase.php | 31 ++++++++++++++++--- ...eRecordToDifferentPageAndChangeSorting.csv | 2 +- ...ndCreatePageRecordAfterMovedPageRecord.csv | 10 ++++++ 6 files changed, 51 insertions(+), 33 deletions(-) create mode 100644 typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/DataSet/Assertion/movePageRecordToDifferentPageAndCreatePageRecordAfterMovedPageRecord.csv diff --git a/typo3/sysext/core/Classes/DataHandling/DataHandler.php b/typo3/sysext/core/Classes/DataHandling/DataHandler.php index 09506bfbb286..f64d834330a4 100644 --- a/typo3/sysext/core/Classes/DataHandling/DataHandler.php +++ b/typo3/sysext/core/Classes/DataHandling/DataHandler.php @@ -6060,6 +6060,10 @@ class DataHandler { if ($lookForLiveVersion = BackendUtility::getLiveVersionOfRecord($table, $row['uid'], $sortRow . ',pid,uid')) { $row = $lookForLiveVersion; } + // Fetch move placeholder, since it might point to a new page in the current workspace + if ($movePlaceholder = BackendUtility::getMovePlaceholder($table, $row['uid'], 'uid,pid,' . $sortRow)) { + $row = $movePlaceholder; + } // If the record should be inserted after itself, keep the current sorting information: if ($row['uid'] == $uid) { $sortNumber = $row[$sortRow]; diff --git a/typo3/sysext/core/Tests/Functional/Fixtures/Frontend/JsonRenderer.ts b/typo3/sysext/core/Tests/Functional/Fixtures/Frontend/JsonRenderer.ts index 35e76d093f73..4e1ff319b921 100644 --- a/typo3/sysext/core/Tests/Functional/Fixtures/Frontend/JsonRenderer.ts +++ b/typo3/sysext/core/Tests/Functional/Fixtures/Frontend/JsonRenderer.ts @@ -26,6 +26,15 @@ page = PAGE page { 10 = CONTENT 10 { + watcher.parentRecordField = __pages + table = pages + select { + orderBy = sorting + pidInList = this + } + } + 20 = CONTENT + 20 { watcher.parentRecordField = __contents table = tt_content select { @@ -103,4 +112,4 @@ page { [globalVar = GP:L = 1] config.sys_language_uid = 1 -[end] \ No newline at end of file +[end] diff --git a/typo3/sysext/version/Classes/Hook/DataHandlerHook.php b/typo3/sysext/version/Classes/Hook/DataHandlerHook.php index 912b69446f44..3c7b019e8a3c 100644 --- a/typo3/sysext/version/Classes/Hook/DataHandlerHook.php +++ b/typo3/sysext/version/Classes/Hook/DataHandlerHook.php @@ -142,32 +142,6 @@ class DataHandlerHook { } } - /** - * Hook that is called after tcemain made most of its decisions. - * - * NOTE: This fixes an issue related to moving/creating initial-placeholders - if such a new page - * is intended to be place behind a move-placeholder tcemain handles the movement/creation, - * but does not respect the wsPlaceholder, which leads the new page to be located at the old location of the - * page where it was intended to be placed behind. - * - * @param string $command - * @param string $table - * @param integer $id - * @param mixed $value - * @param DataHandler $tcemain - */ - public function processCmdmap_postProcess($command, $table, $id, $value, DataHandler $tcemain) { - if ($command === 'move') { - if ($value < 0) { - $movePlaceHolder = BackendUtility::getMovePlaceholder($table, abs($value), 'uid'); - if ($movePlaceHolder !== FALSE) { - $destPid = -$movePlaceHolder['uid']; - $tcemain->moveRecord_raw($table, $id, $destPid); - } - } - } - } - /** * hook that is called AFTER all commands of the commandmap was * executed diff --git a/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/AbstractActionTestCase.php b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/AbstractActionTestCase.php index f18135c97ab5..1729d452c31b 100644 --- a/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/AbstractActionTestCase.php +++ b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/AbstractActionTestCase.php @@ -154,7 +154,6 @@ abstract class AbstractActionTestCase extends \TYPO3\CMS\Core\Tests\Functional\D * @test */ public function moveContentRecordToDifferentPageAndChangeSorting() { - $this->markTestSkipped('Something seems to be wrong here...'); $this->actionService->moveRecord(self::TABLE_Content, self::VALUE_ContentIdLast, self::VALUE_PageIdTarget); $this->actionService->moveRecord(self::TABLE_Content, self::VALUE_ContentIdFirst, -self::VALUE_ContentIdLast); $this->assertAssertionDataSet('moveContentRecordToDifferentPageAndChangeSorting'); @@ -171,7 +170,7 @@ abstract class AbstractActionTestCase extends \TYPO3\CMS\Core\Tests\Functional\D * @test */ public function createPageRecord() { - $newTableIds = $this->actionService->createNewRecord(self::TABLE_Page, self::VALUE_PageId, array('title' => 'Testing #1')); + $newTableIds = $this->actionService->createNewRecord(self::TABLE_Page, self::VALUE_PageId, array('title' => 'Testing #1', 'hidden' => 0)); $this->assertAssertionDataSet('createPageRecord'); $newPageId = $newTableIds[self::TABLE_Page][0]; @@ -256,9 +255,31 @@ abstract class AbstractActionTestCase extends \TYPO3\CMS\Core\Tests\Functional\D $this->actionService->moveRecord(self::TABLE_Page, self::VALUE_PageId, -self::VALUE_PageIdTarget); $this->assertAssertionDataSet('movePageRecordToDifferentPageAndChangeSorting'); - $responseContent = $this->getFrontendResponse(self::VALUE_PageId, 0, self::VALUE_BackendUserId, self::VALUE_WorkspaceId)->getResponseContent(); - $this->assertResponseContentHasRecords($responseContent, self::TABLE_Page, 'title', 'Relations'); - $this->assertResponseContentHasRecords($responseContent, self::TABLE_Content, 'header', array('Regular Element #1', 'Regular Element #2')); + $responseContentPage = $this->getFrontendResponse(self::VALUE_PageId, 0, self::VALUE_BackendUserId, self::VALUE_WorkspaceId)->getResponseContent(); + $this->assertResponseContentHasRecords($responseContentPage, self::TABLE_Page, 'title', 'Relations'); + $this->assertResponseContentHasRecords($responseContentPage, self::TABLE_Content, 'header', array('Regular Element #1', 'Regular Element #2')); + $responseContentWebsite = $this->getFrontendResponse(self::VALUE_PageIdWebsite, 0, self::VALUE_BackendUserId, self::VALUE_WorkspaceId)->getResponseContent(); + $this->assertResponseContentStructureHasRecords( + $responseContentWebsite, self::TABLE_Page . ':' . self::VALUE_PageIdWebsite, '__pages', + self::TABLE_Page, 'title', array('Target', 'Relations', 'DataHandlerTest') + ); + } + + /** + * @test + * @see http://forge.typo3.org/issues/33104 + * @see http://forge.typo3.org/issues/55573 + */ + public function movePageRecordToDifferentPageAndCreatePageRecordAfterMovedPageRecord() { + $this->actionService->moveRecord(self::TABLE_Page, self::VALUE_PageIdTarget, self::VALUE_PageIdWebsite); + $this->actionService->createNewRecord(self::TABLE_Page, -self::VALUE_PageIdTarget, array('title' => 'Testing #1', 'hidden' => 0)); + $this->assertAssertionDataSet('movePageRecordToDifferentPageAndCreatePageRecordAfterMovedPageRecord'); + + $responseContent = $this->getFrontendResponse(self::VALUE_PageIdWebsite, 0, self::VALUE_BackendUserId, self::VALUE_WorkspaceId)->getResponseContent(); + $this->assertResponseContentStructureHasRecords( + $responseContent, self::TABLE_Page . ':' . self::VALUE_PageIdWebsite, '__pages', + self::TABLE_Page, 'title', array('Target', 'Testing #1', 'DataHandlerTest') + ); } } diff --git a/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/DataSet/Assertion/movePageRecordToDifferentPageAndChangeSorting.csv b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/DataSet/Assertion/movePageRecordToDifferentPageAndChangeSorting.csv index 1dc21dea6885..e47a5fc23112 100644 --- a/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/DataSet/Assertion/movePageRecordToDifferentPageAndChangeSorting.csv +++ b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/DataSet/Assertion/movePageRecordToDifferentPageAndChangeSorting.csv @@ -2,7 +2,7 @@ pages ,uid,pid,sorting,deleted,t3ver_wsid,t3ver_state,t3ver_stage,t3ver_oid,t3ver_move_id,title ,1,0,256,0,0,0,0,0,0,FunctionalTest ,88,1,256,0,0,0,0,0,0,DataHandlerTest -,89,1,160,0,0,0,0,0,0,Relations +,89,88,256,0,0,0,0,0,0,Relations ,90,88,512,0,0,0,0,0,0,Target ,91,-1,512,0,1,4,0,90,0,Target ,92,1,128,0,1,3,0,0,90,"[MOVE-TO PLACEHOLDER for #90, WS#1]" diff --git a/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/DataSet/Assertion/movePageRecordToDifferentPageAndCreatePageRecordAfterMovedPageRecord.csv b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/DataSet/Assertion/movePageRecordToDifferentPageAndCreatePageRecordAfterMovedPageRecord.csv new file mode 100644 index 000000000000..4b88eaa02e12 --- /dev/null +++ b/typo3/sysext/workspaces/Tests/Functional/DataHandling/Regular/DataSet/Assertion/movePageRecordToDifferentPageAndCreatePageRecordAfterMovedPageRecord.csv @@ -0,0 +1,10 @@ +pages +,uid,pid,sorting,deleted,t3ver_wsid,t3ver_state,t3ver_stage,t3ver_oid,t3ver_move_id,title +,1,0,256,0,0,0,0,0,0,FunctionalTest +,88,1,256,0,0,0,0,0,0,DataHandlerTest +,89,88,256,0,0,0,0,0,0,Relations +,90,88,512,0,0,0,0,0,0,Target +,91,-1,512,0,1,4,0,90,0,Target +,92,1,128,0,1,3,0,0,90,"[MOVE-TO PLACEHOLDER for #90, WS#1]" +,93,1,192,0,1,1,0,0,0,"Testing #1" +,94,-1,192,0,1,-1,0,93,0,"Testing #1" -- GitLab