Support for getting the results for a location #38

bdunogier · 2020-05-28T09:12:00Z

Question	Answer
JIRA issue	EZP-31659
Type	improvement
Target eZ Platform version	`v2.x`, `v3.x`
BC breaks	no
Tests pass	yes/no
Doc needed	yes

Introduces a new QueryFieldLocationService that allow to get results / results slices / count results based on a location. With the previous methods, only mainLocation was available as a variable, since it was inferred from the content.

Now both can be available, and location will be set to the "current" location. In the context of a content view, it will be the requested location's. For REST and GraphQL, parameters will be added to specify which location should be used.

TODO

Implement PagerFantaAdapter
Implement in QueryResultsInjector
Implement in the REST controller
Implement in GraphQL

Checklist

PR description is updated.
Tests are implemented.
Added code follows Coding Standards (use $ composer fix-cs).
PR is ready for a review.

src/API/QueryFieldLocationService.php

src/API/QueryFieldService.php

src/API/QueryFieldServiceInterface.php

src/API/QueryFieldService.php

src/Controller/QueryFieldRestController.php

adamwojs · 2020-12-22T08:09:04Z

src/Controller/QueryFieldRestController.php

-            $items = $this->queryFieldService->loadContentItemsSlice($content, $fieldDefinitionIdentifier, $offset, $limit);
+            $location = null;
+            $content = $this->contentService->loadContent($contentId, null, $versionNumber);
+            if ($limit === -1 || !method_exists($this->queryFieldService, 'loadContentItemsSlice')) {


Suggested change

if ($limit === -1 || !method_exists($this->queryFieldService, 'loadContentItemsSlice')) {

if ($limit === -1) {

It's guaranteed that loadContentItemsSlicea exists as you expect an \EzSystems\EzPlatformQueryFieldType\API\QueryFieldService (final class) as QueryFieldRestController construct arg.

Well, indeed (it was a backward compatibility check when pagination was introduced).

But that change is not part of that pull-request, it is just moved around :)

src/API/QueryFieldService.php

src/API/QueryFieldServiceInterface.php

src/eZ/ContentView/QueryResultsInjector.php

src/Symfony/Resources/views/fieldtype/field_view.html.twig

bdunogier

I'm done fixing your comments. I'll push the code tomorrow morning, as I worked on the v3 sub-branch for the feature... :)

src/Controller/QueryFieldRestController.php

bdunogier · 2021-01-07T17:41:17Z

src/Controller/QueryFieldRestController.php

-            $items = $this->queryFieldService->loadContentItemsSlice($content, $fieldDefinitionIdentifier, $offset, $limit);
+            $location = null;
+            $content = $this->contentService->loadContent($contentId, null, $versionNumber);
+            if ($limit === -1 || !method_exists($this->queryFieldService, 'loadContentItemsSlice')) {


Well, indeed (it was a backward compatibility check when pagination was introduced).

But that change is not part of that pull-request, it is just moved around :)

src/API/QueryFieldServiceInterface.php

bdunogier · 2021-01-08T15:46:21Z

I think that's it, as long as Travis agrees.

bdunogier · 2021-01-11T08:35:22Z

Ping @ezsystems/php-dev-team.

bdunogier · 2021-01-12T08:12:24Z

All review comments have been fixed, can we move on with this ?

src/API/QueryFieldService.php

src/API/QueryFieldServiceInterface.php

src/Controller/QueryFieldRestController.php

src/eZ/ContentView/QueryResultsInjector.php

bdunogier · 2021-01-12T10:49:34Z

All fixed @Steveb-p (rebased into the correct commits).

Steveb-p

Apart from one minor comment, LGTM ;)

Steveb-p · 2021-01-12T10:56:40Z

src/API/QueryFieldLocationService.php

+    /**
+     * Returns the query results for the given location.
+     */
+    public function loadContentItemsForLocation(Location $location, string $fieldDefinitionIdentifier): iterable;
+
+    /**
+     * Returns a slice of the query results for the given location.
+     */
+    public function loadContentItemsSliceForLocation(Location $location, string $fieldDefinitionIdentifier, int $offset, int $limit): iterable;


Could those methods receive @return notes? I've seen them in concrete implementation, and I think they're even more important to be added here.

…location

…ation with REST An extra 'location' query parameter can be provided with a location href. If provided, that particular location will be used instead of the main one.

…nfiguration()

bdunogier · 2021-01-12T16:25:06Z

Updated again, and CS warning fixed.

bdunogier self-assigned this May 28, 2020

alongosz reviewed May 28, 2020

View reviewed changes

bdunogier force-pushed the ezp31659-results_for_location branch 2 times, most recently from b86885e to abd1f4f Compare June 2, 2020 18:09

bdunogier force-pushed the ezp31659-results_for_location branch from de9c945 to efc0146 Compare December 15, 2020 10:52

bdunogier mentioned this pull request Dec 16, 2020

Added location parameter to ez_render_field ezsystems/ezplatform-admin-ui#1669

Merged

2 tasks

bdunogier force-pushed the ezp31659-results_for_location branch 2 times, most recently from d7f318f to cb336e4 Compare December 21, 2020 14:40

bdunogier marked this pull request as ready for review December 21, 2020 14:43

bdunogier force-pushed the ezp31659-results_for_location branch from cb336e4 to a236233 Compare December 21, 2020 16:17

Steveb-p reviewed Dec 22, 2020

View reviewed changes

src/Controller/QueryFieldRestController.php Outdated Show resolved Hide resolved

adamwojs reviewed Dec 22, 2020

View reviewed changes

alongosz requested changes Dec 22, 2020

View reviewed changes

src/API/QueryFieldService.php Show resolved Hide resolved

src/API/QueryFieldServiceInterface.php Outdated Show resolved Hide resolved

src/eZ/ContentView/QueryResultsInjector.php Outdated Show resolved Hide resolved

Steveb-p reviewed Jan 7, 2021

View reviewed changes

src/Symfony/Resources/views/fieldtype/field_view.html.twig Outdated Show resolved Hide resolved

bdunogier commented Jan 7, 2021

View reviewed changes

bdunogier mentioned this pull request Jan 8, 2021

Added the location parameter to ez_render_field ezsystems/ezplatform-kernel#156

Merged

6 tasks

bdunogier requested a review from alongosz January 8, 2021 15:46

bdunogier force-pushed the ezp31659-results_for_location branch from c194b75 to 7e0aa71 Compare January 11, 2021 08:43

Steveb-p requested changes Jan 12, 2021

View reviewed changes

bdunogier force-pushed the ezp31659-results_for_location branch from 7e0aa71 to 1482904 Compare January 12, 2021 10:48

Steveb-p approved these changes Jan 12, 2021

View reviewed changes

Bertrand Dunogier added 5 commits January 12, 2021 17:24

EZP-31659: Added QueryFieldLocationService for getting results for a …

c0e9cc4

…location

EZP-31659: use location methods from QueryResultsInjector

f9dc06c

Added PaginationSupport

d60a807

Use objects as view controller arguments

f6a6783

EZP-31659: Implemented getting query field results for a specific loc…

6e89f2e

…ation with REST An extra 'location' query parameter can be provided with a location href. If provided, that particular location will be used instead of the main one.

Bertrand Dunogier added 4 commits January 12, 2021 17:24

Signature fix

ee16d14

Fixed typehint and return value of QueryFieldService::getPaginationCo…

8db17a3

…nfiguration()

Removed unused PhpDoc

730673a

Readability improvements in QueryResultsInjector

9bcdc32

bdunogier force-pushed the ezp31659-results_for_location branch from 1482904 to 9bcdc32 Compare January 12, 2021 16:24

lserwatka merged commit e3de453 into master Jan 13, 2021

lserwatka deleted the ezp31659-results_for_location branch January 13, 2021 14:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for getting the results for a location #38

Support for getting the results for a location #38

bdunogier commented May 28, 2020 •

edited

Loading

adamwojs Dec 22, 2020

bdunogier Jan 7, 2021

bdunogier left a comment

bdunogier Jan 7, 2021

bdunogier commented Jan 8, 2021

bdunogier commented Jan 11, 2021

bdunogier commented Jan 12, 2021

bdunogier commented Jan 12, 2021

Steveb-p left a comment

Steveb-p Jan 12, 2021

bdunogier Jan 12, 2021

bdunogier commented Jan 12, 2021

	if ($limit === -1 \|\| !method_exists($this->queryFieldService, 'loadContentItemsSlice')) {
	if ($limit === -1) {

Support for getting the results for a location #38

Support for getting the results for a location #38

Conversation

bdunogier commented May 28, 2020 • edited Loading

TODO

Checklist

adamwojs Dec 22, 2020

Choose a reason for hiding this comment

bdunogier Jan 7, 2021

Choose a reason for hiding this comment

bdunogier left a comment

Choose a reason for hiding this comment

bdunogier Jan 7, 2021

Choose a reason for hiding this comment

bdunogier commented Jan 8, 2021

bdunogier commented Jan 11, 2021

bdunogier commented Jan 12, 2021

bdunogier commented Jan 12, 2021

Steveb-p left a comment

Choose a reason for hiding this comment

Steveb-p Jan 12, 2021

Choose a reason for hiding this comment

bdunogier Jan 12, 2021

Choose a reason for hiding this comment

bdunogier commented Jan 12, 2021

bdunogier commented May 28, 2020 •

edited

Loading