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

fix: properly save all customFields from tax service result #4986

Merged
merged 5 commits into from Feb 14, 2019

Conversation

2 participants
@aldeed
Copy link
Member

commented Feb 14, 2019

Impact: minor
Type: bugfix

Issue

#4955 and #4965 added the ability for tax services to return customFields in the response. A couple bugs were found after merging.

  • When calculating for a cart, customFields on cart items and on taxSummary were not saved to the database
  • When calculating for an order, customFields on cart items were not saved to the database

Solution

All customFields returned by a tax service at any level are now saved in the database, for both cart and order.

On cart and order items, the field name in the database is customTaxFields as opposed to customFields because otherwise it would be unclear that they're related to taxes since they're saved directly on the item object. The tax service should still return them as customFields; the field name change is only in the database.

Breaking changes

None

Testing

  1. Temporarily update the calculateOrderTaxes function in the taxes-rates plugin to include customFields objects in the taxSummary, in one of the itemTaxes objects, and in one of the taxes objects.
  2. Add items to a cart and begin checking out, using a destination address for which the shop collects taxes.
  3. Once you see a tax amount on the checkout page, look in MongoDB to verify that all three expected customFields objects were saved in the Cart document.
  4. Finish checking out. Look in MongoDB to verify that all three expected customFields objects were saved in the Order document.

aldeed added some commits Feb 14, 2019

refactor: move addTaxesToGroup func to its own file
Signed-off-by: Eric Dobbertin <eric@dairystatedesigns.com>
refactor: improve `addTaxesToGroup` func args
Signed-off-by: Eric Dobbertin <eric@dairystatedesigns.com>
test: add unit test for `addTaxesToGroup`
Signed-off-by: Eric Dobbertin <eric@dairystatedesigns.com>
refactor: move `getUpdatedCartItems` func to own file
Signed-off-by: Eric Dobbertin <eric@dairystatedesigns.com>
fix: properly save custom tax fields
Signed-off-by: Eric Dobbertin <eric@dairystatedesigns.com>

@aldeed aldeed requested a review from Akarshit Feb 14, 2019

@aldeed aldeed self-assigned this Feb 14, 2019

@aldeed aldeed added this to the 🏔 Torreys milestone Feb 14, 2019

@aldeed

This comment has been minimized.

Copy link
Member Author

commented Feb 14, 2019

@Akarshit This should fix the remaining issues with saving customFields for taxes. I have unit tests to prove it this time! 😄

@Akarshit Akarshit merged commit 504262b into develop Feb 14, 2019

5 checks passed

DCO DCO
Details
License Compliance All checks passed.
Details
WIP ready for review
Details
build_and_test Workflow: build_and_test
Details
security/snyk - package.json (Reaction Commerce) No manifest changes detected

@aldeed aldeed deleted the fix-aldeed-custom-fields branch Feb 15, 2019

@jeffcorpuz jeffcorpuz referenced this pull request Mar 1, 2019

Merged

Release v2.0.0 rc.10 #5016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.