Merge pull request OCA#472 from i-vyshnevska/fix-master

[11.0][FIX] refactor code to updates in bank-payment

Author: yvaucher

l10n_ch_bank/__init__.py

@@ -1,12 +1,8 @@
 # Copyright 2015 Yannick Vaucher (Camptocamp SA)
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 from . import models
+from .hooks import import_csv_data
 
 
 def post_init(cr, registry):
-    """Import CSV data as it is faster than xml and because we can't use
-    noupdate anymore with csv"""
-    from openerp.tools import convert_file
-    filename = 'data/res.bank.csv'
-    convert_file(cr, 'l10n_ch_bank', filename, None, mode='init',
-                 noupdate=True, kind='init', report=None)
+    import_csv_data(cr, registry)

l10n_ch_bank/hooks.py

@@ -0,0 +1,16 @@
+# Copyright 2019 Camptocamp SA
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl)
+
+from odoo.tools import convert_file
+
+
+def import_csv_data(cr, registry):
+    """Import CSV data as it is faster than xml and because we can't use
+    noupdate anymore with csv"""
+    filenames = ['data/res.bank.csv']
+    for filename in filenames:
+        convert_file(
+            cr, 'l10n_ch_bank',
+            filename, None, mode='init', noupdate=True,
+            kind='init', report=None,
+        )

l10n_ch_base_bank/models/bank.py

@@ -152,8 +152,8 @@ def name_search(self, name, args=None, operator='ilike', limit=80):
             ids = self.search(
                 args,
                 limit=limit
-                ).ids
-        # we sort by occurence
+            ).ids
+        # we sort by occurrence
         to_ret_ids = list(set(ids))
         to_ret_ids = sorted(
             to_ret_ids,
@@ -167,6 +167,31 @@ class ResPartnerBank(models.Model, BankCommon):
     """Inherit res.partner.bank class in order to add swiss specific fields
     and state controls
 
+    Statements:
+    acc_type could be of 3 types:
+        - postal
+        - iban
+        - bank
+
+    if account has ccp and acc_type = 'postal' we dropped acc_number and
+    compute it based on ccp, and partner
+
+    if acc_number given in 'iban' format just transform to iban format, but no
+    further modification on it, and acc_type = 'iban'
+
+    if given bank is a postal (acc_type = 'postal') and ccp != bank.ccp it's
+    mean that ccp in account is ccp of this partner in it's could be different
+    acc_number in this case recomputed by as partner_name + ccp
+
+    if given bank is a bank (acc_type = 'bank') then ccp in account should be
+    the same as bank.ccp and acc_number recomputed
+
+    if given ccp and no bank_id:
+     - check if we already have banks with the same ccp in db, if found set
+      this bank to account, update the rest
+     - if no matches this mean this is a postal type, it has no bank, we
+     set acc_number to this ccp number
+
     """
     _inherit = 'res.partner.bank'
     _compile_check_isr_add_num = re.compile('[0-9]*$')
@@ -177,7 +202,7 @@ class ResPartnerBank(models.Model, BankCommon):
         help="Your Bank adherent number to be printed "
              "in references of your ISR. "
              "This is not a postal account number."
-        )
+    )
     acc_number = fields.Char(
         string='Account/IBAN Number'
     )
@@ -186,6 +211,11 @@ class ResPartnerBank(models.Model, BankCommon):
         store=True
     )
 
+    _sql_constraints = [
+        ('isr_adherent_uniq', 'unique (isr_adherent_num, ccp)',
+         'The ISR adherent number/ccp pair must be unique !'),
+    ]
+
     @api.depends('acc_number')
     def _compute_acc_type(self):
         todo = self.env['res.partner.bank']
@@ -236,34 +266,53 @@ def _check_postal_num(self):
         return True
 
     @api.multi
-    def _get_acc_name(self):
-        """ Return an account name for a bank account
-        to use with a ccp for ISR.
-        This method makes sure to generate a unique name
-        """
-
+    def _update_acc_name(self):
+        """Check if number generated from ccp, if yes replace it on new """
         part_name = self.partner_id.name
         if not part_name and self.env.context.get('default_partner_id'):
             partner_id = self.env.context.get('default_partner_id')
             part_name = self.env['res.partner'].browse(partner_id)[0].name
+        self.acc_number = self._compute_name_ccp(part_name, self.ccp)
 
-        if part_name:
-            acc_name = _("Bank/CCP {}").format(part_name)
+    @api.multi
+    def _compute_name_ccp(self, partner_name, ccp):
+        """This method makes sure to generate a unique name"""
+        if partner_name and ccp:
+            acc_name = _("{}/CCP {}").format(partner_name, ccp)
+        elif ccp:
+            acc_name = _("CCP {}").format(ccp)
         else:
-            acc_name = _("Bank/CCP Undefined")
+            return ''
 
         exist_count = self.env['res.partner.bank'].search_count(
             [('acc_number', '=like', acc_name)])
+        # if acc_number not unique iterate on bank_accounts while not get
+        # unique number
         if exist_count:
             name_exist = exist_count
             while name_exist:
-                new_name = acc_name + " ({})".format(exist_count)
+                new_name = acc_name + " #{}".format(exist_count)
                 name_exist = self.env['res.partner.bank'].search_count(
                     [('acc_number', '=', new_name)])
                 exist_count += 1
             acc_name = new_name
         return acc_name
 
+    @api.model
+    def create(self, vals):
+        """
+        acc_number is mandatory for model, but in localization it could be not
+        mandatory when we have ccp number, so we compute acc_number in onchange
+        methods and check it here also
+        """
+        if not vals.get('acc_number') and vals.get('ccp'):
+            partner = self.env['res.partner'].browse(vals.get('partner_id'))
+            vals['acc_number'] = self._compute_name_ccp(
+                partner.name,
+                vals['ccp']
+            )
+        return super().create(vals)
+
     @api.multi
     def _get_ch_bank_from_iban(self):
         """ Extract clearing number from iban to find the bank """
@@ -273,7 +322,7 @@ def _get_ch_bank_from_iban(self):
         return clearing and self.env['res.bank'].search(
             [('clearing', '=', clearing)], limit=1)
 
-    @api.onchange('acc_number', 'acc_type')
+    @api.onchange('acc_number')
     def onchange_acc_number_set_swiss_bank(self):
         """ Set the bank when possible
         and set ccp when undefined
@@ -287,6 +336,10 @@ def onchange_acc_number_set_swiss_bank(self):
         - a bank account with CCP on the bank, we use ccp of the bank
         - otherwise there is no CCP to use
         """
+        if not self.acc_number:
+            # if account number was flashed in UI
+            self._update_acc_name()
+
         bank = self.bank_id
         ccp = False
         if self.acc_type == 'postal':
@@ -298,8 +351,6 @@ def onchange_acc_number_set_swiss_bank(self):
                 bank or
                 self.env['res.bank'].search([('bic', '=', 'POFICHBEXXX')],
                                             limit=1))
-            if not bank.is_swiss_post():
-                self.acc_number = self._get_acc_name()
         elif self.acc_type == 'iban':
             if not bank:
                 bank = self._get_ch_bank_from_iban()
@@ -308,58 +359,65 @@ def onchange_acc_number_set_swiss_bank(self):
                     ccp = self._convert_iban_to_ccp(self.acc_number.strip())
                 else:
                     ccp = bank.ccp
+
         elif self.bank_id.ccp:
             ccp = self.bank_id.ccp
         self.bank_id = bank
+
         if not self.ccp:
             self.ccp = ccp
 
     @api.onchange('ccp')
-    def onchange_ccp_set_empty_acc_number(self):
-        """ If acc_number is empty and bank ccp is defined fill it """
-        if self.bank_id:
-            if not self.acc_number and self.ccp:
-                if self.bank_id.is_swiss_post():
-                    self.acc_number = self.ccp
-                else:
-                    self.acc_number = self._get_acc_name()
+    def onchange_ccp_set_acc_number(self):
+        """If ccp changes and it's a postal bank update acc_number to ccp
+        we don't want make acc_number as computed to have possibility set it
+        manually and also avoid to shadow other logic on acc_number if exist
+       """
+        if self.acc_type == 'iban':
+            return
+
+        if self.ccp:
+            if self.acc_type == 'postal':
+                # flash bank if it was previously setup, also trigger acc_type
+                # changing
+                self.bank_id = ''
+            self._update_acc_name()
+        else:
+            # flash bank if it was previously setup
+            self.acc_number = ''
+
+        if self.bank_id.is_swiss_post():
+            self.acc_number = self.ccp
             return
 
+        # try to find appropriate bank
         ccp = self.ccp
-        if ccp and self.is_swiss_postal_num(ccp):
-            bank = (
-                self.env['res.bank'].search([('ccp', '=', ccp)], limit=1) or
-                self.env['res.bank'].search([('bic', '=', 'POFICHBEXXX')],
-                                            limit=1))
-            if not self.acc_number:
-                if not bank.is_swiss_post():
-                    self.acc_number = self._get_acc_name()
-                else:
-                    self.acc_number = self.ccp
+        if ccp and self.is_swiss_postal_num(ccp) and not self.bank_id.id:
+            bank = (self.env['res.bank'].search([('ccp', '=', ccp)], limit=1)
+                    or
+                    self.env['res.bank'].search([('bic', '=', 'POFICHBEXXX')],
+                                                limit=1))
+            if not bank.is_swiss_post():
+                self._update_acc_name()
+            else:
+                self.acc_number = self.ccp
             self.bank_id = bank
 
     @api.onchange('bank_id')
     def onchange_bank_set_acc_number(self):
-        """ If acc_number is empty and bank ccp is defined fill it """
-        if not self.bank_id:
+        """ Track bank change to update acc_name if needed"""
+        if not self.bank_id or self.acc_type == 'iban':
             return
         if self.bank_id.is_swiss_post():
-            if not self.acc_number:
-                self.acc_number = self.ccp
-            elif not self.ccp and self.is_swiss_postal_num(self.acc_number):
-                self.ccp = self.acc_number
+            self.acc_number = self.ccp
         else:
-            if not self.acc_number and self.ccp:
-                self.acc_number = self._get_acc_name()
-            elif self.acc_number and self.is_swiss_postal_num(self.acc_number):
-                self.ccp = self.acc_number
-                self.acc_number = self._get_acc_name()
+            self._update_acc_name()
 
     @api.onchange('partner_id')
     def onchange_partner_set_acc_number(self):
+        """
+        when acc_number was computed automatically we call regeneration
+        as partner name is part of acc_number
+        """
         if self.acc_type == 'bank' and self.ccp:
-            if 'Bank/CCP' in self.acc_number:
-                self.acc_number = self._get_acc_name()
-
-    _sql_constraints = [('isr_adherent_uniq', 'unique (isr_adherent_num, ccp)',
-                         'The ISR adherent number/ccp pair must be unique !')]
+            self._update_acc_name()

l10n_ch_base_bank/models/invoice.py

@@ -121,15 +121,40 @@ def _check_isr(self):
                 invoice._is_isr_reference()
         return True
 
+    def write(self, vals):
+        """Override to update partner_bank_id before constraints if needed and
+        to be consistent with create
+        """
+        if not self.partner_bank_id or not vals.get('partner_bank_id'):
+            type_defined = vals.get('type') or self.type
+            if type_defined == 'out_invoice':
+                banks = self.partner_banks_to_show()
+                if banks:
+                    vals['partner_bank_id'] = banks[0].id
+        return super().write(vals)
+
     @api.model
     def create(self, vals):
         """We override create in order to have customer invoices
         generated by the comercial flow as on change partner is
         not systemtically call"""
         type_defined = vals.get('type') or self.env.context.get('type', False)
         if type_defined == 'out_invoice' and not vals.get('partner_bank_id'):
-            user = self.env.user
-            bank_ids = user.company_id.partner_id.bank_ids
-            if bank_ids:
-                vals['partner_bank_id'] = bank_ids[0].id
+            partner = self.env.user.company_id.partner_id
+            vals['partner_bank_id'] = self._get_bank_id(
+                partner, vals.get('journal_id'), vals.get('reference_type'),
+            )
         return super(AccountInvoice, self).create(vals)
+
+    def _get_bank_id(self, partner, journal_id, ref_type):
+        if journal_id:
+            return self.env['account.journal'].browse(journal_id). \
+                bank_account_id.id
+        if ref_type == 'isr':
+            bank_ids = partner.bank_ids.filtered(
+                lambda s: s.acc_type == 'postal'
+            )
+        else:
+            bank_ids = partner.bank_ids
+        if bank_ids:
+            return bank_ids[0].id