[8.0][ADD] Improved module base_bank_account_number_unique. (#117)

base_bank_account_number_unique/README.rst

@@ -8,36 +8,45 @@ It can be desirable to be able to rely on a bank account number identifying
 exactly one partner. This module allows you to enforce this, so that an
 account number is unique in the system.
 
-Installation
-============
+There are some valid corner cases were it is valid to have multiple records
+for the same account number. For instance in a multicompany setup where the
+bank-account linked to one company, is a partner bank account for another
+company.
 
-During installation, the module checks if your bank account numbers are
-unique already. If this is not the case, you won't be able to install the
-module until duplicates are fixed.
+Because of these corner cases, the constraint is no longer implemented as
+a SQL unique index. This has the added advantage, that the module can be
+installed on databases where the bank-account numbers are not unique already.
 
-The error message only shows the first few duplicates, in order to find all
-of them, use the following statement::
+To find records that are not unique, you could use the following SQL
+statement.
 
     with res_partner_bank_sanitized as (
         select
-            id, acc_number,
-            regexp_replace(acc_number, '\W+', '', 'g') acc_number_sanitized
+            id, acc_number, coalesce(company_id, 0) as company_id,
+            sanitized_acc_number
         from res_partner_bank
     ),
     res_partner_bank_sanitized_grouped as (
         select
-            array_agg(id) ids, acc_number_sanitized, count(*) amount
-        from res_partner_bank_sanitized group by acc_number_sanitized
+            array_agg(id) ids, sanitized_acc_number, count(*) kount,
+            company_id
+        from res_partner_bank_sanitized
+        group by sanitized_acc_number, company_id
     )
-    select * from res_partner_bank_sanitized_grouped where amount > 1;
+    select * from res_partner_bank_sanitized_grouped where kount > 1;
+
+Installation
+============
+
+The constraint is active for new and changed numbers, from the moment of
+installation.
+
 
 Bug Tracker
 ===========
 
-Bugs are tracked on `GitHub Issues <https://github.com/OCA/bank-statement-import/issues>`_.
-In case of trouble, please check there if your issue has already been reported.
-If you spotted it first, help us smashing it by providing a detailed and welcomed feedback
-`here <https://github.com/OCA/bank-statement-import/issues/new?body=module:%20base_bank_account_number_unique%0Aversion:%208.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**>`_.
+Bugs are tracked on
+`GitHub Issues <https://github.com/OCA/bank-statement-import/issues>`_.
 
 Credits
 =======
@@ -46,6 +55,7 @@ Contributors
 ------------
 
 * Holger Brunn <hbrunn@therp.nl>
+* Ronald Portier <ronald@therp.nl>
 
 Maintainer
 ----------

base_bank_account_number_unique/__init__.py

@@ -1,21 +1,4 @@
 # -*- coding: utf-8 -*-
-##############################################################################
-#
-#    This module copyright (C) 2015 Therp BV <http://therp.nl>.
-#
-#    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/>.
-#
-##############################################################################
+# © 2015-2017 Therp BV <https://therp.nl>
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 from . import models
-from .hooks import post_init_hook

base_bank_account_number_unique/__openerp__.py

@@ -1,25 +1,9 @@
 # -*- coding: utf-8 -*-
-##############################################################################
-#
-#    Copyright (C) 2015 Therp BV <http://therp.nl>.
-#
-#    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/>.
-#
-##############################################################################
+# © 2015-2017 Therp BV <https://therp.nl>
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 {
     'name': 'Unique bank account numbers',
-    'version': '8.0.1.0.1',
+    'version': '8.0.1.1.0',
     'author': 'Therp BV, Odoo Community Association (OCA)',
     'license': 'AGPL-3',
     'category': 'Banking addons',
@@ -27,7 +11,6 @@
     'depends': [
         'account_bank_statement_import',
     ],
-    'post_init_hook': 'post_init_hook',
     'auto_install': False,
     'installable': True,
 }

base_bank_account_number_unique/hooks.py

@@ -1,55 +0,0 @@
-# -*- coding: utf-8 -*-
-##############################################################################
-#
-#    This module copyright (C) 2015 Therp BV <http://therp.nl>.
-#
-#    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 import _, SUPERUSER_ID
-from openerp.exceptions import Warning as UserError
-
-
-def post_init_hook(cr, pool):
-    """check if your constraint was actually inserted, raise otherwise"""
-    if not pool['ir.model.constraint'].search(cr, SUPERUSER_ID, [
-        ('name', '=', 'res_partner_bank_unique_number'),
-        ('model.model', '=', 'res.partner.bank'),
-    ]):
-        max_account_numbers = 10
-        cr.execute(
-            """
-            with
-            res_partner_bank_sanitized as
-            (select id, acc_number, regexp_replace(acc_number, '\\W+', '', 'g')
-             acc_number_sanitized from res_partner_bank),
-            res_partner_bank_sanitized_grouped as
-            (select array_agg(id) ids, acc_number_sanitized, count(*) amount
-             from res_partner_bank_sanitized group by acc_number_sanitized)
-            select acc_number_sanitized from res_partner_bank_sanitized_grouped
-            where amount > 1 limit %s;
-            """,
-            (max_account_numbers,))
-        duplicates = [acc_number for acc_number, in cr.fetchall()]
-        message = _(
-            "Module installation can't proceed as you have duplicate "
-            "account numbers in your system already. Please clean that up "
-            "and try again.\n"
-            "The following shows the first %d duplicate account numbers\n"
-            "%s\n"
-            "(if you see less than %d, those are the only duplicates)") % (
-                max_account_numbers, '\n'.join(duplicates),
-                max_account_numbers,
-        )
-        raise UserError(message)

base_bank_account_number_unique/models/__init__.py

@@ -1,20 +1,4 @@
 # -*- coding: utf-8 -*-
-##############################################################################
-#
-#    This module copyright (C) 2015 Therp BV <http://therp.nl>.
-#
-#    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/>.
-#
-##############################################################################
+# © 2015-2017 Therp BV <https://therp.nl>
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 from . import res_partner_bank

base_bank_account_number_unique/models/res_partner_bank.py

@@ -1,39 +1,40 @@
 # -*- coding: utf-8 -*-
-##############################################################################
-#
-#    This module copyright (C) 2015 Therp BV (<http://therp.nl>).
-#
-#    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 import models
+# © 2015-2017 Therp BV <https://therp.nl>
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
+from openerp import api, models, _
+from openerp.exceptions import ValidationError
 
 
 class ResPartnerBank(models.Model):
     _inherit = 'res.partner.bank'
 
     def copy_data(self, cr, uid, id, default=None, context=None):
-        if default is None:
-            default = {}
-        if context is None:
-            context = {}
+        default = default or {}
+        context = context or {}
         if 'acc_number' not in default and 'default_acc_number' not in context:
             default['acc_number'] = ''
         return super(ResPartnerBank, self).copy_data(
             cr, uid, id, default=default, context=context)
 
-    _sql_constraints = [
-        ('unique_number', 'unique(sanitized_acc_number)',
-         'Account Number must be unique'),
-    ]
+    @api.constrains('company_id', 'sanitized_acc_number')
+    def _check_unique_account(self):
+        for this in self:
+            check_domain = [
+                ('sanitized_acc_number', '=', this.sanitized_acc_number),
+            ]
+            # No problem if one record has a company and the other not:
+            if this.company_id:
+                check_domain.append(('company_id', '=', this.company_id.id))
+            else:
+                check_domain.append(('company_id', '=', False))
+            # Do not find same record, if existing:
+            if this.exists():
+                check_domain.append(('id', '<>', this.id))
+            already_existing = self.search(check_domain)
+            if already_existing:
+                raise ValidationError(
+                    _("Bank account %s already registered for %s.") %
+                    (this.acc_number,
+                     already_existing.partner_id.display_name or
+                     _("unknown partner"))
+                )

base_bank_account_number_unique/tests/__init__.py

@@ -1,20 +1,4 @@
 # -*- coding: utf-8 -*-
-##############################################################################
-#
-#    This module copyright (C) 2015 Therp BV <http://therp.nl>.
-#
-#    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/>.
-#
-##############################################################################
+# © 2015-2017 Therp BV <https://therp.nl>
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 from . import test_base_bank_account_number_unique

base_bank_account_number_unique/tests/test_base_bank_account_number_unique.py

@@ -1,42 +1,66 @@
 # -*- coding: utf-8 -*-
-##############################################################################
-#
-#    This module copyright (C) 2015 Therp BV <http://therp.nl>.
-#
-#    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/>.
-#
-##############################################################################
+# © 2015-2017 Therp BV <https://therp.nl>
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 from openerp.tests.common import TransactionCase
-from openerp.exceptions import Warning as UserError
-from ..hooks import post_init_hook
+from openerp.exceptions import ValidationError
 
 
 class TestBaseBankAccountNumberUnique(TransactionCase):
+
     def test_base_bank_account_number_unique(self):
-        # drop our constraint, insert nonunique account numbers and see if
-        # the init hook catches this
-        self.env['ir.model.constraint'].search([
-            ('name', '=', 'res_partner_bank_unique_number'),
-            ('model.model', '=', 'res.partner.bank'),
-        ])._module_data_uninstall()
-        self.env['res.partner.bank'].create({
+        """Add a bank account, then try to add another one with the
+        same number."""
+        bank_account_model = self.env['res.partner.bank']
+        bank_account_model.create({
+            'acc_number': 'BE1234567890',
+            'state': 'bank',
+        })
+        with self.assertRaises(ValidationError):
+            bank_account_model.create({
+                'acc_number': 'BE 1234 567 890',
+                'state': 'bank',
+            })
+
+    def test_base_bank_account_number_unique_write(self):
+        """Add a bank account, then try to add another one with the
+        same number."""
+        bank_account_model = self.env['res.partner.bank']
+        bank_account_model.create({
+            'acc_number': 'BE1234567890',
+            'state': 'bank',
+        })
+        acc = bank_account_model.create({
+            'acc_number': 'CH1234567890',
+            'state': 'bank',
+        })
+        with self.assertRaises(ValidationError):
+            acc.acc_number = 'BE1234567890'
+
+    def test_base_bank_account_number_other_company(self):
+        """Add a bank account, then try to add another one with the
+        same number, but belongig to another company."""
+        bank_account_model = self.env['res.partner.bank']
+        company_model = self.env['res.company']
+        company = company_model.create({
+            'name': 'Schmidt AG',
+        })
+        bank_account_model.create({
             'acc_number': 'BE1234567890',
             'state': 'bank',
         })
-        self.env['res.partner.bank'].create({
-            'acc_number': 'BE 1234 567 890',
+        bank_account_model.create({
+            'acc_number': 'CH1234567890',
+            'state': 'bank',
+            'company_id': company.id,
+        })
+
+    def test_bank_account_copy(self):
+        """Copied bank account data should not contain account."""
+        bank_account_model = self.env['res.partner.bank']
+        original_account = bank_account_model.create({
+            'acc_number': 'BE1234567890',
             'state': 'bank',
         })
-        with self.assertRaises(UserError):
-            post_init_hook(self.cr, self.registry)
+        copied_data = original_account.copy_data(default=None)
+        # Calling copy_data from new api returns array!
+        self.assertEqual(copied_data[0]['acc_number'], '')