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

[12.0][IMP] account_tax_balance: multicompany fiscal unit #613

Merged
merged 1 commit into from Dec 2, 2021

Conversation

astirpe
Copy link
Member

@astirpe astirpe commented Sep 16, 2019

In some countries it is allowed to declare to the tax authorities one single tax report that contains data of multiple companies. For example, in The Netherlands a group of companies (one parent company and N children companies) can form a fiscal unit; in this case, the parent company declares one single tax statement for all companies of the fiscal unit (see OCA/l10n-netherlands#237).

In the current account_tax_balance module, you can only see the data regarding move lines of the company you're logged in. This is fine when using the module itself but it could be a bit uncomfortable when extending the module to generate tax statements for multiple companies.

This PR make the extension of this module easier in case of generating reports for multiple companies.
Without this PR, it is still possible to do it, but that requires overriding the methods get_move_line_partial_domain() and _account_tax_ids_with_moves().

Not sure if this patch should set the version of the module to 12.0.2.0.0, since the type of one returned value of get_context_values() method is changed. In any case, I think it's a good idea to include this patch when migrating account_tax_balance module to V13.

@astirpe astirpe changed the title [IMP] account_tax_balance: multicompany fiscal unit [12.0][IMP] account_tax_balance: multicompany fiscal unit Sep 16, 2019
@astirpe astirpe mentioned this pull request Sep 30, 2019
2 tasks
@pedrobaeza pedrobaeza added this to the 12.0 milestone Feb 21, 2020
@pedrobaeza
Copy link
Member

This is incorrect, as it's bypassing the multi-company way of working in version 12. You can change the expression for using child of operator and take parent companies + children, but not to show all the authorized companies at the same time.

@astirpe
Copy link
Member Author

astirpe commented Feb 21, 2020

@pedrobaeza the list of companies ids is passed by context from any localization (or custom) module extending this one. The list of companies depends on how the localization module is supposed to work.

See for example OCA/l10n-netherlands#237 and its proposed solution https://github.com/OCA/l10n-netherlands/pull/238/files.
As you can notice, the list of companies to be reported is calculated and can be different from the classic parent-child. In our module it also depends on the country and the currency.

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

OK, seen

@dreispt
Copy link
Sponsor Member

dreispt commented Dec 2, 2021

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 12.0-ocabot-merge-pr-613-by-dreispt-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 739b3fa into OCA:12.0 Dec 2, 2021
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at afe0910. 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.

None yet

4 participants