From 1da90fea576fb2e6555d728592d0eaa343f354cc Mon Sep 17 00:00:00 2001 From: "Pedro M. Baeza" Date: Sat, 28 Sep 2024 21:57:14 +0200 Subject: [PATCH] [FIX] base_bank_from_iban: Handle correctly non IBAN accounts If any non IBAN account is provided, there's an ugly log with traceback each time, polluting tests and system logs: ``` INFO prod odoo.addons.base_bank_from_iban.models.res_partner_bank: Could not find bank from IBAN Traceback (most recent call last): File ".../addons/base_bank_from_iban/models/res_partner_bank.py", line 34, in _add_bank_vals bank = self._get_bank_from_iban(vals["acc_number"]) File ".../addons/base_bank_from_iban/models/res_partner_bank.py", line 42, in _get_bank_from_iban iban = schwifty.IBAN(acc_number) File ".../python/site-packages/schwifty/iban.py", line 77, in __init__ self.validate(validate_bban) File ".../python/site-packages/schwifty/iban.py", line 175, in validate self._validate_characters() File ".../python/site-packages/schwifty/iban.py", line 185, in _validate_characters raise exceptions.InvalidStructure(f"Invalid characters in IBAN {self!s}") schwifty.exceptions.InvalidStructure: Invalid characters in IBAN XXXXXX ``` This commit removes that traceback catching the proper exception, and handling it accordingly, and also removing an extra INFO log that was not adding value. --- .../models/res_partner_bank.py | 56 ++++++++++--------- .../tests/test_base_bank_from_iban.py | 39 ++++++++++++- 2 files changed, 67 insertions(+), 28 deletions(-) diff --git a/base_bank_from_iban/models/res_partner_bank.py b/base_bank_from_iban/models/res_partner_bank.py index 66281780..62596303 100644 --- a/base_bank_from_iban/models/res_partner_bank.py +++ b/base_bank_from_iban/models/res_partner_bank.py @@ -1,8 +1,7 @@ -# Copyright 2017 Tecnativa - Carlos Dauden +# Copyright 2017 Tecnativa - Carlos Dauden +# Copyright 2024 Tecnativa - Pedro M. Baeza # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl-3). -import logging - import schwifty from odoo import api, models @@ -13,8 +12,6 @@ pretty_iban, ) -_logger = logging.getLogger(__name__) - class ResPartnerBank(models.Model): _inherit = "res.partner.bank" @@ -30,32 +27,37 @@ def write(self, vals): def _add_bank_vals(self, vals): if vals.get("acc_number") and not vals.get("bank_id"): - try: - bank = self._get_bank_from_iban(vals["acc_number"]) - vals["bank_id"] = bank.id - except Exception: - _logger.info("Could not find bank from IBAN", exc_info=True) + vals["bank_id"] = self._get_bank_from_iban(vals["acc_number"]).id return vals @api.model def _get_bank_from_iban(self, acc_number): - iban = schwifty.IBAN(acc_number) - country_code = iban.country_code.lower() - country = self.env.ref("base.%s" % country_code, raise_if_not_found=False) - vals = { - "name": iban.bank["name"], - "bic": iban.bank["bic"], - "code": iban.bank["bank_code"], - "country": country.id, - } - domain = [("code", "=", iban.bank["bank_code"])] - bank = self.env["res.bank"].search(domain) - if bank and len(bank) == 1: - for field in vals: - if not bank[field]: - bank[field] = vals[field] - else: - bank = self.env["res.bank"].create(vals) + try: + iban = schwifty.IBAN(acc_number) + country_code = iban.country_code.lower() + country = self.env.ref("base.%s" % country_code, raise_if_not_found=False) + if iban.bank: + vals = { + "name": iban.bank["name"], + "bic": iban.bank["bic"], + "code": iban.bank["bank_code"], + "country": country.id, + } + domain = [ + ("code", "=", iban.bank["bank_code"]), + ("country", "=", country.id), + ] + bank = self.env["res.bank"].search(domain, limit=1) + if bank: + for field in vals: + if not bank[field]: + bank[field] = vals[field] + else: + bank = self.env["res.bank"].create(vals) + else: + bank = self.env["res.bank"] + except schwifty.exceptions.InvalidStructure: + bank = self.env["res.bank"] return bank @api.onchange("acc_number", "acc_type") diff --git a/base_bank_from_iban/tests/test_base_bank_from_iban.py b/base_bank_from_iban/tests/test_base_bank_from_iban.py index ed96a767..ddd6ce45 100644 --- a/base_bank_from_iban/tests/test_base_bank_from_iban.py +++ b/base_bank_from_iban/tests/test_base_bank_from_iban.py @@ -1,5 +1,5 @@ # Copyright 2017 Tecnativa - Carlos Dauden -# Copyright 2022 Tecnativa - Pedro M. Baeza +# Copyright 2022,2024 Tecnativa - Pedro M. Baeza # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl-3). from odoo.tests import Form, common @@ -44,3 +44,40 @@ def test_onchange_acc_number_iban_wizard(self): self.assertEqual(wizard.bank_id, self.bank) wizard.acc_number = "" self.assertEqual(wizard.bank_id, self.bank) + + def test_create_iban_not_found(self): + partner_bank = self.env["res.partner.bank"].create( + {"acc_number": "es1299999999509999999999", "partner_id": self.partner.id} + ) + self.assertFalse(partner_bank.bank_id) + + def test_create_iban_found(self): + partner_bank = self.env["res.partner.bank"].create( + {"acc_number": "DE89370400440532013000", "partner_id": self.partner.id} + ) + self.assertTrue(partner_bank.bank_id) + self.assertTrue(partner_bank.bank_id.name, "Commerzbank") + self.assertTrue(partner_bank.bank_id.bic, "COBADEFFXXX") + self.assertTrue(partner_bank.bank_id.code, "37040044") + self.assertTrue(partner_bank.bank_id.country.code, "DE") + + def test_create_iban_found_existing_bank(self): + bank = self.env["res.bank"].create( + { + "country": self.env.ref("base.de").id, + "code": "37040044", + "name": "Commerzbank", + } + ) + partner_bank = self.env["res.partner.bank"].create( + {"acc_number": "DE89370400440532013000", "partner_id": self.partner.id} + ) + self.assertEqual(partner_bank.bank_id, bank) + self.assertTrue(bank.bic, "COBADEFFXXX") + + def test_create_invalid_iban(self): + partner_bank = self.env["res.partner.bank"].create( + {"acc_number": "1234567890", "partner_id": self.partner.id} + ) + # The important thing here is to not see any warning in the log + self.assertTrue(partner_bank)