fix: item wise tax details and net amounts

[blaggacao]

• feat: Add net_total to Taxes And Charges doctypes and modify relevant controllers
• test: add test for item tax detail

Net Amount Field Addition for Tax Doctype

This pull request introduces a new "Net Amount" field to various tax-related doctypes in ERPNext, enhancing the granularity of tax calculations and reporting.

Changes Overview

1. Added "Net Amount" and "Base Net Amount" fields to:

   • Advance Taxes and Charges
   • Purchase Taxes and Charges
   • Sales Taxes and Charges
   • [x] PR to HRMS for Expense Taxes and Charges → https://github.com/frappe/hrms/pull/2230

2. Updated the calculate_taxes_and_totals class to incorporate the new net amount fields in tax calculations.

3. Modified the get_current_tax_amount method to get_current_tax_and_net_amount, now returning both net and tax amounts.

4. Adjusted the item-wise tax calculation to include net amount information.

Key Updates

• New Fields: "Net Amount" and "Base Net Amount" added to tax doctypes for more detailed financial tracking[1].
• Calculation Logic: Updated to handle net amounts separately from tax amounts, providing more accurate tax breakdowns[1].
• Item-wise Tax Details: Now includes net amount information, offering a more comprehensive view of tax application per item[1].

Impact

This change will provide users with more detailed tax information, allowing for:

• Better tracking of net amounts before tax application
• More accurate reporting of tax breakdowns
• Enhanced visibility into tax calculations at the item level

Testing Recommendations

• Verify that net amounts are correctly calculated and displayed in all relevant doctypes
• Ensure that existing tax calculations remain accurate with the introduction of the new fields
• Test various tax scenarios to confirm the correct interaction between net amounts and tax amounts

This update significantly improves the granularity and accuracy of tax-related information in ERPNext, providing users with more detailed financial insights. Additionally, it enhances compatibility with UBL2.1 (Universal Business Language) electronic documents, allowing for more precise representation of net amounts and tax calculations in standardized e-invoicing formats. This improvement facilitates better interoperability with various e-invoicing systems and ensures compliance with digital reporting requirements in jurisdictions that mandate UBL2.1 format for electronic invoices.

Migration Recommendations

• DO NOT migrate while you have a POS Invoice Merge pending: the change in json datastructure will be incompatible. Make sure to migrate after a completed merge to only merge invoices with compatible data structures.

cc/ @barredterra

[blaggacao]

In order for this to be merge-able with confidence for migration purposes, we first need a calculate_taxes_and_totals controller who is able to recalculate taxes with a guarantee to not create inconsistent data or data which violates prior (legally settled) state. We need a controller which enough flexibility to:

• add the new amounts to existing data
• provides a dry-run mode for validation before committing changes
• offers detailed logging for audit and troubleshooting purposes

There is currently no reasonable way (I tried to find one for 2 hours, already) to recalculate item-wise net amounts into the item_wise_tax_detail with the current controller. However, any existing json datastructure is incompatible with merge_tax, which might be triggered on old data but post migration.

[barredterra]

This is also a requirement for the UN/CEFACT Cross Industry Invoice, not only UBL.

Btw, the India Compliance app already calculates net amounts per Item (in case that is helpful here).

[barredterra]

DO NOT migrate while you have a POS Invoice Merge pending: the change in json datastructure will be incompatible. Make sure to migrate after a completed merge to only merge invoices with compatible data structures.

This restriction seems hard to enforce (or even communicate). We'll probably have to find a workaround.

[blaggacao]

Indeed, I added a migration script yesterday (wasn't yet able to test). It's not reconstructing the missing value, because that is very difficult to accurately infer in all cases, but it conforms the structure and removes that limitation.

[ruthra-kumar]

@blaggacao

Can you provide a simple example scenario with expected outcomes, along with

1. How the current system handles it
2. How the proposed change will help address it.

[blaggacao]

I've added a unit example of how the new datastructure can be worked with to the end of the PR description.

The key is the addition of net_amount so that UBL2.1 standard of representing item-wise tax breakup in edi documents can be implemented without brittle inference and rather reliably using the source values from the time of first calculation.

Several localization apps, even in-tree ones, have used brittle inference in the past to determine net_amount, but this may not meet the quality requirements for other implementations.

The change from a list to a _dict was overloaded onto this PR in order to use this API change and the baseline necessity for migration and also sanitize and future-proof this datastructure at the occasion. In part, this aided with validating a correct implementation of the migration, because it is easier to audit the explicit datastructure across the repository than a very subtle list and index based logic that is easy to misread.

So to sum it up:

• Higher quality localization code
• Future Proofing the datastructure
• Remove a long standing code bottleneck

Maybe a word on brittleness:

• Rounding logic has evolved over time
• Localization apps didn't catch up (or didn't bother to soundly implement in the first place)
• As a result, already today, certain rounding settings in certain localizations will produce inconsistent results (wether always consequential or not is an entirely different question)

[barredterra]

Several localization apps, even in-tree ones, have used brittle inference in the past to determine net_amount

Can confirm 😁

https://github.com/alyf-de/eu_einvoice/blob/a2fa848d20a55b920eb6c07fab87300543377edc/eu_einvoice/european_e_invoice/custom/sales_invoice.py#L180-L182

[blaggacao]

@ruthra-kumar even if we wouldn't be able to merge it right away, could we lock in an eventual agreement on the design principle, namely:

• Extension of the datastructure by net_amount
• Modification of the datastructure to better future proof the implementation, incl poor-mans typing via type alias

This would currently help a lot as ecosystem downstream work is conditioned (and currently halted) on these hypotheses.

[ruthra-kumar]

We can go ahead with this.

As with most complex logic, edge cases will only come out the more it gets used. Will have to tackle it as and when those arise.

[blaggacao]

Thanks @ruthra-kumar

I've added a priority developer SLA pledge to the beginning of the PR description for the next two weeks.