Chore: shipping methods tax inclusive total #2130

carlos-r-l-rodrigues · 2022-08-30T15:54:33Z

What:
Calculate taxes from shipping methods where the tax is included in the price (includes_tax = true).

FIXES CORE-376

changeset-bot · 2022-08-30T15:54:37Z

⚠️ No Changeset found

Latest commit: f0e4fa2

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

adrien2p

Nice work man 👍

packages/medusa/src/services/__tests__/totals.js

packages/medusa/src/services/cart.ts

adrien2p · 2022-09-02T14:29:30Z

packages/medusa/src/services/totals.ts

+          ) &&
+          shippingMethod.includes_tax
+        ) {
+          const rate =


question: I think that we should have a for loop and call

const totals = await this.getShippingMethodTotals( shippingMethod, cartOrOrder )

on each shipping method. from that total we can get the taxes to remove from the price, in the actual way, we are using the tax_lines which can be missing and we don't take into account the free shipping.

wdyt?

Not sure if I understood it. In case we don't have tax_lines, the taxes will be 0, and free shipping will have price 0 no?

if we have a free shipping discount then it will be 0, but I am wondering why it is not taken into account for the shipping total. specially that the discount total, filter on the discounts that are not free shipping. therefore the getTotal will get all shipping accumulated but not take into account the free shipping.

also, if there is not tax lines it could be due to the backward compatibility of the tax rate being setup on the cartOrOrder, but it is not taken into account. now that there is the inclusive tag, the best way to know the real price of the shipping method would be to call the method getShippingMethodTotals Which take care of the different circumstances (plus the comment on the method too). that way you can compute back the real price. it is not the most performant at the moment, but until it is refactored i think it is the best approach, a bit like i ve done it here getLineItemTotal

@srindom could you confirm if i am missing something just for that part please?

Just to weigh in here:

Think it is a great idea to change this method to just be:

const { shipping_methods } = cartOrOrder return shipping_methods.reduce((acc, next) => { const totals = this.getShippingMethodTotals(next, cartOrOrder) return acc + totals.subtotal }, 0)

It will then be getShippingMethodTotals that is responsible for doing the tax calculation stuff.

Not sure if that was what you meant @adrien2p?

Yes this is exactly what i meant 😊 thanks man

But surrounded by a lot of explaination on why that choice which might be less understandable in the end 🤔

Haha alright :D

@carlos-r-l-rodrigues - does this make sense to you?

packages/medusa/src/strategies/__tests__/tax-calculation.js

packages/medusa/src/strategies/tax-calculation.ts

packages/medusa/src/utils/__tests__/calculate-price-tax-amount.ts

packages/medusa/src/services/totals.ts

* feat(medusa): Update totals and tax calculation way to calculate the totals * feat(medusa): remove region feetching from decorate total * feat(medusa): cleanup * test(medusa): fix tax calculation tests * comment * test(integration): cleanup * test(integration): cleanup * fix(medusa): return service missing await * feat(medusa): cleanup * feat(medusa): cleanup * test(integration): fix data * feat(medusa): improve tax calculation readability * test(medusa): improve tax calculation structure case Co-authored-by: Sebastian Rindom <skrindom@gmail.com>

…ds-tax-inclusive-total

packages/medusa/src/services/__tests__/totals.js

packages/medusa/src/services/pricing.ts

packages/medusa/src/services/totals.ts

packages/medusa/src/strategies/__tests__/tax-calculation.js

packages/medusa/src/strategies/tax-calculation.ts

packages/medusa/src/utils/__tests__/calculate-price-tax-amount.ts

srindom · 2022-09-05T16:29:23Z

packages/medusa/src/services/order.ts

+    const { select, relations, totalsToSelect } = this.transformQueryForTotals(
+      config
+    )


todo: my linter wants this to be like below - can you check your setup?

const { select, relations, totalsToSelect } = this.transformQueryForTotals(config)

Yeah, apparently ESlint and Prettier are behaving differently. I will check the rules, but I hope this is not blocking the PR for now.
At some point we have to apply a lint --fix to the entire project and validate it as the first step in the pipeline.

Have opened an issue on this

srindom

LGTM! 🎉

* add feature flag for tax inclusive pricing * update db model for TIP * add migration * set featureflag column decorators * remove unused prop * update tests to reflect feature_flags as any array * fix types * reference key from featureFlag file * use feature flag key in models * fix copy paste mistake * unify spelling * Create gorgeous-experts-guess.md * feat(medusa): create/update endpoints of currency/region/price-lists/shipping-options should allow to pass includes_tax * test(integration): continue to add some integration test * test(integration): continue to add some integration test * test(unit): Fix region service tests * fix(medusa): API unit tests flags management * feat(medusa): Minor cleanup * style(medusa): Fix typo * fix(medusa): rebase * feat(medusa): Replace old tag with the new one * feat(medusa): revert flag * feat(medusa): Cleanup * feat(medusa): feedback * feat(medusa): Rename currency retrieve method * test(medudsa): fix unit tests * chore(medusa): fix oas * feat(medusa): ShippingMethod should include tax setting from parent option (#2021) * feat(medusa): Shipping method should includes tax from parent options * feat(medusa): Condition the includes tax flag to the availability of the feature and add some other tests * test(integration): Move cart/order ff test in separate files * fix: snapshots folder * fix(integration): snapshots * Create calm-baboons-sit.md * test(integration): file naming Co-authored-by: Carlos R. L. Rodrigues <rodrigolr@gmail.com> * Feat/tax inclusive pricing extend price selection strategy (#2087) * initial changes to price selection strategy including unit tests * typing for tax calculation * update types and remove region and currency from prices results * fix casing * include tax calculation in priceselectionstrategy * integration tests for tax inclusive pricing price calculations * fix build * include tax inclusive considerations when calculating tax fields for variants * include only "includes_tax" fields from currency and region joins * test to see errors in pipelines * conditionally join featureflagged fields * add "includes_tax" to price list factory * add tests for tax inclusive price list prices and currency prices * fix unit tests * refactor pricing array checks to expect arraycontaining * undo error handler * Feat/tax inclusive pricing flag on generated lineitems (#2108) * include tax inclusive pricing flag on generated lineitems * initial addition of tax inclusivity for lineitem service * add generate test to ensure that includes_tax is set when returned from price selection strategy * add integration test for generating lineitem including tax * add test for negative tax inclusion * add tests for mixed pricing * add negative test for setting tax exclusivity * restructure the setting of includes_tax on lineitems * fix: update cwd to be correct in cart test * feat(medusa): Line item totals calculations (#2123) * feat(medusa): Update totals and tax calculation way to calculate the totals * feat(medusa): remove region feetching from decorate total * feat(medusa): cleanup * test(medusa): fix tax calculation tests * comment * test(integration): cleanup * test(integration): cleanup * fix(medusa): return service missing await * feat(medusa): cleanup * feat(medusa): cleanup * test(integration): fix data * feat(medusa): improve tax calculation readability * test(medusa): improve tax calculation structure case Co-authored-by: Sebastian Rindom <skrindom@gmail.com> * Feat(medusa): tax inclusive pricing in shipping method tax (#2125) * initial implementation and test * include tax inclusive calculations for getting shipping options * remove inaccurate comment * remove console log * refactor how prices and taxes are set for shipping methods * fix integration tests * remove verbose flag * fix integration tests * remove console log * format util * use util in price service and tax strategy * fix faulty integration test * undo tax calculation strategy changes in favor or Carlos' pr * undo changes to tax calculation strategy tests * round tax amount * feat(medusa): cleanup calculate tax amount utils and its usage (#2136) * feat(medusa): Refund line totals calculation (#2139) Rely on the update of the following pr #2136 **WIP Missing integration tests** **What** Update the totals calculation on the refund line to include the notion of tax inclusive **Test** - Update and add new tests around the refund Fixes CORE-482 * feat(medusa): Tax inclusive discounts calculations (#2137) **What** - Calculate line adjustments correctly taking into account the tax inclusivity - fix totals getLineItemTotals by adjusting the sub total with the original tax amount instead of the tax amount when the unit price includes the taxes **Tests** - The tests create a cart with a percentage discount of 15%, the cart includes 2 items mixing the tax inclusive and validate the items on the result cart as well as the totals on each item. I ve based my calculation validation based on what we have done + some articles around discount apply on price without taxes to validate the output., FIXES CORE-477 * Chore: shipping methods tax inclusive total (#2130) * chore: calculate tax inclusive shipping methods * chore: additional tests and check undefined tax_rate (#2157) * chore: additional tests and check undefined tax_rate * fix: naming + correct price type check * fix: remove price_includes_tax from type * fix: remove price_includes_tax from type Co-authored-by: Philip Korsholm <philip.korsholm@hotmail.com> Co-authored-by: adrien2p <adrien.deperetti@gmail.com> Co-authored-by: Carlos R. L. Rodrigues <rodrigolr@gmail.com> Co-authored-by: Philip Korsholm <88927411+pKorsholm@users.noreply.github.com> Co-authored-by: Sebastian Rindom <skrindom@gmail.com> Co-authored-by: Carlos R. L. Rodrigues <37986729+carlos-r-l-rodrigues@users.noreply.github.com>

* docs for tax-inclusive pricing * changes based on feedback * changes based on feedback * fix link * fix: make prices optional param when updating a variant (#2155) **Why** - It should be possible to update variant props without having to send the prices array with every update * feat(medusa): Tax-inclusive pricing (#2131) * add feature flag for tax inclusive pricing * update db model for TIP * add migration * set featureflag column decorators * remove unused prop * update tests to reflect feature_flags as any array * fix types * reference key from featureFlag file * use feature flag key in models * fix copy paste mistake * unify spelling * Create gorgeous-experts-guess.md * feat(medusa): create/update endpoints of currency/region/price-lists/shipping-options should allow to pass includes_tax * test(integration): continue to add some integration test * test(integration): continue to add some integration test * test(unit): Fix region service tests * fix(medusa): API unit tests flags management * feat(medusa): Minor cleanup * style(medusa): Fix typo * fix(medusa): rebase * feat(medusa): Replace old tag with the new one * feat(medusa): revert flag * feat(medusa): Cleanup * feat(medusa): feedback * feat(medusa): Rename currency retrieve method * test(medudsa): fix unit tests * chore(medusa): fix oas * feat(medusa): ShippingMethod should include tax setting from parent option (#2021) * feat(medusa): Shipping method should includes tax from parent options * feat(medusa): Condition the includes tax flag to the availability of the feature and add some other tests * test(integration): Move cart/order ff test in separate files * fix: snapshots folder * fix(integration): snapshots * Create calm-baboons-sit.md * test(integration): file naming Co-authored-by: Carlos R. L. Rodrigues <rodrigolr@gmail.com> * Feat/tax inclusive pricing extend price selection strategy (#2087) * initial changes to price selection strategy including unit tests * typing for tax calculation * update types and remove region and currency from prices results * fix casing * include tax calculation in priceselectionstrategy * integration tests for tax inclusive pricing price calculations * fix build * include tax inclusive considerations when calculating tax fields for variants * include only "includes_tax" fields from currency and region joins * test to see errors in pipelines * conditionally join featureflagged fields * add "includes_tax" to price list factory * add tests for tax inclusive price list prices and currency prices * fix unit tests * refactor pricing array checks to expect arraycontaining * undo error handler * Feat/tax inclusive pricing flag on generated lineitems (#2108) * include tax inclusive pricing flag on generated lineitems * initial addition of tax inclusivity for lineitem service * add generate test to ensure that includes_tax is set when returned from price selection strategy * add integration test for generating lineitem including tax * add test for negative tax inclusion * add tests for mixed pricing * add negative test for setting tax exclusivity * restructure the setting of includes_tax on lineitems * fix: update cwd to be correct in cart test * feat(medusa): Line item totals calculations (#2123) * feat(medusa): Update totals and tax calculation way to calculate the totals * feat(medusa): remove region feetching from decorate total * feat(medusa): cleanup * test(medusa): fix tax calculation tests * comment * test(integration): cleanup * test(integration): cleanup * fix(medusa): return service missing await * feat(medusa): cleanup * feat(medusa): cleanup * test(integration): fix data * feat(medusa): improve tax calculation readability * test(medusa): improve tax calculation structure case Co-authored-by: Sebastian Rindom <skrindom@gmail.com> * Feat(medusa): tax inclusive pricing in shipping method tax (#2125) * initial implementation and test * include tax inclusive calculations for getting shipping options * remove inaccurate comment * remove console log * refactor how prices and taxes are set for shipping methods * fix integration tests * remove verbose flag * fix integration tests * remove console log * format util * use util in price service and tax strategy * fix faulty integration test * undo tax calculation strategy changes in favor or Carlos' pr * undo changes to tax calculation strategy tests * round tax amount * feat(medusa): cleanup calculate tax amount utils and its usage (#2136) * feat(medusa): Refund line totals calculation (#2139) Rely on the update of the following pr #2136 **WIP Missing integration tests** **What** Update the totals calculation on the refund line to include the notion of tax inclusive **Test** - Update and add new tests around the refund Fixes CORE-482 * feat(medusa): Tax inclusive discounts calculations (#2137) **What** - Calculate line adjustments correctly taking into account the tax inclusivity - fix totals getLineItemTotals by adjusting the sub total with the original tax amount instead of the tax amount when the unit price includes the taxes **Tests** - The tests create a cart with a percentage discount of 15%, the cart includes 2 items mixing the tax inclusive and validate the items on the result cart as well as the totals on each item. I ve based my calculation validation based on what we have done + some articles around discount apply on price without taxes to validate the output., FIXES CORE-477 * Chore: shipping methods tax inclusive total (#2130) * chore: calculate tax inclusive shipping methods * chore: additional tests and check undefined tax_rate (#2157) * chore: additional tests and check undefined tax_rate * fix: naming + correct price type check * fix: remove price_includes_tax from type * fix: remove price_includes_tax from type Co-authored-by: Philip Korsholm <philip.korsholm@hotmail.com> Co-authored-by: adrien2p <adrien.deperetti@gmail.com> Co-authored-by: Carlos R. L. Rodrigues <rodrigolr@gmail.com> Co-authored-by: Philip Korsholm <88927411+pKorsholm@users.noreply.github.com> Co-authored-by: Sebastian Rindom <skrindom@gmail.com> Co-authored-by: Carlos R. L. Rodrigues <37986729+carlos-r-l-rodrigues@users.noreply.github.com> * Remove unused QueryBuilderService (#2104) **Issue number:** #2068 **What:** - removed unused query-builder service files - medusa/src/services/query-builder.js - medusa/src/services/__mocks__/query-builder.js - deleted export from medusa/src/services/index.ts - (extra) deleted documentation files related to QueryBuilderService (QueryBuilderService.md) * docs for tax-inclusive pricing * changes based on feedback * changes based on feedback * feat(medusa-fulfillment-webshipper): Support personal customs no in orders (#2167) * feat(webshipper): support personal customs no in orders * docs: update readme with personal customs number info * fix(medusa-file-spaces): return `fileKey` for Spaces upload (#2171) **What** - return `fileKey` in the response after the file is uploaded to Spaces Co-authored-by: Oliver Windall Juhl <59018053+olivermrbl@users.noreply.github.com> * fix(medusa): Export/import fixes including export fields that contains new line char (#2150) * feat(medusa-js,medusa-react,medusa): Add missing Currency endpoints (#2185) * fix(medusa): Resent notification replaces parent notification * chore: Bump medusa-file-minio * fix(medusa): allow address updates on carts w/o existing address (#2176) * chore: Bump minor version of plugins * chore: Centralise ESLint rules (#2162) * chore: centrilize eslint rules * feat: order editing data model (#2184) **What** - add order editing entities - add repositories - add a feature flag for the order editing feature - add the migrations file RESOLVES CORE-490 * fix link * fix(medusa): Check for Sales Channel on product import (#2202) * docs: change title in Create a Service documentation (#2201) Change title in Create a Service documentation * chore(release): Publish * added links to sidebar Co-authored-by: Sebastian Rindom <skrindom@gmail.com> Co-authored-by: Oliver Windall Juhl <59018053+olivermrbl@users.noreply.github.com> Co-authored-by: Philip Korsholm <philip.korsholm@hotmail.com> Co-authored-by: adrien2p <adrien.deperetti@gmail.com> Co-authored-by: Carlos R. L. Rodrigues <rodrigolr@gmail.com> Co-authored-by: Philip Korsholm <88927411+pKorsholm@users.noreply.github.com> Co-authored-by: Carlos R. L. Rodrigues <37986729+carlos-r-l-rodrigues@users.noreply.github.com> Co-authored-by: Dorian <46839236+DorianMazur@users.noreply.github.com> Co-authored-by: Frane Polić <16856471+fPolic@users.noreply.github.com> Co-authored-by: Kasper Fabricius Kristensen <45367945+kasperkristensen@users.noreply.github.com> Co-authored-by: Bhargava Prabu Reddy <prabu0reddy777@gmail.com> Co-authored-by: olivermrbl <oliver@mrbltech.com> Co-authored-by: sabakhilji <52318459+sabakhilji@users.noreply.github.com>

adrien2p self-assigned this Aug 31, 2022

carlos-r-l-rodrigues force-pushed the chore/shipping-methods-tax-inclusive-total branch from 57dce1f to 173dc6b Compare August 31, 2022 12:57

carlos-r-l-rodrigues marked this pull request as ready for review September 1, 2022 13:05

carlos-r-l-rodrigues requested a review from a team as a code owner September 1, 2022 13:05

carlos-r-l-rodrigues force-pushed the chore/shipping-methods-tax-inclusive-total branch from 1469282 to 21a7349 Compare September 2, 2022 13:45

adrien2p reviewed Sep 2, 2022

View reviewed changes

packages/medusa/src/services/totals.ts Show resolved Hide resolved

adrien2p reviewed Sep 2, 2022

View reviewed changes

packages/medusa/src/services/totals.ts Outdated Show resolved Hide resolved

adrien2p reviewed Sep 2, 2022

View reviewed changes

packages/medusa/src/services/totals.ts Outdated Show resolved Hide resolved

adrien2p reviewed Sep 2, 2022

View reviewed changes

packages/medusa/src/services/totals.ts Show resolved Hide resolved

carlos-r-l-rodrigues force-pushed the chore/shipping-methods-tax-inclusive-total branch 2 times, most recently from 7efc3c8 to 7b7f5df Compare September 4, 2022 16:24

carlos-r-l-rodrigues and others added 17 commits September 4, 2022 13:31

chore: calculate tax inclusive shipping methods

23b6765

chore: use constant for "free_shipping"

424cb64

chore: tax calculation shipping methods

6c05f11

chore: integration tests

dfc3942

chore: calculate tax inclusive shipping methods

91c2ccd

chore: tax calculation shipping methods

a5407b3

chore: calculate tax inclusive shipping methods

af1ddca

chore: tax calculation shipping methods

1e5c382

fix: import

b9789a2

chore: tests cwd path

8fad96c

fix: calculatePriceTaxAmount

fff4969

fix: tests

4c76880

fix: unit tests

61d0ac4

chore: remove unused file

785500b

chore: check if properties exist

409cc28

chore: additional unit tests

911ffb5

carlos-r-l-rodrigues and others added 6 commits September 4, 2022 14:53

chore: normalize FF.key

4ce6f8b

fix: method name

9a2097d

fix: unit test

4904494

fix: shipping method tax_rate calculation

a8f6c4d

fix: revert commits

3f7dcb2

feat(medusa): cleanup calculate tax amount utils and its usage (#2136)

33024d1

carlos-r-l-rodrigues force-pushed the chore/shipping-methods-tax-inclusive-total branch from 7b7f5df to 33024d1 Compare September 4, 2022 17:55

carlos-r-l-rodrigues and others added 2 commits September 4, 2022 15:01

Merge branch 'feat/tax-inclusive-pricing-2' into chore/shipping-metho…

d4b4b90

…ds-tax-inclusive-total

fix: unit test

09169d4

adrien2p reviewed Sep 5, 2022

View reviewed changes

carlos-r-l-rodrigues added 3 commits September 5, 2022 08:38

fix: tests

81df2a7

fix: test import

17c2d67

chore: shipping subtotal

f0e4fa2

srindom reviewed Sep 5, 2022

View reviewed changes

srindom approved these changes Sep 5, 2022

View reviewed changes

olivermrbl mentioned this pull request Sep 5, 2022

chore: apply prettier on pipeline #2133

Closed

carlos-r-l-rodrigues merged commit 7dd285b into feat/tax-inclusive-pricing-2 Sep 5, 2022

srindom deleted the chore/shipping-methods-tax-inclusive-total branch September 6, 2022 08:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chore: shipping methods tax inclusive total #2130

Chore: shipping methods tax inclusive total #2130

carlos-r-l-rodrigues commented Aug 30, 2022 •

edited

Loading

changeset-bot bot commented Aug 30, 2022 •

edited

Loading

adrien2p left a comment

adrien2p Sep 2, 2022

carlos-r-l-rodrigues Sep 2, 2022

adrien2p Sep 2, 2022 •

edited

Loading

srindom Sep 5, 2022

adrien2p Sep 5, 2022

adrien2p Sep 5, 2022

srindom Sep 5, 2022

srindom Sep 5, 2022

carlos-r-l-rodrigues Sep 5, 2022

adrien2p Sep 5, 2022

srindom Sep 5, 2022

srindom left a comment

Chore: shipping methods tax inclusive total #2130

Chore: shipping methods tax inclusive total #2130

Conversation

carlos-r-l-rodrigues commented Aug 30, 2022 • edited Loading

changeset-bot bot commented Aug 30, 2022 • edited Loading

⚠️ No Changeset found

adrien2p left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrien2p Sep 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srindom left a comment

Choose a reason for hiding this comment

carlos-r-l-rodrigues commented Aug 30, 2022 •

edited

Loading

changeset-bot bot commented Aug 30, 2022 •

edited

Loading

adrien2p Sep 2, 2022 •

edited

Loading