-
Notifications
You must be signed in to change notification settings - Fork 69
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
Disable WooPay first party auth when using adapted extensions #7455
Conversation
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
Size Change: 0 B Total Size: 1.43 MB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes look good and test well 👍
Testing Instructions
- ✅ Disable any adapted extensions. Confirm that the option is now 1.
- ✅ Enable either/all of the extensions listed. Confirm that the option is now set to 0.
- ✅ Go through a WooPay checkout and confirm you get the OTP modal.
- ✅ Disable all the adapted extensions. Confirm that the option is now set to 1.
- ✅ Go through a WooPay checkout and confirm you get the first-party auth flow.
@@ -117,6 +117,12 @@ public function update_compatibility_and_maybe_show_incompatibility_warning() { | |||
public function update_enabled_adapted_extensions( $active_plugins, $adapted_extensions ) { | |||
$enabled_adapted_extensions = $this->get_extensions_in_list( $active_plugins, $adapted_extensions ); | |||
|
|||
if ( count( $enabled_adapted_extensions ) > 0 ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: No changes necessary, but we could also do something like the following:
// ...
$is_first_party_auth_enabled = \WC_Payments_Features::is_woopay_first_party_auth_enabled();
if ( count( $enabled_adapted_extensions ) > 0 && $is_first_party_auth_enabled ) {
update_option( \WC_Payments_Features::WOOPAY_FIRST_PARTY_AUTH_FLAG_NAME, 0 );
} else if ( count( $enabled_adapted_extensions ) === 0 && ! $is_first_party_auth_enabled ) {
update_option( \WC_Payments_Features::WOOPAY_FIRST_PARTY_AUTH_FLAG_NAME, 1 );
}
// ...
This way, we only update the option if it's necessary. Although, this is not necessary since this block of code only executes when plugins are activated/deactivated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Changes proposed in this Pull Request
It was discovered during compatibility testing that the new WooPay first party auth flow would break compatiblity with the WooPay adapted extensions. As a quick fix for this, we are going to disable the first party auth flow flag when we see that any of these extensions are enabled, and enable the flag when none of them are enabled.
This is accomplished in this PR by adding a check when we are updating the list of enabled adapted extensions and then setting the first party auth flag accordingly.
Testing instructions
For testing these changes specifically:
wp_options
table for the option name_wcpay_feature_woopay_first_party_auth
.Testing the plugins still in compatiblity.
npm run changelog
to add a changelog file, choosepatch
to leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge