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

[React Components] update setShippingAddressOnCart to refetch cart details #270

Merged
merged 5 commits into from
May 26, 2020

Conversation

dabe5651
Copy link
Contributor

@dabe5651 dabe5651 commented May 12, 2020

Description

Updated checkout shipping address to rerfetch cart details query now that we have address to use for taxes to fix missing tax from grand total.

Related Issue

Fixes #269.

Motivation and Context

This is required so users aren't able to checkout without taxes being applied to the cart.

How Has This Been Tested?

I tried adding an address as a guest and signed in users and saw the tax apply correctly.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • [x ] I have signed the Adobe Open Source CLA.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes and the overall coverage did not decrease.
  • All unit tests pass on CircleCi.
  • I ran all tests locally and they pass.

@codecov
Copy link

codecov bot commented May 12, 2020

Codecov Report

Merging #270 into master will decrease coverage by 0.08%.
The diff coverage is 12.50%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #270      +/-   ##
============================================
- Coverage     63.20%   63.11%   -0.09%     
- Complexity      754      755       +1     
============================================
  Files           173      172       -1     
  Lines          5354     5366      +12     
  Branches        840      842       +2     
============================================
+ Hits           3384     3387       +3     
- Misses         1855     1862       +7     
- Partials        115      117       +2     
Flag Coverage Δ Complexity Δ
#jest 40.73% <12.50%> (-0.09%) 0.00 <0.00> (ø)
#karma 94.88% <ø> (ø) 0.00 <ø> (ø)
#unittests 85.10% <ø> (-0.10%) 755.00 <ø> (+1.00) ⬇️
Impacted Files Coverage Δ Complexity Δ
...components/src/components/Checkout/editableForm.js 35.71% <12.50%> (-3.35%) 0.00 <0.00> (ø)
...nal/models/v1/searchresults/SearchResultsImpl.java 89.47% <0.00%> (-10.53%) 12.00% <0.00%> (+4.00%) ⬇️
...nternal/models/v1/productlist/ProductListImpl.java 84.84% <0.00%> (-1.16%) 23.00% <0.00%> (+6.00%) ⬇️
...ch/internal/services/SearchResultsServiceImpl.java 85.71% <0.00%> (-0.13%) 35.00% <0.00%> (+1.00%) ⬇️
...core/search/internal/models/SearchOptionsImpl.java 100.00% <0.00%> (ø) 15.00% <0.00%> (ø%)
...oductcollection/clientlibs/js/productcollection.js
...ls/v1/productcollection/ProductCollectionImpl.java
...ctlist/v1/productlist/clientlibs/js/productlist.js 93.87% <0.00%> (ø) 0.00% <0.00%> (?%)
...e/search/internal/models/SearchResultsSetImpl.java 69.44% <0.00%> (+7.28%) 17.00% <0.00%> (+1.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5893c60...39c1119. Read the comment docs.

@herzog31 herzog31 added the bug Something isn't working label May 13, 2020
dispatch({ type: 'error', error: error.toString() });
})
.finally(() => {
dispatch({ type: 'endLoading' });
Copy link
Member

Choose a reason for hiding this comment

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

The beginLoading and endLoading events are only available via the CartContext, so you need to use cartDispatch here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I updated the events to use cartDispatch here.

Copy link
Member

Choose a reason for hiding this comment

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

@dabe5651 please make sure you run tests locally first and they pass. CircleCI reports test failures.

@dabe5651 dabe5651 requested a review from herzog31 May 20, 2020 12:32
Copy link
Contributor

@dplaton dplaton left a comment

Choose a reason for hiding this comment

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

Code looks good to me 👍

@herzog31
Copy link
Member

Somehow CircleCI doesn't like this fork. But I checked out the branch locally and confirmed that all tests pass.

@dplaton
Copy link
Contributor

dplaton commented May 26, 2020

I checked the CI pipeline with another branch, it worked. I'll merge the PR.
Thanks for the contribution, @dabe5651 !

@dplaton dplaton merged commit ac2d42a into adobe:master May 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working verified
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Checkout issue - tax not being added
3 participants