Skip to content
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

#24357 Eav sort order by attribute option_id #24360

Merged
merged 6 commits into from
Dec 18, 2019
Merged

#24357 Eav sort order by attribute option_id #24360

merged 6 commits into from
Dec 18, 2019

Conversation

tnsezer
Copy link
Contributor

@tnsezer tnsezer commented Aug 29, 2019

Referring issue: #24357

Preconditions (*)

  1. Magento 2.3.2
    2.Php 7.2

Steps to reproduce (*)

  1. We set an attribute for a grouped list to order products by attributes
    https://ibb.co/fQCNbVy

But the query orders it by option_id of eav_attribute_option_value table. but it must be ordered by sort_order of eav_attribute_option table. but it doesn't add into the join.

the query is produced by \Magento\Eav\Model\Entity\Attribute\Source\Table::addValueSortToCollection method.

Expected result (*)

It must be ordered by sort_order of eav_attribute_option table

Actual result (*)

It is ordered by option_id of eav_attribute_option_value table to check the query output below

SELECT `e`.*, `price_index`.`price`, `price_index`.`tax_class_id`, `price_index`.`final_price`, IF(price_index.tier_price IS NOT NULL, LEAST(price_index.min_price, price_index.tier_price), price_index.min_price) AS `minimal_price`, `price_index`.`min_price`, `price_index`.`max_price`, `price_index`.`tier_price`, 
IF(a5637144578_option_value_t2.value_id IS NULL, a5637144578_option_value_t1.option_id, a5637144578_option_value_t2.option_id) AS `a5637144578`, 
IF(a5637144609_option_value_t2.value_id IS NULL, a5637144609_option_value_t1.option_id, a5637144609_option_value_t2.option_id) AS `a5637144609`, IF(a5637144582_option_value_t2.value_id IS NULL, a5637144582_option_value_t1.option_id, a5637144582_option_value_t2.option_id) AS `a5637144582`, IF(a5637144596_option_value_t2.value_id IS NULL, a5637144596_option_value_t1.option_id, a5637144596_option_value_t2.option_id) AS `a5637144596`, IF(a5637144607_option_value_t2.value_id IS NULL, a5637144607_option_value_t1.option_id, a5637144607_option_value_t2.option_id) AS `a5637144607` FROM `catalog_product_entity` AS `e`
 INNER JOIN `catalog_product_index_price` AS `price_index` ON price_index.entity_id = e.entity_id AND price_index.website_id = '3' AND price_index.customer_group_id = 0
 LEFT JOIN `catalog_product_entity_int` AS `a5637144578_t1` ON e.row_id=a5637144578_t1.row_id AND a5637144578_t1.attribute_id='371' AND a5637144578_t1.store_id=0
 LEFT JOIN `catalog_product_entity_int` AS `a5637144578_t2` ON e.row_id=a5637144578_t2.row_id AND a5637144578_t2.attribute_id='371' AND a5637144578_t2.store_id='4'
 LEFT JOIN `eav_attribute_option_value` AS `a5637144578_option_value_t1` ON a5637144578_option_value_t1.option_id=IF(a5637144578_t2.value_id > 0, a5637144578_t2.value, a5637144578_t1.value) AND a5637144578_option_value_t1.store_id=0
 LEFT JOIN `eav_attribute_option_value` AS `a5637144578_option_value_t2` ON a5637144578_option_value_t2.option_id=IF(a5637144578_t2.value_id > 0, a5637144578_t2.value, a5637144578_t1.value) AND a5637144578_option_value_t2.store_id=4
 LEFT JOIN `catalog_product_entity_int` AS `a5637144609_t1` ON e.row_id=a5637144609_t1.row_id AND a5637144609_t1.attribute_id='367' AND a5637144609_t1.store_id=0
 LEFT JOIN `catalog_product_entity_int` AS `a5637144609_t2` ON e.row_id=a5637144609_t2.row_id AND a5637144609_t2.attribute_id='367' AND a5637144609_t2.store_id='4'
 LEFT JOIN `eav_attribute_option_value` AS `a5637144609_option_value_t1` ON a5637144609_option_value_t1.option_id=IF(a5637144609_t2.value_id > 0, a5637144609_t2.value, a5637144609_t1.value) AND a5637144609_option_value_t1.store_id=0
 LEFT JOIN `eav_attribute_option_value` AS `a5637144609_option_value_t2` ON a5637144609_option_value_t2.option_id=IF(a5637144609_t2.value_id > 0, a5637144609_t2.value, a5637144609_t1.value) AND a5637144609_option_value_t2.store_id=4
 LEFT JOIN `catalog_product_entity_int` AS `a5637144582_t1` ON e.row_id=a5637144582_t1.row_id AND a5637144582_t1.attribute_id='366' AND a5637144582_t1.store_id=0
 LEFT JOIN `catalog_product_entity_int` AS `a5637144582_t2` ON e.row_id=a5637144582_t2.row_id AND a5637144582_t2.attribute_id='366' AND a5637144582_t2.store_id='4'
 LEFT JOIN `eav_attribute_option_value` AS `a5637144582_option_value_t1` ON a5637144582_option_value_t1.option_id=IF(a5637144582_t2.value_id > 0, a5637144582_t2.value, a5637144582_t1.value) AND a5637144582_option_value_t1.store_id=0
 LEFT JOIN `eav_attribute_option_value` AS `a5637144582_option_value_t2` ON a5637144582_option_value_t2.option_id=IF(a5637144582_t2.value_id > 0, a5637144582_t2.value, a5637144582_t1.value) AND a5637144582_option_value_t2.store_id=4
 LEFT JOIN `catalog_product_entity_int` AS `a5637144596_t1` ON e.row_id=a5637144596_t1.row_id AND a5637144596_t1.attribute_id='369' AND a5637144596_t1.store_id=0
 LEFT JOIN `catalog_product_entity_int` AS `a5637144596_t2` ON e.row_id=a5637144596_t2.row_id AND a5637144596_t2.attribute_id='369' AND a5637144596_t2.store_id='4'
 LEFT JOIN `eav_attribute_option_value` AS `a5637144596_option_value_t1` ON a5637144596_option_value_t1.option_id=IF(a5637144596_t2.value_id > 0, a5637144596_t2.value, a5637144596_t1.value) AND a5637144596_option_value_t1.store_id=0
 LEFT JOIN `eav_attribute_option_value` AS `a5637144596_option_value_t2` ON a5637144596_option_value_t2.option_id=IF(a5637144596_t2.value_id > 0, a5637144596_t2.value, a5637144596_t1.value) AND a5637144596_option_value_t2.store_id=4
 LEFT JOIN `catalog_product_entity_int` AS `a5637144607_t1` ON e.row_id=a5637144607_t1.row_id AND a5637144607_t1.attribute_id='390' AND a5637144607_t1.store_id=0
 LEFT JOIN `catalog_product_entity_int` AS `a5637144607_t2` ON e.row_id=a5637144607_t2.row_id AND a5637144607_t2.attribute_id='390' AND a5637144607_t2.store_id='4'
 LEFT JOIN `eav_attribute_option_value` AS `a5637144607_option_value_t1` ON a5637144607_option_value_t1.option_id=IF(a5637144607_t2.value_id > 0, a5637144607_t2.value, a5637144607_t1.value) AND a5637144607_option_value_t1.store_id=0
 LEFT JOIN `eav_attribute_option_value` AS `a5637144607_option_value_t2` ON a5637144607_option_value_t2.option_id=IF(a5637144607_t2.value_id > 0, a5637144607_t2.value, a5637144607_t1.value) AND a5637144607_option_value_t2.store_id=4 WHERE ((e.entity_id IN (59266, 59149, 58435, 58400, 58398, 58397, 58396, 58395, 58394, 58393, 58251, 58250, 58022, 58021, 58020, 58019, 58018, 53572, 53216, 53215, 51289, 50996, 50946, 50945, 50944, 48833, 48832, 48831, 48830, 48364, 48363, 46791, 46790, 46789, 46529, 46528, 44364, 43767, 43766, 43765, 43764, 43763, 39717, 39572, 39571, 39570, 39569, 39568, 39567, 39566, 34930))) AND (e.created_in <= 1) AND (e.updated_in > 1) ORDER BY `a5637144578` ASC, `a5637144609` ASC, `a5637144582` ASC, `a5637144596` ASC, `a5637144607` ASC

You can see the query orders like
ORDER BY a5637144578 ASC

But the data comes from option_id
IF(a5637144578_option_value_t2.value_id IS NULL, a5637144578_option_value_t1.option_id, a5637144578_option_value_t2.option_id) AS a5637144578

@m2-assistant
Copy link

m2-assistant bot commented Aug 29, 2019

Hi @tnsezer. 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.3-develop instance - deploy vanilla Magento instance

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

@williankeller williankeller changed the title [24357] Eav sort order by attribute option_id #24357 Eav sort order by attribute option_id Sep 3, 2019
@aleron75
Copy link
Contributor

aleron75 commented Sep 5, 2019

Hi @tnsezer thanks for your contribution!

You say:

it must be ordered by sort_order of eav_attribute_option table

I may be wrong but shouldn't it be ordered by value of eav_attribute_option_value?

My interpretation is that the sort_order of eav_attribute_option for an attribute is intended to determine the order in which options are listed but if I want to order by that attribute I mean I want to order by the value of that attribute.

It's just my interpretation, maybe @sidolov or @magento-engcom-team can shed some light on this.

@aleron75 aleron75 self-assigned this Sep 5, 2019
@aleron75 aleron75 self-requested a review September 5, 2019 06:12
@tnsezer
Copy link
Contributor Author

tnsezer commented Sep 5, 2019

Hi @aleron75

There are many type of data that why it would be hard to order by value and be wrong because Magento offers ordering the attributes that why it won’t make sense if we order by value

@sidolov sidolov changed the base branch from 2.3-develop to 2.4-develop December 5, 2019 17:20
…24357

# Conflicts:
#	app/code/Magento/Eav/Model/Entity/Attribute/Source/Table.php
#	app/code/Magento/Eav/Test/Unit/Model/Entity/Attribute/Source/TableTest.php
@tnsezer
Copy link
Contributor Author

tnsezer commented Dec 16, 2019

I merged the new version to this branch

@magento-engcom-team
Copy link
Contributor

Hi @aleron75, thank you for the review.
ENGCOM-6489 has been created to process this Pull Request
✳️ @aleron75, could you please add one of the following labels to the Pull Request?

Label Description
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests

@engcom-Alfa engcom-Alfa self-assigned this Dec 17, 2019
@engcom-Alfa engcom-Alfa added the Auto-Tests: Covered All changes in Pull Request is covered by auto-tests label Dec 17, 2019
@engcom-Golf engcom-Golf self-assigned this Dec 17, 2019
@engcom-Alfa
Copy link
Contributor

✔️ QA Passed

Before:
before

After:
after

@m2-assistant
Copy link

m2-assistant bot commented Dec 18, 2019

Hi @tnsezer, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

@sdzhepa sdzhepa mentioned this pull request May 9, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests Component: Eav Partner: Youwe partners-contribution Pull Request is created by Magento Partner Progress: accept Release Line: 2.4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants