search

OFFLINE-GmbH / oc-mall-plugin

:convenience_store: E-commerce solution for October CMS
https://offline-gmbh.github.io/oc-mall-plugin
MIT License
169 stars 114 forks source link

Clean up PropertyValues when "Use for Variants" is changed #177

Open papppeter opened 5 years ago

papppeter commented 5 years ago

i think this part is bugos.

how to recreate: add a product with variants use multiple property as a variant (i used one dropdown, and one text field) create multiple variants with different property values property values will store the variants wrongly, will pick variant property from another variant as well.

after reindexing it fixed partly. but adding a new variant messed it up again.

tobias-kuendig commented 5 years ago

Thank you for this bug report. I was able to reproduce the issue and will release a fix soon.

tobias-kuendig commented 5 years ago

This should be fixed in 2348abb4502b8b6b143e87d16fa16f112b5cd160. Could you test this change and see if it fixes the index for you? You can apply the patch manually or update to 1.2.5 on the edge updates channel.

papppeter commented 5 years ago

hi, a can confirm it is working.

tobias-kuendig commented 5 years ago

Perfect, thank you very much for the report and the testing! :+1:

papppeter commented 5 years ago

but now it is not including all the properties as it was before, only the variant related properties. is this the expected behavior?

doing the command reindex does produce different result in index. (includes still all the properties)

should reindex use the same method?

tobias-kuendig commented 5 years ago

The reindex command should produce the same output. Let me check...

tobias-kuendig commented 5 years ago

This is fixed in 072d8ab. Would you mind to test the lastest v1.2.6 update again? This was a very nasty bug with how Laravel handles orWhere statements in query scopes.

papppeter commented 5 years ago

Hi,

i tested it and it is better, now both methods are generating the same, but as you can see on the pics, it still picks up some data which should not be there.

i did build up an order by property thats is the reason i need this works perfectly.

thanks for your fixes

Screenshot_54 Screenshot_53

tobias-kuendig commented 5 years ago

Could you by any chance provide me with a database dump of your current setup?This would make debugging this a whole lot easier.

tobias-kuendig commented 5 years ago

Thank you for providing me with the db dump. This is actually another bug and has nothing to do with the indexing functionality.

You changed a property to be "used for variants" after there were products with values for that property already stored in the database.

We need to clean up the property values if the "use for variants" option is changed.

For the time being you can DELETE FROM offline_mall_property_values WHERE property_id = 5 and variant_id is null and run mall:reindex to clean up your database.

papppeter commented 5 years ago

thanks that solved my problem!

would it be good think to do this from the backend, maybe a cleanup button, if it recognize this kind of problem.