Fix emptying cart issue for custom order statuses

[alexedwardjones]

trafficstars

Changes proposed in this Pull Request:

I was experiencing an issue where a custom status wasn't being picked up and causing the cart to be emptied.

After some discussion, I've changed the cart emptying to be based on a boolean value and added a filter for the boolean. The filter takes two parameters, the default boolean value based on the order status and the order in case third-parties want to do further checks.

How to test the changes in this Pull Request:

Recreating the issue

1. Install the Stripe plugin
2. Activate AliPay via Stripe. It doesn't need to configured we just need a payment method that doesn't complete immediately, in this case an external payment gateway.
3. Install the Code Snippets plugin
4. Add and activate the below code snippet which adds a custom order status Foo and makes it the default status.

function register_foo_post_status() {
	register_post_status(
		'wc-foo',
		array(
			'label'   => 'Foo',
			'public'	=> true,
			'show_in_admin_status_list' => true,
			'label_count'               => _n_noop( 'Foo <span class="count">(%s)</span>', 'Foo <span class="count">(%s)</span>' )
		)
	);

}
add_action( 'init', 'register_foo_post_status' );

function register_foo_order_status( $statuses ) {
	$statuses['wc-foo'] = 'Foo';

	return $statuses;
}
add_filter( 'wc_order_statuses', 'register_foo_order_status' );

function register_foo_order_post_status( $statuses ) {
	$statuses['wc-foo'] = 'Foo';
	return $statuses;
}
add_filter( 'woocommerce_register_shop_order_post_statuses', 'register_foo_order_post_status' );

function new_default_order_status( $status ) {
	return 'foo';
}
add_filter( 'woocommerce_default_order_status', 'new_default_order_status' );

function add_foo_as_payment_status( $statuses ) {
	$statuses[] = 'foo';
	return $statuses;
}
add_filter( 'woocommerce_valid_order_statuses_for_payment', 'add_foo_as_payment_status' );

4. Add a product to your cart.
5. Go through checkout with the "Alipay" payment method. The redirect to Alipay should fail because it's not configured but an order will be created in the new default status.
6. Navigating anywhere else on the site will incorrectly clear your cart because the new order status isn't picked up as being for an unpaid order.

Resolving the issue using the new filter

7. Add and activate a code snippet with the following code:

function dont_clear_cart_for_orders_with_foo_status( $did_order_succeed, $order ) {
  if ( 'foo' === $order->get_status() ) {
    return false;
  }

  return $did_order_succeed;
}
add_filter( 'woocommerce_should_clear_cart_after_payment',  'dont_clear_cart_for_orders_with_foo_status', 10, 2 );

8. Change the products in your cart, so that your cart total will be different and a new order will be generated when you attempt to pay.
9. Go through checkout with the "Alipay" payment method. The redirect to Alipay should fail because it's not configured but an order will be created in the new default status.
10. Navigating anywhere else on the site will no longer clear your cart because the new order status is now picked up as being for an unpaid order.

[github-actions[bot]]

Hi ,

Apart from reviewing the code changes, please make sure to review the testing instructions and verify that relevant tests (E2E, Unit, Integration, etc.) have been added or updated as needed.

You can follow this guide to find out what good testing instructions should look like: https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

[alexedwardjones]

Hey @lsinger 👋 Thanks, that was really helpful.

I've gone ahead and added that comment block. Let me know if it looks okay.

I'm not sure how your releases work so I wasn't sure what to put for the @since value. I saw you already have a release candidate for 8.6.0 so I put in 8.6.1.

[alexedwardjones]

Hey @lsinger 👋 How do I go about getting this merged?

[alexedwardjones]

@lsinger @opr @jorgeatorres @Konamiman Hey folks 👋 Is anyone able to help me get this reviewed and merged?

[alexedwardjones]

Hey @lsinger 👋 Thanks for getting back to me. I've gone ahead and made the changes you're proposing. Let me know if it aligns with what you expected.

[lsinger]

@alexedwardjones could you update the testing instructions? I cannot currently reproduce the before and after steps. I'm adding the filter like so add_filter( 'woocommerce_should_clear_cart_after_payment', '__return_false', 10, 2 ); but am not sure what payment method(s) would actually lead to that code path being executed. For things like direct bank transfer etc. there is never a WC()->session->order_awaiting_payment.

[lsinger]

FWIW, I was thinking something like this would be good to have so that in testing we can again validate things work as expected:

1. install the Code Snippets plugin
2. install PAYMENT_METHOD // which one? what are the steps to get into a proper testing mode?
3. add a product to your cart and go through checkout with PAYMENT_METHOD -- the cart should be empty after checkout
4. add and activate a code snippet with the following code: add_filter( 'woocommerce_should_clear_cart_after_payment', '__return_false' );
5. again, add a product to your cart and go through checkout with PAYMENT_METHOD -- the cart should now still contain the product even though you've completed checkout

[alexedwardjones]

@lsinger

You can achieve the necessary state by starting a payment with a method that redirects to an external payment gateway but then cancelling before finishing the payment. The simplest way is probably installing and activating the Stripe plugin, then activating AliPay via Stripe. If you try to pay with AliPay then the redirect will fail but the order will be created in the new default status.

[lsinger]

@alexedwardjones sorry, could you actually go ahead and rewrite the PR description to reflect the new solution? We'll need that in later testing stages even after merging the PR.

[alexedwardjones]

@lsinger Hey, I've updated the test instructions and gone in a slightly different direction to instead test that the code change works by forcing the cart to empty. This avoids the need to have someone install and configure an external payment gateway.

[lsinger]

@lsinger Hey, I've updated the test instructions and gone in a slightly different direction to instead test that the code change works by forcing the cart to empty. This avoids the need to have someone install and configure an external payment gateway.

Thanks @alexedwardjones -- that's more along the lines of what we need. I do find it rather removed from the original intent now, though. Isn't the idea that ...

1. an extension might add a custom order status and
2. the extension might have a need to a) keep that order status from clearing the cart or b) force that order status to clear the cart?

While the above instructions might work, I can't really see the intent of the change in it anymore. I'd prefer if that were still visible so that when many years down the line someone else comes across this PR and wonders why they will be able to understand. 🤔 Do you have a suggestion on how we could rewrite those instructions to ensure the business intent is captured?

[lsinger]

(If communication via this PR is too slow and you'd like to speed things up, please reach out to me in the WooCommerce Community Slack.)

[alexedwardjones]

Hey @lsinger, I've updated the test instructions again. I'll message you on Slack and we can continue this discussion there.

[alexedwardjones]

Hey @vedanshujain 👋 Any chance I can get a review on this?

[alexedwardjones]

Hey @lsinger @barryhughes, I don't seem to be getting any response from @vedanshujain. Can I get another reviewer assigned to this PR.

[barryhughes]

Feel free to request a fresh review once ready (I noticed the recent commits, but wasn't sure if you were perhaps still iterating on this).

[barryhughes]

Running CI, and if all passes we can go ahead and merge (this should then ship as part of the upcoming 9.3 release, scheduled for 2024-09-10).

[lsinger]

Hooray! Congrats, @alexedwardjones! 🎉

[alexedwardjones]

We made it! Thanks for your help @lsinger @barryhughes @vedanshujain

[rodelgc]

Removing impact: high label after clarifying certain aspects with PR reviewers.

[barryhughes]

Alternative testing steps

Since testing with AliPay may be problematic in some cases, here are some alternative steps.

1) Confirm normal behaviour

This PR should not change any of the expected, default checkout flows. So, run through some test purchases: they should work exactly as you expect/exactly as before this change was introduced.

2) Create a test gateway that does not automatically clear the cart

Lots of gateways, including default ones such as BACS or Cheque Payments, take care of clearing the cart after the order is placed. So, they are not suitable for testing this change. Let's instead create a custom payment gateway of our own.

Create a new file in a suitable location. I recommend wp-content/mu-plugins/test-44515.php (remove it when you finish testing this PR), containing the following:

<?php

add_action( 'woocommerce_init', function () {
	class Test_PR_44515_Gateway extends WC_Payment_Gateway {
		public function __construct() {
			$this->id                 = 'test-pr-44515';
			$this->method_title       = 'Test PR 44515';
			$this->method_description = 'A payment gateway that can be used to test PR#44515.';
			$this->has_fields         = false;
			$this->init_form_fields();
			$this->init_settings();
			$this->title       = $this->get_option( 'title' );
			$this->description = $this->get_option( 'description' );
			add_action( 'woocommerce_update_options_payment_gateways_' . $this->id, array(
				$this,
				'process_admin_options'
			) );
		}

		public function init_form_fields() {
			$this->form_fields = array(
				'enabled'     => array(
					'title'   => 'Enable/Disable',
					'type'    => 'checkbox',
					'label'   => 'Enable Test Gateway',
					'default' => 'yes'
				),
				'title'       => array(
					'title'       => 'Title',
					'type'        => 'text',
					'description' => 'This controls the title which the user sees during checkout.',
					'default'     => 'Test PR #44515',
					'desc_tip'    => true,
				),
				'description' => array(
					'title'       => 'Description',
					'type'        => 'textarea',
					'description' => 'This controls the description which the user sees during checkout.',
					'default'     => 'This is our test gateway..',
				),
			);
		}

		public function process_payment( $order_id ) {
			$order = wc_get_order( $order_id );
			$order->update_status( 'complete' );

			return array(
				'result'   => 'success',
				'redirect' => $this->get_return_url( $order ),
			);
		}
	}

	add_filter('woocommerce_payment_gateways', function ( $gateways ) {
		$gateways[] = Test_PR_44515_Gateway::class;
		return $gateways;
	});
} );

Now visit WooCommerce ‣ Settings ‣ Payments and enable the Test PR 44515 gateway. Note that this is a very basic gateway for testing purposes only and, to see it on the frontend, you will need to use the classic checkout.

As a reminder, this is simply a page containing the [woocommerce_checkout] shortcode (you'll need to use a shortcode block to add this from the block editor. Create that if you need to, and copy the page URL.

Now, acting as a shopper, add something to your cart then place the order via the classic checkout. If multiple payment gateways are enabled, be sure to select our new test gateway! Everything should work as expected and if you subsequently visit the cart page, you should see it is empty (again, as expected).

3) Verify the new filter hook works

Amend your test code, and add the following line to the bottom (on a new line, below the final } );):

add_filter( 'woocommerce_should_clear_cart_after_payment', fn () => false );

Now repeat that last test, again taking care to place the order via the classic checkout page. This time, if you visit the cart page after placing the order, you should find the cart is still populated with whatever item(s) you just purchased.