Skip to content

Conversation

wjrosa
Copy link
Contributor

@wjrosa wjrosa commented Oct 8, 2025

Fixes STRIPE-524

Changes proposed in this Pull Request:

This PR removes some unnecessary instantiations of the legacy checkout, as UPE is now the only option available. I am also deprecating the main legacy gateway class to make it easier to remove other references to it later.

Testing instructions

  • Checkout to this branch on your test environment (update/deprecate-and-remove-legacy-checkout-instances)
  • Rebuild your test store (or create a new one using something like JN)
  • Install the extension
  • Connect your Stripe account
  • Confirm the Stripe settings page is working fine
  • Check your settings database key and confirm upe_checkout_experience_enabled is set to yes

  • Covered with tests (or have a good reason not to test in description ☝️)
  • Tested on mobile (or does not apply)

Changelog entry

  • This Pull Request does not require a changelog entry. (Comment required below)
Changelog Entry Comment

Comment

Post merge

@wjrosa wjrosa self-assigned this Oct 8, 2025
@wjrosa wjrosa marked this pull request as ready for review October 8, 2025 19:37
@wjrosa wjrosa requested review from a team, daledupreez and diegocurbelo and removed request for a team October 8, 2025 19:47
Copy link
Member

@diegocurbelo diegocurbelo left a comment

Choose a reason for hiding this comment

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

This looks good. I could not find any other instantiation of the Legacy gateway.

I think we should also update the test suite accordingly. It's still being used in these 4 test files:

  • WC_Stripe_Payment_Gateway_Test.php
  • WC_REST_Stripe_Settings_Controller_Test.php
  • WC_Stripe_Settings_Controller_Test.php
  • WC_Stripe_Express_Checkout_Helper_Test.php

@wjrosa
Copy link
Contributor Author

wjrosa commented Oct 15, 2025

Thanks for the review, Diego! I removed all references from the unit tests in df553ae

@wjrosa wjrosa requested a review from diegocurbelo October 15, 2025 14:46
Copy link
Contributor

@daledupreez daledupreez left a comment

Choose a reason for hiding this comment

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

Changes LGTM! I have some feedback/commentary, but none of it requires immediate action.

* Gateway instance that the controller uses.
*
* @var WC_Gateway_Stripe
* @var WC_Stripe_UPE_Payment_Gateway
Copy link
Contributor

Choose a reason for hiding this comment

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

Non-blocking style note: I prefer using the fully-qualified class name so it's easier to see exactly which class is in use, especially in diffs. I think we should discuss this as a team before making changes to any code, though.

Suggested change
* @var WC_Stripe_UPE_Payment_Gateway
* @var \WC_Stripe_UPE_Payment_Gateway

@wjrosa wjrosa merged commit 31fd315 into develop Oct 16, 2025
40 checks passed
@wjrosa wjrosa deleted the update/deprecate-and-remove-legacy-checkout-instances branch October 16, 2025 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants