[BUGFIX] Category tree when editing a news record no longer filters b…

[derhecht]

…y storage Pid #58

[LeoniePhiline]

Could you describe your approach in words? How are you approaching this differently than me in #59 ?
It is pretty hard to see what you are intending, since the entire pull request is cluttered with white-space and formatting changes.

Issues:

1. In NewsDatabaseTreeDataProvider::__construct() you still use BackendUtility::getTCEFORM_TSconfig()['_STORAGE_PID'], which will never ever give you a storage pid anymore. You'd always get 0.
   I explained this at my initial comment at [BUGFIX] #58 Respect storage_pid if useStoragePid is enabled in EXTCONF #59:

\TYPO3\CMS\Backend\Utility\BackendUtility::getTCEFORM_TSconfig() can no
longer be used for determining the storage_pid, since this field is no
longer loaded by the internally used
\TYPO3\CMS\Backend\Utility\BackendUtility::getPageForRootline().

We are now figuring out the storage Pid ourselves, by traversing the
rootline.

This why I introduced and used tx_ttnews_div::getStoragePid() (See: https://github.com/rupertgermann/tt_news/pull/59/files#diff-5d6751ebc5d47e17180534532a594ad6R225)

2. Even if compatibility6 is installed, you'd still not get a storage pid out of BackendUtility::getTCEFORM_TSconfig(). Still you'll always get 0 for your $tceTSC['_STORAGE_PID'].

3. You disagreed with the need of compatibility6, but you still use the pages.storage_pid field, without defining it yourself (SQL + TCA), so you still need compatibility6.

4. It makes sense that pages.storage_pid is removed (https://docs.typo3.org/typo3cms/extensions/core/Changelog/7.4/Deprecation-65790-PagesStoragePidDeprecated.html). You could use e.g. a pageTSConfig option to replace it. Users with a legacy setup could easily transition their pages.storage_pid fields into a tt_news.categoryStoragePid tsconfig setting.

5. Your div::getCatListWhere() (4098086?w=1#diff-5d6751ebc5d47e17180534532a594ad6R185) still only adds the storage pid constraint users who are not BE admins.
   I intentionally changed that - adding the storage pid constraint for all users, regardless of their admin status (https://github.com/rupertgermann/tt_news/pull/59/files#diff-5d6751ebc5d47e17180534532a594ad6R182), since it simply makes no sense to me the way it is in master and [BUGFIX] Category tree when editing a news record no longer filters b… #60 and looks like an accident.
   It absolutely does make sense to use $excludeArray and $includeCatList only for non-admins, since admins should always be able to see all records without restriction;
   but the pid setting configures not an access restriction but a source definition. Admin or not admin - you'll always want to have the right set of categories in your view.

[derhecht]

regarding your 1.: getTCEFORM_TSconfig() fills _STORAGE_PID if so configured in TS, who cares for getPageForRootline here? => I check in TYPO3 7.6 core!
3. storage_pid is used? Where?
5. ok, here I follow you and updated the change.

[LeoniePhiline]

ad 1. & ad. 3.:

Did you actually test this to base your claims on? Not working for me.
How does this need to look like in your pageTSConfig?
... I think we should look into the core together once more.

both of these cannot work:
TCEFORM.tt_news_cat._STORAGE_PID = 1348
or
TCEFORM.tt_news_cat._STORAGE_PID.types.0 = 1348

Do you define this any different? I guess you cannot, because as far as I can see, you cannot possibly set your $tceTSC['_STORAGE_PID'] from pagesTSconfig.

Let's have a look at the core: https://github.com/TYPO3/TYPO3.CMS/blob/TYPO3_7-6/typo3/sysext/backend/Classes/Utility/BackendUtility.php#L3833

Assume we define TCEFORM.tt_news_cat._STORAGE_PID = 1348 and call \TYPO3\CMS\Backend\Utility\BackendUtility::getTCEFORM_TSconfig():

We will get a $tempConf with $tempConf['value'] = null and $tempConf['properties']['_STORAGE_PID'] = 1348.

In line 3844 (https://github.com/TYPO3/TYPO3.CMS/blob/TYPO3_7-6/typo3/sysext/backend/Classes/Utility/BackendUtility.php#L3844) we will hop out, because $val is 1348, and is_array(1348) fails.
So, at no point is a value of $tempConf written into $res.

Since the code indicates the 'types.' configuration could be used, I tried TCEFORM.tt_news_cat._STORAGE_PID.types.0 = 1348.
But then still in line 3848 (https://github.com/TYPO3/TYPO3.CMS/blob/TYPO3_7-6/typo3/sysext/backend/Classes/Utility/BackendUtility.php#L3848), is_array($val['types.'][$typeVal . '.']) will fail.
So, I can still try to also make .types.0 an array, like TCEFORM.tt_news_cat._STORAGE_PID.types.0._STORAGE_PID = 1348 -- but then I end up with a $res['_STORAGE_PID'] = array('_STORAGE_PID' => 1348);

This is not what you are reading in the extension. What you expect is $res['_STORAGE_PID'] = 1348.

So - do you see any way how to set that in pageTSConfig?

The usual way used to be to edit the page record and select a page at "General storage page":

At line 3859 (https://github.com/TYPO3/TYPO3.CMS/blob/TYPO3_7-6/typo3/sysext/backend/Classes/Utility/BackendUtility.php#L3859),` self::BEgetRootLine()is called, which in line 347 (https://github.com/TYPO3/TYPO3.CMS/blob/TYPO3_7-6/typo3/sysext/backend/Classes/Utility/BackendUtility.php#L347) calledself::getPageForRootline()`.

Then, in line 401, each page record for rootline is loaded (https://github.com/TYPO3/TYPO3.CMS/blob/TYPO3_7-6/typo3/sysext/backend/Classes/Utility/BackendUtility.php#L401), but as you see, the DB field pages.storage_pid (which has been moved into compatibility6) is not anymore loaded.

To compare (and for you to see why this is no longer working):
In TYPO3 6.2, see here https://github.com/TYPO3/TYPO3.CMS/blob/TYPO3_6-2/typo3/sysext/backend/Classes/Utility/BackendUtility.php#L370 , pages.storage_pid was still loaded.

Now at the point of line 3860 (https://github.com/TYPO3/TYPO3.CMS/blob/TYPO3_7-6/typo3/sysext/backend/Classes/Utility/BackendUtility.php#L3860), $res still definitely has no _STORAGE_PID from $tempConf, and we now have an array $rootline.
Each of its elements ($rC) will NOT have a $rC['storage_pid'] (since it's no loaded in getPageForRootline()), so $res['_STORAGE_PID'] = (int)$rC['storage_pid']; will always be 0.

qed - do you disagree?

[LeoniePhiline]

PS: Same issue here: https://github.com/rupertgermann/tt_news/blob/master/pi/class.tx_ttnews.php#L3592
calls $this->tsfe->getStorageSiterootPids().
TSFE uses $this->rootline which is filled by $this->sys_page->getRootLine(), which again uses RootlineUtility::get().
In RootlineUtility you find the following:

/** * Fields to fetch when populating rootline data * * @var array */ protected static $rootlineFields = [ 'pid', 'uid', 't3ver_oid', 't3ver_wsid', 't3ver_state', 'title', 'alias', 'nav_title', 'media', 'layout', 'hidden', 'starttime', 'endtime', 'fe_group', 'extendToSubpages', 'doktype', 'TSconfig', 'tsconfig_includes', 'is_siteroot', 'mount_pid', 'mount_pid_ol', 'fe_login_mode', 'backend_layout_next_level' ];

As you see - no 'storage_pid'.

Even more: in tx_ttnews::initCategoryVars() (https://github.com/rupertgermann/tt_news/blob/master/pi/class.tx_ttnews.php#L3590) not even an attempt is made to load anything from pageTSconfig.
So at least now it should be clear to you that only the DB field pages.storage_pid was ever relevant for tt_news' feature 'useStoragePid'.

3. storage_pid is used? Where?

Everywhere, when it comes to extConf 'useStoragePid' in tt_news. :)

[LeoniePhiline]

Updated:

**See #64 **

• Now complete with warning if pages.storage_pid is not configured.
• Now also the frontend category storage pid is fixed.

For you to consider: You can define pages.storage_pid in tt_news (ext_tables.sql, Configuration/TCA/Overrides/pages.php) to be independent of compatibility6.

However, in that case, I'd rather add not pages.storage_pid, but e.g. pages.tx_ttnews_cat_storage_pid with a proper label (indicating that the field is for configuring the storage pid of tt_news categories only), so that it is clear that other extensions will and should not be using this field.

Then, you can change https://github.com/LeoniePhiline/tt_news/blob/bugfix/58_storagePid_for_categoryTree/lib/class.tx_ttnews_div.php#L239 to use $record['tx_ttnews_cat_storage_pid'] instead.

You can add an update script for easier transition:
UPDATE pages SET tx_ttnews_cat_storage_pid=storage_pid WHERE storage_pid != 0 AND deleted=0

If you do want to use some kind of TSconfig solution, then you have a problem: It's for the Backend.
If you use TS setup (not tsconfig) plugin.tx_ttnews.*, then you will jump through hoops to read the TS setup in the Backend and get the rootline configuration right.
I really recommend using a DB field as it used to be.

[LeoniePhiline]

For you to consider: You can define pages.storage_pid in tt_news (ext_tables.sql, Configuration/TCA/Overrides/pages.php) to be independent of compatibility6.

However, in that case, I'd rather add not pages.storage_pid, but e.g. pages.tx_ttnews_cat_storage_pid with a proper label (indicating that the field is for configuring the storage pid of tt_news categories only), so that it is clear that other extensions will and should not be using this field.

Then, you can change https://github.com/LeoniePhiline/tt_news/blob/bugfix/58_storagePid_for_categoryTree/lib/class.tx_ttnews_div.php#L239 to use $record['tx_ttnews_cat_storage_pid'] instead.

You can add an update script for easier transition:
UPDATE pages SET tx_ttnews_cat_storage_pid=storage_pid WHERE storage_pid != 0 AND deleted=0

Implemented in #65

Tested and works.