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

[13.0][OU-IMP] l10n_be: migration script (account.account.tag) #3675

Closed
wants to merge 1 commit into from

Conversation

remytms
Copy link
Contributor

@remytms remytms commented Jan 4, 2023

Migrate account.account.tag to fit the new account.tax and its repartition lines.

@remytms remytms force-pushed the 13.0-l10n_be-migration branch 2 times, most recently from 239e524 to e3355eb Compare January 4, 2023 16:54
@MiquelRForgeFlow MiquelRForgeFlow added this to the 13.0 milestone Jan 11, 2023
@remytms
Copy link
Contributor Author

remytms commented Apr 20, 2023

@sbidoul this PR might interest you when migrating Belgian accounting. :)

@remytms
Copy link
Contributor Author

remytms commented Feb 13, 2024

@marielejeune Can you give a look at this migration script? Thanks. :)

@marielejeune
Copy link
Contributor

Hi @remytms, I've seen your PR a few days ago, but I'm really not an accounting specialist and I don't think I've the competencies to functionally give any comment :/

@remytms
Copy link
Contributor Author

remytms commented Feb 13, 2024

Thanks @marielejeune , if you know a Belgian accounting person do not hesitate to ping him/her.

@remytms
Copy link
Contributor Author

remytms commented Feb 13, 2024

@luc-demeyer You may be interested by this pull request. :)

@remytms remytms changed the title [13.0] l10n_be: migration script (account.account.tag) [13.0][OU-IMP] l10n_be: migration script (account.account.tag) Feb 15, 2024
@marielejeune
Copy link
Contributor

Hi @remytms
I am trying your script on a customer's database migration. I'll give you a complete feedback when I'll finish the migration. But I already have a functional question, since I'm not an accounting specialist. I got the following error:

There is several account.tax found for the account.tax.template ID 141 '21% EU S.' that are used on account.move.line. Clean up account.tax by renaming your specific account.tax and migrate it via a dedicated script. The matching account.tax ID 133, 551

Indeed, both taxes with ID 133 and 551 have the same name, look the same, but one of them is linked to Company 1 (Belgian company) and the second one is linked to Company 2 (also Belgian company).

Is it a problem to have 2 taxes coming from the same tax template? If yes, what should have been done when initializing invoicing for these 2 companies? Duplicate the tax templates?

@fd-oncodna
Copy link

fd-oncodna commented Feb 26, 2024

Indeed as @marielejeune said I think this does not work if you have multiple companies in Belgium, as the taxes will be duplicated - but not the tax templates, as they are created through XML. You should loop on the companies, change the signature of find_account_tax(env, xmlid, account_tax_template) to find_account_tax(env, xmlid, account_tax_template, company) and search both the taxes and the move lines restricted to company.

Copy link
Contributor

@marielejeune marielejeune left a comment

Choose a reason for hiding this comment

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

I've tried to run your script on a DB with 2 Belgian companies, hence I've adapted it to loop on companies as @fd-oncodna suggested. Here are my comments, please have a look and tell me if it seems good to you:

addons/l10n_be/migrations/13.0.2.0/post-migration.py Outdated Show resolved Hide resolved
addons/l10n_be/migrations/13.0.2.0/post-migration.py Outdated Show resolved Hide resolved
addons/l10n_be/migrations/13.0.2.0/post-migration.py Outdated Show resolved Hide resolved
addons/l10n_be/migrations/13.0.2.0/post-migration.py Outdated Show resolved Hide resolved
@remytms remytms force-pushed the 13.0-l10n_be-migration branch 2 times, most recently from 13496b1 to d33ecea Compare March 13, 2024 14:18
@remytms
Copy link
Contributor Author

remytms commented Mar 13, 2024

@marielejeune @fd-oncodna Thanks a lot for your remark and suggestion about multi-company. Changes added.

@remytms
Copy link
Contributor Author

remytms commented May 22, 2024

I close this PR in favor of this one: #4432

@remytms remytms closed this May 22, 2024
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.

4 participants