From 40ef1232e4caab377e463652102dd1cd898455ff Mon Sep 17 00:00:00 2001 From: Ronald Portier Date: Sun, 12 Aug 2018 17:12:01 +0200 Subject: [PATCH] [8.0][ADD] Improved module base_bank_account_number_unique. (#117) --- base_bank_account_number_unique/README.rst | 42 +++++---- base_bank_account_number_unique/__init__.py | 21 +---- .../__openerp__.py | 23 +---- base_bank_account_number_unique/hooks.py | 55 ------------ .../models/__init__.py | 20 +---- .../models/res_partner_bank.py | 55 ++++++------ .../tests/__init__.py | 20 +---- .../test_base_bank_account_number_unique.py | 86 ++++++++++++------- 8 files changed, 118 insertions(+), 204 deletions(-) delete mode 100644 base_bank_account_number_unique/hooks.py diff --git a/base_bank_account_number_unique/README.rst b/base_bank_account_number_unique/README.rst index 9c3dc23..fd7066a 100644 --- a/base_bank_account_number_unique/README.rst +++ b/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 `_. -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 `_. +Bugs are tracked on +`GitHub Issues `_. Credits ======= @@ -46,6 +55,7 @@ Contributors ------------ * Holger Brunn +* Ronald Portier Maintainer ---------- diff --git a/base_bank_account_number_unique/__init__.py b/base_bank_account_number_unique/__init__.py index 856516e..83d23d7 100644 --- a/base_bank_account_number_unique/__init__.py +++ b/base_bank_account_number_unique/__init__.py @@ -1,21 +1,4 @@ # -*- coding: utf-8 -*- -############################################################################## -# -# This module copyright (C) 2015 Therp BV . -# -# 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 . -# -############################################################################## +# © 2015-2017 Therp BV +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). from . import models -from .hooks import post_init_hook diff --git a/base_bank_account_number_unique/__openerp__.py b/base_bank_account_number_unique/__openerp__.py index 95a8308..3c3a654 100644 --- a/base_bank_account_number_unique/__openerp__.py +++ b/base_bank_account_number_unique/__openerp__.py @@ -1,25 +1,9 @@ # -*- coding: utf-8 -*- -############################################################################## -# -# Copyright (C) 2015 Therp BV . -# -# 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 . -# -############################################################################## +# © 2015-2017 Therp BV +# 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, } diff --git a/base_bank_account_number_unique/hooks.py b/base_bank_account_number_unique/hooks.py deleted file mode 100644 index a8fdb76..0000000 --- a/base_bank_account_number_unique/hooks.py +++ /dev/null @@ -1,55 +0,0 @@ -# -*- coding: utf-8 -*- -############################################################################## -# -# This module copyright (C) 2015 Therp BV . -# -# 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 . -# -############################################################################## -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) diff --git a/base_bank_account_number_unique/models/__init__.py b/base_bank_account_number_unique/models/__init__.py index eeb9f7e..d3d387e 100644 --- a/base_bank_account_number_unique/models/__init__.py +++ b/base_bank_account_number_unique/models/__init__.py @@ -1,20 +1,4 @@ # -*- coding: utf-8 -*- -############################################################################## -# -# This module copyright (C) 2015 Therp BV . -# -# 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 . -# -############################################################################## +# © 2015-2017 Therp BV +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). from . import res_partner_bank diff --git a/base_bank_account_number_unique/models/res_partner_bank.py b/base_bank_account_number_unique/models/res_partner_bank.py index 29ae7ad..8ac88a3 100644 --- a/base_bank_account_number_unique/models/res_partner_bank.py +++ b/base_bank_account_number_unique/models/res_partner_bank.py @@ -1,39 +1,40 @@ # -*- coding: utf-8 -*- -############################################################################## -# -# This module copyright (C) 2015 Therp BV (). -# -# 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 . -# -############################################################################## -from openerp import models +# © 2015-2017 Therp BV +# 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")) + ) diff --git a/base_bank_account_number_unique/tests/__init__.py b/base_bank_account_number_unique/tests/__init__.py index b6ab1e9..0904748 100644 --- a/base_bank_account_number_unique/tests/__init__.py +++ b/base_bank_account_number_unique/tests/__init__.py @@ -1,20 +1,4 @@ # -*- coding: utf-8 -*- -############################################################################## -# -# This module copyright (C) 2015 Therp BV . -# -# 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 . -# -############################################################################## +# © 2015-2017 Therp BV +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). from . import test_base_bank_account_number_unique diff --git a/base_bank_account_number_unique/tests/test_base_bank_account_number_unique.py b/base_bank_account_number_unique/tests/test_base_bank_account_number_unique.py index 0c87638..b21ebcc 100644 --- a/base_bank_account_number_unique/tests/test_base_bank_account_number_unique.py +++ b/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 . -# -# 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 . -# -############################################################################## +# © 2015-2017 Therp BV +# 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'], '')