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

feat: Add support for fallback tax service #4871

Merged
merged 8 commits into from
Jan 9, 2019

Conversation

impactmass
Copy link
Contributor

@impactmass impactmass commented Dec 12, 2018

Resolves #4817

Issue:

Add ability to have two tax services configured and enabled, one primary and one fallback. The fallback should be used only when the primary is down, to ensure that orders can still be placed with taxes.

Changes

  1. Updates Shop Tax Settings form to allow selection of a fallback tax service. Stores the value in the package DB entry under settings field as fallbackTaxServiceName.

  2. Changed previous active-tax service nomenclature to primary, to reflect the addition of a fallback

  3. Changed existing getActiveTaxServiceForShop to getTaxServicesForShop. Now, getTaxServicesForShop returns both the primary and fallback tax services enabled for the shop.

  4. Add tests for getTaxServicesForShop capturing how it should behave with different inputs

  5. Update tax calculation step to use fallback config when primary returns null. This happens before falling back to default 0 tax (if forceZeroes is set)

Breaking Changes

None. From my checks, the modified functions and names are all private to the core tax setup.

A migration is included for the schema field name change. An app's tax setting running previous release should work same even after getting this update.

Testing

Requirements: Two tax services should be provided. Reaction already includes a "custom-rates" tax plugin. I made a clone of the custom rates plugin and updated parts of it to enable using it as an extra tax service. You can pull that from here and add it as a custom plugin.

  1. Visit the Tax Admin UI. Select "Rapid" as primary tax service. Select "Custom Rates" as the fallback; then save.
  2. Configure the tax services:
    • For custom tax, configure a rate with values like these: Country: US, Region: CA, Postal: 90405, Rate: 6
    • For rapid-tax (i.e the custom test plugin), configure a rate with values like these: Country: US, Region: CA, Postal: 90405, Rate: 100
  3. On the storefront app, add a product* to cart and proceed to checkout (either as guest or logged in). [* Use a product that doesn't have a specific tax code set. More on that below]
  4. Add in a shipping address. Use an address with the postal code you set for the tax service. Save. Confirm that the tax shown reflects what rate is set in the primary tax service
  5. Open the calculateOrderTaxes function in the custom plugin. Here you will change this line of code to make the plugin return null. This is so that the fallback tax config can kick in. After changing that, wait for the reaction app to restart.
  6. Visit the storefront again. Click "change" to edit the shipping address. Change only the name and save. The tax calculation should reload. Confirm that the new tax reflects the fallback rate/setting.

Note about Tax Codes

  • We may have a situation where when editing a product, the primary tax service provided tax codes, then during a checkout the fallback might be the one providing tax calculation. This means that when Tax Codes are active, both the primary and fallback tax services must provide (and process) a similar tax code list.

@aldeed aldeed changed the base branch from release-2.0.0-rc.8 to develop December 14, 2018 20:09
@impactmass impactmass force-pushed the feat-4817-impactmass-fallback-tax-service branch from 8353192 to 6c1db77 Compare December 18, 2018 21:40
@impactmass impactmass changed the title WIP - feat 4817 impactmass fallback tax service feat: Add support for fallback tax service Jan 7, 2019
@Akarshit
Copy link
Contributor

Akarshit commented Jan 8, 2019

@impactmass So there could be a case when the tax plugin is triggered but there was no shipping entered because of which the tax-service returned null, will the fallback service calculate a tax in this case too?

@impactmass
Copy link
Contributor Author

impactmass commented Jan 8, 2019

@Akarshit thanks for raising this scenario. As it is now, the fallback will also be called. And if fallback service also requires shipping info, it will also return null.

Some thoughts:
(A). Is shipping info always required for tax calculations?
(B). If the answer to (A) is yes, then we should not call calculateTaxes at all if no shipping info is supplied?
(C). If the answer to (A) is no, should we allow the fallback to kick-in when the primary returns null?

Copy link
Collaborator

@brent-hoover brent-hoover left a comment

Choose a reason for hiding this comment

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

Tested. Verified working per test instructions.

@impactmass
Copy link
Contributor Author

Merging this though I see a CI test error timeout of 2000ms exceeded. The test failure showed up after updating this branch with develop. I see the issue is happening on develop. I'll see it I can open a separate PR to fix it there.

@impactmass impactmass merged commit cd61e11 into develop Jan 9, 2019
@impactmass impactmass deleted the feat-4817-impactmass-fallback-tax-service branch January 9, 2019 12:51
@spencern spencern mentioned this pull request Jan 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants