Skip to content

Conversation

tdgroot
Copy link
Member

@tdgroot tdgroot commented Sep 20, 2018

Fix productlink collection provider missing items when some positions are null.

Description

There were several issues with the productlink collection provider.

When several items have the position value of null, the position will get converted to '0'. When sorting, the items get sorted by indexing the items by position value and sort them by a key sort. Multiple '0' values for position, means duplicate indexes.

The same applies with every case where multiple items have the same position value.

Manual testing scenarios

  1. Create some configurable products
  2. Add related products to one of them
  3. Try sorting the related products

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 @tdgroot. 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

@sidolov
Copy link
Contributor

sidolov commented Sep 27, 2018

Hi @tdgroot , looks like changes related to your PR was merged to 2.3-develop. Please, resolve merge conflicts and check if the issue still present.
Thank you!

@tdgroot
Copy link
Member Author

tdgroot commented Sep 27, 2018

@sidolov thank you for informing. I think @hostep's work solves my problem! Will check soon.

@hostep
Copy link
Contributor

hostep commented Sep 27, 2018

Nice @tdgroot 👍 , our changes are nearly identical, makes me feel more confident that the solution is a good one 🙂
Feel free to double check though.

@tdgroot
Copy link
Member Author

tdgroot commented Sep 27, 2018

@hostep exactly what I was thinking! Magento contribution are great sanity checks for your solutions :).

@tdgroot tdgroot closed this Sep 28, 2018
@tdgroot tdgroot deleted the fix_productlink_collection_sorting branch November 26, 2018 13:15
@tdgroot
Copy link
Member Author

tdgroot commented Nov 26, 2018

Fixed in #18207

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.

4 participants