Removing the UPE checkout enabled feature flag usages #4805
Conversation
wjrosa
requested review from
a team,
daledupreez and
diegocurbelo
and removed request for
a team
daledupreez
left a comment
daledupreez
left a comment
There was a problem hiding this comment.
Thanks for taking this on, @wjrosa!
Based on inspection, this looking really good, with the exception of some legitimate test failures in WC_Stripe_Inbox_Notes_Test.
It may also be worth triggering a manual e2e test run that includes the secondary LPMs. I will also aim to do some more manual testing over the next few hours.
includes/payment-methods/class-wc-stripe-upe-payment-gateway.php
Outdated
Show resolved
Hide resolved
https://github.com/woocommerce/woocommerce-gateway-stripe into dev/removing-upe-checkout-enabled-feature-flag-usages
|
Thanks for the review, Dale! I have fixed tests in bfa000d and also deprecated the |
daledupreez
left a comment
daledupreez
left a comment
There was a problem hiding this comment.
This is looking good to me, though I don't know how much I covered with my smoke tests. Still, given that we can no longer disable UPE, I think these changes should be relatively safe.
includes/admin/class-wc-stripe-rest-upe-flag-toggle-controller.php
Outdated
Show resolved
Hide resolved
diegocurbelo
left a comment
diegocurbelo
left a comment
There was a problem hiding this comment.
The changes look good. I left two optional suggestions.
| * @version 4.0.0 | ||
| */ | ||
| public function __construct() { | ||
| return; |
There was a problem hiding this comment.
This looks a bit odd; I think we should move the return below the add_action (as it was before), or remove the unreachable code (or the entire constructor).
| add_action( 'admin_notices', [ $this, 'admin_notices' ] ); | ||
| add_action( 'wp_loaded', [ $this, 'hide_notices' ] ); | ||
| add_action( 'woocommerce_stripe_updated', [ $this, 'stripe_updated' ] ); | ||
| add_action( 'after_plugin_row_woocommerce-gateway-stripe/woocommerce-gateway-stripe.php', [ $this, 'display_legacy_deprecation_notice' ], 10, 1 ); |
There was a problem hiding this comment.
display_legacy_deprecation_notice() does nothing now; there is no need to register this anymore:
Changes proposed in this Pull Request:
In this simple cleanup PR, I am removing all references to the UPE-enabled feature flag, as that is no longer necessary, since UPE is now the only version available.
I am also deprecating
WC_Stripe_UPE_Availability_NoteandWC_Stripe_REST_UPE_Flag_Toggle_Controlleras they do not make sense anymore.Testing instructions
Code review. Check if the tests are still passing. Perform some basic smoke testing and confirm no regression was introduced.
Changelog entry
Changelog Entry Comment
Comment
Post merge