-
Notifications
You must be signed in to change notification settings - Fork 146
Add explat variation for wc pay promotion #7554
Conversation
2de04bf
to
298c0b6
Compare
c501f1e
to
9cfca4f
Compare
298c0b6
to
7aae799
Compare
9cfca4f
to
21722b0
Compare
42e4bec
to
9a1f054
Compare
21722b0
to
23da7cf
Compare
2c77007
to
f5c9ce3
Compare
@louwie17 When this has been rebased, please ping me and I'll take a look. |
23da7cf
to
02d90fc
Compare
02d90fc
to
219e732
Compare
This has been rebased and updated to work with the latest wc.com data structure. |
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.
Looks great and is testing well! Left a couple minor comments, but pre-approving. The property check is the main one worth considering since this call will live in perpetuity in older WCA versions.
Not directly related to this PR, but noticed this error when initially fetching transients. Seems to be stemming from merge_specs()
:
$allow_tracking | ||
); | ||
|
||
$variation_name = $abtest->get_variation( 'woocommerce_wc_pay_promotion_payment_methods_table_' . $wc_pay_spec->additional_info->experiment_version ); |
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.
Do we need to double check that experiment_version
exists above first or are we sure it will always exist on this API going forward?
This will be a public API, so anything we add here will continue to be used for the same request in the future with this version of WooCommerce Admin.
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.
Yeah I should probably do that as well
@@ -0,0 +1,173 @@ | |||
<?php | |||
/** | |||
* A class that interacts with Explat A/B tests. |
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.
Should we add some comments here with the source URL this was copied from and any notes about when we can remove this in the future (if applicable)?
…min#7554) * Add experimental abtest class and use it in WCPayPromotion * Update experiment version data structure * Address PR feedback
Changes for #7319
Adds an experimental ab test class to allow running explat experiments in PHP, this is temporary and will be replace by a Jetpack library once Jetpack has been updated. I talked with @adrianduffell about this (same is happening on WC Pay at the moment).
Making use of the experiment in the WC Pay promotion.
No changelog added, as it will be in the final PR enabling it on all environments.
Detailed test instructions:
*Make sure you don't have WooCommerce Payments installed or enabled.
*Make sure you have logging enabled
add/payment-method-recommendation-endpoint
) -> 11084-gh-Automattic/woocommerce.comDataSourcePoller.php
to point to your local version with tohttp://woocommerce.test
or using your local ip might work as well -http://192....
you might have to remove the
woocommerce_admin_payment_method_promotion_specs
transient.explat_test_v3_
here so it reads -explat_test_v3_woocommerce_wc_pay_promotion_payment_methods_table_
control
, meaning the WooCommerce Payments should not be displayed in the payment method table in WooCommerce > Settings > Paymentsabtest_variation_explat_test_v3_woocommerce_wc_pay_promotion_payment_methods_table_v1
transient, you can usewp transient delete ...