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

Supplement the implementation of API Pull for the Settings page #2779

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

eason9487
Copy link
Member

Changes proposed in this Pull Request:

This is a preliminary change before granting API Pull access is added to the onboarding flow.

To avoid too many file changes for subsequent PRs, this PR changes a few things first:

  • Remove the unused prop hideNotificationService from MerchantCenterAccountInfoCard.
    • The use of hideNotificationService has been removed by PR #2639
  • Add the two missing properties to the GoogleMCAccount type.
  • Stay with the loading state of EnableNewProductSyncButton during the page redirection.
  • Adjust two event tracking and add their doc.
    • gla_enable_product_sync_click: Add the page event property to indicate which page the event happened to facilitate further breakdown after the onboarding page also triggers this tracking in a later PR.
    • gla_disable_product_sync_click: Change its trigger time to when the confirm button in the confirmation modal is clicked rather than when the button to open the confirmation modal is clicked.

Detailed test instructions:

  1. Open the Console tab in the browser DevTool
    • Configure it as below
      0
    • Run localStorage.setItem( 'debug', 'wc-admin:*' ) to enable tracking debugging mode
  2. Go to the Settings page.
  3. Check if the functions to enable and disable the new product sync (API Pull) work well as before
  4. View the Console tab in the browser DevTool and check if the following event tracking is triggered
    • 1
    • 2
    • 3

Changelog entry

Tweak - Supplement the implementation of API Pull for the Settings page.

…AccountInfoCard`.

The use of `hideNotificationService` has been removed by:
- #2639
… doc.

- Add the `page` event property to indicate which page the event happened
…s doc.

- Change its trigger time to when the confirm button in the confirmation modal
  is clicked rather than when the button to open the confirmation modal is
  clicked.
@eason9487 eason9487 requested a review from a team January 23, 2025 10:12
@eason9487 eason9487 self-assigned this Jan 23, 2025
@github-actions github-actions bot added the changelog: tweak Small change, that isn't actually very important. label Jan 23, 2025
Copy link

codecov bot commented Jan 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.7%. Comparing base (6e15a75) to head (e922ae1).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             develop   #2779      +/-   ##
============================================
- Coverage       67.2%   66.7%    -0.5%     
============================================
  Files            480     316     -164     
  Lines          19569    4921   -14648     
  Branches           0    1204    +1204     
============================================
- Hits           13155    3282    -9873     
+ Misses          6414    1502    -4912     
- Partials           0     137     +137     
Flag Coverage Δ
js-unit-tests 66.7% <100.0%> (?)
php-unit-tests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...s/src/components/enable-new-product-sync-button.js 60.0% <100.0%> (ø)
...s/src/components/enable-new-product-sync-notice.js 100.0% <ø> (ø)
js/src/data/selectors.js 57.1% <ø> (ø)
js/src/hooks/useGoogleMCAccount.js 12.5% <ø> (ø)

... and 792 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: tweak Small change, that isn't actually very important.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant