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

[ADD] account: compute taxes line on account.move with onchange #19029

Closed
wants to merge 2 commits into from

Conversation

smetl
Copy link
Contributor

@smetl smetl commented Aug 24, 2017

Allow the user to set taxes on lines when creating a new account.move.

task: https://www.odoo.com/web#id=33802&view_type=form&model=project.task&action=333&active_id=967&menu_id=4720

I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@smetl smetl added Accounting RD research & development, internal work labels Aug 24, 2017
@smetl smetl requested a review from qdp-odoo August 24, 2017 06:58
@nhomar
Copy link
Collaborator

nhomar commented Aug 24, 2017

I will never understand WHY you guys insists on create this feature on view level when this is at model level (this an old topic).

I am glad this feature is comming up, But IMHO, this must be a model aware feature, I mean it does not matter where the move line comes from if you wire a tax this must happen at model level also (may be a create/write overwrite with an update process).

With that even API level this will be consistent.

@@ -446,6 +492,7 @@ def _get_counterpart(self):
analytic_tag_ids = fields.Many2many('account.analytic.tag', string='Analytic tags')
company_id = fields.Many2one('res.company', related='account_id.company_id', string='Company', store=True)
counterpart = fields.Char("Counterpart", compute='_get_counterpart', help="Compute the counter part accounts of this journal item for this journal entry. This can be needed in reports.")
is_tax_line = fields.Boolean(string='Is a tax line')
Copy link
Contributor

Choose a reason for hiding this comment

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

useless field: use "tax_line_id is True" instead

line with 150 debit.
'''
# Retrieve the existing tax lines and drop them.
tax_line_ids = self.line_ids.filtered(lambda l: l.is_tax_line)
Copy link
Contributor

Choose a reason for hiding this comment

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

why???

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't remember well but it was the only way I found to avoid some problems when dealing with real ids on db + 'fake ids' corresponding to newly created (but not committed) lines... to recheck!

Copy link
Contributor

@qdp-odoo qdp-odoo left a comment

Choose a reason for hiding this comment

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

changes needed


@tagged('post_install', '-at_install')
class TestAccountMoveTaxesEdition(AccountingTestCase):
def _check_move_lines_taxes(self, lines):
Copy link
Contributor

Choose a reason for hiding this comment

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

use check_complete_move(self, move, theorical_lines) defined in AccountingTestCase.

adapt that function in order to allow testing of any columns (currently only name, debit, credit are tested but we need tax_ids and tax_line_id as well)

Then, later in the test, use a tax that you create yourself so that you're sure of the rate and and tax amount


amount = tax.compute_all(2000)['total_included']

with move_form.line_ids.new() as credit_line2:
Copy link
Contributor

Choose a reason for hiding this comment

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

that last part of the 3th test is unclear.

So far, you should have

Name debit credit tax_ids tax_line_id
debit line 1 1000 10%
tax line 105 10%
credit line 1 1105

When adding the second debit line, assert the debit amount of tax line is set to 300 and assert the number of lines is now 4 (no fifth line added)

Then set the last credit line 2 manually to (3300 - 1105) and check the whole move is now

Name debit credit tax_ids tax_line_id
debit line 1 1000 10%
tax line 300 10%
credit line 1 1105
debit line 2 2000 10%
credit line 2 2195

def _get_tax_account(tax, amount):
if tax.tax_exigibility == 'on_payment' and tax.cash_basis_account:
return tax.cash_basis_account
return tax.refund_account_id if amount < 0 else tax.account_id
Copy link
Contributor

Choose a reason for hiding this comment

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

mmh that part is dubious: a tax line being a credit is a regular sale tax line, or a refund purchase tax line. I don't think it'll work as, somehow, you need to consider the type of tax used, I guess

@@ -21,15 +22,73 @@ def setUp(self):
_logger.warn('Test skipped because there is no chart of account defined ...')
self.skipTest("No Chart of account found")

def check_complete_move(self, move, theorical_lines):
def check_complete_move(self, move, theorical_lines, fields_name=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, I was not clear on my first comment :-s

theorical_lines should be a list of dict, and move should be a recordset of account.move.line, that would be much more convenient.

The idea is, in a next iteration, to remove codes doing similar thing such as check_results() in test_reconciliation.py or check_journal_items() in test_payment.py

Actually, the parameter fields_name is useless as it can be deducted from theorical_lines[0].keys()

Can you change that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in last commit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

stuff moved to #25129

@smetl smetl force-pushed the master-tax-jour-las branch 3 times, most recently from ba7463c to af1e210 Compare June 7, 2018 07:23
pniederlag pushed a commit to pniederlag/odoo that referenced this pull request Jun 8, 2018
Allow the user to set taxes on lines when creating manually a new journal entry, and tax lines will be computed automatically thanks to an onchange based on new unstored fields.

Was task: 33802
Was PR odoo#19029
@qdp-odoo
Copy link
Contributor

qdp-odoo commented Jun 8, 2018

landed in master, for saas 11.4, at revision d49ab56

@qdp-odoo qdp-odoo closed this Jun 8, 2018
nvn-odoo pushed a commit to odoo-dev/odoo that referenced this pull request Jul 18, 2018
Allow the user to set taxes on lines when creating manually a new journal entry, and tax lines will be computed automatically thanks to an onchange based on new unstored fields.

Was task: 33802
Was PR odoo#19029
@smetl smetl deleted the master-tax-jour-las branch November 6, 2018 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accounting RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants