Skip to content

add tax_calculation_method :ITEM_BASED which changes tax amount validation to item based#25

Merged
halfbyte merged 1 commit into
halfbyte:mainfrom
SubandiK:add-tax_calculation_method-ITEM_BASED
Jun 3, 2025
Merged

add tax_calculation_method :ITEM_BASED which changes tax amount validation to item based#25
halfbyte merged 1 commit into
halfbyte:mainfrom
SubandiK:add-tax_calculation_method-ITEM_BASED

Conversation

@SubandiK
Copy link
Copy Markdown
Contributor

@SubandiK SubandiK commented Apr 9, 2025

Another of those specific use-cases I'm afraid:

We are rounding taxes on a per-line-item basis, not like Secretariat currently validates for on a per-taxrate basis.
The new :NONE option doesn't work here, because it actually changes the taxes. What we need is to just change / skip the"Base amount and summed tax base amount" validation.

By using the new :ITEM_BASED option, the validation now compares the invoice's tax_amount with the sum of the item tax amounts.

@halfbyte
Copy link
Copy Markdown
Owner

halfbyte commented Jun 3, 2025

Hmmmpfff, this feels like we need to overhaul this but maybe it is also just a function of the inherent complexity.

I'll merge this for now, but I'm not happy with amassing all of these various modes :)

@halfbyte halfbyte merged commit 4692781 into halfbyte:main Jun 3, 2025
5 checks passed
@RST-J
Copy link
Copy Markdown
Contributor

RST-J commented Jan 26, 2026

I stumbled over this by accident while looking into an issue I had with one of our invoices.

@halfbyte and @SubandiK I'm convinced that the :ITEM_BASED variant introduced here is meant to have the same semantics as the :HORIZONTAL I added just before in this PR

If there is an issue with validation then we should have a fixture/test case that illustrates the problem with the current implementation and then look into the cause for the issue and how to fix it (or work out, why there is an error in the fixture if there would be any).

Furthermore I think that the check in L117 repeats the exact same check as L106 does, as the taxes tax_amount are nothing else than a summation of the tax_amounts of line_items as per L83.

From what I can tell by analyzing the check, I think everything that would be needed here instead of this PR is to change if tax_calculation_method != :NONE to if tax_calculation_method == :VERTICAL because the vertical method is the one (and only) in which the tax is calculated as a rate of the net total of a tax group.

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