#198 Feature/add graphql dynamic filters introspection #223

kevin-mitchell · 2020-03-11T21:13:18Z

New feature added allowing for dynamic filtering (or "faceting") of attributes on search and product list pages.

Description

This PR represents the work required to provide filtering functionality on the search and product list pages, powered by Magento 2.3.4's new GraphQL filtering functionality.

There are quite a few changes here a number of key items to call out.

SearchResultsService

The SearchResultsService is intended to be useful way of querying for Magento products that acts as the translation layer between "frontend" code / Sling Models / etc and the GraphQL query layer. The hopeful goal of this class is to make it usable in a number of potential different cases - it's original use was in the SearchResults component, however it was easily incorporated into ProductList as well.

DRY-ness

These changes were originally made specifically for search results, NOT for the category pages (aka ProductList and related). Later, we needed to support the filtering on the category page as well. As mentioned above, the flexibility of the SearchResultsService and the fact that the category page is VERY similar to the search results made this fairly simple, HOWEVER the result is that there is quite a bit of code that is nearly identical between the two components SearchResults and ProductList.

There are enough differences between the two designs that I didn't want to totally steamroll over the ProductList component but it would likely be possible to refactor to reduce some of the code duplication at some point.

Certain aspects of the category page vs search are unique, e.g. category specific images and other marketing related features made possible by the differences in ProductList.

Dependency on GraphQL Introspection additions

The PR "#91 add css rules for search results component relating to filter bar" has changes that are REQUIRED for this PR to land. Most likely we'll need to bump version numbers somewhere in here to make sure that this change lands in such a way that the updated GraphQL introspection changes are available.

Related Issue

#198

Motivation and Context

Motivated by Magento 2.3.4 and it's ability to dynamically filter search results.

How Has This Been Tested?

There are a large number of unit tests that have been added as part of this change. Where possible I've used the converter pattern when changing GraphQL layer data to Sling Model friendly POJOs and the like. This makes testing quite a bit easier.

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I have signed the Adobe Open Source CLA.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes and the overall coverage did not decrease.
All unit tests pass on CircleCi.
I ran all tests locally and they pass.

kevin-mitchell · 2020-03-12T10:58:18Z

...va/com/adobe/cq/commerce/core/components/internal/models/v1/productlist/ProductListImpl.java

@@ -15,10 +15,10 @@
 package com.adobe.cq.commerce.core.components.internal.models.v1.productlist;


The changes made to this class are almost entirely in an effort to port the functionality from the SearchResults component here, so that essentially the same functionality available in search is available on the product listing pages (basically the category pages)

kevin-mitchell · 2020-03-12T11:00:05Z

...es/core/src/main/java/com/adobe/cq/commerce/core/components/client/MagentoGraphqlClient.java

@@ -29,7 +29,9 @@
 import com.adobe.cq.commerce.graphql.client.RequestOptions;


Changes to this file are made so that introspection queries can be made. If memory serves originally I had created a separate MagentoIntrospectionGraphqlClient or something similar but the team thought it would be better to just have the existing class handle both queries types (with the default behavior remaining in tact)

kevin-mitchell · 2020-03-12T11:02:26Z

...om/adobe/cq/commerce/core/components/internal/models/v1/searchresults/SearchResultsImpl.java

@@ -11,14 +11,12 @@
 *    governing permissions and limitations under the License.


This was refactored to use the SearchResultsService, which hopefully hides most of the details about how to actually query for search results.

kevin-mitchell · 2020-03-12T11:03:41Z

...core/src/main/java/com/adobe/cq/commerce/core/components/models/productlist/ProductList.java

@@ -71,21 +71,15 @@
    @Nullable
    String getTitle();

-    int getTotalCount();


Here again an attempt was made to DRY things out a bit by moving all pagination / search result information and pagination functionality into the search result class(es).

kevin-mitchell · 2020-03-12T11:06:18Z

.../src/main/java/com/adobe/cq/commerce/core/components/models/searchresults/SearchResults.java

+    /**
+     * Name of the String resource property indicating number of products to render on front-end.
+     */
+    String PN_PAGE_SIZE = "pageSize";


Some of this functionality was borrowed from the ProductList component to allow the same authoring options (at least those that are relevant for search result pages)

codecov · 2020-03-12T11:42:38Z

Codecov Report

Merging #223 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #223   +/-   ##
=========================================
  Coverage     62.28%   62.28%           
  Complexity      724      724           
=========================================
  Files           171      171           
  Lines          5236     5236           
  Branches        817      817           
=========================================
  Hits           3261     3261           
  Misses         1864     1864           
  Partials        111      111

Flag	Coverage Δ	Complexity Δ
#jest	`40.21% <0.00%> (ø)`	`0.00% <0.00%> (ø%)`
#karma	`94.88% <0.00%> (ø)`	`0.00% <0.00%> (ø%)`
#unittests	`84.74% <0.00%> (ø)`	`724.00% <0.00%> (ø%)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a4a5e7c...a4a5e7c. Read the comment docs.

...jcr_root/apps/core/cif/components/commerce/searchresults/v1/searchresults/paginationbar.html

...ent/jcr_root/apps/core/cif/components/commerce/productlist/v1/productlist/paginationbar.html

Ran `mvn clean install -Pformat-code -s ~/.m2/aem-settings.xml`

I failed to realize I didn't remove the parameters from the method signature

…ary.

Most of the changes here are cosmetic, but I did add a few new classes to support pagination / removing pagination logic from the search results set itself. This greatly simplifies each class and makes it a lot easier to test the pagination logic itself.

kevin-mitchell · 2020-04-06T19:50:48Z

.../com/adobe/cq/commerce/core/components/internal/models/v1/productlist/CategoryRetriever.java


 class CategoryRetriever extends AbstractCategoryRetriever {

    CategoryRetriever(MagentoGraphqlClient client) {
        super(client);
    }

-    private ProductPriceQueryDefinition generatePriceQuery() {


This class no longer is responsible for generating product searches, this is all being handled in the search service

kevin-mitchell · 2020-04-06T19:55:43Z

...n/java/com/adobe/cq/commerce/core/components/models/retriever/AbstractCategoryRetriever.java

-     *
-     * @param productQueryHook Lambda that extends the product query
-     */
-    public void extendProductQueryWith(Consumer<ProductInterfaceQuery> productQueryHook) {


I moved this same functionality into the SearchService, and the extended product query hook is supplied as a parameter in the search method.

kevin-mitchell · 2020-04-06T19:57:21Z

...om/adobe/cq/commerce/core/components/internal/models/v1/productlist/ProductListImplTest.java

-        }
-    }
-
-    @Test


These tests (or similar tests) have been moved to the Pager class which is responsible for computing pagination.

...va/com/adobe/cq/commerce/core/components/internal/models/v1/productlist/ProductListImpl.java

.../core/src/main/java/com/adobe/cq/commerce/core/search/internal/models/SearchOptionsImpl.java

* I've reverted the parseCategoryId call to use the current (`master`) version of this method without the Optional or value checking. * I also remove the call that sanitized the search term query string per PR feedback (it's likely better to allow this sort of heavy handed decision to be decided by the implementer or allow Magento's API to respond appropriately if strange search terms are used)

- removed usage of "selectors" to build urls - remove duplicate html templates

… content

- fixed issue when product cannot be converted

- modified code and added tests to support missing introspection results

…pection # Conflicts: # bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/models/v1/productlist/ProductListImpl.java # bundles/core/src/main/java/com/adobe/cq/commerce/core/components/internal/models/v1/searchresults/SearchResultsImpl.java # bundles/core/src/test/java/com/adobe/cq/commerce/core/components/internal/models/v1/productlist/ProductListImplTest.java # bundles/core/src/test/java/com/adobe/cq/commerce/core/components/internal/models/v1/searchresults/SearchResultsImplTest.java # ui.apps/src/main/content/jcr_root/apps/core/cif/components/commerce/productlist/v1/productlist/paginationbar.html

- enforce non empty search term in search component - only display pagination when there are products

herzog31 · 2020-04-16T12:30:46Z

bundles/core/src/main/java/com/adobe/cq/commerce/core/search/models/Pager.java

+    Map<String, String> getPreviousPageParameters();
+
+    /**
+     * Get the mext page parameters for supporting linking to the mext page.


Typo mext.

herzog31 · 2020-04-16T13:16:39Z

...va/com/adobe/cq/commerce/core/components/internal/models/v1/productlist/ProductListImpl.java

+    @Override
+    public SearchResultsSet getSearchResultsSet() {
+        if (searchResultsSet == null) {
+            searchResultsSet = searchResultsService.performSearch(searchOptions, resource, productPage, request);


Setting a query hook for the product query is supported via the 5th argument of the performSearch search method. However, currently a customer has no way of using it in any way using the Sling model delegation pattern. Replacing the whole query is also not possible.

The same applies for the SearchResults component.

Right I missed that, I had this in my radar and then forgot to check it ... We definitely have to support this.

…pection

kevin-mitchell changed the title ~~Feature/add graphql dynamic filters introspection~~ #198 Feature/add graphql dynamic filters introspection Mar 11, 2020

kevin-mitchell commented Mar 12, 2020

View reviewed changes

mhaack added the feature New feature or request label Mar 23, 2020

dplaton reviewed Mar 25, 2020

View reviewed changes

...jcr_root/apps/core/cif/components/commerce/searchresults/v1/searchresults/paginationbar.html Outdated Show resolved Hide resolved

LSantha reviewed Mar 25, 2020

View reviewed changes

...ent/jcr_root/apps/core/cif/components/commerce/productlist/v1/productlist/paginationbar.html Outdated Show resolved Hide resolved

id-mitchell and others added 20 commits April 3, 2020 11:09

add dynamic filtering of product attributes with new search service

336d5b1

Fix unit tests

e827257

Fix test and remove unused file

0c0a592

Fix code style

8a7fc9e

Ran `mvn clean install -Pformat-code -s ~/.m2/aem-settings.xml`

add component template files and fix some rebase issues

d084f62

fix parent pom

7a5587d

resolve merge issues and add some styling specific changes classes

48d09fb

Break out search filter service as separate service and remove page

8fd5b99

move to converter pattern instead of adapting graphql models

4f2ee1d

add service calls for retrieving attribute metadata

a7f274f

move code to more organized package structure for services and models

0618bc7

fix previous and next page buttons on pagination bar

89f94f2

I failed to realize I didn't remove the parameters from the method signature

adding a bunch of unit tests and fix some link issues

03c84d2

Fix issue with category id selector being an Optional and not a String

e6bdc3f

add some additional tests and basic code cleanup

99069f9

add some missing comments and add page size and live prices options

0a33900

change graphql and connector versions back to original

0e4a8c2

add two additional tests and some missing comments

fa0dafd

Changes to use the introspection features of the magento-graphql libr…

022bd68

…ary.

Minor code formatting

d7df933

kevin-mitchell force-pushed the feature/add-graphql-dynamic-filters-introspection branch from 82294c5 to 5f2ee2c Compare April 3, 2020 11:55

kevin-mitchell commented Apr 6, 2020

View reviewed changes

cjelger reviewed Apr 14, 2020

View reviewed changes

...va/com/adobe/cq/commerce/core/components/internal/models/v1/productlist/ProductListImpl.java Outdated Show resolved Hide resolved

cjelger reviewed Apr 14, 2020

View reviewed changes

.../core/src/main/java/com/adobe/cq/commerce/core/search/internal/models/SearchOptionsImpl.java Outdated Show resolved Hide resolved

id-mitchell and others added 7 commits April 14, 2020 14:18

- removed obsolete code and refactored some common constants

5cb8eb0

- removed usage of "selectors" to build urls - remove duplicate html templates

- let searchresults inherit from productlist and removed duplicate js…

777a9f5

… content

Missing change

647973f

- changed unit tests to use mocked Magento JSON responses

843f47f

- fixed issue when product cannot be converted

- fixed broken tests with java11

89c5717

- modified code and added tests to support missing introspection results

cjelger requested review from herzog31 and mhaack April 16, 2020 11:06

- fixed broken productlist "placeholder" data in editor

39df164

- enforce non empty search term in search component - only display pagination when there are products

herzog31 reviewed Apr 16, 2020

View reviewed changes

- re-enable support for product query customisations

62c6a13

herzog31 approved these changes Apr 17, 2020

View reviewed changes

cjelger added 2 commits April 17, 2020 10:33

Merge branch 'master' into feature/add-graphql-dynamic-filters-intros…

5f0afed

…pection

Merge branch 'master' into feature/add-graphql-dynamic-filters-intros…

af2331e

…pection

cjelger added the To Verify label Apr 22, 2020

mhaack added 3 commits April 23, 2020 15:12

Merge branch 'master' into feature/add-graphql-dynamic-filters-intros…

0f699ea

…pection

CIF-1085 - update search result readme

2a0ea95

CIF-1085 - update search result readme

a4a5e7c

mhaack merged commit c0c9080 into adobe:master Apr 24, 2020

mhaack added verified and removed To Verify labels Apr 24, 2020

mhaack mentioned this pull request Apr 24, 2020

Feature Request: Allow dynamic filtering of search results #198

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#198 Feature/add graphql dynamic filters introspection #223

#198 Feature/add graphql dynamic filters introspection #223

kevin-mitchell commented Mar 11, 2020

kevin-mitchell Mar 12, 2020

kevin-mitchell Mar 12, 2020

kevin-mitchell Mar 12, 2020

kevin-mitchell Mar 12, 2020

kevin-mitchell Mar 12, 2020

codecov bot commented Mar 12, 2020 •

edited

Loading

kevin-mitchell Apr 6, 2020

kevin-mitchell Apr 6, 2020

kevin-mitchell Apr 6, 2020

herzog31 Apr 16, 2020

herzog31 Apr 16, 2020

cjelger Apr 16, 2020

		@@ -15,10 +15,10 @@
		package com.adobe.cq.commerce.core.components.internal.models.v1.productlist;

		@@ -29,7 +29,9 @@
		import com.adobe.cq.commerce.graphql.client.RequestOptions;

		@@ -11,14 +11,12 @@
		* governing permissions and limitations under the License.

-                      }
-                  }
-                  @Test

#198 Feature/add graphql dynamic filters introspection #223

#198 Feature/add graphql dynamic filters introspection #223

Conversation

kevin-mitchell commented Mar 11, 2020

Description

SearchResultsService

DRY-ness

Dependency on GraphQL Introspection additions

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Mar 12, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Mar 12, 2020 •

edited

Loading