Skip to content
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

Mark customize store task complete when user redirects to the site ed… #1307

Merged
merged 7 commits into from
Oct 24, 2023

Conversation

moon0326
Copy link
Contributor

@moon0326 moon0326 commented Sep 26, 2023

…itor from a theme details page

Changes proposed in this Pull Request:

Closes https://github.com/woocommerce/team-ghidorah/issues/264

  • This PR is a very minor change/addition and does not require testing instructions (if checked you can ignore/remove the next section).

How to test the changes in this Pull Request:

  1. Create a new ECOM/Free trial site and make it a WoA dev site.
  2. Make sure to install WooCommerce Beta Tester and enable customize-store feature flag from Tools -> WCA Test Helper -> Features
  3. Copy over the changes in this branch to your WoA dev site.
  4. Visit https://your-woa-dev-site/wp-admin/site-editor.php?from=theme-browser
  5. Confirm woocommerce_admin_customize_store_completed option is set to yes

For step 4, you can install SQL Executioner plugin and run the following query.

select * from wp_options where option_name = 'woocommerce_admin_customize_store_completed'

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

@moon0326 moon0326 marked this pull request as draft September 26, 2023 01:38
@github-actions
Copy link

github-actions bot commented Sep 26, 2023

Size Change: 0 B

Total Size: 196 kB

ℹ️ View Unchanged
Filename Size
./build/53.js 1.05 kB
./build/index.css 883 B
./build/index.js 121 kB
./build/marketing.js 58 kB
./build/payment-gateway-suggestions.css 1.25 kB
./build/payment-gateway-suggestions.js 6.45 kB
./build/plugins.js 3.92 kB
./build/style-index.css 2 kB
./build/style-marketing.css 805 B

compressed-size-action

@moon0326 moon0326 requested review from a team, psealock and adrianduffell September 26, 2023 20:31
@moon0326 moon0326 marked this pull request as ready for review September 26, 2023 20:31
Copy link
Contributor

@adrianduffell adrianduffell left a comment

Choose a reason for hiding this comment

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

It's looking nice @moon0326 , thank you! It tested well and I left a couple of suggestions to consider.

Pre-approving! 🚀

For other reviewers: I used my local wp-env site to test the PR and confirmed the option with wp-env run cli wp option get woocommerce_admin_customize_store_completed

private function __construct() {

// Only in Ecommerce or Free Trial
if ( ! wc_calypso_bridge_has_ecommerce_features() && ! wc_calypso_bridge_is_ecommerce_trial_plan() ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

How about checking the feature flag instead? I think it would keep us in sync with the planned rollout without needing to make changes in the future. The rollout currently looks like:

October: Woo Express only
December: Everywhere (Core, all WP.com hosted plans)

Between Oct-Dec, e-commerce plans could still be used for testing by changing the feature flag on the site.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good 👍 I'll make an update.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wp-env run cli wp option get woocommerce_admin_customize_store_completed

I always forget we have access to CLI in WoA dev site :) Thank you for the tip 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adrianduffell Updated in dee4b25

Could you run the test again, please? I've also updated the test instructions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @moon0326 -- I tested it again and it's all working nice!

includes/class-wc-calypso-bridge-customize-store.php Outdated Show resolved Hide resolved
@moon0326 moon0326 merged commit e58d4e7 into master Oct 24, 2023
5 checks passed
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.

2 participants