Skip to content

Conversation

pepijn-vanvlaanderen
Copy link
Contributor

Summary

What — What changes are introduced in this PR?

Added shipping method data to the normalize tax module context.

Why — Why are these changes relevant or necessary?

We need to override the tax provider to set taxes based on the shipping method (when it is a pickup). I.e. we have pickup locations in the Netherlands, but we also ship abroad. When someone from Germany chooses to pickup, the tax needs to be overriden to the Dutch VAT.

To make this possible we need the shipping method data in the taxCalculationContext of the getTaxLines in the CustomTaxProvider.

How — How have these changes been implemented?

Extensions have been made to the normalizeTaxModuleContext function

Testing — How have these changes been tested, or how can the reviewer test the feature?

We implemented the patch locally and seen the data passed through properly.


Examples

Provide examples or code snippets that demonstrate how this feature works, or how it can be used in practice.
This helps with documentation and ensures maintainers can quickly understand and verify the change.

  async getTaxLines(
    itemLines: TaxTypes.ItemTaxCalculationLine[],
    shippingLines: TaxTypes.ShippingTaxCalculationLine[],
    taxCalculationContext: TaxTypes.TaxCalculationContext
  ): Promise<(TaxTypes.ItemTaxLineDTO | TaxTypes.ShippingTaxLineDTO)[]> {
    const shippingOptionIds = taxCalculationContext.shipping_methods?.map(sm => sm.shipping_option_id) ?? [];
    
    // If a customer chooses pickup in shop/warehouse, use Dutch tax rate
    let hasPickOption = false;
    if (shippingOptionIds.length > 0) {
      const result = await this.getShippingOptions(shippingOptionIds);
      hasPickOption = result.length > 0;
    }
    
    ...
}

Checklist

Please ensure the following before requesting a review:

  • I have added a changeset for this PR
    • Every non-breaking change should be marked as a patch
    • To add a changeset, run yarn changeset and follow the prompts
  • The changes are covered by relevant tests
  • I have verified the code works as intended locally
  • I have linked the related issue(s) if applicable

@pepijn-vanvlaanderen pepijn-vanvlaanderen requested a review from a team as a code owner October 14, 2025 15:10
Copy link

changeset-bot bot commented Oct 14, 2025

🦋 Changeset detected

Latest commit: 5b572c9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 74 packages
Name Type
@medusajs/core-flows Patch
@medusajs/types Patch
@medusajs/medusa Patch
integration-tests-http Patch
@medusajs/draft-order Patch
@medusajs/framework Patch
@medusajs/js-sdk Patch
@medusajs/modules-sdk Patch
@medusajs/orchestration Patch
@medusajs/utils Patch
@medusajs/workflows-sdk Patch
@medusajs/medusa-oas-cli Patch
@medusajs/admin-bundler Patch
@medusajs/dashboard Patch
@medusajs/test-utils Patch
@medusajs/analytics Patch
@medusajs/api-key Patch
@medusajs/auth Patch
@medusajs/cache-inmemory Patch
@medusajs/cache-redis Patch
@medusajs/caching Patch
@medusajs/cart Patch
@medusajs/currency Patch
@medusajs/customer Patch
@medusajs/event-bus-local Patch
@medusajs/event-bus-redis Patch
@medusajs/file Patch
@medusajs/fulfillment Patch
@medusajs/index Patch
@medusajs/inventory Patch
@medusajs/link-modules Patch
@medusajs/locking Patch
@medusajs/notification Patch
@medusajs/order Patch
@medusajs/payment Patch
@medusajs/pricing Patch
@medusajs/product Patch
@medusajs/promotion Patch
@medusajs/region Patch
@medusajs/sales-channel Patch
@medusajs/settings Patch
@medusajs/stock-location Patch
@medusajs/store Patch
@medusajs/tax Patch
@medusajs/user Patch
@medusajs/workflow-engine-inmemory Patch
@medusajs/workflow-engine-redis Patch
@medusajs/analytics-local Patch
@medusajs/analytics-posthog Patch
@medusajs/auth-emailpass Patch
@medusajs/auth-github Patch
@medusajs/auth-google Patch
@medusajs/caching-redis Patch
@medusajs/file-local Patch
@medusajs/file-s3 Patch
@medusajs/fulfillment-manual Patch
@medusajs/locking-postgres Patch
@medusajs/locking-redis Patch
@medusajs/notification-local Patch
@medusajs/notification-sendgrid Patch
@medusajs/payment-stripe Patch
@medusajs/oas-github-ci Patch
@medusajs/cli Patch
@medusajs/deps Patch
@medusajs/telemetry Patch
@medusajs/admin-sdk Patch
@medusajs/admin-shared Patch
@medusajs/admin-vite-plugin Patch
@medusajs/icons Patch
@medusajs/toolbox Patch
@medusajs/ui-preset Patch
create-medusa-app Patch
medusa-dev-cli Patch
@medusajs/ui Patch

Not sure what this means? Click here to learn what changesets are.

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

Copy link

vercel bot commented Oct 14, 2025

@pepijn-vanvlaanderen is attempting to deploy a commit to the medusajs Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

This PR is being reviewed by Cursor Bugbot

Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

name: method.name,
shipping_option_id: method.shipping_option_id,
amount: method.amount,
})),
Copy link

Choose a reason for hiding this comment

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

Bug: Tax Context Mismatch Causes Incorrect Calculations

The tax context is populated from orderOrCart.shipping_methods, but the step's shippingMethods parameter is used for tax calculation. This inconsistency can lead to the tax context containing different shipping methods than those actually being taxed, potentially causing incorrect calculations.

Fix in Cursor Fix in Web

Copy link
Contributor

@willbouch willbouch left a comment

Choose a reason for hiding this comment

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

LGTM. Will be merged for the next release. If you can in the meantime, it would be nice to have this covered by a test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants