From 182ea2893a303c2d306d3abb9588464aabd33d5e Mon Sep 17 00:00:00 2001 From: "Ronald Portier (Therp BV)" Date: Mon, 22 Jun 2015 15:44:39 +0200 Subject: [PATCH] [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.py | 22 +-- .../tests/__init__.py | 2 + .../tests/test_import_file.py | 132 ++++++++++++++++++ bank_statement_parse/parserlib.py | 8 +- .../tests/test_import_bank_statement.py | 57 +++----- .../tests/test_import_bank_statement.py | 66 +++------ .../account_bank_statement_import.py | 3 +- bank_statement_parse_nl_rabo_mt940/mt940.py | 4 +- .../tests/test_import_bank_statement.py | 66 +++------ 9 files changed, 208 insertions(+), 152 deletions(-) create mode 100644 account_bank_statement_import/tests/test_import_file.py diff --git a/account_bank_statement_import/account_bank_statement_import.py b/account_bank_statement_import/account_bank_statement_import.py index d75ee7ce..6d4dff68 100644 --- a/account_bank_statement_import/account_bank_statement_import.py +++ b/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 diff --git a/account_bank_statement_import/tests/__init__.py b/account_bank_statement_import/tests/__init__.py index 899646a8..9d91a6a8 100644 --- a/account_bank_statement_import/tests/__init__.py +++ b/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 diff --git a/account_bank_statement_import/tests/test_import_file.py b/account_bank_statement_import/tests/test_import_file.py new file mode 100644 index 00000000..08afdcd2 --- /dev/null +++ b/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 . +# +# 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 . +# +############################################################################## +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) diff --git a/bank_statement_parse/parserlib.py b/bank_statement_parse/parserlib.py index c52accdc..eeee36b4 100644 --- a/bank_statement_parse/parserlib.py +++ b/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): diff --git a/bank_statement_parse_camt/tests/test_import_bank_statement.py b/bank_statement_parse_camt/tests/test_import_bank_statement.py index fdfbdb81..fa845cae 100644 --- a/bank_statement_parse_camt/tests/test_import_bank_statement.py +++ b/bank_statement_parse_camt/tests/test_import_bank_statement.py @@ -4,8 +4,6 @@ # # Copyright (C) 2015 Therp BV . # -# 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 . # ############################################################################## -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 ) diff --git a/bank_statement_parse_nl_ing_mt940/tests/test_import_bank_statement.py b/bank_statement_parse_nl_ing_mt940/tests/test_import_bank_statement.py index da734478..09c5fd1a 100644 --- a/bank_statement_parse_nl_ing_mt940/tests/test_import_bank_statement.py +++ b/bank_statement_parse_nl_ing_mt940/tests/test_import_bank_statement.py @@ -20,64 +20,34 @@ # along with this program. If not, see . # ############################################################################## -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 ) diff --git a/bank_statement_parse_nl_rabo_mt940/account_bank_statement_import.py b/bank_statement_parse_nl_rabo_mt940/account_bank_statement_import.py index 4b75257c..1d21c08f 100644 --- a/bank_statement_parse_nl_rabo_mt940/account_bank_statement_import.py +++ b/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.") diff --git a/bank_statement_parse_nl_rabo_mt940/mt940.py b/bank_statement_parse_nl_rabo_mt940/mt940.py index 0161d7ba..4533f6ee 100644 --- a/bank_statement_parse_nl_rabo_mt940/mt940.py +++ b/bank_statement_parse_nl_rabo_mt940/mt940.py @@ -30,7 +30,7 @@ class MT940Parser(MT940): tag_61_regex = re.compile( r'^(?P\d{6})(?P[CD])(?P\d+,\d{2})N(?P.{3})' r'(?PMARF|EREF|PREF|NONREF)\s*' - r'\n?(?P\w{1,16})?' + r'\n?(?P\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']) diff --git a/bank_statement_parse_nl_rabo_mt940/tests/test_import_bank_statement.py b/bank_statement_parse_nl_rabo_mt940/tests/test_import_bank_statement.py index 4c9337c0..bfddeeca 100644 --- a/bank_statement_parse_nl_rabo_mt940/tests/test_import_bank_statement.py +++ b/bank_statement_parse_nl_rabo_mt940/tests/test_import_bank_statement.py @@ -19,58 +19,28 @@ # along with this program. If not, see . # ############################################################################## -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 )