-
-
Notifications
You must be signed in to change notification settings - Fork 254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FEATURE] RFC: Enable dataProcessing on Results #2763
base: release-11.5.x
Are you sure you want to change the base?
[FEATURE] RFC: Enable dataProcessing on Results #2763
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please explain some exact use case for this stuff?
|
||
public function __construct() | ||
{ | ||
$this->contentDataProcessor = GeneralUtility::makeInstance(ContentDataProcessor::class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use Injector style, to be able to mock this stuff in tests.
@@ -113,6 +123,11 @@ public function resultsAction() | |||
]; | |||
|
|||
$values = $this->emitActionSignal(__CLASS__, __FUNCTION__, [$values]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this signal unusable for the things you want?
If there is not all what you want, pack this in array.
Would propose to extend all the signals and pack the stuff in the docs and examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Main difference is that Data Processing is configured via typoscript, allowing to plug different configs in places.
Instead of Signals which are global.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, i mean that we could plug this stuff via this signal[s], via something like "Facade/Gateway" or some other design pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ContentDataProcessor
only needs the ContentObjectRenderer
which we can get pretty much anywhere injection works. And the configuration from Typoscript of course.
So yeah we could plug it via the signals.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI: signal/slot is deprecated
https://docs.typo3.org/m/typo3/reference-coreapi/master/en-us/ApiOverview/Hooks/EventDispatcher/Index.html#eventdispatcher
c443899
to
88bf0d5
Compare
2bfc641
to
b3a9fef
Compare
b0a618c
to
ec66f49
Compare
caabbba
to
e4519e9
Compare
What this pr does
Typo3 7.4 Introduced Data Processing for the
FLUIDTEMPLATE
cObject. I have found it to be quite useful but it is not globally available in extensions.This PR Enables the feature on the Results Action and should be easy to be expanded to the other actions in the
SearchController
.Location of the
dataProcessing
block in the setup is just my first idea, and retrieval of the value probably should be packed into a getter.How to test
Configuration/TypoScript/Examples/DataProcessing/setup.txt
has an example for TypoScript setup. A DataProcessor has to be Provided for testing, either one of the ones Supplied by Typo3 or a custom one.