Bug #52245

Revert BackendUtility::BEenableFields in FAL

Added by Fabien Udriot almost 6 years ago. Updated almost 6 years ago.

Status:
Rejected
Priority:
Should have
Assignee:
-
Category:
File Abstraction Layer (FAL)
Target version:
Start date:
2013-09-25
Due date:
% Done:

0%

TYPO3 Version:
6.2
PHP Version:
Tags:
Complexity:
medium
Is Regression:
No
Sprint Focus:

Description

Quite recently, in https://review.typo3.org/#/c/23742/, method
BackendUtility::BEenableFields was introduced in FAL.

This has a drawback for third party extensions which are introducing
additional enabledFields for a File. As instance, Media has the "hidden"
flag enabled. Consequently, FAL is not anymore capable of finding a
File that has hidden = 1 ; $fileRepository->findByUid(123) is raising
exception #1314354065 whenever file "123" is marked as hidden.

Since we don't have Query Settings like in Extbase, the patch
removes this constraint following the List module practice which
displays all records except the deleted ones.

Releases: 6.0, 6.1, 6.2
Fixes: #52236


Related issues

Related to Media Management - Bug #52236: Fatal error when record is hidden Resolved 2013-09-24
Related to TYPO3 Core - Bug #51562: sys_file records marked as deleted crash file list (AKA: remove sys_file.deleted flag) Closed 2013-08-30

History

#1 Updated by Gerrit Code Review almost 6 years ago

  • Status changed from New to Under Review

Patch set 1 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/24040

#2 Updated by Gerrit Code Review almost 6 years ago

Patch set 2 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/24040

#3 Updated by Gerrit Code Review almost 6 years ago

Patch set 3 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/24040

#4 Updated by Gerrit Code Review almost 6 years ago

Patch set 4 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/24040

#5 Updated by Gerrit Code Review almost 6 years ago

Patch set 5 for branch master has been pushed to the review server.
It is available at https://review.typo3.org/24040

#6 Updated by Markus Klein almost 6 years ago

I guess this approach will break extensions, relying on this behaviour.
What if you need the "not-enabled" records in FE too?

I think we should add a possibility to modify this behaviour on the fly by introducing some new API.

#7 Updated by Fabien Udriot almost 6 years ago

I guess this approach will break extensions, relying on this behaviour.
What if you need the "not-enabled" records in FE too?

FE? Please read carefully the commit message. This patch only addresses the BE (and does not touch the FE in any means).

Look at the method getWhereClauseForEnabledFields in this patch which clearly makes the distinction btw BE and FE.
https://review.typo3.org/#/c/23742/3/typo3/sysext/core/Classes/Resource/AbstractRepository.php

Furthermore, before the patch was merged (14 days ago) the behaviour was the opposite and is currently breaking Media @see #52236

I think we should add a possibility to modify this behaviour on the fly by introducing some new API.

As somehow expressed in the commit message, we could have this but it is the kind of thing that requires time. To quickly have Media fixed (and other extensions if they use this in the BE), I would prefer to have the default behaviour of the List module: list everything regardless of starttime, endtime, disable except the deleted flag.

#8 Updated by Fabien Udriot almost 6 years ago

This issue can be abandoned. It has been discussed that enable fields don't suit in the FAL concept where a file record is a strict representation of a file. With the new "sys_file_metadata" in 6.2, some fields like "visible" have been introduced for controlling visibility on the FE by a gallery plugin as example.

#9 Updated by Steffen Ritter almost 6 years ago

  • Status changed from Under Review to Rejected

Also available in: Atom PDF