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

Make sure all linked products (related, upsells, crosssells) show up … #17885

Merged
merged 1 commit into from
Sep 22, 2018

Conversation

hostep
Copy link
Contributor

@hostep hostep commented Sep 1, 2018

…in the backend grids and in the correct order. Fixes #13720

Description

This is a re-take of #15251
This fixes #13720 & #14050
The problem was introduced in 168701c & 308b70b

The fact that product link positions can be the same probably happens by importing products, you probably can't get to this state by just managing related, upsell or crosssell products in the adminhtml.

Next to the fix, I've also added two extra cases in the unit tests for when either the position isn't defined or doesn't exist, these are edge cases which will probably never happen, but I feel like this makes the code more robust. When the position isn't defined or doesn't exist, I've opted to set the position to 0 and this will cause those products to show up as the first ones in the list.

There is a small change in the result of the $sorterItems variable, in that the array keys no longer follow the position exactly. But in my testing this doesn't seem to have any negative effect.

Fixed Issues (if relevant)

  1. Only 2 related products are showing in backend . #13720: Only 2 related products are showing in backend
  2. Import related products issue #14050: Import related products issue

Manual testing scenarios

  1. Have a product with 5 to 10 related products
  2. Go into the database and look at the table catalog_product_link_attribute_int
  3. All positions (value column, for product_link_attribute_id == 1) should be unique if you created those related products through the backend
  4. To simulate a product import where the positions can end up being non-unique, run the following query: UPDATE catalog_product_link_attribute_int SET value = 1 WHERE product_link_attribute_id = 1;
  5. Take a look at the product in the adminhtml and open the Related Products, Up-Sells, and Cross-Sells section.
  6. Without this fix, you'll only see 2 products, not the full list, with this fix all products show up in the correct sort order.
  7. Also check the frontend, before and after the fix, the sort order will be the same over there, so frontend sort order isn't affected by this change.

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 on Travis CI are green)

@magento-engcom-team
Copy link
Contributor

Hi @hostep. 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-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me {$VERSION} instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

…in the backend grids and in the correct order. Fixes magento#13720
@hostep hostep force-pushed the fix-for-issue-13720 branch from 041e7d9 to a3f1c38 Compare September 1, 2018 14:58
@magento-engcom-team
Copy link
Contributor

Hi @sidolov, thank you for the review.
ENGCOM-2930 has been created to process this Pull Request

@magento-engcom-team
Copy link
Contributor

Hi @hostep. Thank you for your contribution.
We will aim to release these changes as part of 2.2.8.
Please check the release notes for final confirmation.

Please, consider to port this solution to 2.3 release line.
You may use Porting tool to port commits automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants