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.

Sign up for GitHub

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

Jump to bottom

Tax categories that are still in use can be soft deleted, creating silent failures #4683

Open
tmtrademarked opened this issue Oct 20, 2022 · 5 comments · May be fixed by #6059
Open

Tax categories that are still in use can be soft deleted, creating silent failures #4683

tmtrademarked opened this issue Oct 20, 2022 · 5 comments · May be fixed by #6059
Labels
changelog:solidus_core Changes to the solidus_core gem type:bug Error, flaw or fault

Comments

@tmtrademarked
Copy link
Contributor

tmtrademarked commented Oct 20, 2022 •
edited
Loading

Tax categories which are still in use by products or line items are allowed to be deleted. This creates a silent failure mode when attempting to update the taxes for any order referring to these objects.

Solidus Version:
3.2.2

To Reproduce

  1. Create a tax category and associated rate, assign it to a product, and create an order with that product.
  2. Now create a new tax category, and update the rate to have only the new category and not the old.
  3. Delete the old tax category via the UI.
  4. Update taxes on the order, and notice that no taxes are calculated - and all existing taxes are cleared.

Current behavior
The code which selects the rates for the line item will return no available rates:

solidus/core/app/models/spree/tax/tax_helpers.rb

Line 11 in 425c82f

@rates_for_item.select do |rate|

This will result in the behavior in the bug where we clear the taxes on the order.

Expected behavior
There's two things we could do to make this more obvious:

  • Consider perhaps warning or raising if the tax category associated with the line item is soft deleted. That would turn this from a silent failure into a loud failure that would be more noticeable.
  • Consider disallowing archiving a tax category that is still "in use" by products/variants/line_items/shipping_methods. That would stop this bug from ever occurring.
@gsmendoza
Copy link
Contributor

Hi @tmtrademarked ! I tried to test this but I wasn't able to reproduce Step 4: Update taxes on the order. Please see the video below. Am I missing something?

https://www.loom.com/share/ec029cc36b39472d98308c8df9a7a1bb

@tmtrademarked
Copy link
Contributor Author

Hey @gsmendoza - thanks for the detailed test video! That was super helpful. I think the missing piece here is that you have to cause taxes to be recalculated on the order. That's what I meant by "update taxes on the order".

So you can trigger this by starting a reimbursement for an item, for instance - or I think by simply doing order.recalculate in a rails console. At that point, it should cause the taxes to be zeroed out on the line item.

@gsmendoza
Copy link
Contributor

@tmtrademarked I've done the following:

  1. Create a tax category and associated rate, assign it to a product, and create an order with that product.
  2. Now create a new tax category, and update the rate to have only the new category and not the old.
  3. Assign the product to the new category.
  4. Delete the old tax category via the UI.
  5. Update taxes on the order.

I've confirmed that even if I assigned the product to the new category before deleting the old category, the tax rate is no longer applied to the order.

I'll check with the team what should be the expected behavior for this issue 👍

@kennyadsl
Copy link
Member

@gsmendoza did you already bring this to the team's attention?

@gsmendoza
Copy link
Contributor

@kennyadsl Yes, I brought it up in slack and created a ticket for it.

@kennyadsl kennyadsl added type:bug Error, flaw or fault changelog:solidus_core Changes to the solidus_core gem labels Nov 24, 2022
@harmonymjb harmonymjb linked a pull request Jan 7, 2025 that will close this issue
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
changelog:solidus_core Changes to the solidus_core gem type:bug Error, flaw or fault
Projects
None yet
Milestone
No milestone
3 participants
@gsmendoza @kennyadsl @tmtrademarked