Forum Replies Created

Viewing 15 replies - 1 through 15 (of 17 total)
  • Thread Starter pumka

    (@pumka)

    Hi, @lovingbro!

    As I was testing the issue again, I found there is another plugin involved, Zoho CRM Lead Magnet, which I somehow overlooked during previous tests. That’s odd as I was sure I disabled everything else but now I couldn’t reproduce the issue w/o that plugin enabled. Well, it turns out CartCheckoutUtils::is_page_type checks for both, the pre_get_posts event and $post global variable. And what the Zoho CRM Lead Magnet did was setting the $post global variable to an empty array, which confused CartCheckoutUtils::is_page_type even further.
    Well, I admit it’s getting a bit out of control and we have three different bugs interfering here:
    * WooCommerce using a wrong condition to check if the main WP query was executed.
    * Stripe plugin calling is_checkout_page() too early, when the current page is not decided yet.
    * Zoho CRM Lead Magnet plugin initializing $post global variable for absolutely no reason and finally making CartCheckoutUtils::is_page_type return a result, when it actually shouldn’t.

    I’m afraid it’s too much and I already spend a lot of time figuring it all out. For now, I’ll just remove the Zoho CRM Lead Magnet plugin as my client doesn’t use it anymore and this should resolve the issue.

    It’s up to you if you to decide on my suggested fix for changing WC_Stripe::init_express_checkout to run on template_redirect with a priority more that 10, when CartCheckoutUtils::is_page_type finally could return a meaningful result. Though this wouldn’t affect our site anymore, I believe it’s still a bug as the Stripe gateway could be enabled on the pages it shouldn’t, because the WooCommerce page type check doesn’t work at the time of init event yet.

    Thank you for your help with investigating this!

    Thread Starter pumka

    (@pumka)

    Hi, @lovingbro!
    I don’t think ACF is doing anything wrong and any change should be done from their side. The pre_get_posts filter is normally called during a get_posts execution. In my opinion it’s rather a bad choice for the condition in CartCheckoutUtils::is_page_type. They use the filter to ensure that the main WordPress query was already executed but that event is triggered by ANY post query, not necessarily be the main one. A better option could be checking for the pre_handle_404 event because it is triggered immediately after main WordPress query in \WP::handle_404 or by just checking global $wp_query object properties. For example, a non-null value of posts should indicate that $wp_query::get_posts was indeed executed. I beleive it’s a WooCommerce core bug and should be addressed in WooCommerce.

    Thread Starter pumka

    (@pumka)

    Hi, @lovingbro!
    Oh, how did I never thought about disabling everything else myself? Yes, it doesn’t happen if I deactivate all plugins other than WooCommerce and Stripe. And I’ve isolated the issue down to the “Advanced Custom Fields Pro” plugin. It’s a paid one but their free version has the same effect: https://www.advancedcustomfields.com/latest/.

    The thing is \Automattic\WooCommerce\Blocks\Utils\CartCheckoutUtils::is_page_type has this check at the very beginning:

    if ( ! did_action( 'pre_get_posts' ) ) {
    return null;
    }

    And it likely means “WordPress query wasn’t executed yet”. Otherwise checking for the current page doesn’t really make any sense. The problem is calling get_posts triggers this action/filter too, which is what the Advanced Custom Fields plugin does at its initialization. Well, I would say it’s a bad choice of a condition to block is_page_type execution as it should be something related to the main WP query. So we shouldn’t really blame the Advanced Custom Fields plugin for that. But it explains why you couldn’t reproduce the problem.

    While it seems to be a WooCommerce bug, I still believe moving WC_Stripe::init_express_checkout to the template_redirect event to run after wc_template_redirect is a right solution here because is_checkout, is_product and is_cart checks in \WC_Stripe_Express_Checkout_Helper::is_enabled_for_current_context should be all returning false regardless of the actual request URI before a wc_template_redirect call. Therefore the Stripe plugin could make an incorrect decision on whether it is enabled or not.

    • This reply was modified 2 months, 3 weeks ago by pumka.
    • This reply was modified 2 months, 3 weeks ago by pumka.
    Thread Starter pumka

    (@pumka)

    Hi, Frank!

    The caching happens right inside CartCheckoutUtils::is_checkout_page but I assume we do have something special as nobody else seems captured it yet.

    	public static function is_checkout_page(): bool {
    if ( null === self::$is_checkout_page ) {
    self::$is_checkout_page = self::is_page_type( 'checkout' );
    }
    return true === self::$is_checkout_page;
    }

    Answering your questions:
    1. https://gist.github.com/pumka/35cd22e1d6ce738ee7320ec44ebaba1e
    2. It’s a classic checkout with the [woocommerce_checkout] shortcode.
    3. No, the URL looks pretty stock like /checkout/order-received/XXXXX/?key=wc_order_DeaDBeeF1234
    4. On every order I beleive. I’ve placed an order for a free product w/o any payment method involved and got the same.
    5. No. We have a Speed Optimizer (sg-sitepress) plugin but it happens even if I turn it off and it happens on my local site copy as well, where obviously no CDN or another proxy is involved.

    Let me know if you have more questions. I can collect variable values etc. on each stage of the call stack for example.

    Thread Starter pumka

    (@pumka)

    It worked just fine! Thank you for the fast update.

    Thread Starter pumka

    (@pumka)

    Hi!
    Swedish translation is “Kan restnoteras”, at least that’s how WooCommerce translates it. But if you could not filter it, that’s the best option. I will test the new version tomorrow.

    Thread Starter pumka

    (@pumka)

    Yes, it fixed the issue. Thanks!

    Thread Starter pumka

    (@pumka)

    Yes, it works fine now. Thanks!

    Thread Starter pumka

    (@pumka)

    Sorry about the later reply. Looks like the update fixed the “Array” string. It broke a compatibility with the 1.6.0 “pro” counterpart of the plugin though and that’s why I hesitated to install the update. I’m asking my client to update the pro plugin to check if it resolves the compatibility issue.

    • This reply was modified 1 year, 4 months ago by pumka.
    Thread Starter pumka

    (@pumka)

    This happens only if the physical order of array elements in _product_attributes meta field doesn’t match their “position” values, because the “position” is used for extra sorting. At the moment I’m not sure what might cause the order of “position” values to differ because regular product save reorders the elements. Yet something did that, we just don’t know what.

    To reproduce the issue, you can edit the _product_attributes meta of the product directly in the database and adjust the “position” values to not match the physical order of the array elements. They usually go in natural order like 0, 1, 2… but in our case they sometimes differ:

    You can use this tool to edit a serialized array value: https://sciactive.com/phpserialeditor.php.

    • This reply was modified 1 year, 6 months ago by pumka.
    Thread Starter pumka

    (@pumka)

    Thank you!

    PS. Also required me to disable “User Level Filter” option in plugin settings as it still not allowed me to edit user roles. Actually disabling the option alone fixes the issue, though I believe the option’s implementation should exclude superadmins.

    Seems to be a missing check for superadmin user when it filters user roles available.
    Adding this code at the beginning of \AAM_Service_UserLevelFilter::filterRoles() at application/Service/UserLevelFilter.php:129 fixed the issue for me:

    if (is_super_admin()) {
        return $roles;
    }

    As a temporary workaround, you can try assigning yourself to the site explicitly as administrator. But you will likely need to deactivate the plugin while adding your user record to the site as it will restrict “administrator” role while active.

    • This reply was modified 6 years, 4 months ago by pumka.
    Thread Starter pumka

    (@pumka)

    Sure, no problem at all as my fix was relative easy to implement once I find out what caused the reset. 😉
    Emailed you another issue with the Bulk Cloner add-on as I believe it does not belong here. Let me know if add-ons have their issue tracker(s) as I couldn’t find one.

    Thread Starter pumka

    (@pumka)

    Hi, Edward!

    Thanks for the links! As I wrote I was able to implement a workaround myself by unsetting post_parent property in ‘threewp_broadcast_broadcasting_after_switch_to_blog’ action handler in my custom plugin. If the property is not found in the post object, WordPress would not update the corresponding field and thus existing parent would be kept.

    I just thought that if Broadcast couldn’t find a correct parent itself, it shouldn’t really reset the existing parent, and that’s quite easy to implement.
    But no problem if you assume that resetting the parent is a correct behavior here.

    • This reply was modified 7 years, 5 months ago by pumka.
Viewing 15 replies - 1 through 15 (of 17 total)