search

EmicoEcommerce / Magento2TweakwiseExport-archived

Magento 2 module for Tweakwise export
Other
6 stars 16 forks source link

Bugfix: fixed a issue where the category node for products was empty #54

Closed chefbuitenhuis closed 6 years ago

chefbuitenhuis commented 6 years ago

Magento 2.2.5 introduced parallel indexers which caused the catalog-product relationship to be saved to different table for each storeview. The table name used in the query to retrieve those nodes had to be changed so it would pick the appropiate table for the collection.

The table name is now generated by using the store id of the current collection.

hostep commented 6 years ago

Watch out, the TableMaintainer class was introduced in Magento 2.2.5, so it doesn exist in Magento 2.2.4 and lower.

So if this PR gets approved, it will make the module incompatible with Magento 2.2.4 and lower.

So this is probably not the best solution?

chefbuitenhuis commented 6 years ago

@hostep I think it's a future-proof solution - but you're right that we will lose backwards compatibility. I'd love to see your suggestion so I can perhaps update this PR.

hostep commented 6 years ago

Maybe there is an internal API which can be used to fetch this information instead of doing a SQL query. It will probably be a bit more overhead, but using raw SQL is bound to cause issues like this.

If raw SQL is still needed, you can probably try to check for the version of Magento being used (or the version of the Magento_Catalog module itself, or maybe even do feature detecting to see of the class TableMaintainer exists) and let the code execute differently based on that.

But best solution in my opinion is to ditch the SQL queries and look for an internal API from Magento to get that same information. Since that's a much more stable solution. (We just discovered another big problem in this module with direct SQL queries, which is fetching wrong data in a Magento Enterprise version, since the database structure for configurable/simple products is different then the Magento Community version. So the wrong simple products are associated with configurables in the export of this module. A colleague of mine is writing a new bug report for this at the moment.)

edwinljacobs commented 6 years ago

Hello,

thanks for reporting this and for the pull request.

I agree that using the TableMaintainer class is preferable but indeed backwards compatibility is an issue. For now the issue is fixed with the following by checking if the store_{id} tables exist if not return the base index table.

This will be integrated in the next release (1.2.0)

edwinljacobs commented 6 years ago

Release 1.2.0 has been published