Merge lp:~camptocamp-reviewer/banking-addons/fix-type-and-account-in-bk-st-line into lp:banking-addons/bank-statement-reconcile-70

[Nicolas Bessi - Camptocamp]

Major refactoring of statement import and completion:

Fix the way to look default account on partner:
- If master account is provided in profile it will be forced
 - If the customer checkbox is checked on the found partner, type and account will be customer and receivable
 - If the supplier checkbox is checked on the found partner, type and account will be supplier and payable
 - If both checkbox are checked or none of them, it'll be based on the amount :
    If amount is positif the type and account will be customer and receivable,
    If amount is negativ, the type and account will be supplier and payable

Completion:
-Fix and refactor the the invoices lookup for completion
-Various fixes in completion rules
- Non matches lines are not mark as completed.

Optimisation:

Refactoring of statement import:
We by pass ORM to increase performances.
TODO support of sparse field

Refactoring of completion.
We have done some structural changes in order to avoid a lot of un needed call to ORM.
Bypass orm when writing to database.

These merge is required in order to fix transaction id completion rules and import +
Handle the new semantic change in OpenERP partner model

[Vincent Renaville@camptocamp (vrenaville-c2c)]

Hello,

just a little typo error at line 23, remove q letter before tag

review: Needs Fixing

[lp:~camptocamp-reviewer/banking-addons/fix-type-and-account-in-bk-st-line]

116. By Nicolas Bessi - Camptocamp on 2013-04-24

[FIX] transaction_id completion to support optimization

117. By Nicolas Bessi - Camptocamp on 2013-04-24

[FIX] transaction id import styling

118. By Nicolas Bessi - Camptocamp on 2013-04-24

[FIX] incoherence in type lookup behavior

[Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c)]

General: no space before "!" "?" ":" or ";" in english -> fix needed for the error messages

line 148-149: I find this message confusing, since we are not searching by partner. This message is repeated in several places in the code.

line 418: is there not a security rules bypass here ? Eg in a multicompany context with non shares res.partners, I can imagine the query returning several rows, yet only one partner being availalble to the current company.

line 424: the else False part will never execute : the function has returned on line 419.
line 425: this test is always true (the execution of line 424 sets res to an non empty dictionary, which is True)

line 487-492: there is still a little cleanup to be done here: the previous logic was handling several lines in a loop (hence the need for error_stack). The current logic deals with a single line, so error_stack can be removed and the exception raised in the except block. In the same function, I'd change the "return res" statements to "return {}" as res is never changed.

line 519: use log_line.insert(0, value) to insert a single element at the beginning of the line (I have not seen anything indicating the the RHS arg is multi element list).

line 598: leftover print statement

line 874: no batch update is performed. Docstring update required :-)

line 952: any reason for doing this in a loop rather than using UPDATE account_bank_statement_line SET sequence = account_bank_statement_line.id + 1 WHERE statement_id in %(list_of_ids)s" ?

review: Needs Fixing (code review, no test)

[Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount)]

Hello,

Many thanks for your review.

Line 148-149 the message is done this way because even if the lookup is done on invoice, it's the partner we try to complete. But that right we can add the name of the rule inside the message.

For the rest I agree with you I will do the fixes ASAP.

[lp:~camptocamp-reviewer/banking-addons/fix-type-and-account-in-bk-st-line]

119. By Nicolas Bessi - Camptocamp on 2013-04-25

[IMP] completion messages

120. By Nicolas Bessi - Camptocamp on 2013-04-25

[FIX] by partner name completion rule if statement and wrong variable

121. By Nicolas Bessi - Camptocamp on 2013-04-25

[FIX] dead if statement

122. By Nicolas Bessi - Camptocamp on 2013-04-25

[FIX] dead error management code

123. By Nicolas Bessi - Camptocamp on 2013-04-25

[IMP] return value

124. By Nicolas Bessi - Camptocamp on 2013-04-25

[IMP] log code cleanup

125. By Nicolas Bessi - Camptocamp on 2013-04-25

[FIX] comment and forgotten print statement

126. By Nicolas Bessi - Camptocamp on 2013-04-25

[IMP] no loop when computing sequence

127. By Nicolas Bessi - Camptocamp on 2013-04-25

[TYPO]

128. By Nicolas Bessi - Camptocamp on 2013-04-25

[FIX] partner name lookup now respect security rules

129. By Nicolas Bessi - Camptocamp on 2013-04-25

[TYPO]

130. By Nicolas Bessi - Camptocamp on 2013-04-25

[FIX] po file

131. By Nicolas Bessi - Camptocamp on 2013-04-25

[IMP] message logging

[Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount)]

Corrections applied.

Alexandre please feel free to do an english correction round. This way I can regenerate PO files.

Regards

Nicolas

[lp:~camptocamp-reviewer/banking-addons/fix-type-and-account-in-bk-st-line]

132. By Alexandre Fayolle - camptocamp on 2013-04-25

[IMP] fix a few docstrings

small code improvement in account_statement_transactionid_completion/statement.py AccountStatementCompletionRule.get_from_transaction_id_and_so)

[Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c)]

account_statement_base_completion/statement.py, AccountStatementProfil._find_values_from_rules

* the docstring does not match the arguments of the method (no st_line, and calls and line are not described)
* the "if not calls branch" will crash, it uses 'id' which is a Python built in function, not shadowed by a local variable here

account_statement_base_completion/statement.py, AccountStatementCompletionRule.get_from_label_and_partner_name:

* security rule bypass still there AFAICT

I've pushed a few changes and improvements, don't forget to pull before fixing the points above)

review: Needs Fixing (code review, no test)

[lp:~camptocamp-reviewer/banking-addons/fix-type-and-account-in-bk-st-line]

133. By Nicolas Bessi - Camptocamp on 2013-04-25

[FIX] wrong argument id instead of line['profile_id']

[Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount)]

Hello,

Many thanks for the attentive review. I have pushed the last fixes

Normally get_from_label_and_partner_name should not bypass security rules as the memoizer partner set is limited by the first search statement:
 partner_ids = partner_obj.search(cr,
                                  uid,
                                  [('bank_statement_label', '!=', False)])

[Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c)]

LGTM.

review: Approve (code review, no test)

=== modified file 'account_statement_base_completion/__openerp__.py'
--- account_statement_base_completion/__openerp__.py	2013-01-10 15:09:55 +0000
+++ account_statement_base_completion/__openerp__.py	2013-04-25 11:52:27 +0000
@@ -52,6 +52,10 @@
 
  You can use it with  our account_advanced_reconcile module to automatize the reconciliation process.
 
+
+ TODO: The rules that look for invoices to find out the partner should take back the payable / receivable
+       account from there directly instead of retrieving it from partner properties !
+
  """,
  'website': 'http://www.camptocamp.com',
  'init_xml': [],
 
=== modified file 'account_statement_base_completion/statement.py'
--- account_statement_base_completion/statement.py	2013-03-22 08:31:27 +0000
+++ account_statement_base_completion/statement.py	2013-04-25 11:52:27 +0000
@@ -18,15 +18,21 @@
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 ##############################################################################
+# TODO replace customer supplier by package constant
+import traceback
+import sys
+import logging
+
 from collections import defaultdict
 import re
-
 from tools.translate import _
-from openerp.osv.orm import Model, fields
+from openerp.osv import osv, orm, fields
 from openerp.tools import DEFAULT_SERVER_DATETIME_FORMAT
 from operator import attrgetter
 import datetime
 
+_logger = logging.getLogger(__name__)
+
 
 class ErrorTooManyPartner(Exception):
     """
@@ -40,7 +46,7 @@
         return repr(self.value)
 
 
-class AccountStatementProfil(Model):
+class AccountStatementProfil(orm.Model):
     """
     Extend the class to add rules per profile that will match at least the partner,
     but it could also be used to match other values as well.
@@ -49,11 +55,11 @@
     _inherit = "account.statement.profile"
 
     _columns = {
-        # @Akretion : For now, we don't implement this features, but this would probably be there:
+        # @Akretion: For now, we don't implement this features, but this would probably be there:
         # 'auto_completion': fields.text('Auto Completion'),
         # 'transferts_account_id':fields.many2one('account.account', 'Transferts Account'),
         # => You can implement it in a module easily, we design it with your needs in mind
-        # as well !
+        # as well!
 
         'rule_ids': fields.many2many(
             'account.statement.completion.rule',
@@ -61,36 +67,45 @@
             rel='as_rul_st_prof_rel'),
     }
 
-    def find_values_from_rules(self, cr, uid, id, line_id, context=None):
+    def _get_callable(self, cr, uid, profile, context=None):
+        if isinstance(profile, (int, long)):
+            prof = self.browse(cr, uid, profile, context=context)
+        else:
+            prof = profile
+        # We need to respect the sequence order
+        sorted_array = sorted(prof.rule_ids, key=attrgetter('sequence'))
+        return tuple((x.function_to_call for x in sorted_array))
+
+    def _find_values_from_rules(self, cr, uid, calls, line, context=None):
         """
         This method will execute all related rules, in their sequence order,
         to retrieve all the values returned by the first rules that will match.
-
-        :param int/long line_id: id of the concerned account.bank.statement.line
+        :param calls: list of lookup function name available in rules
+        :param dict line: read of the concerned account.bank.statement.line
         :return:
             A dict of value that can be passed directly to the write method of
             the statement line or {}
            {'partner_id': value,
-            'account_id' : value,
+            'account_id: value,
 
             ...}
         """
         if context is None:
             context = {}
-        res = {}
+        if not calls:
+            calls = self._get_callable(cr, uid, line['profile_id'], context=context)
         rule_obj = self.pool.get('account.statement.completion.rule')
-        profile = self.browse(cr, uid, id, context=context)
-        # We need to respect the sequence order
-        sorted_array = sorted(profile.rule_ids, key=attrgetter('sequence'))
-        for rule in sorted_array:
-            method_to_call = getattr(rule_obj, rule.function_to_call)
-            result = method_to_call(cr, uid, line_id, context)
+
+        for call in calls:
+            method_to_call = getattr(rule_obj, call)
+            result = method_to_call(cr, uid, line, context)
             if result:
+                result['already_completed'] = True
                 return result
-        return res
-
-
-class AccountStatementCompletionRule(Model):
+        return None
+
+
+class AccountStatementCompletionRule(orm.Model):
     """
     This will represent all the completion method that we can have to
     fullfill the bank statement lines. You'll be able to extend them in you own module
@@ -109,12 +124,11 @@
         List of available methods for rules. Override this to add you own.
         """
         return [
-            ('get_from_ref_and_invoice', 'From line reference (based on invoice number)'),
+            ('get_from_ref_and_invoice', 'From line reference (based on customer invoice number)'),
             ('get_from_ref_and_supplier_invoice', 'From line reference (based on supplier invoice number)'),
             ('get_from_ref_and_so', 'From line reference (based on SO number)'),
             ('get_from_label_and_partner_field', 'From line label (based on partner field)'),
-            ('get_from_label_and_partner_name', 'From line label (based on partner name)'),
-            ]
+            ('get_from_label_and_partner_name', 'From line label (based on partner name)')]
 
     _columns = {
         'sequence': fields.integer('Sequence', help="Lower means parsed first."),
@@ -126,134 +140,129 @@
         'function_to_call': fields.selection(_get_functions, 'Method'),
     }
 
-    def get_from_ref_and_supplier_invoice(self, cr, uid, line_id, context=None):
+    def _find_invoice(self, cr, uid, st_line, inv_type, context=None):
+        """Find invoice related to statement line"""
+        inv_obj = self.pool.get('account.invoice')
+        if inv_type == 'supplier':
+            type_domain = ('in_invoice', 'in_refund')
+            number_field = 'supplier_invoice_number'
+        elif inv_type == 'customer':
+            type_domain = ('out_invoice', 'out_refund')
+            number_field = 'number'
+        else:
+            raise osv.except_osv(_('System error'),
+                                 _('Invalid invoice type for completion: %') % inv_type)
+
+        inv_id = inv_obj.search(cr, uid,
+                                [(number_field, '=', st_line['ref'].strip()),
+                                 ('type', 'in', type_domain)],
+                                context=context)
+        if inv_id:
+            if len(inv_id) == 1:
+                inv = inv_obj.browse(cr, uid, inv_id[0], context=context)
+            else:
+                raise ErrorTooManyPartner(_('Line named "%s" (Ref:%s) was matched by more '
+                                            'than one partner while looking on %s invoices') %
+                                          (st_line['name'], st_line['ref'], inv_type))
+            return inv
+        return False
+
+    def _from_invoice(self, cr, uid, line, inv_type, context):
+        """Populate statement line values"""
+        if not inv_type in ('supplier', 'customer'):
+            raise osv.except_osv(_('System error'),
+                                 _('Invalid invoice type for completion: %') % inv_type)
+        res = {}
+        inv = self._find_invoice(cr, uid, line, inv_type, context=context)
+        if inv:
+            res = {'partner_id': inv.partner_id.id,
+                   'account_id': inv.account_id.id,
+                   'type': inv_type}
+            override_acc = line['master_account_id']
+            if override_acc:
+                res['account_id'] = override_acc
+        return res
+
+    # Should be private but data are initialised with no update XML
+    def get_from_ref_and_supplier_invoice(self, cr, uid, line, context=None):
         """
         Match the partner based on the invoice supplier invoice number and the reference of the statement
         line. Then, call the generic get_values_for_line method to complete other values.
         If more than one partner matched, raise the ErrorTooManyPartner error.
 
-        :param int/long line_id: id of the concerned account.bank.statement.line
+        :param dict line: read of the concerned account.bank.statement.line
         :return:
             A dict of value that can be passed directly to the write method of
             the statement line or {}
            {'partner_id': value,
-            'account_id' : value,
+            'account_id': value,
 
             ...}
         """
-        st_obj = self.pool['account.bank.statement.line']
-        st_line = st_obj.browse(cr, uid, line_id, context=context)
-        res = {}
-        inv_obj = self.pool.get('account.invoice')
-        if st_line:
-            inv_id = inv_obj.search(cr,
-                                    uid,
-                                    [('supplier_invoice_number', '=', st_line.ref),
-                                     ('type', 'in', ('in_invoice', 'in_refund'))],
-                                    context=context)
-            if inv_id:
-                if len(inv_id) == 1:
-                    inv = inv_obj.browse(cr, uid, inv_id[0], context=context)
-                    res['partner_id'] = inv.partner_id.id
-                else:
-                    raise ErrorTooManyPartner(_('Line named "%s" (Ref:%s) was matched by more '
-                                                'than one partner.') % (st_line.name, st_line.ref))
-                st_vals = st_obj.get_values_for_line(cr,
-                                                     uid,
-                                                     profile_id=st_line.statement_id.profile_id.id,
-                                                     partner_id=res.get('partner_id', False),
-                                                     line_type="supplier",
-                                                     amount=st_line.amount,
-                                                     context=context)
-                res.update(st_vals)
-        return res
+        return self._from_invoice(cr, uid, line, 'supplier', context=context)
 
-    def get_from_ref_and_invoice(self, cr, uid, line_id, context=None):
+    # Should be private but data are initialised with no update XML
+    def get_from_ref_and_invoice(self, cr, uid, line, context=None):
         """
         Match the partner based on the invoice number and the reference of the statement
         line. Then, call the generic get_values_for_line method to complete other values.
         If more than one partner matched, raise the ErrorTooManyPartner error.
 
-        :param int/long line_id: id of the concerned account.bank.statement.line
+        :param dict line: read of the concerned account.bank.statement.line
         :return:
             A dict of value that can be passed directly to the write method of
             the statement line or {}
            {'partner_id': value,
-            'account_id' : value,
+            'account_id': value,
             ...}
         """
-        st_obj = self.pool.get('account.bank.statement.line')
-        st_line = st_obj.browse(cr, uid, line_id, context=context)
-        res = {}
-        if st_line:
-            inv_obj = self.pool.get('account.invoice')
-            inv_id = inv_obj.search(cr,
-                                    uid,
-                                    [('number', '=', st_line.ref)],
-                                    context=context)
-            if inv_id:
-                if len(inv_id) == 1:
-                    inv = inv_obj.browse(cr, uid, inv_id[0], context=context)
-                    res['partner_id'] = inv.partner_id.id
-                else:
-                    raise ErrorTooManyPartner(_('Line named "%s" (Ref:%s) was matched by more '
-                                                'than one partner.') % (st_line.name, st_line.ref))
-                st_vals = st_obj.get_values_for_line(cr,
-                                                     uid,
-                                                     profile_id=st_line.statement_id.profile_id.id,
-                                                     partner_id=res.get('partner_id', False),
-                                                     line_type=st_line.type,
-                                                     amount=st_line.amount,
-                                                     context=context)
-                res.update(st_vals)
-        return res
+        return self._from_invoice(cr, uid, line, 'customer', context=context)
 
-    def get_from_ref_and_so(self, cr, uid, line_id, context=None):
+    # Should be private but data are initialised with no update XML
+    def get_from_ref_and_so(self, cr, uid, st_line, context=None):
         """
         Match the partner based on the SO number and the reference of the statement
         line. Then, call the generic get_values_for_line method to complete other values.
         If more than one partner matched, raise the ErrorTooManyPartner error.
 
-        :param int/long line_id: id of the concerned account.bank.statement.line
+        :param int/long st_line: read of the concerned account.bank.statement.line
         :return:
             A dict of value that can be passed directly to the write method of
             the statement line or {}
            {'partner_id': value,
-            'account_id' : value,
+            'account_id': value,
 
             ...}
         """
         st_obj = self.pool.get('account.bank.statement.line')
-        st_line = st_obj.browse(cr, uid, line_id, context=context)
         res = {}
         if st_line:
             so_obj = self.pool.get('sale.order')
-            so_id = so_obj.search(
-                    cr,
-                    uid,
-                    [('name', '=', st_line.ref)],
-                    context=context)
+            so_id = so_obj.search(cr,
+                                  uid,
+                                  [('name', '=', st_line['ref'])],
+                                  context=context)
             if so_id:
                 if so_id and len(so_id) == 1:
                     so = so_obj.browse(cr, uid, so_id[0], context=context)
                     res['partner_id'] = so.partner_id.id
                 elif so_id and len(so_id) > 1:
-                    raise ErrorTooManyPartner(
-                            _('Line named "%s" (Ref:%s) was matched by more '
-                              'than one partner.') %
-                            (st_line.name, st_line.ref))
-                st_vals = st_obj.get_values_for_line(
-                        cr,
-                        uid,
-                        profile_id=st_line.statement_id.profile_id.id,
-                        partner_id=res.get('partner_id', False),
-                        line_type=st_line.type,
-                        amount=st_line.amount,
-                        context=context)
+                    raise ErrorTooManyPartner(_('Line named "%s" (Ref:%s) was matched by more '
+                                                'than one partner while looking on SO by ref.') %
+                                              (st_line['name'], st_line['ref']))
+                st_vals = st_obj.get_values_for_line(cr,
+                                                     uid,
+                                                     profile_id=st_line['profile_id'],
+                                                     master_account_id=st_line['master_account_id'],
+                                                     partner_id=res.get('partner_id', False),
+                                                     line_type='customer',
+                                                     amount=st_line['amount'] if st_line['amount'] else 0.0,
+                                                     context=context)
                 res.update(st_vals)
         return res
 
-    def get_from_label_and_partner_field(self, cr, uid, line_id, context=None):
+    # Should be private but data are initialised with no update XML
+    def get_from_label_and_partner_field(self, cr, uid, st_line, context=None):
         """
         Match the partner based on the label field of the statement line
         and the text defined in the 'bank_statement_label' field of the partner.
@@ -261,12 +270,12 @@
         get_values_for_line method to complete other values.
         If more than one partner matched, raise the ErrorTooManyPartner error.
 
-        :param int/long line_id: id of the concerned account.bank.statement.line
+        :param dict st_line: read of the concerned account.bank.statement.line
         :return:
             A dict of value that can be passed directly to the write method of
             the statement line or {}
            {'partner_id': value,
-            'account_id' : value,
+            'account_id': value,
 
             ...}
             """
@@ -283,7 +292,7 @@
             partner_ids = partner_obj.search(cr,
                                              uid,
                                              [('bank_statement_label', '!=', False)])
-            line_ids = tuple(x.id for x in context.get('line_ids', []))
+            line_ids = context.get('line_ids', [])
             for partner in partner_obj.browse(cr, uid, partner_ids, context=context):
                 vals = '|'.join(re.escape(x.strip()) for x in partner.bank_statement_label.split(';'))
                 or_regex = ".*%s*." % vals
@@ -294,70 +303,71 @@
                 pairs = cr.fetchall()
                 for pair in pairs:
                     context['label_memoizer'][pair[0]].append(partner)
-        st_line = st_obj.browse(cr, uid, line_id, context=context)
-        if st_line and st_line.id in context['label_memoizer']:
-            found_partner = context['label_memoizer'][st_line.id]
+        if st_line['id'] in context['label_memoizer']:
+            found_partner = context['label_memoizer'][st_line['id']]
             if len(found_partner) > 1:
                 raise ErrorTooManyPartner(_('Line named "%s" (Ref:%s) was matched by '
-                                            'more than one partner.') %
-                                          (st_line.name, st_line.ref))
+                                            'more than one partner while looking on partner label') %
+                                          (st_line['name'], st_line['ref']))
             res['partner_id'] = found_partner[0].id
             st_vals = st_obj.get_values_for_line(cr,
                                                  uid,
-                                                 profile_id=st_line.statement_id.profile_id.id,
+                                                 profile_id=st_line['profile_id'],
+                                                 master_account_id=st_line['master_account_id'],
                                                  partner_id=found_partner[0].id,
-                                                 line_type=st_line.type,
-                                                 amount=st_line.amount,
+                                                 line_type=False,
+                                                 amount=st_line['amount'] if st_line['amount'] else 0.0,
                                                  context=context)
             res.update(st_vals)
         return res
 
-    def get_from_label_and_partner_name(self, cr, uid, line_id, context=None):
+    def get_from_label_and_partner_name(self, cr, uid, st_line, context=None):
         """
         Match the partner based on the label field of the statement line
         and the name of the partner.
         Then, call the generic get_values_for_line method to complete other values.
         If more than one partner matched, raise the ErrorTooManyPartner error.
 
-        :param int/long line_id: id of the concerned account.bank.statement.line
+        :param dict st_line: read of the concerned account.bank.statement.line
         :return:
             A dict of value that can be passed directly to the write method of
             the statement line or {}
            {'partner_id': value,
-            'account_id' : value,
+            'account_id': value,
 
             ...}
             """
-        # This Method has not been tested yet !
         res = {}
+        # We memoize allowed partner
+        if not context.get('partner_memoizer'):
+            context['partner_memoizer'] = tuple(self.pool['res.partner'].search(cr, uid, []))
+        if not context['partner_memoizer']:
+            return res
         st_obj = self.pool.get('account.bank.statement.line')
-        st_line = st_obj.browse(cr, uid, line_id, context=context)
-        if st_line:
-            sql = "SELECT id FROM res_partner WHERE name ~* %s"
-            pattern = ".*%s.*" % re.escape(st_line.label)
-            cr.execute(sql, (pattern,))
-            result = cr.fetchall()
-            if not result:
-                return res
-            if len(result) > 1:
-                raise ErrorTooManyPartner(_('Line named "%s" (Ref:%s) was matched by more '
-                                            'than one partner.') %
-                                          (st_line.name, st_line.ref))
-            res['partner_id'] = result[0][0] if result else False
-            if res:
-                st_vals = st_obj.get_values_for_line(
-                        cr,
-                        uid,
-                        profile_id=st_line.statement_id.profile_id.id,
-                        partner_id=res['partner_id'],
-                        line_type=st_line.type,
-                        amount=st_line.amount,
-                        context=context)
-                res.update(st_vals)
+        sql = "SELECT id FROM res_partner WHERE name ~* %s and id in %s"
+        pattern = ".*%s.*" % re.escape(st_line['name'])
+        cr.execute(sql, (pattern, context['partner_memoizer']))
+        result = cr.fetchall()
+        if not result:
+            return res
+        if len(result) > 1:
+            raise ErrorTooManyPartner(_('Line named "%s" (Ref:%s) was matched by more '
+                                        'than one partner while looking on partner by name') %
+                                      (st_line['name'], st_line['ref']))
+        res['partner_id'] = result[0][0]
+        st_vals = st_obj.get_values_for_line(cr,
+                                             uid,
+                                             profile_id=st_line['porfile_id'],
+                                             master_account_id=st_line['master_account_id'],
+                                             partner_id=res['partner_id'],
+                                             line_type=False,
+                                             amount=st_line['amount'] if st_line['amount'] else 0.0,
+                                             context=context)
+        res.update(st_vals)
         return res
 
 
-class AccountStatementLine(Model):
+class AccountStatementLine(orm.Model):
     """
     Add sparse field on the statement line to allow to store all the
     bank infos that are given by a bank/office. You can then add you own in your
@@ -390,7 +400,7 @@
         'already_completed': False,
     }
 
-    def get_line_values_from_rules(self, cr, uid, ids, context=None):
+    def _get_line_values_from_rules(self, cr, uid, line, rules, context=None):
         """
         We'll try to find out the values related to the line based on rules setted on
         the profile.. We will ignore line for which already_completed is ticked.
@@ -401,36 +411,17 @@
             {117009: {'partner_id': 100997, 'account_id': 489L}}
         """
         profile_obj = self.pool.get('account.statement.profile')
-        st_obj = self.pool.get('account.bank.statement.line')
-        res = {}
-        errors_stack = []
-        for line in self.browse(cr, uid, ids, context=context):
-            if line.already_completed:
-                continue
-            try:
-                # Take the default values
-                res[line.id] = st_obj.get_values_for_line(
-                        cr,
-                        uid,
-                        profile_id=line.statement_id.profile_id.id,
-                        line_type=line.type,
-                        amount=line.amount,
-                        context=context)
-                # Ask the rule
-                vals = profile_obj.find_values_from_rules(
-                        cr, uid, line.statement_id.profile_id.id, line.id, context)
-                # Merge the result
-                res[line.id].update(vals)
-            except ErrorTooManyPartner, exc:
-                msg = "Line ID %s had following error: %s" % (line.id, exc.value)
-                errors_stack.append(msg)
-        if errors_stack:
-            msg = u"\n".join(errors_stack)
-            raise ErrorTooManyPartner(msg)
-        return res
-
-
-class AccountBankSatement(Model):
+        if line.get('already_completed'):
+            return {}
+        # Ask the rule
+        vals = profile_obj._find_values_from_rules(cr, uid, rules, line, context)
+        if vals:
+            vals['id'] = line['id']
+            return vals
+        return {}
+
+
+class AccountBankSatement(orm.Model):
     """
     We add a basic button and stuff to support the auto-completion
     of the bank statement once line have been imported or manually fullfill.
@@ -450,59 +441,82 @@
         :param int/long stat_id: ID of the account.bank.statement
         :param char error_msg: Message to add
         :number_imported int/long: Number of lines that have been completed
-        :return : True
+        :return True
         """
-        error_log = ""
-        user_name = self.pool.get('res.users').read(
-                cr, uid, uid, ['name'], context=context)['name']
-        log = self.read(
-                cr, uid, stat_id, ['completion_logs'], context=context)['completion_logs']
-        log_line = log and log.split("\n") or []
+        user_name = self.pool.get('res.users').read(cr, uid, uid,
+                                                    ['name'], context=context)['name']
+
+        log = self.read(cr, uid, stat_id, ['completion_logs'],
+                        context=context)['completion_logs']
+        log = log if log else ""
+
         completion_date = datetime.datetime.now().strftime(DEFAULT_SERVER_DATETIME_FORMAT)
-        if error_msg:
-            error_log = error_msg
-        log_line[0:0] = [completion_date + ' : '
-            + _("Bank Statement ID %s has %s lines completed by %s") % (stat_id, number_imported, user_name)
-            + "\n" + error_log + "-------------" + "\n"]
-        log = "\n".join(log_line)
-        self.write(cr, uid, [stat_id], {'completion_logs': log}, context=context)
-        self.message_post(
-                cr, uid,
-                [stat_id],
-                body=_('Statement ID %s auto-completed for %s lines completed') % (stat_id, number_imported),
-                context=context)
+        message = (_("%s Bank Statement ID %s has %s lines completed by %s \n%s\n") %
+                   (completion_date, stat_id, number_imported, user_name, log))
+        self.write(cr, uid, [stat_id], {'completion_logs': message}, context=context)
+
+        body = (_('Statement ID %s auto-completed for %s lines completed') %
+                (stat_id, number_imported)),
+        self.message_post(cr, uid,
+                          [stat_id],
+                          body=body,
+                          context=context)
         return True
 
     def button_auto_completion(self, cr, uid, ids, context=None):
         """
         Complete line with values given by rules and tic the already_completed
-        checkbox so we won't compute them again unless the user untick them !
+        checkbox so we won't compute them again unless the user untick them!
         """
         if context is None:
             context = {}
-        stat_line_obj = self.pool.get('account.bank.statement.line')
+        stat_line_obj = self.pool['account.bank.statement.line']
+        profile_obj = self.pool.get('account.statement.profile')
         compl_lines = 0
+        stat_line_obj.check_access_rule(cr, uid, [], 'create')
+        stat_line_obj.check_access_rights(cr, uid, 'create', raise_exception=True)
         for stat in self.browse(cr, uid, ids, context=context):
             msg_lines = []
             ctx = context.copy()
-            ctx['line_ids'] = stat.line_ids
-            for line in stat.line_ids:
-                res = {}
+            ctx['line_ids'] = tuple((x.id for x in stat.line_ids))
+            b_profile = stat.profile_id
+            rules = profile_obj._get_callable(cr, uid, b_profile, context=context)
+            profile_id = b_profile.id  # Only for perfo even it gains almost nothing
+            master_account_id = b_profile.receivable_account_id
+            master_account_id = master_account_id.id if master_account_id else False
+            res = False
+            for line in stat_line_obj.read(cr, uid, ctx['line_ids']):
                 try:
-                    res = stat_line_obj.get_line_values_from_rules(
-                            cr, uid, [line.id], context=ctx)
+                    # performance trick
+                    line['master_account_id'] = master_account_id
+                    line['profile_id'] = profile_id
+                    res = stat_line_obj._get_line_values_from_rules(cr, uid, line,
+                                                                    rules, context=ctx)
                     if res:
                         compl_lines += 1
                 except ErrorTooManyPartner, exc:
                     msg_lines.append(repr(exc))
                 except Exception, exc:
                     msg_lines.append(repr(exc))
-                # vals = res and res.keys() or False
+                    error_type, error_value, trbk = sys.exc_info()
+                    st = "Error: %s\nDescription: %s\nTraceback:" % (error_type.__name__, error_value)
+                    st += ''.join(traceback.format_tb(trbk, 30))
+                    _logger.error(st)
                 if res:
-                    vals = res[line.id]
-                    vals['already_completed'] = True
-                    stat_line_obj.write(cr, uid, [line.id], vals, context=ctx)
+                    #stat_line_obj.write(cr, uid, [line.id], vals, context=ctx)
+                    try:
+                        stat_line_obj._update_line(cr, uid, res, context=context)
+                    except Exception as exc:
+                        msg_lines.append(repr(exc))
+                        error_type, error_value, trbk = sys.exc_info()
+                        st = "Error: %s\nDescription: %s\nTraceback:" % (error_type.__name__, error_value)
+                        st += ''.join(traceback.format_tb(trbk, 30))
+                        _logger.error(st)
+                    # we can commit as it is not needed to be atomic
+                    # commiting here adds a nice perfo boost
+                    if not compl_lines % 500:
+                        cr.commit()
             msg = u'\n'.join(msg_lines)
             self.write_completion_log(cr, uid, stat.id,
-                                  msg, compl_lines, context=context)
+                                      msg, compl_lines, context=context)
         return True
 
=== modified file 'account_statement_base_import/parser/generic_file_parser.py'
--- account_statement_base_import/parser/generic_file_parser.py	2013-02-26 08:22:25 +0000
+++ account_statement_base_import/parser/generic_file_parser.py	2013-04-25 11:52:27 +0000
@@ -29,6 +29,7 @@
 except:
     raise Exception(_('Please install python lib xlrd'))
 
+
 def float_or_zero(val):
     """ Conversion function used to manage
     empty string into float usecase"""
@@ -82,14 +83,12 @@
         In this generic parser, the commission is given for every line, so we store it
         for each one.
         """
-        return {
-            'name': line.get('label', line.get('ref', '/')),
-            'date': line.get('date', datetime.datetime.now().date()),
-            'amount': line.get('amount', 0.0),
-            'ref': line.get('ref', '/'),
-            'label': line.get('label', ''),
-            'commission_amount': line.get('commission_amount', 0.0),
-        }
+        return {'name': line.get('label', line.get('ref', '/')),
+                'date': line.get('date', datetime.datetime.now().date()),
+                'amount': line.get('amount', 0.0),
+                'ref': line.get('ref', '/'),
+                'label': line.get('label', ''),
+                'commission_amount': line.get('commission_amount', 0.0)}
 
     def _post(self, *args, **kwargs):
         """
 
=== modified file 'account_statement_base_import/statement.py'
--- account_statement_base_import/statement.py	2013-02-26 08:45:03 +0000
+++ account_statement_base_import/statement.py	2013-04-25 11:52:27 +0000
@@ -18,14 +18,16 @@
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 ##############################################################################
+import sys
+import traceback
+
+import psycopg2
 
 from openerp.tools.translate import _
 import datetime
 from openerp.osv.orm import Model
 from openerp.osv import fields, osv
 from parser import new_bank_statement_parser
-import sys
-import traceback
 
 
 class AccountStatementProfil(Model):
@@ -43,7 +45,8 @@
             help="Tic that box to automatically launch the completion "
                  "on each imported file using this profile."),
         'last_import_date': fields.datetime("Last Import Date"),
-        'rec_log': fields.text('log', readonly=True, deprecated=True),
+        #  we remove deprecated as it floods logs in standard/warning level sob...
+        'rec_log': fields.text('log', readonly=True),  # Deprecated
         'import_type': fields.selection(
             get_import_type_selection,
             'Type of import',
@@ -64,7 +67,8 @@
         self.message_post(cr,
                           uid,
                           ids,
-                          body=_('Statement ID %s have been imported with %s lines.') % (statement_id, num_lines),
+                          body=_('Statement ID %s have been imported with %s lines.') %
+                                (statement_id, num_lines),
                           context=context)
         return True
 
@@ -79,7 +83,7 @@
             :param:    browse_record of the current parser
             :param:    result_row_list: [{'key':value}]
             :param:    profile: browserecord of account.statement.profile
-            :param:    statement_id : int/long of the current importing statement ID
+            :param:    statement_id: int/long of the current importing statement ID
             :param:    context: global context
             return:    dict of vals that will be passed to create method of statement line.
         """
@@ -98,7 +102,7 @@
                 'account_id': commission_account_id,
                 'ref': 'commission',
                 'analytic_account_id': commission_analytic_id,
-                # !! We set the already_completed so auto-completion will not update those values !
+                # !! We set the already_completed so auto-completion will not update those values!
                 'already_completed': True,
             }
         return comm_values
@@ -116,18 +120,32 @@
         :param int/long account_payable: ID of the receivable account to use
         :param int/long account_receivable: ID of the payable account to use
         :param int/long statement_id: ID of the concerned account.bank.statement
-        :return : dict of vals that will be passed to create method of statement line.
+        :return: dict of vals that will be passed to create method of statement line.
         """
         statement_obj = self.pool.get('account.bank.statement')
         values = parser_vals
         values['statement_id'] = statement_id
-        values['account_id'] = statement_obj.get_account_for_counterpart(
-                cr,
-                uid,
-                parser_vals['amount'],
-                account_receivable,
-                account_payable
-        )
+        values['account_id'] = statement_obj.get_account_for_counterpart(cr,
+                                                                         uid,
+                                                                         parser_vals['amount'],
+                                                                         account_receivable,
+                                                                         account_payable)
+
+        date = values.get('date')
+        period_memoizer = context.get('period_memoizer')
+        if not period_memoizer:
+            period_memoizer = {}
+            context['period_memoizer'] = period_memoizer
+        if period_memoizer.get(date):
+            values['period_id'] = period_memoizer[date]
+        else:
+            # This is awfully slow...
+            periods = self.pool.get('account.period').find(cr, uid,
+                                                           dt=values.get('date'),
+                                                           context=context)
+            values['period_id'] = periods[0]
+            period_memoizer[date] = periods[0]
+        values['type'] = 'general'
         return values
 
     def statement_import(self, cr, uid, ids, profile_id, file_stream, ftype="csv", context=None):
@@ -148,75 +166,85 @@
         attachment_obj = self.pool.get('ir.attachment')
         prof_obj = self.pool.get("account.statement.profile")
         if not profile_id:
-            raise osv.except_osv(
-                    _("No Profile !"),
-                    _("You must provide a valid profile to import a bank statement !"))
+            raise osv.except_osv(_("No Profile!"),
+                                 _("You must provide a valid profile to import a bank statement!"))
         prof = prof_obj.browse(cr, uid, profile_id, context=context)
 
         parser = new_bank_statement_parser(prof.import_type, ftype=ftype)
         result_row_list = parser.parse(file_stream)
-        # Check all key are present in account.bank.statement.line !!
+        # Check all key are present in account.bank.statement.line!!
+        if not result_row_list:
+            raise osv.except_osv(_("Nothing to import"),
+                                 _("The file is empty"))
         parsed_cols = parser.get_st_line_vals(result_row_list[0]).keys()
         for col in parsed_cols:
             if col not in statement_line_obj._columns:
-                raise osv.except_osv(
-                        _("Missing column !"),
-                        _("Column %s you try to import is not "
-                          "present in the bank statement line !") % col)
+                raise osv.except_osv(_("Missing column!"),
+                                     _("Column %s you try to import is not "
+                                       "present in the bank statement line!") % col)
 
-        statement_id = statement_obj.create(
-                cr, uid, {'profile_id': prof.id}, context=context)
-        account_receivable, account_payable = statement_obj.get_default_pay_receiv_accounts(
-                cr, uid, context)
+        statement_id = statement_obj.create(cr, uid,
+                                            {'profile_id': prof.id},
+                                            context=context)
+        if prof.receivable_account_id:
+            account_receivable = account_payable = prof.receivable_account_id.id
+        else:
+            account_receivable, account_payable = statement_obj.get_default_pay_receiv_accounts(
+                                                       cr, uid, context)
         try:
             # Record every line in the bank statement and compute the global commission
             # based on the commission_amount column
             statement_store = []
             for line in result_row_list:
                 parser_vals = parser.get_st_line_vals(line)
-                values = self.prepare_statetement_lines_vals(
-                        cr, uid, parser_vals, account_payable,
-                        account_receivable, statement_id, context)
-                # we finally create the line in system
-                statement_store.append((0, 0, values))
+                values = self.prepare_statetement_lines_vals(cr, uid, parser_vals, account_payable,
+                                                             account_receivable, statement_id, context)
+                statement_store.append(values)
+            # Hack to bypass ORM poor perfomance. Sob...
+            statement_line_obj._insert_lines(cr, uid, statement_store, context=context)
+
             # Build and create the global commission line for the whole statement
-            statement_obj.write(cr, uid, [statement_id],
-                                {'line_ids': statement_store}, context=context)
             comm_vals = self.prepare_global_commission_line_vals(cr, uid, parser, result_row_list,
                                                                  prof, statement_id, context)
             if comm_vals:
                 statement_line_obj.create(cr, uid, comm_vals, context=context)
-
-            attachment_obj.create(
-                    cr,
-                    uid,
-                    {
-                        'name': 'statement file',
-                        'datas': file_stream,
-                        'datas_fname': "%s.%s" % (
-                            datetime.datetime.now().date(),
-                            ftype),
-                        'res_model': 'account.bank.statement',
-                        'res_id': statement_id,
-                    },
-                    context=context
-                )
-            # If user ask to launch completion at end of import, do it !
+            else:
+                # Trigger store field computation if someone has better idea
+                start_bal = statement_obj.read(cr, uid, statement_id,
+                                               ['balance_start'],
+                                               context=context)
+                start_bal = start_bal['balance_start']
+                statement_obj.write(cr, uid, [statement_id],
+                                    {'balance_start': start_bal})
+
+            attachment_obj.create(cr,
+                                  uid,
+                                  {'name': 'statement file',
+                                   'datas': file_stream,
+                                   'datas_fname': "%s.%s" % (
+                                       datetime.datetime.now().date(),
+                                       ftype),
+                                   'res_model': 'account.bank.statement',
+                                   'res_id': statement_id},
+                                  context=context)
+
+            # If user ask to launch completion at end of import, do it!
             if prof.launch_import_completion:
                 statement_obj.button_auto_completion(cr, uid, [statement_id], context)
 
             # Write the needed log infos on profile
-            self.write_logs_after_import(
-                    cr, uid, prof.id, statement_id, len(result_row_list), context)
+            self.write_logs_after_import(cr, uid, prof.id,
+                                         statement_id,
+                                         len(result_row_list),
+                                         context)
 
         except Exception:
             statement_obj.unlink(cr, uid, [statement_id], context=context)
             error_type, error_value, trbk = sys.exc_info()
             st = "Error: %s\nDescription: %s\nTraceback:" % (error_type.__name__, error_value)
             st += ''.join(traceback.format_tb(trbk, 30))
-            raise osv.except_osv(
-                    _("Statement import error"),
-                    _("The statement cannot be created : %s") % st)
+            raise osv.except_osv(_("Statement import error"),
+                                 _("The statement cannot be created: %s") % st)
         return statement_id
 
 
@@ -228,6 +256,45 @@
     """
     _inherit = "account.bank.statement.line"
 
+    def _get_available_columns(self, statement_store):
+        """Return writeable by SQL columns"""
+        statement_line_obj = self.pool['account.bank.statement.line']
+        model_cols = statement_line_obj._columns
+        avail = [k for k, col in model_cols.iteritems() if not hasattr(col, '_fnct')]
+        keys = [k for k in statement_store[0].keys() if k in avail]
+        keys.sort()
+        return keys
+
+    def _insert_lines(self, cr, uid, statement_store, context=None):
+        """ Do raw insert into database because ORM is awfully slow
+            when doing batch write. It is a shame that batch function
+            does not exist"""
+        statement_line_obj = self.pool['account.bank.statement.line']
+        statement_line_obj.check_access_rule(cr, uid, [], 'create')
+        statement_line_obj.check_access_rights(cr, uid, 'create', raise_exception=True)
+        cols = self._get_available_columns(statement_store)
+        tmp_vals = (', '.join(cols), ', '.join(['%%(%s)s' % i for i in cols]))
+        sql = "INSERT INTO account_bank_statement_line (%s) VALUES (%s);" % tmp_vals
+        try:
+            cr.executemany(sql, tuple(statement_store))
+        except psycopg2.Error as sql_err:
+            cr.rollback()
+            raise osv.except_osv(_("ORM bypass error"),
+                                 sql_err.pgerror)
+
+    def _update_line(self, cr, uid, vals, context=None):
+        """ Do raw update into database because ORM is awfully slow
+            when cheking security."""
+        cols = self._get_available_columns([vals])
+        tmp_vals = (', '.join(['%s = %%(%s)s' % (i, i) for i in cols]))
+        sql = "UPDATE account_bank_statement_line SET %s where id = %%(id)s;" % tmp_vals
+        try:
+            cr.execute(sql, vals)
+        except psycopg2.Error as sql_err:
+            cr.rollback()
+            raise osv.except_osv(_("ORM bypass error"),
+                                 sql_err.pgerror)
+
     _columns = {
         'commission_amount': fields.sparse(
             type='float',
 
=== modified file 'account_statement_ext/__openerp__.py'
--- account_statement_ext/__openerp__.py	2013-02-14 08:13:49 +0000
+++ account_statement_ext/__openerp__.py	2013-04-25 11:52:27 +0000
@@ -25,11 +25,9 @@
  'maintainer': 'Camptocamp',
  'category': 'Finance',
  'complexity': 'normal',
- 'depends': [
-     'account',
-     'report_webkit',
-     'account_voucher'
-     ],
+ 'depends': ['account',
+             'report_webkit',
+             'account_voucher'],
  'description': """
  Improve the basic bank statement, by adding various new features,
  and help dealing with huge volume of reconciliation through payment offices such as Paypal, Lazer,
@@ -65,7 +63,6 @@
  4) Remove the period on the bank statement, and compute it for each line based on their date instead.
     It also adds this feature in the voucher in order to compute the period correctly.
 
-
  5) Cancelling a bank statement is much more easy and will cancel all related entries, unreconcile them,
     and finally delete them.
 
@@ -88,4 +85,4 @@
  'auto_install': False,
  'license': 'AGPL-3',
  'active': False,
-}
+ }
 
=== modified file 'account_statement_ext/i18n/fr.po'
--- account_statement_ext/i18n/fr.po	2012-12-13 13:57:29 +0000
+++ account_statement_ext/i18n/fr.po	2013-04-25 11:52:27 +0000
@@ -41,7 +41,7 @@
 #. module: account_statement_ext
 #: code:addons/account_statement_ext/statement.py:361
 #, python-format
-msgid "Configuration Error !"
+msgid "Configuration Error!"
 msgstr "Erreur de configuration !"
 
 #. module: account_statement_ext
@@ -64,7 +64,7 @@
 #: code:addons/account_statement_ext/statement.py:307
 #: code:addons/account_statement_ext/statement.py:372
 #, python-format
-msgid "Error !"
+msgid "Error!"
 msgstr "Erreur !"
 
 #. module: account_statement_ext
 
=== modified file 'account_statement_ext/statement.py'
--- account_statement_ext/statement.py	2013-03-01 14:33:32 +0000
+++ account_statement_ext/statement.py	2013-04-25 11:52:27 +0000
@@ -1,4 +1,4 @@
-# -*- coding: utf-8 -*-
+#-*- coding: utf-8 -*-
 ##############################################################################
 #
 #    Author: Nicolas Bessi, Joel Grand-Guillaume
@@ -18,12 +18,26 @@
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 ##############################################################################
-
+import openerp.addons.account.account_bank_statement as stat_mod
 from openerp.osv.orm import Model
 from openerp.osv import fields, osv
 from openerp.tools.translate import _
 
 
+# Monkey patch to fix bad write implementation...
+def fixed_write(self, cr, uid, ids, vals, context=None):
+    """ Fix performance desing of original function
+    Ideally we should use a real PostgreSQL sequence or serial fields.
+    I will do it when I have time."""
+    res = super(stat_mod.account_bank_statement, self).write(cr, uid, ids,
+                                                             vals, context=context)
+    cr.execute("UPDATE account_bank_statement_line"
+               " SET sequence = account_bank_statement_line.id + 1"
+               " where statement_id in %s", (tuple(ids),))
+    return res
+stat_mod.account_bank_statement.write = fixed_write
+
+
 class AccountStatementProfil(Model):
     """
     A Profile will contain all infos related to the type of
@@ -44,38 +58,45 @@
                   "commission move (and optionaly on the counterpart "
                   "of the intermediate/banking move if you tick the "
                   "corresponding checkbox)."),
+
         'journal_id': fields.many2one(
             'account.journal',
              'Financial journal to use for transaction',
              required=True),
+
         'commission_account_id': fields.many2one(
             'account.account',
              'Commission account',
              required=True),
+
         'commission_analytic_id': fields.many2one(
             'account.analytic.account',
             'Commission analytic account'),
+
         'receivable_account_id': fields.many2one(
             'account.account',
             'Force Receivable/Payable Account',
             help="Choose a receivable account to force the default "
                  "debit/credit account (eg. an intermediat bank account "
                  "instead of default debitors)."),
+
         'force_partner_on_bank': fields.boolean(
             'Force partner on bank move',
             help="Tick that box if you want to use the credit "
                  "institute partner in the counterpart of the "
                  "intermediate/banking move."),
+
         'balance_check': fields.boolean(
             'Balance check',
             help="Tick that box if you want OpenERP to control "
                  "the start/end balance before confirming a bank statement. "
-                 "If don't ticked, no balance control will be done."
-            ),
-        'bank_statement_prefix': fields.char(
-            'Bank Statement Prefix', size=32),
-        'bank_statement_ids': fields.one2many(
-            'account.bank.statement', 'profile_id', 'Bank Statement Imported'),
+                 "If don't ticked, no balance control will be done."),
+
+        'bank_statement_prefix': fields.char('Bank Statement Prefix', size=32),
+
+        'bank_statement_ids': fields.one2many('account.bank.statement',
+                                              'profile_id',
+                                              'Bank Statement Imported'),
         'company_id': fields.many2one('res.company', 'Company'),
     }
 
@@ -86,7 +107,7 @@
         return True
 
     _constraints = [
-        (_check_partner, "You need to put a partner if you tic the 'Force partner on bank move' !", []),
+        (_check_partner, "You need to put a partner if you tic the 'Force partner on bank move'!", []),
     ]
 
 
@@ -166,11 +187,11 @@
         """
         for statement in self.browse(cr, uid, ids, context=context):
             if (statement.period_id and
-                statement.company_id.id != statement.period_id.company_id.id):
+                    statement.company_id.id != statement.period_id.company_id.id):
                 return False
             for line in statement.line_ids:
                 if (line.period_id and
-                    statement.company_id.id != line.period_id.company_id.id):
+                        statement.company_id.id != line.period_id.company_id.id):
                     return False
         return True
 
@@ -244,8 +265,10 @@
                   create the move from.
            :return: int/long of the res.partner to use as counterpart
         """
-        bank_partner_id = super(AccountBankSatement, self).\
-                _get_counter_part_partner(cr, uid, st_line, context=context)
+        bank_partner_id = super(AccountBankSatement, self)._get_counter_part_partner(cr,
+                                                                                     uid,
+                                                                                     st_line,
+                                                                                     context=context)
         # get the right partner according to the chosen profil
         if st_line.statement_id.profile_id.force_partner_on_bank:
             bank_partner_id = st_line.statement_id.profile_id.partner_id.id
@@ -285,7 +308,7 @@
         We have to copy paste a big block of code, changing the error
         stack + managing period from date.
 
-        TODO: Log the error in a bank statement field instead of using a popup !
+        TODO: Log the error in a bank statement field instead of using a popup!
         """
         for st in self.browse(cr, uid, ids, context=context):
 
@@ -297,8 +320,8 @@
             self.balance_check(cr, uid, st.id, journal_type=j_type, context=context)
             if (not st.journal_id.default_credit_account_id) \
                     or (not st.journal_id.default_debit_account_id):
-                raise osv.except_osv(_('Configuration Error !'),
-                        _('Please verify that an account is defined in the journal.'))
+                raise osv.except_osv(_('Configuration Error!'),
+                                     _('Please verify that an account is defined in the journal.'))
 
             if not st.name == '/':
                 st_number = st.name
@@ -308,20 +331,24 @@
 # End Changes
             for line in st.move_line_ids:
                 if line.state != 'valid':
-                    raise osv.except_osv(_('Error !'),
-                            _('The account entries lines are not in valid state.'))
+                    raise osv.except_osv(_('Error!'),
+                                         _('The account entries lines are not in valid state.'))
 # begin changes
             errors_stack = []
             for st_line in st.line_ids:
                 try:
                     if st_line.analytic_account_id:
                         if not st.journal_id.analytic_journal_id:
-                            raise osv.except_osv(_('No Analytic Journal !'),
-                                             _("You have to assign an analytic journal on the '%s' journal!") % (st.journal_id.name,))
+                            raise osv.except_osv(_('No Analytic Journal!'),
+                                                 _("You have to assign an analytic"
+                                                   " journal on the '%s' journal!") % st.journal_id.name)
                     if not st_line.amount:
                         continue
                     st_line_number = self.get_next_st_line_number(cr, uid, st_number, st_line, context)
-                    self.create_move_from_st_line(cr, uid, st_line.id, company_currency_id, st_line_number, context)
+                    self.create_move_from_st_line(cr, uid, st_line.id,
+                                                  company_currency_id,
+                                                  st_line_number,
+                                                  context)
                 except osv.except_osv, exc:
                     msg = "Line ID %s with ref %s had following error: %s" % (st_line.id, st_line.ref, exc.value)
                     errors_stack.append(msg)
@@ -332,37 +359,97 @@
                 msg = u"\n".join(errors_stack)
                 raise osv.except_osv(_('Error'), msg)
 #end changes
-            self.write(cr, uid, [st.id], {
-                    'name': st_number,
-                    'balance_end_real': st.balance_end
-            }, context=context)
-            self.message_post(cr, uid, [st.id], body=_('Statement %s confirmed, journal items were created.') % (st_number,), context=context)
+            self.write(cr, uid, [st.id],
+                       {'name': st_number,
+                        'balance_end_real': st.balance_end},
+                       context=context)
+            body = _('Statement %s confirmed, journal items were created.') % st_number
+            self.message_post(cr, uid, [st.id],
+                              body,
+                              context=context)
         return self.write(cr, uid, ids, {'state': 'confirm'}, context=context)
 
-    def get_account_for_counterpart(
-            self, cr, uid, amount, account_receivable, account_payable):
+    def get_account_for_counterpart(self, cr, uid, amount, account_receivable, account_payable):
+        """For backward compatibility."""
+        account_id, type = self.get_account_and_type_for_counterpart(cr, uid, amount,
+                                                                     account_receivable,
+                                                                     account_payable)
+        return account_id
+
+    def _compute_type_from_partner_profile(self, cr, uid, partner_id,
+                                           default_type, context=None):
+        """Compute the statement line type
+           from partner profile (customer, supplier)"""
+        obj_partner = self.pool.get('res.partner')
+        part = obj_partner.browse(cr, uid, partner_id, context=context)
+        if part.supplier == part.customer:
+            return default_type
+        if part.supplier:
+            return 'supplier'
+        else:
+            return 'customer'
+
+    def _compute_type_from_amount(self, cr, uid, amount):
+        """Compute the statement type based on amount"""
+        if amount in (None, False):
+            return 'general'
+        if amount < 0:
+            return 'supplier'
+        return 'customer'
+
+    def get_type_for_counterpart(self, cr, uid, amount, partner_id=False):
+        """Give the amount and receive the type to use for the line.
+        The rules are:
+         - If the customer checkbox is checked on the found partner, type customer
+         - If the supplier checkbox is checked on the found partner, typewill be supplier
+         - If both checkbox are checked or none of them, it'll be based on the amount :
+              If amount is positif the type customer,
+              If amount is negativ, the type supplier
+        :param float: amount of the line
+        :param int/long: partner_id the partner id
+        :return: type as string: the default type to use: 'customer' or 'supplier'.
+        """
+        s_line_type = self._compute_type_from_amount(cr, uid, amount)
+        if partner_id:
+            s_line_type = self._compute_type_from_partner_profile(cr, uid,
+                                                                  partner_id, s_line_type)
+        return s_line_type
+
+    def get_account_and_type_for_counterpart(self, cr, uid, amount, account_receivable,
+                                             account_payable, partner_id=False):
         """
         Give the amount, payable and receivable account (that can be found using
         get_default_pay_receiv_accounts method) and receive the one to use. This method
         should be use when there is no other way to know which one to take.
+        The rules are:
+         - If the customer checkbox is checked on the found partner, type and account will be customer and receivable
+         - If the supplier checkbox is checked on the found partner, type and account will be supplier and payable
+         - If both checkbox are checked or none of them, it'll be based on the amount :
+              If amount is positive, the type and account will be customer and receivable,
+              If amount is negative, the type and account will be supplier and payable
+        Note that we return the payable or receivable account from agrs and not from the optional partner_id
+        given!
 
         :param float: amount of the line
         :param int/long: account_receivable the  receivable account
         :param int/long: account_payable the payable account
-        :return: int/long :the default account to be used by statement line as the counterpart
-                 of the journal account depending on the amount.
+        :param int/long: partner_id the partner id
+        :return: dict with [account_id as int/long,type as string]: the default account to be used by
+            statement line as the counterpart of the journal account depending on the amount and the type
+            as 'customer' or 'supplier'.
         """
         account_id = False
-        if amount >= 0:
+        ltype = self.get_type_for_counterpart(cr, uid, amount, partner_id=partner_id)
+        if ltype == 'supplier':
+            account_id = account_payable
+        else:
             account_id = account_receivable
-        else:
-            account_id = account_payable
         if not account_id:
             raise osv.except_osv(
                 _('Can not determine account'),
                 _('Please ensure that minimal properties are set')
             )
-        return account_id
+        return [account_id, ltype]
 
     def get_default_pay_receiv_accounts(self, cr, uid, context=None):
         """
@@ -386,14 +473,11 @@
              ('model', '=', 'res.partner')],
             context=context
         )
-        property_ids = property_obj.search(
-                    cr,
-                    uid,
-                    [('fields_id', 'in', model_fields_ids),
-                     ('res_id', '=', False),
-                    ],
-                    context=context
-        )
+        property_ids = property_obj.search(cr,
+                                           uid,
+                                           [('fields_id', 'in', model_fields_ids),
+                                            ('res_id', '=', False)],
+                                           context=context)
 
         for erp_property in property_obj.browse(
                 cr, uid, property_ids, context=context):
@@ -433,13 +517,10 @@
         journal_id = import_config.journal_id.id
         account_id = import_config.journal_id.default_debit_account_id.id
         credit_partner_id = import_config.partner_id and import_config.partner_id.id or False
-        return {'value':
-                    {'journal_id': journal_id,
-                     'account_id': account_id,
-                     'balance_check': import_config.balance_check,
-                     'credit_partner_id': credit_partner_id,
-                    }
-                }
+        return {'value': {'journal_id': journal_id,
+                          'account_id': account_id,
+                          'balance_check': import_config.balance_check,
+                          'credit_partner_id': credit_partner_id}}
 
 
 class AccountBankSatementLine(Model):
@@ -472,18 +553,24 @@
         'account_id': _get_default_account,
     }
 
-    def get_values_for_line(self, cr, uid, profile_id=False, partner_id=False, line_type=False, amount=False, context=None):
+    def get_values_for_line(self, cr, uid, profile_id=False, partner_id=False, line_type=False, amount=False, master_account_id=None, context=None):
         """
         Return the account_id to be used in the line of a bank statement. It'll base the result as follow:
             - If a receivable_account_id is set in the profile, return this value and type = general
-            - Elif line_type is given, take the partner receivable/payable property (payable if type= supplier, receivable
+            # TODO
+            - Elif how_get_type_account is set to force_supplier or force_customer, will take respectively payable and type=supplier,
+              receivable and type=customer otherwise
+            # END TODO
+            - Elif line_type is given, take the partner receivable/payable property (payable if type=supplier, receivable
               otherwise)
-            - Elif amount is given, take the partner receivable/payable property (receivable if amount >= 0.0,
-              payable otherwise). In that case, we also fullfill the type (receivable = customer, payable = supplier)
-              so it is easier for the accountant to know why the receivable/payable has been chosen
+            - Elif amount is given:
+                 - If the customer checkbox is checked on the found partner, type and account will be customer and receivable
+                 - If the supplier checkbox is checked on the found partner, type and account will be supplier and payable
+                 - If both checkbox are checked or none of them, it'll be based on the amount :
+                      If amount is positive, the type and account will be customer and receivable,
+                      If amount is negative, the type and account will be supplier an payable
             - Then, if no partner are given we look and take the property from the company so we always give a value
               for account_id. Note that in that case, we return the receivable one.
-
         :param int/long profile_id of the related bank statement
         :param int/long partner_id of the line
         :param char line_type: a value from: 'general', 'supplier', 'customer'
@@ -499,79 +586,77 @@
         res = {}
         obj_partner = self.pool.get('res.partner')
         obj_stat = self.pool.get('account.bank.statement')
-        line_type = receiv_account = pay_account = account_id = False
+        receiv_account = pay_account = account_id = False
         # If profile has a receivable_account_id, we return it in any case
-        if profile_id:
+        if master_account_id:
+            res['account_id'] = master_account_id
+            # We return general as default instead of get_type_for_counterpart
+            # for perfomance reasons as line_type is not a meaningfull value
+            # as account is forced
+            res['type'] = line_type if line_type else 'general'
+            return res
+        # To optimize we consider passing false means there is no account
+        # on profile
+        if profile_id and master_account_id is None:
             profile = self.pool.get("account.statement.profile").browse(
-                    cr, uid, profile_id, context=context)
+                                       cr, uid, profile_id, context=context)
             if profile.receivable_account_id:
-                account_id = profile.receivable_account_id.id
-                line_type = 'general'
+                res['account_id'] = profile.receivable_account_id.id
+                # We return general as default instead of get_type_for_counterpart
+                # for perfomance reasons as line_type is not a meaningfull value
+                # as account is forced
+                res['type'] = line_type if line_type else 'general'
                 return res
-        # If partner -> take from him
+        # If no account is available on profile you have to do the lookup
+        # This can be quite a performance killer as we read ir.properity fields
         if partner_id:
             part = obj_partner.browse(cr, uid, partner_id, context=context)
             pay_account = part.property_account_payable.id
             receiv_account = part.property_account_receivable.id
         # If no value, look on the default company property
         if not pay_account or not receiv_account:
-            receiv_account, pay_account = obj_stat.get_default_pay_receiv_accounts(
-                    cr, uid, context=None)
-        # Now we have both pay and receive account, choose the one to use
-        # based on line_type first, then amount, otherwise take receivable one.
-        if line_type is not False:
-            if line_type == 'supplier':
-                account_id = pay_account
-        elif amount is not False:
-            if amount >= 0:
-                account_id = receiv_account
-                line_type = 'customer'
-            else:
-                account_id  = pay_account
-                line_type = 'supplier'
+            receiv_account, pay_account = obj_stat.get_default_pay_receiv_accounts(cr, uid, context=None)
+        account_id, comp_line_type = obj_stat.get_account_and_type_for_counterpart(cr, uid, amount,
+                                                                                   receiv_account, pay_account,
+                                                                                   partner_id=partner_id)
         res['account_id'] = account_id if account_id else receiv_account
-        res['type'] = line_type
+        res['type'] = line_type if line_type else comp_line_type
         return res
 
     def onchange_partner_id(self, cr, uid, ids, partner_id, profile_id=None, context=None):
         """
         Override of the basic method as we need to pass the profile_id in the on_change_type
         call.
+        Moreover, we now call the get_account_and_type_for_counterpart method now to get the
+        type to use.
         """
-        obj_partner = self.pool.get('res.partner')
+        obj_stat = self.pool.get('account.bank.statement')
         if not partner_id:
             return {}
-        part = obj_partner.browse(cr, uid, partner_id, context=context)
-        if not part.supplier and not part.customer:
-            type = 'general'
-        elif part.supplier and part.customer:
-            type = 'general'
-        else:
-            if part.supplier == True:
-                type = 'supplier'
-            if part.customer == True:
-                type = 'customer'
-        res_type = self.onchange_type(cr, uid, ids, partner_id, type, profile_id, context=context)  # Chg
+        line_type = obj_stat.get_type_for_counterpart(cr, uid, 0.0, partner_id=partner_id)
+        res_type = self.onchange_type(cr, uid, ids, partner_id, line_type, profile_id, context=context)
         if res_type['value'] and res_type['value'].get('account_id', False):
-            return {'value': {'type': type,
+            return {'value': {'type': line_type,
                               'account_id': res_type['value']['account_id'],
                               'voucher_id': False}}
-        return {'value': {'type': type}}
+        return {'value': {'type': line_type}}
 
-    def onchange_type(self, cr, uid, line_id, partner_id, type, profile_id, context=None):
+    def onchange_type(self, cr, uid, line_id, partner_id, line_type, profile_id, context=None):
         """
         Keep the same features as in standard and call super. If an account is returned,
         call the method to compute line values.
         """
-        res = super(AccountBankSatementLine, self).onchange_type(
-                cr, uid, line_id, partner_id, type, context=context)
+        res = super(AccountBankSatementLine, self).onchange_type(cr, uid,
+                                                                 line_id,
+                                                                 partner_id,
+                                                                 line_type,
+                                                                 context=context)
         if 'account_id' in res['value']:
-            result = self.get_values_for_line(
-                    cr, uid,
-                    profile_id=profile_id,
-                    partner_id=partner_id,
-                    line_type=type,
-                    context=context)
+            result = self.get_values_for_line(cr, uid,
+                                              profile_id=profile_id,
+                                              partner_id=partner_id,
+                                              line_type=line_type,
+                                              context=context)
             if result:
                 res['value'].update({'account_id': result['account_id']})
         return res
 
=== modified file 'account_statement_transactionid_completion/statement.py'
--- account_statement_transactionid_completion/statement.py	2012-12-20 13:37:01 +0000
+++ account_statement_transactionid_completion/statement.py	2013-04-25 11:52:27 +0000
@@ -32,7 +32,7 @@
 
     def _get_functions(self, cr, uid, context=None):
         res = super(AccountStatementCompletionRule, self)._get_functions(
-                cr, uid, context=context)
+                                                           cr, uid, context=context)
         res.append(('get_from_transaction_id_and_so',
                     'From line reference (based on SO transaction ID)'))
         return res
@@ -41,12 +41,12 @@
         'function_to_call': fields.selection(_get_functions, 'Method'),
     }
 
-    def get_from_transaction_id_and_so(self, cr, uid, line_id, context=None):
+    def get_from_transaction_id_and_so(self, cr, uid, st_line, context=None):
         """
         Match the partner based on the transaction ID field of the SO.
         Then, call the generic st_line method to complete other values.
         In that case, we always fullfill the reference of the line with the SO name.
-        :param int/long line_id: ID of the concerned account.bank.statement.line
+        :param dict st_line: read of the concerned account.bank.statement.line
         :return:
             A dict of value that can be passed directly to the write method of
             the statement line or {}
@@ -55,33 +55,28 @@
             ...}
             """
         st_obj = self.pool.get('account.bank.statement.line')
-        st_line = st_obj.browse(cr, uid, line_id, context=context)
         res = {}
-        if st_line:
-            so_obj = self.pool.get('sale.order')
-            so_id = so_obj.search(
-                    cr,
-                    uid,
-                    [('transaction_id', '=', st_line.transaction_id)],
-                    context=context)
-            if so_id and len(so_id) == 1:
-                so = so_obj.browse(cr, uid, so_id[0], context=context)
-                res['partner_id'] = so.partner_id.id
-                res['ref'] = so.name
-            elif so_id and len(so_id) > 1:
-                raise ErrorTooManyPartner(
-                        _('Line named "%s" (Ref:%s) was matched by more than '
-                          'one partner.') % (st_line.name, st_line.ref))
-            if so_id:
-                st_vals = st_obj.get_values_for_line(
-                        cr,
-                        uid,
-                        profile_id=st_line.statement_id.profile_id.id,
-                        partner_id=res.get('partner_id', False),
-                        line_type=st_line.type,
-                        amount=st_line.amount,
-                        context=context)
-                res.update(st_vals)
+        so_obj = self.pool.get('sale.order')
+        so_id = so_obj.search(cr,
+                              uid,
+                              [('transaction_id', '=', st_line['transaction_id'])],
+                              context=context)
+        if len(so_id) > 1:
+            raise ErrorTooManyPartner(_('Line named "%s" (Ref:%s) was matched by more than '
+                                        'one partner.') % (st_line['name'], st_line['ref']))
+        if len(so_id) == 1:
+            so = so_obj.browse(cr, uid, so_id[0], context=context)
+            res['partner_id'] = so.partner_id.id
+            res['ref'] = so.name
+            st_vals = st_obj.get_values_for_line(cr,
+                                                 uid,
+                                                 profile_id=st_line['profile_id'],
+                                                 master_account_id=st_line['master_account_id'],
+                                                 partner_id=res.get('partner_id', False),
+                                                 line_type=st_line['type'],
+                                                 amount=st_line['amount'] if st_line['amount'] else 0.0,
+                                                 context=context)
+            res.update(st_vals)
         return res
 
 
 
=== modified file 'account_statement_transactionid_import/parser/transactionid_file_parser.py'
--- account_statement_transactionid_import/parser/transactionid_file_parser.py	2013-02-26 08:19:44 +0000
+++ account_statement_transactionid_import/parser/transactionid_file_parser.py	2013-04-25 11:52:27 +0000
@@ -17,8 +17,6 @@
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 ##############################################################################
-
-from openerp.tools.translate import _
 import datetime
 from account_statement_base_import.parser.file_parser import FileParser
 
@@ -30,13 +28,11 @@
     """
 
     def __init__(self, parse_name, ftype='csv'):
-        conversion_dict = {
-                            'transaction_id': unicode,
-                            'label': unicode,
-                            'date': datetime.datetime,
-                            'amount': float,
-                            'commission_amount': float
-                          }
+        conversion_dict = {'transaction_id': unicode,
+                           'label': unicode,
+                           'date': datetime.datetime,
+                           'amount': float,
+                           'commission_amount': float}
         # Order of cols does not matter but first row of the file has to be header
         keys_to_validate = ['transaction_id', 'label', 'date', 'amount', 'commission_amount']
         super(TransactionIDFileParser, self).__init__(parse_name, keys_to_validate=keys_to_validate,
@@ -70,15 +66,13 @@
         In this generic parser, the commission is given for every line, so we store it
         for each one.
         """
-        return {
-            'name': line.get('label', line.get('ref', '/')),
-            'date': line.get('date', datetime.datetime.now().date()),
-            'amount': line.get('amount', 0.0),
-            'ref': line.get('transaction_id', '/'),
-            'label': line.get('label', ''),
-            'transaction_id': line.get('transaction_id', '/'),
-            'commission_amount': line.get('commission_amount', 0.0),
-        }
+        return {'name': line.get('label', line.get('ref', '/')),
+                'date': line.get('date', datetime.datetime.now().date()),
+                'amount': line.get('amount', 0.0),
+                'ref': line.get('transaction_id', '/'),
+                'label': line.get('label', ''),
+                'transaction_id': line.get('transaction_id', '/'),
+                'commission_amount': line.get('commission_amount', 0.0)}
 
     def _post(self, *args, **kwargs):
         """