[FIX] Restructure and expand tests. Fix some bugs.

There is now a common class to test import of statements and their transactions.
Fixes:
- Field in regular expression for mt940 rabo parser was to short for bank-account.
- Wrong name used for reference attribute in mt940 rabo parser;
- Attribute transferred_amount should not have been renamed amount in BankTransaction.

account_bank_statement_import/account_bank_statement_import.py

@@ -1,13 +1,13 @@
 # -*- coding: utf-8 -*-
 """Framework for importing bank statement files."""
+import logging
 import base64
 
 from openerp import api, models, fields
 from openerp.tools.translate import _
 from openerp.exceptions import Warning
 
-import logging
-_logger = logging.getLogger(__name__)
+_LOGGER = logging.getLogger(__name__)
 
 
 class AccountBankStatementLine(models.Model):
@@ -223,7 +223,7 @@ class AccountBankStatementImport(models.TransientModel):
                 journal_currency_id = journal_obj.currency.id
                 if currency_id != journal_currency_id:
                     # ALso log message with id's for technical analysis:
-                    _logger.warn(
+                    _LOGGER.warn(
                         _('Statement currency id is %d,'
                           ' but journal currency id = %d.'),
                         currency_id,
@@ -237,7 +237,7 @@ class AccountBankStatementImport(models.TransientModel):
                 company_currency_id = self.env.user.company_id.currency_id.id
                 if currency_id != company_currency_id:
                     # ALso log message with id's for technical analysis:
-                    _logger.warn(
+                    _LOGGER.warn(
                         _('Statement currency id is %d,'
                           ' but company currency id = %d.'),
                         currency_id,
@@ -288,9 +288,9 @@ class AccountBankStatementImport(models.TransientModel):
             unique_import_id = line_vals.get('unique_import_id', False)
             if unique_import_id:
                 line_vals['unique_import_id'] = (
-                    account_number and account_number + '-' or '') + \
+                    (account_number and account_number + '-' or '') +
                     unique_import_id
-
+                )
             if not line_vals.get('bank_account_id'):
                 # Find the partner and his bank account or create the bank
                 # account. The partner selected during the reconciliation
@@ -298,17 +298,17 @@ class AccountBankStatementImport(models.TransientModel):
                 # closed.
                 partner_id = False
                 bank_account_id = False
-                identifying_string = line_vals.get('account_number')
-                if identifying_string:
+                account_number = line_vals.get('account_number')
+                if account_number:
                     bank_model = self.env['res.partner.bank']
                     banks = bank_model.search(
-                        [('acc_number', '=', identifying_string)], limit=1)
+                        [('acc_number', '=', account_number)], limit=1)
                     if banks:
                         bank_account_id = banks[0].id
                         partner_id = banks[0].partner_id.id
                     else:
-                        bank_account_id = self._create_bank_account(
-                            identifying_string).id
+                        bank_obj = self._create_bank_account(account_number)
+                        bank_account_id = bank_obj and bank_obj.id or False
                 line_vals['partner_id'] = partner_id
                 line_vals['bank_account_id'] = bank_account_id
         return stmt_vals

account_bank_statement_import/tests/__init__.py

@@ -1,3 +1,5 @@
 # -*- encoding: utf-8 -*-
+"""Define tests to be run."""
 from . import test_res_partner_bank
 from . import test_import_bank_statement
+from .test_import_file import TestStatementFile

account_bank_statement_import/tests/test_import_file.py

@@ -0,0 +1,132 @@
+# -*- coding: utf-8 -*-
+"""Provide common base for bank statement import tests."""
+##############################################################################
+#
+#    Copyright (C) 2015 Therp BV <http://therp.nl>.
+#
+#    All other contributions are (C) by their respective contributors
+#
+#    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/>.
+#
+##############################################################################
+import logging
+
+from openerp.tests.common import TransactionCase
+from openerp.modules.module import get_module_resource
+
+
+_LOGGER = logging.getLogger(__name__)
+
+
+class TestStatementFile(TransactionCase):
+    """Check wether statements with transactions correctly imported.
+
+    No actual tests are done in this class, implementations are in
+    subclasses in actual import modules.
+    """
+
+    def _test_transaction(
+            self, statement_obj, remote_account=False,
+            transferred_amount=False, value_date=False, ref=False):
+        """Check wether transaction with attributes passed was created.
+
+        Actually this method also tests wether automatic creation of
+        partner bank accounts is working.
+        """
+        transaction_model = self.env['account.bank.statement.line']
+        partner_bank_model = self.env['res.partner.bank']
+        domain = [('statement_id', '=', statement_obj.id)]
+        if remote_account:
+            bids = partner_bank_model.search(
+                [('acc_number', '=', remote_account)])
+            self.assertTrue(
+                bids,
+                'Bank-account %s not found after parse.' % remote_account
+            )
+            domain.append(('bank_account_id', '=', bids[0].id))
+        if transferred_amount:
+            domain.append(('amount', '=', transferred_amount))
+        if value_date:
+            domain.append(('date', '=', value_date))
+        if ref:
+            domain.append(('ref', '=', ref))
+        ids = transaction_model.search(domain)
+        if not ids:
+            # We will get assertion error, but to solve we need to see
+            # what transactions have been added:
+            self.cr.execute(
+                "select name, date, amount, ref, bank_account_id"
+                " from account_bank_statement_line"
+                " where statement_id=%d" % statement_obj.id)
+            _LOGGER.error(
+                "Transaction not found in %s" %
+                str(self.cr.fetchall())
+            )
+        self.assertTrue(
+            ids,
+            'Transaction %s not found after parse.' % str(domain)
+        )
+
+    def _test_statement_import(
+            self, module_name, file_name, statement_name, local_account=False,
+            start_balance=False, end_balance=False, transactions=None):
+        """Test correct creation of single statement."""
+        import_model = self.env['account.bank.statement.import']
+        partner_bank_model = self.env['res.partner.bank']
+        statement_model = self.env['account.bank.statement']
+        statement_path = get_module_resource(
+            module_name,
+            'test_files',
+            file_name
+        )
+        statement_file = open(
+            statement_path, 'rb').read().encode('base64')
+        bank_statement_id = import_model.create(
+            dict(
+                data_file=statement_file,
+            )
+        )
+        bank_statement_id.import_file()
+        # Check wether bank account has been created:
+        if local_account:
+            bids = partner_bank_model.search(
+                [('acc_number', '=', local_account)])
+            self.assertTrue(
+                bids,
+                'Bank account %s not created from statement' % local_account
+            )
+        # statement name is account number + '-' + date of last 62F line:
+        ids = statement_model.search([('name', '=', statement_name)])
+        self.assertTrue(
+            ids,
+            'Statement %s not found after parse.' % statement_name
+        )
+        statement_obj = ids[0]
+        #statement_obj = statement_model.browse(statement_id)
+        if start_balance:
+            self.assertTrue(
+                abs(statement_obj.balance_start - start_balance) < 0.00001,
+                'Start balance %f not equal to expected %f' %
+                (statement_obj.balance_start, start_balance)
+            )
+        if end_balance:
+            self.assertTrue(
+                abs(statement_obj.balance_end_real - end_balance) < 0.00001,
+                'End balance %f not equal to expected %f' %
+                (statement_obj.balance_end_real, end_balance)
+            )
+        # Maybe we need to test transactions?
+        if transactions:
+            for transaction in transactions:
+                self._test_transaction(statement_obj, **transaction)

bank_statement_parse/parserlib.py

@@ -44,14 +44,14 @@ class BankTransaction(dict):
         self['name'] = name
 
     @property
-    def amount(self):
+    def transferred_amount(self):
         """property getter"""
         return self['amount']
 
-    @amount.setter
-    def amount(self, amount):
+    @transferred_amount.setter
+    def transferred_amount(self, transferred_amount):
         """property setter"""
-        self['amount'] = amount
+        self['amount'] = transferred_amount
 
     @property
     def eref(self):

bank_statement_parse_camt/tests/test_import_bank_statement.py

@@ -4,8 +4,6 @@
 #
 #    Copyright (C) 2015 Therp BV <http://therp.nl>.
 #
-#    All other contributions are (C) by their respective contributors
-#
 #    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
@@ -20,45 +18,28 @@
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 ##############################################################################
-from openerp.tests.common import TransactionCase
-from openerp.modules.module import get_module_resource
+from openerp.addons.account_bank_statement_import.tests import (
+    TestStatementFile)
 
 
-class TestStatementFile(TransactionCase):
-    """Run test to import camt.053 import."""
+class TestImport(TestStatementFile):
+    """Run test to import camt import."""
 
     def test_statement_import(self):
         """Test correct creation of single statement."""
-        import_model = self.registry('account.bank.statement.import')
-        statement_model = self.registry('account.bank.statement')
-        cr, uid = self.cr, self.uid
-        statement_path = get_module_resource(
-            'bank_statement_parse_camt',
-            'test_files',
-            'test-camt053.xml'
-        )
-        statement_file = open(
-            statement_path, 'rb').read().encode('base64')
-        bank_statement_id = import_model.create(
-            cr, uid,
-            dict(
-                data_file=statement_file,
-            )
-        )
-        import_model.import_file(cr, uid, [bank_statement_id])
-        ids = statement_model.search(
-            cr, uid, [('name', '=', '1234Test/1')])
-        self.assertTrue(ids, 'Statement not found after parse.')
-        statement_id = ids[0]
-        statement_obj = statement_model.browse(
-            cr, uid, statement_id)
-        self.assertTrue(
-            abs(statement_obj.balance_start - 15568.27) < 0.00001,
-            'Start balance %f not equal to 15568.27' %
-            statement_obj.balance_start
-        )
-        self.assertTrue(
-            abs(statement_obj.balance_end_real - 15121.12) < 0.00001,
-            'Real end balance %f not equal to 15121.12' %
-            statement_obj.balance_end_real
+        transactions = [
+            {
+                'remote_account': 'NL46ABNA0499998748',
+                'transferred_amount': -754.25,
+                'value_date': '2013-01-05',
+                'ref': '435005714488-ABNO33052620',
+            },
+        ]
+        # statement name is account number + '-' + date of last 62F line:
+        self._test_statement_import(
+            'bank_statement_parse_camt', 'test-camt053.xml',
+            '1234Test/1',
+            local_account='NL77ABNA0574908765',
+            start_balance=15568.27, end_balance=15121.12,
+            transactions=transactions
         )

bank_statement_parse_nl_ing_mt940/tests/test_import_bank_statement.py

@@ -20,64 +20,34 @@
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 ##############################################################################
-from openerp.tests.common import TransactionCase
-from openerp.modules.module import get_module_resource
+from openerp.addons.account_bank_statement_import.tests import (
+    TestStatementFile)
 
 
-class TestStatementFile(TransactionCase):
+class TestImport(TestStatementFile):
     """Run test to import MT940 ING import."""
 
-    def _test_statement_import(
-            self, file_name, statement_name, start_balance, end_balance):
-        """Test correct creation of single statement."""
-        import_model = self.registry('account.bank.statement.import')
-        statement_model = self.registry('account.bank.statement')
-        cr, uid = self.cr, self.uid
-        statement_path = get_module_resource(
-            'bank_statement_parse_nl_ing_mt940',
-            'test_files',
-            file_name
-        )
-        statement_file = open(
-            statement_path, 'rb').read().encode('base64')
-        bank_statement_id = import_model.create(
-            cr, uid,
-            dict(
-                data_file=statement_file,
-            )
-        )
-        import_model.import_file(cr, uid, [bank_statement_id])
-        # statement name is account number + '-' + date of last 62F line:
-        ids = statement_model.search(
-            cr, uid, [('name', '=', statement_name)])
-        self.assertTrue(
-            ids,
-            'Statement %s not found after parse.' % statement_name
-        )
-        statement_id = ids[0]
-        statement_obj = statement_model.browse(
-            cr, uid, statement_id)
-        self.assertTrue(
-            abs(statement_obj.balance_start - start_balance) < 0.00001,
-            'Start balance %f not equal to expected %f' %
-            (statement_obj.balance_start, start_balance)
-        )
-        self.assertTrue(
-            abs(statement_obj.balance_end_real - end_balance) < 0.00001,
-            'Start balance %f not equal to expected %f' %
-            (statement_obj.balance_end_real, end_balance)
-        )
-
     def test_old_statement_import(self):
         """Test correct creation of single statement from old format."""
         self._test_statement_import(
-            'test-ing-old.940', 'NL77INGB0574908765-2014-01-20',
-            662.23, 564.35
+            'bank_statement_parse_nl_ing_mt940', 'test-ing-old.940',
+            'NL77INGB0574908765-2014-01-20',
+            start_balance=662.23, end_balance=564.35
         )
 
     def test_statement_import(self):
         """Test correct creation of single statement."""
+        transactions = [
+            {
+                'remote_account': 'NL32INGB0000012345',
+                'transferred_amount': 1.56,
+                'value_date': '2014-02-20',
+                'ref': 'EV12341REP1231456T1234',
+            },
+        ]
         self._test_statement_import(
-            'test-ing.940', 'NL77INGB0574908765-2014-02-20',
-            662.23, 564.35
+            'bank_statement_parse_nl_ing_mt940', 'test-ing.940',
+            'NL77INGB0574908765-2014-02-20',
+            start_balance=662.23, end_balance=564.35,
+            transactions=transactions
         )

bank_statement_parse_nl_rabo_mt940/account_bank_statement_import.py

@@ -36,7 +36,8 @@ class AccountBankStatementImport(models.TransientModel):
         parser = Parser()
         try:
             _LOGGER.debug("Try parsing with MT940 RABO.")
-            return parser.parse(data_file)
+            statements = parser.parse(data_file)
+            return statements
         except ValueError:
             # Returning super will call next candidate:
             _LOGGER.debug("Statement file was not a MT940 RABO file.")

bank_statement_parse_nl_rabo_mt940/mt940.py

@@ -30,7 +30,7 @@ class MT940Parser(MT940):
     tag_61_regex = re.compile(
         r'^(?P<date>\d{6})(?P<sign>[CD])(?P<amount>\d+,\d{2})N(?P<type>.{3})'
         r'(?P<reference>MARF|EREF|PREF|NONREF)\s*'
-        r'\n?(?P<remote_account>\w{1,16})?'
+        r'\n?(?P<remote_account>\w{1,34})?'
     )
 
     def __init__(self):
@@ -57,7 +57,7 @@ class MT940Parser(MT940):
         parsed_data = self.tag_61_regex.match(data).groupdict()
         self.current_transaction.transferred_amount = (
             str2amount(parsed_data['sign'], parsed_data['amount']))
-        self.current_transaction.reference = parsed_data['reference']
+        self.current_transaction.eref = parsed_data['reference']
         if parsed_data['remote_account']:
             self.current_transaction.remote_account = (
                 parsed_data['remote_account'])

bank_statement_parse_nl_rabo_mt940/tests/test_import_bank_statement.py

@@ -19,58 +19,28 @@
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 ##############################################################################
-from openerp.tests.common import TransactionCase
-from openerp.modules.module import get_module_resource
+from openerp.addons.account_bank_statement_import.tests import (
+    TestStatementFile)
 
 
-class TestStatementFile(TransactionCase):
+class TestImport(TestStatementFile):
     """Run test to import MT940 RABO import."""
 
     def test_statement_import(self):
-        """Test correct creation of single statement.
-
-        For this test there is NOT an existing bank-account. Therefore a
-        bank account should automatically be created in the main company.
-        """
-        partner_bank_model = self.env['res.partner.bank']
-        import_model = self.registry('account.bank.statement.import')
-        statement_model = self.registry('account.bank.statement')
-        cr, uid = self.cr, self.uid
-        statement_path = get_module_resource(
-            'bank_statement_parse_nl_rabo_mt940',
-            'test_files',
-            'test-rabo.swi'
-        )
-        statement_file = open(
-            statement_path, 'rb').read().encode('base64')
-        bank_statement_id = import_model.create(
-            cr, uid,
-            dict(
-                data_file=statement_file,
-            )
-        )
-        import_model.import_file(cr, uid, [bank_statement_id])
-        # Check wether bank account has been created:
-        vals = partner_bank_model.search(
-            [('acc_number', '=', 'NL34RABO0142623393')])
-        self.assertEquals(
-            1, len(vals),
-            'Bank account not created from statement'
-        )
+        """Test correct creation of single statement."""
+        transactions = [
+            {
+                'remote_account': 'NL66RABO0160878799',
+                'transferred_amount': 400.00,
+                'value_date': '2014-01-02',
+                'ref': 'NONREF',
+            },
+        ]
         # statement name is account number + '-' + date of last 62F line:
-        ids = statement_model.search(
-            cr, uid, [('name', '=', 'NL34RABO0142623393-2014-01-07')])
-        self.assertTrue(ids, 'Statement not found after parse.')
-        statement_id = ids[0]
-        statement_obj = statement_model.browse(
-            cr, uid, statement_id)
-        self.assertTrue(
-            abs(statement_obj.balance_start - 4433.52) < 0.00001,
-            'Start balance %f not equal to 4433.52' %
-            statement_obj.balance_start
-        )
-        self.assertTrue(
-            abs(statement_obj.balance_end_real - 4798.91) < 0.00001,
-            'Real end balance %f not equal to 4798.91' %
-            statement_obj.balance_end_real
+        self._test_statement_import(
+            'bank_statement_parse_nl_rabo_mt940', 'test-rabo.swi',
+            'NL34RABO0142623393-2014-01-07',
+            local_account='NL34RABO0142623393',
+            start_balance=4433.52, end_balance=4798.91,
+            transactions=transactions
         )