Skip to content

Commit

Permalink
[FIX] base_bank_from_iban: Handle correctly non IBAN accounts
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
pedrobaeza committed Sep 28, 2024
1 parent 207d18d commit 1da90fe
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 28 deletions.
56 changes: 29 additions & 27 deletions base_bank_from_iban/models/res_partner_bank.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
# Copyright 2017 Tecnativa - Carlos Dauden <[email protected]>
# 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
Expand All @@ -13,8 +12,6 @@
pretty_iban,
)

_logger = logging.getLogger(__name__)


class ResPartnerBank(models.Model):
_inherit = "res.partner.bank"
Expand All @@ -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")
Expand Down
39 changes: 38 additions & 1 deletion base_bank_from_iban/tests/test_base_bank_from_iban.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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)

0 comments on commit 1da90fe

Please sign in to comment.