Bug #83179

Epic #86307: Extbase allows to fetch deleted/hidden records (respects ignoreEnableFields)

getSysLanguageStatement builder does not respect enable fields

Added by Andi Peh about 4 years ago. Updated about 3 years ago.

Status:
New
Priority:
Should have
Assignee:
-
Category:
Extbase + l10n
Target version:
-
Start date:
2017-11-30
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
7
PHP Version:
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

I stumbled upon a problem while listing translated rows from a custom table with config.sys_language_mode=strict

Steps to reproduce:
  • create a new repository, with [ctrl] transOrigPointerField, languageField and enablecolumns
  • add some records to the repository in the default language
  • add a new language
  • create translated entries for the existing records
  • hide/disable the second last translated entry
  • list those entries in the frontend limited to count(*)-1 (so if you have created 5 records, limit it to 4)
  • switch to the new language
what happends:
  • a findAll in the repository will return the first 4 records from the default language
  • the language overlay will query each uid to check if there is a translated record available
  • typo3 correctly excludes the one hidden record (the second last)
  • the problem: the list does not show 4 items as requests and available, but only 3

the generated queries look something like this:

SELECT tx_test_domain_model_item.* FROM tx_test_domain_model_item 
WHERE 1=1 AND 
(tx_test_domain_model_item.sys_language_uid=-1 OR 
    (tx_test_domain_model_item.sys_language_uid = 2 AND tx_test_domain_model_item.l10n_parent=0) OR 
    (tx_test_domain_model_item.sys_language_uid=0 AND tx_test_domain_model_item.uid IN 
        (SELECT tx_test_domain_model_item.l10n_parent 
        FROM tx_test_domain_model_item 
        WHERE tx_test_domain_model_item.l10n_parent>0 AND 
        tx_test_domain_model_item.sys_language_uid=2 AND 
        tx_test_domain_model_item.deleted=0
        )
    )
) AND tx_test_domain_model_item.pid IN (37) AND tx_test_domain_model_item.deleted=0 
AND tx_test_domain_model_item.hidden=0
ORDER BY tx_test_domain_model_item.sorting ASC
LIMIT 4;

-- followed by this for each found record:

SELECT * FROM tx_test_domain_model_item
WHERE pid=37 AND sys_language_uid=2 AND l10n_parent=1 AND 
tx_test_domain_model_item.deleted=0 AND tx_test_domain_model_item.hidden=0 
LIMIT 1

the problem goes down to:

Typo3DbQueryParser->getSysLanguageStatement

there the generator does only respected deleted fields in the subquery, but not hidden or outdated fields.

https://git.typo3.org/Packages/TYPO3.CMS.git/blob/dea1e9643567e99a6f7cee6edda6d8000893b743:/typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbQueryParser.php#l705

therefor this

// Add delete clause to ensure all entries are loaded
if (isset($GLOBALS['TCA'][$tableName]['ctrl']['delete'])) {
    $additionalWhereClause .= ' AND ' . $tableName . '.' . $GLOBALS['TCA'][$tableName]['ctrl']['delete'] . '=0';
}

should be replaced by this:

// Add visibility constraints to ensure all entries are loaded and enabled
$enableFields = $this->getVisibilityConstraintStatement($querySettings, $tableName, $tableName);
if(!empty($enableFields)) {
    $additionalWhereClause .= ' AND '. $enableFields;
}


Related issues

Related to TYPO3 Core - Feature #82986: Extbase Model - includeDeleted and ignoreEnableFieldsNew2017-11-13

Actions
Related to TYPO3 Core - Bug #76054: Extbase ignores parent record's enableFields when using ContentFallbackNew2016-05-05

Actions
#1

Updated by Andi Peh about 4 years ago

  • Category set to Localization
#2

Updated by Gerrit Code Review almost 4 years ago

  • Status changed from New to Under Review

Patch set 1 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/55030

#3

Updated by Gerrit Code Review almost 4 years ago

Patch set 1 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/55031

#4

Updated by Gerrit Code Review almost 4 years ago

Patch set 2 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/55031

#5

Updated by Andi Peh almost 4 years ago

I made a mistake in the first patch (55030) and removed it.

#6

Updated by Andi Peh almost 4 years ago

Has someone checked this or did I miss something?

#7

Updated by Tymoteusz Motylewski about 3 years ago

  • Category changed from Localization to Extbase + l10n
#8

Updated by Tymoteusz Motylewski about 3 years ago

  • Related to Feature #82986: Extbase Model - includeDeleted and ignoreEnableFields added
#9

Updated by Stephan Großberndt about 3 years ago

As the patch at https://review.typo3.org/55031 is currently not available (maybe due to being marked as draft?) here are its contents for reference:

commit 3676bb0aebb2711745d490504077f83fb2aab338
Author: Andreas Pauschenwein <hugo_2000@gmx.at>
Date:   Mon Dec 11 15:43:01 2017 +0100

    [BUGFIX] Respect enable fields in getSysLanguageStatement

    getSysLanguageStatement builder currently only respect deleted fields
    but does not check for hidden or outdated fields. Subsequent
    localization queries do respect all enable fields, can therefor return a
    different subset. This leads to issues in paginations.

    Change-Id: If5050c40688491816a7735f81a65fd300c3eb586
    Releases: 7.6
    Resolves: #83179

diff --git a/typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbQueryParser.php b/typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbQueryParser.php
index de0f5f2..7b3b711 100644
--- a/typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbQueryParser.php
+++ b/typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbQueryParser.php
@@ -711,9 +711,10 @@ class Typo3DbQueryParser implements SingletonInterface
                             ' AND ' . $tableName . '.' . $GLOBALS['TCA'][$tableName]['ctrl']['languageField'] . '=' . (int)$querySettings->getLanguageUid();
                     }

-                    // Add delete clause to ensure all entries are loaded
-                    if (isset($GLOBALS['TCA'][$tableName]['ctrl']['delete'])) {
-                        $additionalWhereClause .= ' AND ' . $tableName . '.' . $GLOBALS['TCA'][$tableName]['ctrl']['delete'] . '=0';
+                    // Add visibility constraints to ensure all entries are loaded and enabled
+                    $enableFields = $this->getVisibilityConstraintStatement($querySettings, $tableName, $tableName);
+                    if (!empty($enableFields)) {
+                        $additionalWhereClause .= ' AND '. $enableFields;
                     }
                     $additionalWhereClause .= '))';
                 }

#10

Updated by Andi Peh about 3 years ago

Sorry - I just published the patch .

#11

Updated by Tymoteusz Motylewski about 3 years ago

  • Parent task set to #86307
#12

Updated by Tymoteusz Motylewski about 3 years ago

  • Related to Bug #76054: Extbase ignores parent record's enableFields when using ContentFallback added
#13

Updated by Tymoteusz Motylewski about 3 years ago

  • Status changed from Under Review to New
#14

Updated by Tymoteusz Motylewski about 3 years ago

Hi Andi
Thanks for the issue report and the patch.
In order to be able to fix issues like that, we need to cover mentioned case with tests.
We already have plenty of tests for extbase, see e.g.
typo3/sysext/extbase/Tests/Functional/Persistence/TranslatedContentTest.php
or
typo3/sysext/extbase/Tests/Functional/Persistence/QueryLocalizedDataTest.php
or
typo3/sysext/extbase/Tests/Functional/Persistence/TranslationTest.php

These tests are using "blog_example" fixture extension which is shipped with the core:
typo3/sysext/extbase/Tests/Functional/Fixtures/Extensions/blog_example

It would be very helpful if you could write a test which is fixed with your patch.
Or at last, describe the case using models from the blog_example.

Thanks a lot!

Also available in: Atom PDF