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

Somehow, each of the line items in one order have 3 tax_lines with identical data. #1901

Closed
dwene opened this issue Jul 25, 2022 · 8 comments
Assignees

Comments

@dwene
Copy link
Contributor

dwene commented Jul 25, 2022

Bug report

We use a 3rd party plugin to calculate taxes, using the Medusa tax plugin system.

I noticed one of my recent orders had 3x the tax I was expecting. Looking at the order it appears that each line_item has 3 tax lines, each with the same percentage added. Seems like somehow the tax lines were added 3 times. I would expect that if any part of the order failed, it would be rolled back in a transaction, but that didn't appear to happen.

image

Describe the bug

https://www.loom.com/share/2f6ce6cc7ba34a08b8296ad505804052

Tax is 3x higher than it should be

System information

Medusa version (including plugins): .1.3.4
Node.js version: 14
Database: PG
Operating system: Linux
Browser (if relevant):

Steps to reproduce the behavior

See above.

Expected behavior

Screenshots

Code snippets

Additional context

Add any other context about the problem here

@olivermrbl
Copy link
Contributor

Thanks for reporting it in @dwene! We'll attend to it immediately and release a fix asap. Will keep you posted :)

@adrien2p
Copy link
Member

adrien2p commented Jul 26, 2022

Also, @dwene @olivermrbl that pr #1894 will probably fix some issues as well around transaction and wrong object usage. Might not fix everything but it could help

@olivermrbl
Copy link
Contributor

@dwene Latest release includes a bunch of fixes around transactions in the core. Let us know if the issue persists, when you get a chance to update to latest

@akramaskar
Copy link

akramaskar commented Oct 29, 2023

I've identified a potential issue in the order creation process, where the tax rate triples after transforming a cart into an order. Below is the relevant code and logs that highlight the problem:

Path: OrderService - createFromCart

 console.log("[DEBUGGING] ::: Start of createFromCart");
      console.log(
        "[DEBUGGING] ::: order :: create from cart : items",
        cart?.items.map((item) => item.tax_lines.map((tl) => tl.rate)),
      );

      const itemsReturn = await Promise.all(
        [
          cart.items.map(async (lineItem): Promise<unknown[]> => {
            console.log("[DEBUGGING] ::: Processing line item:", lineItem.id);

            const toReturn: unknown[] = [
              lineItemServiceTx.update(lineItem.id, { order_id: order.id }),
            ];

            console.log(
              "[DEBUGGING] ::: After updating line item:",
              lineItem.id,
              "tax_lines:",
              lineItem.tax_lines.map((tl) => tl.rate),
            );

            if (lineItem.is_giftcard) {
              toReturn.push(
                this.createGiftCardsFromLineItem_(order, lineItem, manager),
              );
            }

            return toReturn;
          }),
          cart.shipping_methods.map(async (method) => {
            console.log(
              "[DEBUGGING] ::: Processing shipping method:",
              method.id,
            );

            (method.tax_lines as any) = undefined;
            return shippingOptionServiceTx.updateShippingMethod(method.id, {
              order_id: order.id,
            });
          }),
        ].flat(Infinity),
      );

      console.log("[DEBUGGING] ::: After all Promise.all operations");

      orderTemp = await this.retrieve(order.id, {
        relations: ["items", "items.tax_lines"],
      });

      console.log(
        "[DEBUGGING] ::: order :: create from cart : after itemsReturn",
        orderTemp?.items.map((item) => item.tax_lines.map((tl) => tl.rate)),
      );

Logs

[DEBUGGING] ::: Start of createFromCart
[DEBUGGING] ::: order :: create from cart : items [ [ 19 ] ]
[DEBUGGING] ::: Processing line item: item_01HDY2B3BVH8VDMYADA22TD0G8
[DEBUGGING] ::: Start of lineItemService.update
[DEBUGGING] ::: line item update ::: idOrSelector:  item_01HDY2B3BVH8VDMYADA22TD0G8
[DEBUGGING] ::: line item update ::: data:  { order_id: 'order_01HDY2D5G5SKV1SR5NHJNDSDN3' }
[DEBUGGING] ::: Inside atomicPhase_
[DEBUGGING] ::: After updating line item: item_01HDY2B3BVH8VDMYADA22TD0G8 tax_lines: [ 19 ]
[DEBUGGING] ::: Processing shipping method: sm_01HDY2CQA5ZZ6Y5E9PGV58N8PK
[DEBUGGING] ::: Before modifying line items: [ { id: 'item_01HDY2B3BVH8VDMYADA22TD0G8', tax_lines: undefined } ]
[DEBUGGING] ::: After modifying line items: [ { id: 'item_01HDY2B3BVH8VDMYADA22TD0G8', tax_lines: undefined } ]
[DEBUGGING] ::: After all Promise.all operations
[DEBUGGING] ::: order :: create from cart : after itemsReturn [ [ 19, 19, 19 ] ]
[DEBUGGING] ::: End of createFromCart

Observation:

  • Tax rate is recorded as [19] initially.
  • Post order creation, it becomes [19, 19, 19].

It's worth noting that the "line item update" log is from the line-item service, which I suspect isn't the root of the problem.

Would appreciate insights on potential fixes or workarounds.

@akramaskar
Copy link

akramaskar commented Oct 29, 2023

Found the in our case

We are using a custom cart-completion strategy based on the one provided in the strategies folder in the medusa repository.

In here, in the handlePaymentAuthorized as well as handleTaxLinesCreated, the following code is called again to recreate the tax lines. After removing it in both cases above, the tax calculation works as it is supposed to.


const res = await this.handleCreateTaxLines(id, { manager })
    if (res.response_code) {
      return res
    }

@nickmonad
Copy link
Contributor

We are hitting this issue as well. Pretty big blocker for us to use a custom tax provider. I mentioned it on discord, and only just now saw this issue

@nickmonad
Copy link
Contributor

We don't have a custom cart completion strategy, but rather a custom tax provider

@nickmonad
Copy link
Contributor

Ok, figured it out. Because of the fix in #1262, code cannot be null. There is a unique constraint that apparently depends on code not being null. After setting code in the line items returned from our customer tax provider, we are left with only one line applied to both the item and the shipping method

kodiakhq bot pushed a commit that referenced this issue Nov 6, 2023
While I was developing a custom tax provider, I ran into a pretty frustrating issue related to multiple tax lines and incorrect / unexpected tax total calculations. I outlined the issue in discord, before finding the solution: https://discord.com/channels/876835651130097704/1169705457804398662

After some digging, I found this issue from a while back: #1262, where it states there is now a unique constraint on `item_id, code` and `shipping_method_id, code`. But, in the current documentation for creating a custom tax provider, it states these fields as part of the tax line items returned are optional.

If these `code` values are left out, it can cause tax lines to be applied multiple times (as seen here as well: #1901)

I'm not entirely sure how this should be phrased in the documentation, so I just wanted to get this up and on your radar for resolution. If it ends up being that `code` truly should not be optional, I suspect some type definitions would need to change as well?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants