-
Notifications
You must be signed in to change notification settings - Fork 10.7k
Ensure subscriptions menu (experiment) is only visible to eligible stores #32867
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
Ensure subscriptions menu (experiment) is only visible to eligible stores #32867
Conversation
plugins/woocommerce/src/Internal/Admin/WcPaySubscriptionsPage.php
Outdated
Show resolved
Hide resolved
plugins/woocommerce/src/Internal/Admin/WcPaySubscriptionsPage.php
Outdated
Show resolved
Hide resolved
james-allan
left a comment
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.
Nice work on this so far @Jinksi. I haven't had a chance to test it yet but gave some initial feedback about the wc_get_orders() query.
james-allan
left a comment
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.
Changes look good and work well in testing. I've just left a comment about adding 1 additional slug for other subscription plugin options.
We may need to confirm internally the sale criteria but otherwise this works for me.
Nice work @Jinksi !
plugins/woocommerce/src/Internal/Admin/WcPaySubscriptionsPage.php
Outdated
Show resolved
Hide resolved
plugins/woocommerce/src/Internal/Admin/WcPaySubscriptionsPage.php
Outdated
Show resolved
Hide resolved
…eriment-subscriptions-admin-menu-eligibility # Conflicts: # plugins/woocommerce/src/Internal/Admin/WcPaySubscriptionsPage.php
plugins/woocommerce/src/Internal/Admin/WcPaySubscriptionsPage.php
Outdated
Show resolved
Hide resolved
|
LGTM! Nice work @Jinksi Here's the set of tests I've run locally: Location:
Store Age:
Recent Sales:
Subscription plugins:
The two plugins noted with a 🔴 I wasn't able to test without getting my hands on the plugin itself. IMO it's reasonably safe to assume those slugs are correct after we found them referenced in the internal data sources. |
|
Hi @Jinksi, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:
|
All Submissions:
Changes proposed in this Pull Request:
This PR prevents rendering the WooCommerce > Subscriptions menu item for WooCommerce stores that are ineligible for the experiment, as defined in #32682.
A store is ineligible for the experiment if:
Conflicting subscription plugins, installed and confirmed to ensure ineligibility:
woocommerce-subscriptions-pro(confirmed via Jetpack Site Profiles)xa-woocommerce-subscriptions(confirmed via Jetpack Site Profiles)Part of #32682
How to test the changes in this Pull Request:
wp-envfrom theplugins/woocommercedirectory, so you can delete plugins in an isolated environment.woocommerce_admin_install_timestampoption in thewp_optionstable to a Unix timestamp that is >= 6 months in the past.I installed the WP Console plugin and used it to run the following:
update_option( 'woocommerce_admin_install_timestamp', strtotime( '-6 months' ) );Delete the existing cached transient: Using WP Console, run
delete_transient( 'woocommerce-wcpay-subscriptions_recent_sales_eligibility' );update_option( 'woocommerce_admin_install_timestamp', strtotime( '-1 month' ) );Using WP Console, run
delete_transient( 'woocommerce-wcpay-subscriptions_recent_sales_eligibility' );Other information:
pnpm nx affected --target=changelog?FOR PR REVIEWER ONLY: