Added a filter to sort categories by position by default #29301

Dnyomo · 2020-07-28T00:23:33Z

Description (*)

This fixed a bug where the default GraphQL Category method does not sort by category position. By adding this filter results are returned in the correct order.

Related Pull Requests

magento/catalog-storefront#173

Fixed Issues (if relevant)

Fixes Default sort order catalog-storefront#104

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds are green)

m2-assistant · 2020-07-28T00:23:35Z

Hi @Dnyomo. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE,
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

Dnyomo · 2020-07-28T00:24:01Z

@magento run WebAPI Tests

Dnyomo · 2020-07-28T11:37:54Z

@magento run WebAPI Tests

mslabko · 2020-07-28T20:07:53Z

app/code/Magento/CatalogGraphQl/Model/Category/CategoryFilter.php

@@ -71,6 +72,7 @@ public function getResult(array $criteria, StoreInterface $store)
        $categoryIds = [];
        $criteria[Filter::ARGUMENT_NAME] = $this->formatMatchFilters($criteria['filters'], $store);
        $criteria[Filter::ARGUMENT_NAME][CategoryInterface::KEY_IS_ACTIVE] = ['eq' => 1];
+        $criteria[Sort::ARGUMENT_NAME][CategoryInterface::KEY_POSITION] = ['ASC'];


@Dnyomo can you please verify and confirm that for categories (e.g in menu) also have a sort order by position ASC ?

…CriteriaBuilder.php Remove the _id from the order that is being passed to the product collection in ProductSearch.php Updated the position to be DESC passed to the product collection in ProductSearch.php Updated the ProductSearchTest.php to have the items in the attended order

Dnyomo · 2020-07-30T19:43:31Z

@magento run WebAPI Tests, Integration Tests

mslabko · 2020-07-30T20:56:16Z

app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php

    {
        $ordersArray = [];
        $sortOrders = $searchCriteria->getSortOrders();
        if (is_array($sortOrders)) {
            foreach ($sortOrders as $sortOrder) {
-                $ordersArray[$sortOrder->getField()] = $sortOrder->getDirection();
+                if ($sortOrder->getField() !== '_id') {


Why do we need this check? It will ignore any sort order defined by user in GQL query

mslabko · 2020-07-30T21:22:43Z

app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php

@@ -104,6 +104,7 @@ public function build(array $args, bool $includeAggregation): SearchCriteriaInte
            $this->addDefaultSortOrder($searchCriteria, $isSearch);
        }

+        $this->addEntityIdSort($searchCriteria, $isSearch);


could you please verify the behavior for GQL query which will filter by category id/ids, e.g.

{ products(filter: {category_id: {in: ["3", "5"]}}) { items { sku } } } { products(filter: {category_id: {eq:"3"}}) { items { sku } } }

In these examples we should add order by "position ASC" only for the second case.
And only if the sort was provided explicitly in GQL query it should be added as is.

I'm afraid that current implementation in addDefaultSortOrder method don't take this into account

I think we dont' have specific tests for that and it make sense to add this for GQL coverage
(@yaroslavGoncharuk can you help us with steps?)

I did test this and because this method (\Magento\Catalog\Model\ResourceModel\Product\Collection::addAttributeToSort line 1749) converts the attribute position to entity_id. I think it makes sense to always have a position set as a sort.

Dnyomo · 2020-07-31T14:22:25Z

@magento run WebAPI Tests

Dnyomo · 2020-07-31T17:14:56Z

@magento run WebAPI Tests

Dnyomo · 2020-07-31T17:51:40Z

@magento run WebAPI Tests

yaroslavGoncharuk · 2020-08-01T01:54:57Z

@Dnyomo Query with condition eq returns "Internal server error"

{
  products(filter: {category_id: {eq:"3"}}) {
   items {    
      sku
    }
  }
}

Works well if eq is replaced by in OR sort: {price: ASC} argument is added

…array. Reverted the tests back to check on products

Dnyomo · 2020-08-02T14:00:13Z

@magento run WebAPI Tests

…array. Reverted the tests back to check on products

Dnyomo · 2020-08-02T17:28:10Z

@magento run WebAPI Tests

…er_b

mslabko · 2020-08-03T16:21:05Z

@magento run all tests

mslabko · 2020-08-03T22:37:40Z

@Dnyomo can you please check static tests?
https://github.com/magento/magento2/runs/941354354

mslabko · 2020-08-03T22:42:01Z

@magento run Functional Tests CE,Functional Tests B2B

mslabko · 2020-08-04T14:59:07Z

@magento run Functional Tests B2B

…m:Dnyomo/magento2 into storefront-issue_104_default_sort_order_b

Dnyomo · 2020-08-04T17:20:02Z

@magento run Static Tests

Dnyomo · 2020-08-04T18:06:11Z

@magento run Static Tests

Dnyomo · 2020-08-04T19:05:49Z

@magento run Static Tests

Dnyomo · 2020-08-04T19:12:04Z

@magento run Static Tests

Dnyomo · 2020-08-04T19:15:30Z

@magento run Static Tests

…er_b

mslabko · 2020-08-04T19:54:54Z

@magento run all tests

mslabko · 2020-08-05T03:50:47Z

@magento run Functional Tests B2B

Added a filter to sort categories by position by default

605d847

ghost added the Progress: pending review label Jul 28, 2020

magento-engcom-team added Component: CatalogGraphQl Release Line: 2.4 Partner: Briteskies partners-contribution Pull Request is created by Magento Partner labels Jul 28, 2020

Test Fixes for the new filter that sorts categories by position

262445f

mslabko reviewed Jul 28, 2020

View reviewed changes

ghost assigned mslabko Jul 28, 2020

dnyomo added 2 commits July 30, 2020 15:17

mslabko suggested changes Jul 30, 2020

View reviewed changes

ghost added Progress: needs update and removed Progress: pending review labels Jul 30, 2020

Updated tests to cover changes to the sort orders

3400017

Remove the position from the filter and replaced it with Entity Id

aad9bff

Removed bad code that was not needed

30c79f3

Updated the default sort to check if the category id is passed as an …

c137e0e

…array. Reverted the tests back to check on products

Updated the default sort to check if the category id is passed as an …

34cf76d

…array. Reverted the tests back to check on products

Updated test

d3aa6b6

Merge branch '2.4-develop' into storefront-issue_104_default_sort_ord…

f02b7ef

…er_b

dnyomo added 2 commits August 4, 2020 13:15

Code style fixes

056cb38

Merge branch 'storefront-issue_104_default_sort_order_b' of github.co…

d0ac893

…m:Dnyomo/magento2 into storefront-issue_104_default_sort_order_b

Code style fixes

2f1b09c

Code style fixes and added a message around converting _id to entity_id

e1f1247

Updated the message with the related method

a50dae1

Remvoed @SuppressWarnings(PHPMD.CouplingBetweenObjects)

1c340c3

Dnyomo mentioned this pull request Aug 4, 2020

Issue 104 default sort order Fixes magento/catalog-storefront/104 magento/catalog-storefront#173

Closed

3 tasks

mslabko approved these changes Aug 4, 2020

View reviewed changes

ghost added Progress: ready for testing and removed Progress: needs update labels Aug 4, 2020

Merge branch '2.4-develop' into storefront-issue_104_default_sort_ord…

d0c5577

…er_b

ghost added Progress: testing in progress Progress: to approve and removed Progress: ready for testing Progress: testing in progress labels Aug 4, 2020

magento-engcom-team merged commit 351a90c into magento:2.4-develop Aug 6, 2020

magento deleted a comment from m2-assistant bot Aug 6, 2020

Dnyomo mentioned this pull request Aug 28, 2020

Default sort order magento/catalog-storefront#104

Open

Added a filter to sort categories by position by default #29301

Added a filter to sort categories by position by default #29301

Uh oh!

Conversation

Dnyomo commented Jul 28, 2020

Description (*)

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

Uh oh!

m2-assistant bot commented Jul 28, 2020

Uh oh!

Dnyomo commented Jul 28, 2020

Uh oh!

Dnyomo commented Jul 28, 2020

Uh oh!

mslabko Jul 28, 2020

Choose a reason for hiding this comment

Uh oh!

Dnyomo commented Jul 30, 2020

Uh oh!

mslabko Jul 30, 2020

Choose a reason for hiding this comment

Uh oh!

mslabko Jul 30, 2020

Choose a reason for hiding this comment

Uh oh!

Dnyomo Jul 31, 2020

Choose a reason for hiding this comment

Uh oh!

Dnyomo commented Jul 31, 2020

Uh oh!

Dnyomo commented Jul 31, 2020

Uh oh!

Dnyomo commented Jul 31, 2020

Uh oh!

yaroslavGoncharuk commented Aug 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dnyomo commented Aug 2, 2020

Uh oh!

Dnyomo commented Aug 2, 2020

Uh oh!

mslabko commented Aug 3, 2020

Uh oh!

mslabko commented Aug 3, 2020

Uh oh!

mslabko commented Aug 3, 2020

Uh oh!

mslabko commented Aug 4, 2020

Uh oh!

Dnyomo commented Aug 4, 2020

Uh oh!

Dnyomo commented Aug 4, 2020

Uh oh!

Dnyomo commented Aug 4, 2020

Uh oh!

Dnyomo commented Aug 4, 2020

Uh oh!

Dnyomo commented Aug 4, 2020

Uh oh!

mslabko commented Aug 4, 2020

Uh oh!

mslabko commented Aug 5, 2020

Uh oh!

Uh oh!

yaroslavGoncharuk commented Aug 1, 2020 •

edited

Loading