-
-
Notifications
You must be signed in to change notification settings - Fork 449
Fixed getAttributeRawValue when there are no value in the "default" store #2964
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
Conversation
is this a situation that happens often? to have attributes at store-level but without the default? do we know the impact it has on the query? anywa, if M2 merged this same code, we could probably use it too. |
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.
Yes, we can't be lower than M2. |
@luigifab do you use this in your stores? are we safe to merge it? |
Yes I use it. I there are issues, I don't find them. |
I know it will be controversial but I think that technically we're modifying what's returned by a method that's probably used in custom code, so for that reason I think although it's a bugfix it should be treated as a breaking change. that's why I merged it in next. |
Description
Reworked query in
getAttributeRawValue
so that it returns a store specific value even if there is no default value.See also #23369 from Magento2.
I tested this PR with #572, and with PHP 8.0 / 8.1 / 8.2.
Manual testing scenarios
bonjour
code andstore view
scopeapp/design/frontend/rwd/default/template/catalog/product/view.phtml
, add:Then go to the product page, without the PR, your value is out there, with the PR, the value is displayed.
If you use now the same attribute in
checkout/cart/item/default.phtml
with:and:
And now it works!
SQL queries
The order of UNIQUE index is: entity_id, attribute_id, store_id
in catalog_product_entity_varchar...
the data
getAttributeRawValue
getLoadAttributesSelect (for cart and collections)
Problem: for my test (for getLoadAttributesSelect) I lost ~5 ms.
The EXPLAIN says:
Contribution checklist