[!!!][TASK] Remove 4th parameter of enableFields 73/61073/4
authorBenni Mack <benni@typo3.org>
Mon, 17 Jun 2019 13:26:44 +0000 (15:26 +0200)
committerGeorg Ringer <georg.ringer@gmail.com>
Fri, 28 Jun 2019 11:05:33 +0000 (13:05 +0200)
Since Versioning is completely handled via
the Context API and set within PageRepository
directly since TYPO3 v9, using the fourth
parameter would result in an invalid scenario.

Instead PageRepository->where_hid_del in
a live scenario now always contains (pid!=-1)
which filters out all non-versioned records.

This is a breaking change to avoid confusion,
however in regular scenarios this does
not affect the system, as PageRepository->versionOL()
filters out these cases in live workspace
anyways.

Resolves: #88574
Releases: master
Change-Id: I538c04997cb67e30c58a4dfa515acd751f868e9c
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/61073
Tested-by: Daniel Goerz <daniel.goerz@posteo.de>
Tested-by: Georg Ringer <georg.ringer@gmail.com>
Reviewed-by: Daniel Goerz <daniel.goerz@posteo.de>
Reviewed-by: Georg Ringer <georg.ringer@gmail.com>
typo3/sysext/core/Documentation/Changelog/master/Breaking-88574-4thParameterOfPageRepository-enableFieldsRemoved.rst [new file with mode: 0644]
typo3/sysext/frontend/Classes/Page/PageRepository.php
typo3/sysext/frontend/Tests/Functional/Page/PageRepositoryTest.php
typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodArgumentDroppedMatcher.php

diff --git a/typo3/sysext/core/Documentation/Changelog/master/Breaking-88574-4thParameterOfPageRepository-enableFieldsRemoved.rst b/typo3/sysext/core/Documentation/Changelog/master/Breaking-88574-4thParameterOfPageRepository-enableFieldsRemoved.rst
new file mode 100644 (file)
index 0000000..06a8f6d
--- /dev/null
@@ -0,0 +1,40 @@
+.. include:: ../../Includes.txt
+
+========================================================================
+Breaking: #88574 - 4th parameter of PageRepository->enableFields removed
+========================================================================
+
+See :issue:`88574`
+
+Description
+===========
+
+The fourth parameter of :php:`PageRepository->enableFields()` was meant to filter out versioned records
+which are in Live Workspace (versioning, not workspaces). Although the method has largely been superseded
+with Doctrine DBAL's Restrictions, it is still used in some places.
+
+With the introduction of the Context API, new PageRepository instances can be created to fetch multiple variants
+of certain aspects, instead of modifying existing public properties. Therefore the fourth argument has been removed.
+
+
+Impact
+======
+
+Calling the method above with the fourth parameter set to true has no effect anymore, and will
+trigger a PHP Notice.
+
+
+Affected Installations
+======================
+
+Any TYPO3 installation dealing with non-workspace versioning in Frontend requests with third-party extension
+still relying on non-workspace versioning.
+
+
+Migration
+=========
+
+The fourth parameter on any method call can be removed (if set to "false"), or should be replaced with a
+separate instance of PageRepository with a custom Context.
+
+.. index:: Frontend, PHP-API, FullyScanned
\ No newline at end of file
index 70bec45..65b7637 100644 (file)
@@ -174,7 +174,7 @@ class PageRepository implements LoggerAwareInterface
             // versioning preview (that means we are online!)
             $this->where_hid_del = ' AND ' . (string)$expressionBuilder->andX(
                 QueryHelper::stripLogicalOperatorPrefix(
-                    $this->enableFields('pages', $show_hidden, ['fe_group' => true], true)
+                    $this->enableFields('pages', $show_hidden, ['fe_group' => true])
                 ),
                 $expressionBuilder->lt('pages.doktype', 200)
             );
@@ -1222,11 +1222,10 @@ class PageRepository implements LoggerAwareInterface
      * @param string $table Table name found in the $GLOBALS['TCA'] array
      * @param int $show_hidden If $show_hidden is set (0/1), any hidden-fields in records are ignored. NOTICE: If you call this function, consider what to do with the show_hidden parameter. Maybe it should be set? See ContentObjectRenderer->enableFields where it's implemented correctly.
      * @param array $ignore_array Array you can pass where keys can be "disabled", "starttime", "endtime", "fe_group" (keys from "enablefields" in TCA) and if set they will make sure that part of the clause is not added. Thus disables the specific part of the clause. For previewing etc.
-     * @param bool $noVersionPreview If set, enableFields will be applied regardless of any versioning preview settings which might otherwise disable enableFields
      * @throws \InvalidArgumentException
      * @return string The clause starting like " AND ...=... AND ...=...
      */
-    public function enableFields($table, $show_hidden = -1, $ignore_array = [], $noVersionPreview = false)
+    public function enableFields($table, $show_hidden = -1, $ignore_array = [])
     {
         if ($show_hidden === -1) {
             // If show_hidden was not set from outside, use the current context
@@ -1244,7 +1243,7 @@ class PageRepository implements LoggerAwareInterface
                 $constraints[] = $expressionBuilder->eq($table . '.' . $ctrl['delete'], 0);
             }
             if ($ctrl['versioningWS'] ?? false) {
-                if (!$this->versioningWorkspaceId > 0) {
+                if ($this->versioningWorkspaceId === 0) {
                     // Filter out placeholder records (new/moved/deleted items)
                     // in case we are NOT in a versioning preview (that means we are online!)
                     $constraints[] = $expressionBuilder->lte(
@@ -1261,7 +1260,7 @@ class PageRepository implements LoggerAwareInterface
                 }
 
                 // Filter out versioned records
-                if (!$noVersionPreview && empty($ignore_array['pid'])) {
+                if (empty($ignore_array['pid'])) {
                     $constraints[] = $expressionBuilder->neq($table . '.pid', -1);
                 }
             }
@@ -1270,7 +1269,7 @@ class PageRepository implements LoggerAwareInterface
             if (is_array($ctrl['enablecolumns'])) {
                 // In case of versioning-preview, enableFields are ignored (checked in
                 // versionOL())
-                if ($this->versioningWorkspaceId <= 0 || !$ctrl['versioningWS'] || $noVersionPreview) {
+                if ($this->versioningWorkspaceId === 0 || !$ctrl['versioningWS']) {
                     if (($ctrl['enablecolumns']['disabled'] ?? false) && !$show_hidden && !($ignore_array['disabled'] ?? false)) {
                         $field = $table . '.' . $ctrl['enablecolumns']['disabled'];
                         $constraints[] = $expressionBuilder->eq($field, 0);
index e2326c1..f0a5485 100644 (file)
@@ -359,9 +359,10 @@ class PageRepositoryTest extends \TYPO3\TestingFramework\Core\Functional\Functio
 
         $connection = GeneralUtility::makeInstance(ConnectionPool::class)->getConnectionForTable('pages');
         $expectedSQL = sprintf(
-            ' AND ((%s = 0) AND (%s <= 0) AND (%s = 0) AND (%s <= 123) AND ((%s = 0) OR (%s > 123))) AND (%s < 200)',
+            ' AND ((%s = 0) AND (%s <= 0) AND (%s <> -1) AND (%s = 0) AND (%s <= 123) AND ((%s = 0) OR (%s > 123))) AND (%s < 200)',
             $connection->quoteIdentifier('pages.deleted'),
             $connection->quoteIdentifier('pages.t3ver_state'),
+            $connection->quoteIdentifier('pages.pid'),
             $connection->quoteIdentifier('pages.hidden'),
             $connection->quoteIdentifier('pages.starttime'),
             $connection->quoteIdentifier('pages.endtime'),
@@ -507,37 +508,6 @@ class PageRepositoryTest extends \TYPO3\TestingFramework\Core\Functional\Functio
         );
     }
 
-    /**
-     * @test
-     */
-    public function enableFieldsDoesNotHideVersionedRecordsWhenCheckingVersionOverlays()
-    {
-        $table = $this->getUniqueId('aTable');
-        $GLOBALS['TCA'][$table] = [
-            'ctrl' => [
-                'versioningWS' => true
-            ]
-        ];
-
-        $subject = new PageRepository(new Context([
-            'workspace' => new WorkspaceAspect(23)
-        ]));
-
-        $conditions = $subject->enableFields($table, -1, [], true);
-        $connection = GeneralUtility::makeInstance(ConnectionPool::class)->getConnectionForTable($table);
-
-        $this->assertThat(
-            $conditions,
-            $this->logicalNot($this->stringContains(' AND (' . $connection->quoteIdentifier($table . '.t3ver_state') . ' <= 0)')),
-            'No versioning placeholders'
-        );
-        $this->assertThat(
-            $conditions,
-            $this->logicalNot($this->stringContains(' AND (' . $connection->quoteIdentifier($table . '.pid') . ' <> -1)')),
-            'No records from page -1'
-        );
-    }
-
     protected function assertOverlayRow($row)
     {
         $this->assertIsArray($row);
index ac72a48..ba91f88 100644 (file)
@@ -223,4 +223,10 @@ return [
             'Breaking-87193-DeprecatedFunctionalityRemoved.rst',
         ],
     ],
+    'TYPO3\CMS\Core\Frontend\Page\PageRepository->enableFields' => [
+        'maximumNumberOfArguments' => 3,
+        'restFiles' => [
+            'Breaking-88574-4thParameterOfPageRepository-enableFieldsRemoved.rst',
+        ],
+    ],
 ];