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

[BUGFIX] [issue-13720] Allow more then 2 products with the same posit… #15251

Conversation

lewisvoncken
Copy link
Contributor

…ion in the Admin for Related Products

Description

Only 2 related products are showing in backend

Fixed Issues (if relevant)

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

Steps to reproduce

  1. Update products related products using csv and update via magento default import.
  2. Flush the cache storage and reindexing .
  3. Check the frontend and backend

Expected result

  1. Updated related products should come in the frontend of product page and backend of product edit page

Actual result

  1. The update related products are showing in the frontend .But only 2 related products are showing in the backend

Apply changes

Now the expected result will be the outcome.

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)

@VladimirZaets
Copy link
Contributor

Hi @lewisvoncken, thank you for collaboration.

I think to use while(true) it's not a better solution. Maybe you can find another solution.
Anyway, I think to use the recursive function in the current case looks better. Are you agree?

Can you please cover your changes by tests?

@robbertstevens
Copy link

@VladimirZaets i agree the solutions isn't the best, but atleast it works now.

@okorshenko okorshenko removed this from the May 2018 milestone May 31, 2018
@VladimirZaets
Copy link
Contributor

@robbertstevens can you refactor this code uses recursive function instead of while(true)

@robbertstevens
Copy link

@VladimirZaets no i cant, and what do you mean you think this is not a better solution? Now it doesnt even work. With his solutions atleast the feature works.

It's like going to the garage and you need some air in your tires, but the electric pump is broken so you need to do in manually, and you say well what ever, ill drive with empty tires

@VladimirZaets
Copy link
Contributor

@robbertstevens we try to not introduce new not reliable code. This code is not reliable, it can introduce infinite loop and other bugs. I provided information how to refactor this code and this fix should be covered by tests.

Please reopen and update if you wish to continue.
Thank you for collaboration

@orlangur
Copy link
Contributor

@lewisvoncken I don't think there is any use for recursion here and I don't think a loop is infinite sometimes, however, old implementation was O(n) and your one is O(n*n) which needs to be fixed.

@n0kit31
Copy link

n0kit31 commented Aug 31, 2018

How come this bug is closed? Issue still happens in 2.2.5!

@hostep
Copy link
Contributor

hostep commented Sep 1, 2018

I've opened a new PR using a different way for fixing this, which will hopefully get accepted by the Magento devs: #17885

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.

8 participants