-
-
Notifications
You must be signed in to change notification settings - Fork 55
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
8.0 l10n nl tax declaration reporting dev #18
Conversation
Hey @RichDijk, thank you for your Pull Request. It looks like some users haven't signed our Contributor License Agreement, yet.
Appreciation of efforts, |
Hi @RichDijk, thanks! Please check the Travis CI output. It will give you some comments on style and inconsistancy that you need to follow up. And please register at the OCA with your Github login as a member of the Onestein team (no membership fee required for that). |
@StefanRijnhart, Thank you for your feedback. I'ĺl take care for the subscription. |
Now the module should be ready to be reviewed |
|
||
i = 0 | ||
while i < len(res): | ||
# TODO refactor, e.g. using map or switch case? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use a mapping, indeed. That allows you to raise properly when you encounter an unrecognized code as well.
|
||
from . import report | ||
from . import wizard | ||
# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The vim: lines are optional, and can be better removed in new modules
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually they are completely useless: you should configure your vim to use 4 spaces for all python files, and it probably does already.
I think this should be removed.
Hi, I just join this team to give some input. |
I think there is a bug when you make a supplier invoice for importing outside the EU. You pay 21% tax and that tax is returned in the 'voorbelasting'. The result is zero. But on the report you print a negative tax amount in 'rubriek 4a' and then extract the 'voorbelasting'. This leads in a tax return and not a zero result. Can you check? |
@erwin, |
I think before we doubt to the report. Let's check the settings of the tax at 4a. Therefore I think the issue is in the Tax definition. could you check that? |
fy = self.cr.fetchall() | ||
self.cr.execute( | ||
"select id from account_period where fiscalyear_id = %s", | ||
(fy[0][0],)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean to select all periods from the current fiscal year? In that case, you need do make sure you select the right fiscal year (in this case the order seems to be undefined). Also, make the query company aware (or use the ORM to select the fiscal year, which applies standard company filters).
@RichDijk thanks for the updates so far! @erwin-bas-solutions can you confirm that the results are correct now? |
@RichDijk please rebase |
Are there still any issues with this? I think it's time to merge 👍 |
there's still a couple of comments by @StefanRijnhart pending |
The report has been rewritten into a Qweb report in order to make ik easy to maintain.