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

[10.0][MIG] Port l10n_nl_tax_declaration_reporting #63

Closed

Conversation

astirpe
Copy link
Member

@astirpe astirpe commented Mar 7, 2017

Porting module l10n_nl_tax_declaration_reporting to V10.
This is a work in progress...

@hbrunn hbrunn added this to the 10.0 milestone Mar 7, 2017
hbrunn added a commit to hbrunn/l10n-netherlands that referenced this pull request Mar 12, 2017
hbrunn added a commit to hbrunn/l10n-netherlands that referenced this pull request Mar 12, 2017
@astirpe astirpe force-pushed the 10_port_l10n_nl_tax_declaration_reporting branch from 6388400 to d94324b Compare March 13, 2017 14:12
@astirpe
Copy link
Member Author

astirpe commented Mar 13, 2017

Rebased PR, in order to apply the latest updates made on .travis.yml

JOIN account_invoice_line inv_line
on inv_line.id = inv_line_tax.invoice_line_id
JOIN account_invoice inv
on inv.id = inv_line.invoice_id
Copy link
Member

Choose a reason for hiding this comment

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

AND inv.id = inv_tax.invoice_id

Copy link
Member

Choose a reason for hiding this comment

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

in my version for 9, I changed the query entirely, because the way the query stands here, it will return the amounts multiplied by the amount of lines which have the same tax (note that inv_tax.amount is the sum of all lines with the tax in the invoice, not just the amount of some specific line): #68 (comment)

@StefanRijnhart
Copy link
Member

I see that the current version of the report does not include taxes posted directly on move lines. Is that the WIP-part?

@astirpe
Copy link
Member Author

astirpe commented Mar 16, 2017

@StefanRijnhart As far as I know, the VAT reporting should be only calculated based on invoices. But since I'm not accountant I'm going to ask to some financial consultant :)

Thank you for the tip on the query, I will update it ASAP!

And yes, I consider this PR still in an early WIP stage...

@StefanRijnhart
Copy link
Member

That assumption is just wrong.

But I think I would prefer a different approach altogether. Maybe this module could be refactored to reformat the results of https://github.com/OCA/account-financial-reporting/tree/10.0/account_tax_balance?

@astirpe
Copy link
Member Author

astirpe commented Mar 16, 2017

@StefanRijnhart Indeed it would be a better approach! Thank you for mentioning that module, I will work on this PR tomorrow...

@StefanRijnhart
Copy link
Member

@astirpe that would be great! You might want to check this PR of mine to fix the reporting of taxes on bank transactions in this module OCA/account-financial-reporting#284

@astirpe
Copy link
Member Author

astirpe commented Mar 21, 2017

If the Community agrees, this PR is superseded by #70

@astirpe astirpe closed this Mar 24, 2017
@astirpe astirpe deleted the 10_port_l10n_nl_tax_declaration_reporting branch March 27, 2017 10:18
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

3 participants