Merge lp:~acsone-openerp/account-financial-tools/account_partner_required-sbi into lp:~account-core-editors/account-financial-tools/7.0

[Stéphane Bidoul (Acsone)]

New module to control the partner field in account move lines.

In many cases, it is desirable to enforce the presence of the
partner field in account move lines, but not always.

This module provides such a mechanism based on the account user type.

Cfr mailing list discussion for reference.

[1] https://lists.launchpad.net/openerp-community/msg05502.html

[lp:~acsone-openerp/account-financial-tools/account_partner_required-sbi]

175. By Stéphane Bidoul (Acsone) on 2014-04-19

[IMP] add policy column in account type tree view

176. By Stéphane Bidoul (Acsone) on 2014-04-19

[IMP] local import

177. By Stéphane Bidoul (Acsone) on 2014-04-19

[IMP] header comments

[Lorenzo Battistini (elbati)]

Hello Stéphane,

why not using '_constraints' member instead of overriding create and write methods?

Is there any technical reason to avoid YAML tests?

Thanks!

review: Needs Information

[Stéphane Bidoul (Acsone) (sbi)]

Hello Lorenzo,

About the constraints. This module is heavily inspired by account_analytic_required which used the same technique. I must confess I did not question the approach. Initially account_analytic_required was working mainly on the vals dictionary so it would have been faster than constraints. While working on account_partner_required I identified a bug in account_analytic_required which required a refactoring [1] which possibly rendered the optimization less obvious. Now since there are really two constraints to be checked based on the policy, the current approach is probably still slightly faster.

Would you agree to keep the current approach and postpone a possible performance test / refactoring with constraints to another MP?

Regarding yml vs unittest, it's only a matter of personal taste in this case. I notice the unittest2 framework seems better documented [2]. But hey, at least I have tests, right :)

[1] https://code.launchpad.net/~acsone-openerp/account-analytic/account_analytic_required-test_suite-sbi
[2] https://doc.openerp.com/trunk/server/05_test_framework/

[Lorenzo Battistini (elbati)]

Ok :-)

review: Approve (code review)

[Joël Grand-Guillaume @ camptocamp (jgrandguillaume-c2c)]

Hi Stéphane,

Thanks for the contrib, and as usual, thanks for providing tests :)

LGMT,

Regards,

Joël

review: Approve (code review, no tests)

[Omar (Pexego) (omar7r)]

LGTM

review: Approve (code review)

=== added directory 'account_partner_required'
=== added file 'account_partner_required/__init__.py'
--- account_partner_required/__init__.py	1970-01-01 00:00:00 +0000
+++ account_partner_required/__init__.py	2014-04-19 10:33:22 +0000
@@ -0,0 +1,23 @@
+# -*- encoding: utf-8 -*-
+##############################################################################
+#
+#    Account partner required module for OpenERP
+#    Copyright (C) 2014 Acsone (http://acsone.eu).
+#    @author Stéphane Bidoul <[e-mail]>
+#
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU Affero General Public License as
+#    published by the Free Software Foundation, either version 3 of the
+#    License, or (at your option) any later version.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU Affero General Public License for more details.
+#
+#    You should have received a copy of the GNU Affero General Public License
+#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+##############################################################################
+
+from . import account
 
=== added file 'account_partner_required/__openerp__.py'
--- account_partner_required/__openerp__.py	1970-01-01 00:00:00 +0000
+++ account_partner_required/__openerp__.py	2014-04-19 10:33:22 +0000
@@ -0,0 +1,46 @@
+# -*- encoding: utf-8 -*-
+##############################################################################
+#
+#    Account partner required module for OpenERP
+#    Copyright (C) 2014 Acsone (http://acsone.eu).
+#    @author Stéphane Bidoul <[e-mail]>
+#
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU Affero General Public License as
+#    published by the Free Software Foundation, either version 3 of the
+#    License, or (at your option) any later version.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU Affero General Public License for more details.
+#
+#    You should have received a copy of the GNU Affero General Public License
+#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+##############################################################################
+
+{
+    'name': 'Account partner required',
+    'version': '0.1',
+    'category': 'Generic Modules/Accounting',
+    'license': 'AGPL-3',
+    'description': """This module adds an option "partner policy"
+on account types.
+
+You have the choice between 3 policies : optional (the default),
+always (require a partner), and never (forbid a partner).
+
+This module is useful to enforce a partner on account move lines on
+customer and supplier accounts.
+
+Module developed by Stéphane Bidoul <[e-mail]>,
+inspired by Alexis de Lattre <[e-mail]>'s
+account_analytic_required module.
+""",
+    'author': 'ACSONE SA/NV',
+    'website': 'http://acsone.eu/',
+    'depends': ['account'],
+    'data': ['account_view.xml'],
+    'installable': True,
+}
 
=== added file 'account_partner_required/account.py'
--- account_partner_required/account.py	1970-01-01 00:00:00 +0000
+++ account_partner_required/account.py	2014-04-19 10:33:22 +0000
@@ -0,0 +1,96 @@
+# -*- encoding: utf-8 -*-
+##############################################################################
+#
+#    Account partner required module for OpenERP
+#    Copyright (C) 2014 Acsone (http://acsone.eu).
+#    @author Stéphane Bidoul <[e-mail]>
+#
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU Affero General Public License as
+#    published by the Free Software Foundation, either version 3 of the
+#    License, or (at your option) any later version.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU Affero General Public License for more details.
+#
+#    You should have received a copy of the GNU Affero General Public License
+#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+##############################################################################
+
+from openerp.osv import orm, fields
+from openerp.tools.translate import _
+
+
+class account_account_type(orm.Model):
+    _inherit = "account.account.type"
+
+    _columns = {
+        'partner_policy': fields.selection([
+            ('optional', 'Optional'),
+            ('always', 'Always'),
+            ('never', 'Never')
+            ], 'Policy for partner field',
+            help="Set the policy for the partner field : if you select "
+                 "'Optional', the accountant is free to put a partner "
+                 "on an account move line with this type of account ; "
+                 "if you select 'Always', the accountant will get an error "
+                 "message if there is no partner ; if you select 'Never', "
+                 "the accountant will get an error message if a partner "
+                 "is present."),
+    }
+
+    _defaults = {
+        'partner_policy': 'optional',
+    }
+
+
+class account_move_line(orm.Model):
+    _inherit = "account.move.line"
+
+    def check_partner_required(self, cr, uid, ids, vals, context=None):
+        if 'account_id' in vals or 'partner_id' in vals or \
+                'debit' in vals or 'credit' in vals:
+            if isinstance(ids, (int, long)):
+                ids = [ids]
+            for move_line in self.browse(cr, uid, ids, context):
+                if move_line.debit == 0 and move_line.credit == 0:
+                    continue
+                policy = move_line.account_id.user_type.partner_policy
+                if policy == 'always' and not move_line.partner_id:
+                    raise orm.except_orm(_('Error :'),
+                                         _("Partner policy is set to 'Always' "
+                                           "with account %s '%s' but the "
+                                           "partner is missing in the account "
+                                           "move line with label '%s'." %
+                                           (move_line.account_id.code,
+                                            move_line.account_id.name,
+                                            move_line.name)))
+                elif policy == 'never' and move_line.partner_id:
+                    raise orm.except_orm(_('Error :'),
+                                         _("Partner policy is set to 'Never' "
+                                           "with account %s '%s' but the "
+                                           "account move line with label '%s' "
+                                           "has a partner '%s'." %
+                                           (move_line.account_id.code,
+                                            move_line.account_id.name,
+                                            move_line.name,
+                                            move_line.partner_id.name)))
+
+    def create(self, cr, uid, vals, context=None, check=True):
+        line_id = super(account_move_line, self).create(cr, uid, vals,
+                                                        context=context,
+                                                        check=check)
+        self.check_partner_required(cr, uid, line_id, vals, context=context)
+        return line_id
+
+    def write(self, cr, uid, ids, vals, context=None, check=True,
+              update_check=True):
+        res = super(account_move_line, self).write(cr, uid, ids, vals,
+                                                   context=context,
+                                                   check=check,
+                                                   update_check=update_check)
+        self.check_partner_required(cr, uid, ids, vals, context=context)
+        return res
 
=== added file 'account_partner_required/account_view.xml'
--- account_partner_required/account_view.xml	1970-01-01 00:00:00 +0000
+++ account_partner_required/account_view.xml	2014-04-19 10:33:22 +0000
@@ -0,0 +1,35 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+  Account partner required module for OpenERP
+  Copyright (C) 2014 Acsone (http://acsone.eu).
+  @author Stéphane Bidoul <[e-mail]>
+  The licence is in the file __openerp__.py
+-->
+
+<openerp>
+<data>
+
+<record id="account_partner_required_account_type_form" model="ir.ui.view">
+    <field name="name">account_partner_required.account_type_form</field>
+    <field name="model">account.account.type</field>
+    <field name="inherit_id" ref="account.view_account_type_form" />
+    <field name="arch"  type="xml">
+        <field name="code" position="after">
+                <field name="partner_policy" />
+        </field>
+    </field>
+</record>
+
+<record id="view_account_type_tree" model="ir.ui.view">
+    <field name="name">account_partner_required.account_type_tree</field>
+    <field name="model">account.account.type</field>
+    <field name="inherit_id" ref="account.view_account_type_tree" />
+    <field name="arch"  type="xml">
+        <field name="code" position="after">
+            <field name="partner_policy" />
+        </field>
+    </field>
+</record>
+
+</data>
+</openerp>
 
=== added directory 'account_partner_required/tests'
=== added file 'account_partner_required/tests/__init__.py'
--- account_partner_required/tests/__init__.py	1970-01-01 00:00:00 +0000
+++ account_partner_required/tests/__init__.py	2014-04-19 10:33:22 +0000
@@ -0,0 +1,30 @@
+# -*- encoding: utf-8 -*-
+##############################################################################
+#
+#    Account partner required module for OpenERP
+#    Copyright (C) 2014 Acsone (http://acsone.eu).
+#    @author Stéphane Bidoul <[e-mail]>
+#
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU Affero General Public License as
+#    published by the Free Software Foundation, either version 3 of the
+#    License, or (at your option) any later version.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU Affero General Public License for more details.
+#
+#    You should have received a copy of the GNU Affero General Public License
+#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+##############################################################################
+
+from . import test_account_partner_required
+
+fast_suite = [
+]
+
+checks = [
+    test_account_partner_required,
+]
 
=== added file 'account_partner_required/tests/test_account_partner_required.py'
--- account_partner_required/tests/test_account_partner_required.py	1970-01-01 00:00:00 +0000
+++ account_partner_required/tests/test_account_partner_required.py	2014-04-19 10:33:22 +0000
@@ -0,0 +1,121 @@
+# -*- encoding: utf-8 -*-
+##############################################################################
+#
+#    Account partner required module for OpenERP
+#    Copyright (C) 2014 Acsone (http://acsone.eu).
+#    @author Stéphane Bidoul <[e-mail]>
+#
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU Affero General Public License as
+#    published by the Free Software Foundation, either version 3 of the
+#    License, or (at your option) any later version.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU Affero General Public License for more details.
+#
+#    You should have received a copy of the GNU Affero General Public License
+#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+##############################################################################
+
+from datetime import datetime
+
+from openerp.tests import common
+from openerp.osv import orm
+
+
+class test_account_partner_required(common.TransactionCase):
+
+    def setUp(self):
+        super(test_account_partner_required, self).setUp()
+        self.account_obj = self.registry('account.account')
+        self.account_type_obj = self.registry('account.account.type')
+        self.move_obj = self.registry('account.move')
+        self.move_line_obj = self.registry('account.move.line')
+
+    def _create_move(self, with_partner, amount=100):
+        date = datetime.now()
+        period_id = self.registry('account.period').find(
+            self.cr, self.uid, date,
+            context={'account_period_prefer_normal': True})[0]
+        move_vals = {
+            'journal_id': self.ref('account.sales_journal'),
+            'period_id': period_id,
+            'date': date,
+        }
+        move_id = self.move_obj.create(self.cr, self.uid, move_vals)
+        self.move_line_obj.create(self.cr, self.uid,
+                                  {'move_id': move_id,
+                                   'name': '/',
+                                   'debit': 0,
+                                   'credit': amount,
+                                   'account_id': self.ref('account.a_sale')})
+        move_line_id = self.move_line_obj.create(
+            self.cr, self.uid,
+            {'move_id': move_id,
+             'name': '/',
+             'debit': amount,
+             'credit': 0,
+             'account_id': self.ref('account.a_recv'),
+             'partner_id': self.ref('base.res_partner_1') if with_partner else False})
+        return move_line_id
+
+    def _set_partner_policy(self, policy, aref='account.a_recv'):
+        account_type = self.account_obj.browse(self.cr, self.uid,
+                                               self.ref(aref)).user_type
+        self.account_type_obj.write(self.cr, self.uid, account_type.id,
+                                    {'partner_policy': policy})
+
+    def test_optional(self):
+        self._create_move(with_partner=False)
+        self._create_move(with_partner=True)
+
+    def test_always_no_partner(self):
+        self._set_partner_policy('always')
+        with self.assertRaises(orm.except_orm):
+            self._create_move(with_partner=False)
+
+    def test_always_no_partner_0(self):
+        # accept missing partner when debit=credit=0
+        self._set_partner_policy('always')
+        self._create_move(with_partner=False, amount=0)
+
+    def test_always_with_partner(self):
+        self._set_partner_policy('always')
+        self._create_move(with_partner=True)
+
+    def test_never_no_partner(self):
+        self._set_partner_policy('never')
+        self._create_move(with_partner=False)
+
+    def test_never_with_partner(self):
+        self._set_partner_policy('never')
+        with self.assertRaises(orm.except_orm):
+            self._create_move(with_partner=True)
+
+    def test_never_with_partner_0(self):
+        # accept partner when debit=credit=0
+        self._set_partner_policy('never')
+        self._create_move(with_partner=True, amount=0)
+
+    def test_always_remove_partner(self):
+        # remove partner when policy is always
+        self._set_partner_policy('always')
+        line_id = self._create_move(with_partner=True)
+        with self.assertRaises(orm.except_orm):
+            self.move_line_obj.write(self.cr, self.uid, line_id,
+                                     {'partner_id': False})
+
+    def test_change_account(self):
+        self._set_partner_policy('always', aref='account.a_pay')
+        line_id = self._create_move(with_partner=False)
+        # change account to a_pay with policy always but missing partner
+        with self.assertRaises(orm.except_orm):
+            self.move_line_obj.write(self.cr, self.uid, line_id,
+                                     {'account_id': self.ref('account.a_pay')})
+        # change account to a_pay with policy always with partner -> ok
+        self.move_line_obj.write(self.cr, self.uid, line_id,
+                                 {'account_id': self.ref('account.a_pay'),
+                                  'partner_id': self.ref('base.res_partner_1')})