Skip to content
This repository has been archived by the owner on Mar 17, 2022. It is now read-only.

WooCommerce 3.6 issues #408

Closed
Jon007 opened this issue Apr 2, 2019 · 19 comments
Closed

WooCommerce 3.6 issues #408

Jon007 opened this issue Apr 2, 2019 · 19 comments

Comments

@Jon007
Copy link
Contributor

Jon007 commented Apr 2, 2019

Opening this ticket for potential WooCommerce 3.6 compatibility issues.

See the post for enhancements in 3.6:
https://woocommerce.wordpress.com/2019/04/01/performance-improvements-in-3-6/

The extensive changes are likely to introduce some subtle bugs.
For example Product Meta synchronisation uses the standard Polylang post meta synchronisation, it does not use the Product api, so updated meta for secondary languages would not be copied to the new woo3.6 product lookup tables which might create subtle bugs on product sorting and reports when not using site primary language.
Similarly, cached product data for secondary languages may not be flushed on updating product in primary language.

@fkoomek
Copy link

fkoomek commented Apr 30, 2019

Hello. Here is a bug related to what you mention above.

Can you reproduce this issue on default Wordpress theme (eg Storefront)?
YES

Can you reproduce this issue when all other plugins are disabled except WooCommerce, Polylang and Hyyan WooCommerce Polylang Integration?
YES

What product versions and settings are you using when this issue occurs?
PHP: 7.3.1
WordPress: 5.1.1
WooCommerce: 3.6.1
Polylang: 2.5.3
Hyyan WooCommerce Polylang Integration: 1.3.0
Browser: Chrome,Firefox

Steps to Reproduce
Set Polylang with at least 2 languages
Create Variable product in the default language
Create a translation of the product

What I Expected
Translated product is also variable.

What Happened Instead
When I click to add a translation of the product, product type of that translation is set to SIMPLE.
Product type is not synchronized.

@Jon007
Copy link
Contributor Author

Jon007 commented May 6, 2019

Product Type is Confirmed 3.6.x issue - I can also reproduce this and is also reported on https://wordpress.org/support/topic/variable-products-change-to-simple-in-translated-version/

The product type issue is likely caused by the caching added to product type woocommerce/woocommerce@57ccde6
this means that where the product type is updated by woopoly, the cache is not invalidated so it then appears as Simple..

This is a different but similar issue to the new Product Data Lookup Tables: essentially all updates now need to go through the woocommerce api to ensure the caching and lookup tables are updated.
Currently the plugin extends Polylang capabilities for copying post meta and taxonomies, with the minimum necessary understanding of which meta and taxonomies require copying or translating.
Now the predefined WooCommerce items should be handled via the WooCommerce api itself.

@mrleemon
Copy link
Contributor

mrleemon commented May 11, 2019

I don't have much experience dealing with WooCommerce code, but is there any info on the WooCommerce caching somewhere so we can try to find a way to fix these issues? According to your findings, does the copyTerms() function in Meta.php need to be updated?

public function copyTerms($old, $new, $lang, $taxonomies)

@Jon007
Copy link
Contributor Author

Jon007 commented May 11, 2019

Ideally, all the updates should use the woocommerce Product objects rather than the wordpress Post objects, to ensure any woocommerce level caching and intermediate tables (and future Product tables) are always consistent. This may not be as difficult as it sounds.

Alternatively it should also be possible to update as is and force woocommerce to re-cache and recalculate the relevant objects, but updating through the api would be more future-proof, and we could all do with less maintenance and fewer issues from future woocommerce releases.

The info on the changes is all there in the first link in this thread and release notes of the point versions since then. The woocommerce github link above I found by looking through the release notes which are linked to github fixes.

@mrleemon
Copy link
Contributor

In really old versions of the plugin, there was a call to $this->syncSelectedproductType($ID); at the end of the syncProductsMeta() function at Meta.php. If I re-add this, new translations of Variable products select the correct option in the product type dropdown.

@Jon007
Copy link
Contributor Author

Jon007 commented May 12, 2019

@mrleemon yep that fixes the variation product type issue, well done!

It is a bit of hack job - it doesn't actually correct the underlying problem but instead uses a bit of javascript to synchronise the translated new product form with a bit of woopoly meta, so that it works fine when the product is saved.

The general product synchronisation seems to be ok too (subject to more testing), only the new wc_product_meta_lookup table is not updated and I believe that currently only affects sorting.

@mrleemon
Copy link
Contributor

So, we need to directly copy product properties using WooCommerce CRUD functions instead of relying on the pll_copy_post_metas filter like now, right?

@Jon007
Copy link
Contributor Author

Jon007 commented May 13, 2019

Well the meta itself seems to be working, just there is a risk that it could be cached for example here is the woocommerce caching added for variation attributes :

class-wc-product-variable-data-store-cpt
	public function read_variation_attributes( &$product ) {
		global $wpdb;

		$variation_attributes = array();
		$attributes           = $product->get_attributes();
		$child_ids            = $product->get_children();
		$cache_key            = WC_Cache_Helper::get_cache_prefix( 'product_' . $product->get_id() ) . 'product_variation_attributes_' . $product->get_id();
		$cache_group          = 'products';
		$cached_data          = wp_cache_get( $cache_key, $cache_group );

Somehow I think the cache is getting cleared and this is working, that may be luck rather than design.
Still the wc_product_meta_lookup table needs to be updated and this could be done separately by updating the specific fields via the product class, but would be more efficient if all the updates were done via the relevant product class to avoid repeated database calls. It probably needs to be the relevant product class though as different product types have different handling..

@mrleemon
Copy link
Contributor

I don't know if the following WC function can be useful to copy product data when creating new translations:

https://docs.woocommerce.com/wc-apidocs/source-class-WC_Admin_Duplicate_Product.html#134

It uses a woocommerce_duplicate_product_exclude_meta filter to exclude meta fields from being copied and a woocommerce_product_duplicate_before_save hook to modify the product object further before it is created.

@Jon007
Copy link
Contributor Author

Jon007 commented May 19, 2019

@mrleemon thanks.. we don't duplicate the product like that but perhaps we could.....?

In the meantime I do have another solution which I will check in.

Jon007 added a commit that referenced this issue May 19, 2019
- when saving product, ensures translated product caches are flushed
and product_meta_lookup is kept updated, especially variations
- fixes one deprecated in 3.6 message for update_woocommerce_term_meta
@Jon007
Copy link
Contributor Author

Jon007 commented May 19, 2019

@mrleemon I accepted your suggestion for $this->syncSelectedproductType($ID); and added a function to ensure that translation product caches are cleared and that the lookup tables are updated.

This addresses all 3.6 issues reported so far.
But it is not a full code review...

@mrleemon
Copy link
Contributor

@mrleemon thanks.. we don't duplicate the product like that but perhaps we could.....?

Yes, I know. But, probably in the long run it will be necessary to consider duplicating products using WC core functions, given that the WC team plans to move all product metadata out of the wp_postmeta table in the future.

@Jon007
Copy link
Contributor Author

Jon007 commented May 21, 2019

The new product post is created by Polylang as a blank post with taxonomy data for language and linked translations and selected meta and this plugin extends that in a generic way to handle meta options and additional terms and taxonomies.

It's actually better that the terms and taxonomies are copied in a generic way because then they (generally) work (or can be made to work with filters) on any product type (including product types we don't know about) and any plugin that adds meta data or taxonomies to the product (which the standard woocommerce objects do not know about).

WooCommerce long term goal seemed to be to move the product data from the posts table itself, but that breaks every extension plugin so that's why they've come up with this lookup table to mitigate the performance limitations for the main data fields used for sorting.

Jon007 added a commit that referenced this issue May 25, 2019
merged in updates from 3.6.3 WooCommerce cart-fragments.js
Jon007 added a commit that referenced this issue May 30, 2019
woocommmerce pull request 23820 currently labelled for 3.7.0 milestone
should make update_lookup_table() public in future.
This checking handles that by upgrading itself to use
update_lookup_table() if the functionality is available.
@Jon007 Jon007 unpinned this issue Jun 12, 2019
@giomasce
Copy link

giomasce commented Jun 22, 2019

Hi! Has this bug been fixed? Last changelog entries mention that compatibility with WC 3.6 has been fixed, but this issue is still open. What is the status? Also, is it possible to have the plugin distributed by WP (https://wordpress.org/plugins/woo-poly-integration/) updated?

BTW, loads of thanks for maintaining this plugin to all involved people!

@Jon007
Copy link
Contributor Author

Jon007 commented Jun 22, 2019

This is a general topic for woo3.6 issues not referring to a single issue. Specific issues reported so far were fixed from the first 3.4 release of this plugin on github. The source is now 3.4.3 which I will release on github after rechecking a few more points.

It is always preferable to have more feedback from early adopters before a full release on WordPress: there are so many different settings and usage scenarios that full testing is actually impossible especially when other plugins are considered - I already had to revert one change after accepting it because a compatibility change for the benefit of one plugin broke compatibility for other plugins (name your price vs currency switchers).

Yesterday WooCommerce rejected one of my related pull requests to WooCommerce 3.6+ after previously accepting it - on consideration they don't want to allow other plugins to able resynchronise the product lookup table - so in the long term any wordpress plugin that treats product as a post using the WordPress API will have issues. At the same time the only reliable way so far of copying product data for any product (including custom product types which have data we do not know about) is to copy all the meta and taxonomy data that there is (according to settings and filterable of course).
The workaround added in 3.4 will still work but I'll have another look at it before any release.

@Oclair
Copy link

Oclair commented Jul 17, 2019

I am still experiencing this bug on Hyyan WooCommerce Polylang Integration Version 1.4.3 on wp5.2.2

Loading the editor for a variable product removes the variable product data, even re-saving the variable product data is ineffective.

Disabling and re-enabling Hyyan did not change this behaviour
there was recently an update to Polylang
https://wordpress.org/plugins/polylang/#developers
2.6.2 (2019-07-16)
Pro: Fix slow admin in case the translations update server can’t be reached
Pro: Fix value not correctly translated for ACF clone fields in repeater
Fix strings translations mixed when registered via the WPML compatibility. #381

@Jon007
Copy link
Contributor Author

Jon007 commented Jul 17, 2019

Hi, @Oclair please note that the fix is still working even with the latest Polylang and WooCommerce updates, so if you are experiencing a problem please raise as a separate github issue with full details.

Please note that the workaround does include javascript so in addition for checking for server side errors, it is worth Inspecting with Chrome developer tools and checking the javascript console for any errors.

It is possible that another problem or other plugin is affecting this in some way.

@Jon007 Jon007 closed this as completed Jul 17, 2019
@Oclair
Copy link

Oclair commented Jul 17, 2019

Hey thanks for the response and solution, not a trivial caveat enabling javascript!
Maybe the plugin might notify the admin if this javascript is having an issue? because most folks do not check product variables if they only need to update some text....

Thanks again, have a nice one!

Hi, @Oclair please note that the fix is still working even with the latest Polylang and WooCommerce updates, so if you are experiencing a problem please raise as a separate github issue with full details.

Please note that the workaround does include javascript so in addition for checking for server side errors, it is worth Inspecting with Chrome developer tools and checking the javascript console for any errors.

It is possible that another problem or other plugin is affecting this in some way.

@Jon007
Copy link
Contributor Author

Jon007 commented Jul 17, 2019

Maybe the plugin might notify the admin if this javascript is having an issue?
it may be a difficult situation to detect and act on: if the javascript issue is that it never executes then it wouldn't be able to make an alert..

I actually removed this workaround a few versions ago as I don't like it and found it was not necessary. Unfortunately WooCommerce changes made it necessary again and I couldn't find a better alternative..

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants