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

[14.0][IMP][FIX] l10n_nl_account_tax_unece: Update account_tax_codes #404

Merged
merged 2 commits into from
Mar 26, 2024

Conversation

bosd
Copy link
Contributor

@bosd bosd commented Nov 13, 2023

Update the UNECE tax codes according to https://www.softwarepakketten.nl/wiki_uitleg/10&bronw=7/UBL_en_BTW_algemene_informatie.htm

As discovered in:
OCA/edi#323 (comment)
Current mapping is incorrect.

Todo:

  • Update 6,9,21% tax rates to S code:

/ocabot add-lebal help-wanted

  • Check other tax codes
  • G = Free export item, tax not charged, code specifying that the item is free export
    and taxes are not charged.

Marked as done: Implemented in update of code

  • K = VAT exempt for EEA intracommunity supply of goods and services. A tax category code indicating the item is VAT exempt
    due to an intracommunity supply in the European Economic Area.
    Wordt gebruikt bij de facturering van intracommunautaire goederen (ICP).

Marked as done: Implemented in update of code

  • F = Value Added Tax (VAT) margin scheme second hand goods. Indication that the VAT margin scheme for second hand
    goods is applied. Wordt gebruikt bij facturering van tweede hands goederen volgens de margeregeling.

Marked as done because: categoriecode F (Margin Scheme) niet is toegestaan in de basisfactuur. Gebruik code E
Met excempt reasong

  • Create migration script

cc: @gjotten , @thomaspaulb , @astirpe

@bosd
Copy link
Contributor Author

bosd commented Nov 13, 2023

Please also check if these are still correct:

Check, is this correct? I assume first one should be K

    <record id="l10n_nl.btw_I_overig" model="account.tax.template">
        <field name="unece_type_id" eval="ref('account_tax_unece.tax_type_vat')" />
        <field name="unece_categ_id" eval="ref('account_tax_unece.tax_categ_s')" />
    </record>

    <record id="l10n_nl.btw_I_overig_d" model="account.tax.template">
        <field name="unece_type_id" eval="ref('account_tax_unece.tax_type_vat')" />
        <field name="unece_categ_id" eval="ref('account_tax_unece.tax_categ_s')" />
    </record>

This is not according to peppol standard.
Should be AE (binnenland) or K (ICP)? What do you think?

    <record id="l10n_nl.btw_verk_0" model="account.tax.template">
        <field name="unece_type_id" eval="ref('account_tax_unece.tax_type_vat')" />
        <field name="unece_categ_id" eval="ref('account_tax_unece.tax_categ_b')" />
    </record>

@gjotten
Copy link
Contributor

gjotten commented Nov 14, 2023

@bosd good catch!

Based on the 'UBL en BTW' information linked, I'd agree with the changes to code S you've made up until now. As for how I'm reading the rest:

  • what's tax_categ_b in 'Buiten de EU / Verkoop' right now could be categ_g. However, we're not making a goederen/diensten distinction in our account.tax.template records yet. I suspect diensten would need to be separated under code Z maybe? See the 0% BTW page
  • regarding l10n_nl.btw_I_overig[_d] I'm a bit confused, the linked pages tell us about sales but I'm missing info on purchases and their VAT regimes
  • regarding l10n_nl.btw_verk_0: my intuition says AE since the 'verlegd' regime has mostly been applied to intra-NL transactions with special circumstances in my experience. But that's a bit thin since N=1 so ideally we have this checked somehow

The more I look at this the less sure I am what's supposed to be correct. A quick search on TaxCategory implementation at other vendors netted me nothing; might contacting peppolautoriteit.nl make sense? I'm suspecting they're just some hollow shell 'facilitating' private entities who 'support' this standard 😞

@bosd
Copy link
Contributor Author

bosd commented Nov 14, 2023

Found some usefull info on: https://www.forumstandaardisatie.nl/open-standaarden/nlcius
Which links to this pdf: https://www.forumstandaardisatie.nl/sites/default/files/Downloads/Bijlagen%20OS/NLCIUS/E-Factureren-Gebruiksinstructie-voor-de-basisfactuur-v1.0.2.pdf

TL;DR all.
But adjusted tags according to info from page 37.

Also found a gap in implementation.
Currently the odoo CoA / Tax Tags does not have a seperate tag for:
Z Nultarief (0%)
E BTW Vrij

This could mean we would be unable to parse some peppol / ubl invoices
(Checked odoo version 14 CE )

Not sure why tests are failling after last commit. Should be ok.

@gjotten
Copy link
Contributor

gjotten commented Nov 15, 2023

@bosd self.assertTrue(tax.unece_categ_id.id in unece_categ_ids) will lead to a false if it won't find them in the references no?

@NL66278 NL66278 changed the title [IMP][FIX] l10n_nl_account_tax_unece: Update account_tax_codes [14.0][IMP][FIX] l10n_nl_account_tax_unece: Update account_tax_codes Nov 15, 2023
@bosd bosd force-pushed the patch-1 branch 3 times, most recently from 273a1a5 to 468ca8f Compare December 30, 2023 17:45
@bosd bosd marked this pull request as ready for review December 30, 2023 17:45
@bosd
Copy link
Contributor Author

bosd commented Jan 12, 2024

@astirpe , @thomaspaulb Can you please review?

@bosd
Copy link
Contributor Author

bosd commented Mar 26, 2024

@NL66278 Can you please review?

Copy link
Contributor

@NL66278 NL66278 left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@NL66278
Copy link
Contributor

NL66278 commented Mar 26, 2024

@bosd Can you rebase this one?

Update the UNECE tax codes according to https://www.softwarepakketten.nl/wiki_uitleg/10&bronw=7/UBL_en_BTW_algemene_informatie.htm

Update account_tax_template.xml

Update account_tax_template.xml

Update test_nl_account_tax_unece.py

[IMP][FIX] l10n_nl_account_tax_unece: Update mapping
@bosd
Copy link
Contributor Author

bosd commented Mar 26, 2024

Can we proceed with this pr and to other ones. To get some more traction in this repo

@thomaspaulb
Copy link
Contributor

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-404-by-thomaspaulb-bump-patch, awaiting test results.

@thomaspaulb
Copy link
Contributor

Can we proceed with this pr and to other ones. To get some more traction in this repo

If you want to speed things up, the only way really is review swapping - you review other people's PR's, they review yours. It doesn't help to become impatient, unfortunately.

@OCA-git-bot OCA-git-bot merged commit 07d98f3 into OCA:14.0 Mar 26, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at d85630e. Thanks a lot for contributing to OCA. ❤️

@bosd
Copy link
Contributor Author

bosd commented Mar 26, 2024

If you want to speed things up, the only way really is review swapping - you review other people's PR's, they review yours. It doesn't help to become impatient, unfortunately.

Many thanks ❤️
Noted
I'm already reviewing a lot of pr's. But mainly those users are not active in this localization repo 😉
Understandable they won't swap.
I've patiently waited 3 months.. 😄

@thomaspaulb
Copy link
Contributor

3 months is nothing :) everyone has the same problem

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.

5 participants