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

add: core profiler fetch extensions #38270

Merged
merged 1 commit into from May 18, 2023
Merged

Conversation

rjchow
Copy link
Contributor

@rjchow rjchow commented May 12, 2023

Submission Review Guidelines:

Changes proposed in this Pull Request:

Adds a prefetch call in the background for fetching extensions and countries in the init state without blocking the UI state.

This makes it so that subsequently when these calls are made, there won't be any loading time

Closes #37992.

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

Intermediate PR without user facing feature, QA testing not required

  1. Clone repo
  2. Go to new profiler page
  3. Note that there is no loading time for the 'skip flow'
  4. Reload profiler
  5. Note that Extensions page has the extensions populated, and also without loading time

@github-actions github-actions bot added focus: react admin [team:Ghidorah] plugin: woocommerce Issues related to the WooCommerce Core plugin. labels May 12, 2023
@rjchow rjchow changed the base branch from trunk to dev/add-core-profiler-state-tests May 12, 2023 08:01
@rjchow rjchow force-pushed the add/core-profiler-fetch-extensions branch from 0adba7f to c5d9f01 Compare May 12, 2023 08:05
@github-actions github-actions bot removed the plugin: woocommerce Issues related to the WooCommerce Core plugin. label May 12, 2023
@rjchow rjchow marked this pull request as ready for review May 12, 2023 08:17
@github-actions
Copy link
Contributor

github-actions bot commented May 12, 2023

Test Results Summary

Commit SHA: fed506a

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests26700202690m 52s
E2E Tests1890010019914m 46s

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.

@rjchow rjchow requested review from a team, chihsuan and adrianduffell May 12, 2023 09:30
@github-actions
Copy link
Contributor

Hi @chihsuan, @adrianduffell, @woocommerce/ghidorah

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

@rjchow rjchow force-pushed the dev/add-core-profiler-state-tests branch from 29520cd to c7f46ff Compare May 15, 2023 01:26
moon0326
moon0326 previously approved these changes May 16, 2023
Copy link
Contributor

@moon0326 moon0326 left a comment

Choose a reason for hiding this comment

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

LGTM and tested well 👍 🚀

I think we need a rebase.

Pre-approving

Base automatically changed from dev/add-core-profiler-state-tests to trunk May 16, 2023 00:56
@rjchow rjchow force-pushed the add/core-profiler-fetch-extensions branch from 8f14774 to bcc9cb9 Compare May 16, 2023 02:28
@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label May 16, 2023
@codecov
Copy link

codecov bot commented May 16, 2023

Codecov Report

Merging #38270 (bcc9cb9) into trunk (e077c4c) will decrease coverage by 0.1%.
The diff coverage is n/a.

❗ Current head bcc9cb9 differs from pull request most recent head fed506a. Consider uploading reports for the commit fed506a to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             trunk   #38270     +/-   ##
==========================================
- Coverage     51.6%    51.5%   -0.1%     
+ Complexity   17444    17443      -1     
==========================================
  Files          440      440             
  Lines        80297    80291      -6     
==========================================
- Hits         41401    41346     -55     
- Misses       38896    38945     +49     

see 4 files with indirect coverage changes

Copy link
Member

@chihsuan chihsuan left a comment

Choose a reason for hiding this comment

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

Looks good to me! 👍 😃

@rjchow rjchow force-pushed the add/core-profiler-fetch-extensions branch from bcc9cb9 to acfb160 Compare May 17, 2023 03:03
- prefetch it once in the machine init so that it's instant when used later
- fetch it again in preExtensions state and store it to context
- added prefetch for getCountries as well
@rjchow rjchow force-pushed the add/core-profiler-fetch-extensions branch from acfb160 to fed506a Compare May 18, 2023 02:16
@rjchow rjchow merged commit 00952e8 into trunk May 18, 2023
17 checks passed
@rjchow rjchow deleted the add/core-profiler-fetch-extensions branch May 18, 2023 02:51
@github-actions github-actions bot added this to the 7.8.0 milestone May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin: woocommerce Issues related to the WooCommerce Core plugin.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Core Profiler - Extensions Fetching Async Task
3 participants