Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Fix - "Order received" page does not display the payment method information. #9092

Merged
merged 5 commits into from
Apr 19, 2023

Conversation

iamdharmesh
Copy link
Member

This PR contains a minor change to fix #8448. Currently, when we place an order using the checkout block, the payment method title information not getting saved to the order and it results in payment method information is not showing on Order received page and backed order list page as described in the reported issue.

This was happening due to we are passing the payment method ID in set_payment_method() in the update_order_from_request function so, Payment method title is not getting updated for orders. This PR updates it to pass the payment method object to update the payment method title to order.

Fixes #8448

Accessibility

Other Checks

  • This PR adds/removes a feature flag & I've updated this doc.
  • This PR adds/removes an experimental interfaces and I've updated this doc.
  • I tagged two reviewers because this PR makes queries to the database or I think it might have some security impact.

Screenshots

Before After
Screenshot 2023-04-19 at 11 06 06 AM Screenshot 2023-04-19 at 11 05 00 AM
image image

Testing

Automated Tests

  • Changes in this PR are covered by Automated Tests.
    • Unit tests
    • E2E tests

User Facing Testing

  1. Checkout to this PR branch.
  2. Create a product and add it to the site's shopping cart.
  3. Proceed to checkout using the "Checkout" Block
  4. Complete the checkout process and confirm the order.
  5. Verify that payment method information is there on the "Order received" page.
  • Do not include in the Testing Notes

WooCommerce Visibility

  • WooCommerce Core
  • Feature plugin
  • Experimental

Performance Impact

Changelog

Fix - "Order received" page does not display the payment method information.

@nielslange nielslange self-requested a review April 19, 2023 06:01
Copy link
Member

@nielslange nielslange left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for working on this, @iamdharmesh! 🙌

While the fix works as expected, I'd like to get a second review from @mikejolley, who has deeper knowledge about the Store API. What surprises me in this particular case, is the history of the line you changed:

This PR

$this->order->set_payment_method( $this->get_request_payment_method( $request ) );

Jan 5, 2022

$this->order->set_payment_method( $this->get_request_payment_method_id( $request ) );

Oct 26, 2021

$this->order->set_payment_method( $request['payment_method'] ?? '' );

Sep 28, 2021

$this->order->set_payment_method( $this->get_request_payment_method( $request ) );

Dec 2, 2020

$this->order->set_payment_method( $this->order->needs_payment() ? $this->get_request_payment_method( $request ) : '' );

@mikejolley Would you mind taking a look at this case?

@nielslange nielslange added type: bug The issue/PR concerns a confirmed bug. block: checkout Issues related to the checkout block. Contributor Day - H1 2023 labels Apr 19, 2023
@ankitguptaindia
Copy link
Member

QA/Test Report-

Testing Environment -

  • WordPress: 6.2
  • Theme: Storefront Version: 4.2.0
  • WooCommerce - Version 7.6.0
  • PHP: 8.0.22
  • Web Server: Nginx
  • Browser: Chrome - Version 111.0.5563.64
  • OS: macOS Monterey V 12.3.1

Steps to Test-

Steps to test this PR:

  1. Install the WooCommerce Block plugin on a WordPress site.
  2. Create a product and add it to the site's shopping cart.
  3. Proceed to checkout using the "Checkout" Block, which can be added to a page using the "WooCommerce" block category.
  4. Complete the checkout process and confirm the order.
  5. On the "Order received" page, observe that the payment method information is available.
  6. Also check payment method on New Order Email, Order Listing page in wp-admin

Test Results -

This pull request has been tested and found to be functional in resolving the aforementioned issue. Payment method is now appearing consistently across all relevant areas, including the Order Confirmation Thank You Page, Order Listing page in wp-admin, and Email sent to the customer upon successful payment. All relevant test cases have passed successfully.

Functional Demo / Screencast -

Before Fix-

Woo.Contri.-.Before.Fix.mp4

After Fix-

Woo.Contri-.After.Fix.mp4

Negative Test - To ensure new change doesn't break already working functionality.

Negative.Test.mp4

QA Status- Pass ✅

@mikejolley
Copy link
Member

@nielslange @iamdharmesh Looking back at the last major change in #5440, both functions were changed so that get_request_payment_method_id uses get_request_payment_method internally, so flipping these again is fine and should work with the core function which I'm pasting below for reference:

   /**
    * Set the payment method.
    *
    * @param string $payment_method Supports WC_Payment_Gateway for bw compatibility with < 3.0.
    * @throws WC_Data_Exception Throws exception when invalid data is found.
    */
   public function set_payment_method( $payment_method = '' ) {
   	if ( is_object( $payment_method ) ) {
   		$this->set_payment_method( $payment_method->id );
   		$this->set_payment_method_title( $payment_method->get_title() );
   	} elseif ( '' === $payment_method ) {
   		$this->set_prop( 'payment_method', '' );
   		$this->set_prop( 'payment_method_title', '' );
   	} else {
   		$this->set_prop( 'payment_method', $payment_method );
   	}
   }

However, while this does state that the method is for bw compatibility and not deprecated, the type defined by the docblock suggests that we should be passing a string.

The other concern is that there may be an edge case issue where there is no payment method. This would result is us passing null to the above set_payment_method function. This is unhandled and could result in breakage.

I think there are 2 options to resolve this.

  1. Handle the potential null value coming from get_request_payment_method inline, and if null, pass an empty string. This would fix the issue but could result in IDE warnings due to the unexpected object type rather than string as documented.

  2. Leave this line as is and instead add a second call to $this->order->set_payment_method_title(). There we can pass the title instead. We could introduce a get_request_payment_method_title helper method in our Checkout class to obtain this and handle null (return empty string if null).

How does this sound?

@iamdharmesh
Copy link
Member Author

Thanks for the detailed explanation @mikejolley, I have updated PR as per 2nd option. Could you please help to review it once?

Thanks

Copy link
Member

@mikejolley mikejolley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making those changes. This fixes the original issues and any potential edge cases. 👍🏻

@mikejolley
Copy link
Member

e2e tests passing. The failing tests are relating to forks/permission issues, so ignoring. Merging this now 👍🏻

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
block: checkout Issues related to the checkout block. Contributor Day - H1 2023 type: bug The issue/PR concerns a confirmed bug.
Projects
None yet
4 participants