Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pre-orders status when UPE is enabled (with a new payment method) #2911

Merged
merged 6 commits into from
Feb 20, 2024

Conversation

wjrosa
Copy link
Contributor

@wjrosa wjrosa commented Feb 16, 2024

Fixes #2572

Changes proposed in this Pull Request:

This PR fixes an issue when checking out pre-order products with a new payment method. Currently, there's a bug that puts pre-orders into the processing status on this case (the status should remain as pre-ordered).

The fix consists in replicating part of the non-UPE purchase flow and not calling the process_response or payment_complete for pre-orders.

Testing instructions

  • Checkout to this branch on your local environment
  • Run npm install, npm build:webpack and npm run up
  • Access your admin and install pre-orders plugin
  • Create a pre-order product
  • Purchase it using a new payment method
  • Confirm that the order status will be pre-ordered as expected

  • Covered with tests (or have a good reason not to test in description ☝️)
  • Added changelog entry in both changelog.txt and readme.txt (or does not apply)
  • Tested on mobile (or does not apply)

Post merge

@wjrosa wjrosa self-assigned this Feb 16, 2024
@wjrosa wjrosa changed the title Fix pre-orders status when UPE is enabled Fix pre-orders status when UPE is enabled (with a new payment method) Feb 16, 2024
*/
public function get_latest_charge_from_intent( $intent ) {
if ( ! empty( $intent->charges->data ) ) {
return end( $intent->charges->data );
} else {
} elseif ( ! empty( $intent->latest_charge ) ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This just fixes a warning shown at the top of the page for pre-orders, the latest_charge property does not exist in this case (per my tests). Also, can be nullable per Stripe's documentation.

Copy link
Contributor

Choose a reason for hiding this comment

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

The latest_charge issue has been fixed in #2910. Sharing this in case you would like to update this to avoid conflicts later.

@wjrosa wjrosa requested review from a team and Mayisha and removed request for a team February 16, 2024 18:23
@a-danae
Copy link
Contributor

a-danae commented Feb 19, 2024

Thanks for your work here @wjrosa! Could we please point these changes to the add/deferred-intent branch instead of develop?

The UPE files we're changing in this PR are heavily updated in that branch. By implementing these changes there directly, we'd save ourselves some potential conflicts and extra testing when updating that branch with the latest develop.

@wjrosa wjrosa changed the base branch from develop to add/deferred-intent February 19, 2024 12:27
@wjrosa
Copy link
Contributor Author

wjrosa commented Feb 19, 2024

Thanks for your work here @wjrosa! Could we please point these changes to the add/deferred-intent branch instead of develop?

The UPE files we're changing in this PR are heavily updated in that branch. By implementing these changes there directly, we'd save ourselves some potential conflicts and extra testing when updating that branch with the latest develop.

Hi @a-danae ! Sure, just did it 😄

@a-danae
Copy link
Contributor

a-danae commented Feb 19, 2024

Thanks Wesley! 🙌

Copy link
Contributor

@Mayisha Mayisha left a comment

Choose a reason for hiding this comment

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

The fix works as described 🎉

Before this change: status of a pre-order is set as Processing

In this branch:

  • Status of a pre-order is set as Pre-ordered ✔️
  • Status of a normal order is set as processing ✔️

*/
public function get_latest_charge_from_intent( $intent ) {
if ( ! empty( $intent->charges->data ) ) {
return end( $intent->charges->data );
} else {
} elseif ( ! empty( $intent->latest_charge ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The latest_charge issue has been fixed in #2910. Sharing this in case you would like to update this to avoid conflicts later.

@wjrosa wjrosa merged commit 5276c93 into add/deferred-intent Feb 20, 2024
32 checks passed
@wjrosa wjrosa deleted the fix/pre-orders-when-upe-is-enabled branch February 20, 2024 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants