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

Reorganizes the frontend code related to the calls of settings APIs #2742

Merged
merged 5 commits into from
Dec 30, 2024

Conversation

eason9487
Copy link
Member

Changes proposed in this Pull Request:

This is one of the preliminaries for the shipping Improvements.

To avoid too many file changes for subsequent PRs, this PR reorganizes the code related to the calls of settings APIs:

  • Add the saveSettings action as one of the return values to the useSettings hook and update the related uses.
    • Since they are always used together, and there will be a third use case in the subsequent PR.
  • Create a new action syncSettings, add it as one of the return values to the useSettings hook and update the related uses.
    • There will be a third use case of calling to mc/settings/sync API in the subsequent PR.
    • They are often used together.
  • Correct the JSDoc description for the saveSettings action.

Detailed test instructions:

  1. Go to step 2 of the onboarding flow
    1. Open the Network tab of browser's DevTool
    2. Change the Shipping rates setting
      image
    3. Inspect Network tab. Check if there is a request issued to wc/gla/mc/settings API and if the request is successful
    4. Select the Preserve log option on the Network tab
    5. Continue to complete the onboarding flow
    6. Inspect Network tab. Check if there is a request issued to wc/gla/mc/settings/sync API and if the request is successful
  2. Go to edit free listing page
    1. Open the Network tab of browser's DevTool
    2. Change the Tax rate or Shipping rates setting
    3. Save changes
    4. Inspect Network tab.
    5. Check if there is a request issued to wc/gla/mc/settings API and if the request is successful
    6. Check if there is a request issued to wc/gla/mc/settings/sync API and if the request is successful
      image

Changelog entry

@eason9487 eason9487 self-assigned this Dec 20, 2024
@github-actions github-actions bot added the changelog: dev Developer-facing only change. label Dec 20, 2024
Copy link

codecov bot commented Dec 20, 2024

Codecov Report

Attention: Patch coverage is 18.18182% with 9 lines in your changes missing coverage. Please review.

Project coverage is 63.5%. Comparing base (9c3514c) to head (bc91758).
Report is 29 commits behind head on develop.

Files with missing lines Patch % Lines
js/src/hooks/useSettings.js 0.0% 4 Missing ⚠️
js/src/pages/edit-free-listings/index.js 0.0% 3 Missing ⚠️
js/src/data/actions.js 0.0% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #2742     +/-   ##
=========================================
- Coverage     63.5%   63.5%   -0.0%     
=========================================
  Files          337     337             
  Lines         5193    5195      +2     
  Branches      1273    1273             
=========================================
  Hits          3299    3299             
- Misses        1721    1723      +2     
  Partials       173     173             
Flag Coverage Δ
js-unit-tests 63.5% <18.2%> (-<0.1%) ⬇️

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

Files with missing lines Coverage Δ
...es/onboarding/setup-stepper/saved-setup-stepper.js 87.2% <100.0%> (ø)
js/src/data/actions.js 15.7% <0.0%> (-0.1%) ⬇️
js/src/pages/edit-free-listings/index.js 2.1% <0.0%> (+<0.1%) ⬆️
js/src/hooks/useSettings.js 20.0% <0.0%> (-5.0%) ⬇️

@eason9487 eason9487 requested a review from a team December 20, 2024 11:51
Copy link
Member

@tomalec tomalec left a comment

Choose a reason for hiding this comment

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

Thank you for the changes @eason9487 !
I reviewed the code and run both flows, all LGTM.

@eason9487 eason9487 merged commit 2f71864 into develop Dec 30, 2024
9 checks passed
@eason9487 eason9487 deleted the dev/reorganize-frontend-settings-apis branch December 30, 2024 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: dev Developer-facing only change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants