Fix orders by shop

[lmeyer1]

Questions	Answers
Description?	In multishop environment, we must take into account the shop where the orders were placed. Create an index to optimize the query.
Type?	bug fix
BC breaks?	no
Deprecations?	no
Fixed ticket?	Fixes PrestaShop/PrestaShop#29136.
How to test?	See the behavior explained in PrestaShop/PrestaShop#29136

[kpodemski]

@lmeyer1

I think we should move the optimization part to the core itself. It does make sense to have an index on this field, but the native module shouldn't be responsible for core optimization.

btw. great job with finding all these issues with multi-store, love it ❤️

[lmeyer1]

@kpodemski

I think we should move the optimization part to the core itself.

You suggest just to delete the new upgrade_module file. Right?
And where should I add the index in the core project ?

[kpodemski]

@lmeyer1

yes :)

The problem is that DB changes are not possible in patch versions, so the next version for this could be either 8.0 or 8.1 - and I'm pretty sure for a store with many orders, this change is critical? I'm unsure if we can put it in 8.0 as it has been feature-frozen.

About the process. You need to modify the DB structure in the core + add this upgrade script to the autoupgrade module. Example:
https://github.com/PrestaShop/autoupgrade/pull/494/files

Before doing that, we need to make sure if it's a good change + if we can do that for 8.1 or somehow put it to 8.0.

I'll ping other maintainers to help us out @PrestaShop/prestashop-maintainers

[lmeyer1]

@kpodemski
I'm not really sure if the combination index-query is the best for performance in all configurations.
What really counts for performance, is to restrict the number of analyzed carts (#41). When reducing from 2078 to 24 carts, even without the index, it is already literally thousands of times better than before (reducing query time form 10 s to 5 ms).
Meanwhile, I will just remove the upgrade file with the index declaration.

[atomiix]

@kpodemski I think @lmeyer1 is right, we can already merge this fix while we can work on the index of the core db structure for later?

[MhiriFaten]

Hello @lmeyer1 ,

As mentioned in the issue, I didn't reproduce it without PR.
PrestaShop/PrestaShop#29136 (comment)

The behavior without PR is the same as with PR, so I am waiting for your feedback before testing it.

Thank you ! 🙏

[lmeyer1]

@MhiriFaten As stated in the issue, you didn't observe the behavior of the crossselling module in the product details in the FO. But the difference in behavior is there, where you didn't look at!

[Hlavtox]

@kpodemski @lmeyer1 I think we can merge it right away, it works fine.

But I have one last question - what do you think about an idea of adding a switch for this behavior? Maybe it does not matter on which store the people bought the products together, no? You would maybe get a bigger sample with the original behavior.

Something like Use all shops to get frequently bought products - YES/NO.

[kpodemski]

@Hlavtox it makes sense if you have shared catalog between shops

@lmeyer1, what do you think?

[lmeyer1]

@Hlavtox is right. There are scenarios, where it makes sense to use all shop's data to get the list of products to display, and others where we want to avoid to mix the data from different shops.