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

[MIG][15.0] Migration datev_export_xml #126

Merged
merged 14 commits into from
Mar 14, 2024

Conversation

fkantelberg
Copy link
Member

Migration of datev_export_xml and the common module to 15.0 including fixes for pre-commit

@marylla
Copy link

marylla commented Jul 20, 2023

@fkantelberg Can you please check the xml files for corrections of invoices?

The xml files should have negative amounts in case of out_refund and in_refund

It was like this in version 12 and I can confirm that Datev worked fine with these negative values:

image

(Only the "tax line node" should have positive values.)

In version 14 and in this PR for version 15 it has positive values and Datev complains.
We might need to multiply with -1 in case of refunds.

Can you fix this in this PR directly and can you create a PR for version 14, too?

Thank you :)

@tv-openbig FYI

@fkantelberg
Copy link
Member Author

fkantelberg commented Jul 21, 2023

Hi @marylla
I'm currently not 100% sure that I can simply do it because Odoo seems to lack the "Rechnungskorrektur" in 14 and 15.0 and only implements "Gutschrift § 14 UStG". With 16.0 they added the Storno accounting which negates those numbers. From my perspective (but I'm not an expert here) the invoice_type is wrong and should be Gutschrift § 14 UStG instead of Gutschrift/Rechnungskorrektur without additional changes to implement the storno accounting within Odoo.

I don't hope that it's a bigger rabbit hole but I'm also quite confused when the accountant people are calling everything "Gutschrift" in german. I try to raise it internally and see what the experts say.

@fkantelberg fkantelberg force-pushed the 15.0-add-datev_export_xml branch 2 times, most recently from dacb898 to d40317a Compare August 9, 2023 12:11
@fkantelberg
Copy link
Member Author

@marylla I negated some attributes according to your wish.

@OSevangelist
Copy link
Contributor

@marylla @tv-openbig @olaf-wagner can either one of you give that another review to allow us to merge that timely? @tv-openbig coming back on a discussion we had lately... we shall be faster in the German community ;-)

@fkantelberg fkantelberg force-pushed the 15.0-add-datev_export_xml branch from 01a5cf3 to 278f93c Compare January 11, 2024 07:08
t-if="doc.move_type in ['out_invoice', 'out_refund']"
/>
<t t-set="partner" t-value="doc.company_id.partner_id" t-else="" />
<t t-set="account" t-value="partner.property_account_receivable_id" />
Copy link

Choose a reason for hiding this comment

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

This should follow similar t-if-else pattern as in supplier_party. Currently it's always setting doc.company_id.partner_id as the partner.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jsierp Did you test this end-to-end with a proper example ? If i remember correctly we needed to have this different, as otherwise we didn't get the correct partners in supplier bills / supplier refunds and customer invoices / customer refunds (f.e. issuing party).

@fkantelberg @marylla Can you confirm this ?

Copy link

Choose a reason for hiding this comment

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

Unfortunately I don't have a possibility to test it end-to-end atm, but setting the account in line 96 to doc.company_id.partner_id.property_account_receivable_id just seems wrong.
Consider a vendor bill. In line 95 the partner is set to doc.company_id.partner_id. Shouldn't the receivable account be always taken from the invoice partner (and not the company, because of line 95), i.e. doc.partner_id.property_account_receivable_id?
It was at least like this in previous versions of this module and I don't see why something would change in Odoo 15.

Copy link
Member Author

Choose a reason for hiding this comment

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

I finally got some time to reply here and it's fine because there are t-if and t-else available. You don't have to introduce extra <t> tags.

The current code is equal to this if you insert the extra <t> tags.

<invoice_party t-call="datev_export_xml.export_party">
    <t t-if="doc.move_type in ['out_invoice', 'out_refund']">
        <t t-set="partner" t-value="doc.partner_id" />
    </t>
    <t t-else="">
        <t t-set="partner" t-value="doc.company_id.partner_id" />
    </t>
    <t t-set="account" t-value="partner.property_account_receivable_id" />
</invoice_party>

The difference between invoice_party and supplier_party is that the account is always set for invoice_party and only set for incoming invoices for supplier_party.

Copy link

@jsierp jsierp Mar 22, 2024

Choose a reason for hiding this comment

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

Yes, thank you for your explanation. My concern was, that in version 14.0 the account for invoice_party was set only for outgoing invoices. I'm wondering why this was changed in this version, or am I still missing something?

Copy link
Member Author

Choose a reason for hiding this comment

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

We noticed it in 15.0 and I backported it recently.

Copy link

Choose a reason for hiding this comment

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

In which commit? Here: https://github.com/OCA/l10n-germany/blob/14.0/datev_export_xml/views/templates.xml#L89C1-L101C1 the account gets set only for outgoing invoices:

<invoice_party t-call="datev_export_xml.export_party">
    <t t-if="doc.move_type in ['out_invoice', 'out_refund']">
        <t t-set="partner" t-value="doc.partner_id" />
        <t
            t-set="account"
            t-value="partner.property_account_receivable_id"
        />
    </t>
    <t t-else="">
        <t t-set="partner" t-value="doc.company_id.partner_id" />
    </t>
</invoice_party>

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for noticing me. Seems like I failed to cherry pick it. Either way 15.0 is the most tested version there is and should be the template for the migration not 14.0.

@hbrunn
Copy link
Member

hbrunn commented Mar 14, 2024

/ocabot rebase

@OCA-git-bot
Copy link
Contributor

@hbrunn The rebase process failed, because command git push --force initOS tmp-pr-126:15.0-add-datev_export_xml failed with output:

remote: Permission to initOS/l10n-germany.git denied to OCA-git-bot.
fatal: unable to access 'https://github.com/initOS/l10n-germany/': The requested URL returned error: 403

@OSevangelist
Copy link
Contributor

@fkantelberg can you have a look into what @hbrunn found out concerning the credentials (OCA git bot)

@fkantelberg fkantelberg force-pushed the 15.0-add-datev_export_xml branch from 278f93c to 4e7fcf7 Compare March 14, 2024 09:35
@fkantelberg
Copy link
Member Author

I don't know the oca bot doesn't have access to our repos. We can discuss it internally. I rebased manually now.

@tv-openbig
Copy link
Contributor

@fkantelberg @marylla @hbrunn I just finished a end-to-end test within DATEV Unternehmen Online and especially the changes from positive to negative amounts is correct from DATEV perspective for the in and out refunds.

@tv-openbig
Copy link
Contributor

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 15.0-ocabot-merge-pr-126-by-tv-openbig-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 09dba3a into OCA:15.0 Mar 14, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

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.

9 participants