Skip to content

Allow plugins_page_skipped parameter in Onboarding API endpoint #38764

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.

Sign up for GitHub

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

Jump to bottom
Merged
ilyasfoo merged 5 commits into from
Jun 16, 2023
Merged

Allow plugins_page_skipped parameter in Onboarding API endpoint #38764

ilyasfoo merged 5 commits into from
Jun 16, 2023

Conversation

@ilyasfoo
Copy link
Contributor

@ilyasfoo ilyasfoo commented Jun 16, 2023

Changes proposed in this Pull Request:

Closes #38659.

How to test the changes in this Pull Request:

  1. Install and activate WooCommerce on a brand new site
  2. Install the WooCommerce Beta Tester plugin
  3. Go to /wp-admin/tools.php?page=woocommerce-admin-test-helper
  4. Enable the 'core-profiler' feature flag
  5. Visit your public URL /wp-admin/admin.php?page=wc-admin&path=%2Fsetup-wizard
  6. Go through the new core profiler flow until the plugins step
  7. Skip the plugins step by clicking on Skip this step
  8. Go to Tools > WCA Test Helper
  9. In the Options tab, search for woocommerce_onboarding_profile
  10. Click on Edit
  11. Observe the value plugins_page_skipped: true exists

@ilyasfoo ilyasfoo requested review from a team and moon0326 June 16, 2023 08:11
@ilyasfoo ilyasfoo requested a review from adrianduffell June 16, 2023 08:11
@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Jun 16, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 16, 2023 •
edited
Loading

Hi @moon0326, @adrianduffell,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

adrianduffell
adrianduffell reviewed Jun 16, 2023
View reviewed changes
plugins/woocommerce/src/Admin/API/OnboardingProfile.php Outdated
'readonly' => true,
'validate_callback' => 'rest_validate_request_arg',
),
'plugins_page_skipped' => array(
Copy link
Contributor

@adrianduffell adrianduffell Jun 16, 2023

Choose a reason for hiding this comment

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

Suggested change
'plugins_page_skipped' => array(
'is_plugins_page_skipped' => array(

Nit: the other boolean params have an is_ prefix

Copy link
Contributor Author

@ilyasfoo ilyasfoo Jun 16, 2023

Choose a reason for hiding this comment

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

Good call! Renamed it in 2486c2b

@github-actions
Copy link
Contributor

github-actions bot commented Jun 16, 2023 •
edited
Loading

Test Results Summary

Commit SHA: 461d732

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202610m 50s
E2E Tests1950010020513m 38s
To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

@adrianduffell
Copy link
Contributor

adrianduffell commented Jun 16, 2023

@ilyasfoo Looks like there's a test that needs an updated assertion:

1) WC_Admin_Tests_API_Onboarding_Profiles::test_schema
Failed asserting that actual size 18 matches expected size 17.

/var/www/html/wp-content/plugins/woocommerce/tests/legacy/unit-tests/woocommerce-admin/api/onboarding-profile.php:134

@adrianduffell adrianduffell self-requested a review June 16, 2023 08:44
adrianduffell
adrianduffell approved these changes Jun 16, 2023
View reviewed changes
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.

Tested well for me and looks good! Thanks @ilyasfoo 🚀

I left a nit and also it looks like a minor change is needed for a test to pass

@github-actions github-actions bot added the package: @woocommerce/data issues related to @woocommerce/data label Jun 16, 2023
@ilyasfoo
Copy link
Contributor Author

ilyasfoo commented Jun 16, 2023

Thanks, Adrian! Merging since I'm confident with 1 review.

@ilyasfoo ilyasfoo merged commit 1e72bb0 into trunk Jun 16, 2023
@ilyasfoo ilyasfoo deleted the fix/38659-core-profiler-skipped-option-plugins-step branch June 16, 2023 10:07
@github-actions github-actions bot added this to the 7.9.0 milestone Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@moon0326 moon0326 Awaiting requested review from moon0326

1 more reviewer

@adrianduffell adrianduffell adrianduffell approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

package: @woocommerce/data issues related to @woocommerce/data plugin: woocommerce Issues related to the WooCommerce Core plugin.

Projects

None yet

Milestone

7.9.0

Development

Successfully merging this pull request may close these issues.

Core Profiler skip plugins page not saving onboarding options correctly

3 participants

@ilyasfoo @adrianduffell