Merge lp:~therp-nl/banking-addons/6.1_lp1117319_abnamro_sepa_line into lp:banking-addons/6.1

[Ronald Portier (Therp)]

Make it possible to import abnamro statement files with extended sepa attributes in line.

Taking into account the remarks by Stefan I streamlined the code , hopefully solving the errors.

[Stefan Rijnhart (Opener) (stefan-opener)]

Thanks for fixing this!

l.8,28 Maybe combine in a single dictionary of possible keys?
l.62 Looks like the last key/value pair is not stored in sepa_dict. You could copy this line after the end of the 'while' loop.

review: Needs Fixing

[Stefan Rijnhart (Opener) (stefan-opener)]

I'm afraid your solution fails when the separator occurs in the field value as is allowed by the specs. E.g.

'/TRTP/SEPA OVERBOEKING/IBAN/CH999999999999F/BIC/UBSWCHZH80A/NAME/ORIG/INAL ART/REMI/CH9999999999/EREF/NOTPROVIDED/O\
RDP//ID/UBS AG'

Comes out as

{'ORIG//REMI': 'CH9999999999', 'NAME': '', 'BIC': 'UBSWCHZH80A', 'TRTP': 'SEPA OVERBOEKING', 'EREF': 'NOTPROVIDED', 'ORDP//ID': 'UBS AG', 'IBAN': 'CH999999999999F'}

when using your test script.

review: Needs Fixing

[lp:~therp-nl/banking-addons/6.1_lp1117319_abnamro_sepa_line]

163. By Ronald Portier (Therp) on 2013-02-08

[FIX] Updated parsing of abnamro sepa string to handle even more unlikely cases.

164. By Ronald Portier (Therp) on 2013-04-17

[FIX] More meaningfull names for index variable names.

Unmerged revisions

=== modified file 'account_banking_nl_abnamro/abnamro.py'
--- account_banking_nl_abnamro/abnamro.py	2012-10-27 19:53:50 +0000
+++ account_banking_nl_abnamro/abnamro.py	2013-02-08 09:36:20 +0000
@@ -155,28 +155,53 @@
             The string consists of slash separated KEY/VALUE pairs,
             but the slash is allowed to and known to occur in VALUE as well!
             """
-            items = field[1:].split('/') # skip leading slash
+            def _sepa_message(field, reason):
+                return _(
+                    'unable to parse SEPA string: %s - %s' % (field, reason))
+
+            def _get_next_key(items, start):
+                '''Find next key, starting from start, returns the key found,
+                the start position in the array and the end position + 1'''
+                known_keys = [
+                    'TRTP', 'IBAN', 'BIC', 'NAME', 'RTRN', 'EREF', 'SWOC',
+                    'REMI', 'ADDR', 'CPRP', 'CREF', 'CSID', 'ISDT', 'MARF',
+                    'NRTX', 'NRTXR', 'PREF', 'PURP', 'REFOB', 'RREF', 'RTYP',
+                    'SVCL', 'SWOD', 'BENM//ID', 'ORDP//ID', 'ORDP//RID',
+                    'ORIG//CSID', 'ORIG//MARF', 'ULTD//NAME', 'ULTD//ID',
+                    'ULTB//NAME', 'ULTB//ID'
+                ]
+                items_len = len(items)
+                si = start
+                # Search until start after end of items
+                while si < items_len:
+                    ei = si + 1
+                    while ei < items_len:
+                        key = '/'.join(items[si:ei])
+                        if  key in known_keys:
+                            return (key, si, ei)
+                        ei += 1
+                    si += 1
+                return False
+
+            items = field[1:].split('/')
+            assert len(items) > 1, _sepa_message(field, _('too few items'))
             sepa_dict = {}
-            prev_key = False
-            known_keys = ['TRTP', 'IBAN', 'BIC', 'NAME', 'RTRN', 'EREF',
-                          'SWOC', 'REMI', ]
-            while items:
-                if len(items) == 1:
-                    raise osv.except_osv(
-                        _('Error !'),
-                        _("unable to parse SEPA string: %s") % field)
-                key = items.pop(0)
-                if key not in known_keys:
-                    # either an unknown key or a value containing a slash
-                    if prev_key:
-                        sepa_dict[prev_key] = sepa_dict[prev_key] + '/' + key
-                    else:
-                        raise osv.except_osv(
-                            _('Error !'),
-                            _("unable to parse SEPA string: %s") % field)
-                else:
-                    sepa_dict[key] = items.pop(0).strip()
-                    prev_key = key
+            item_index = 0
+            items_len = len(items)
+            key_info = _get_next_key(items, item_index)
+            assert key_info, _sepa_message(
+                field, _('no key found for start %d') % item_index)
+            assert key_info[1] == 0, _sepa_message(
+                field, _('invalid data found before key %s') % key_info[0])
+            while key_info:
+                sepa_key = key_info[0]
+                item_index = key_info[2]
+                # Find where next key - if any - starts
+                key_info = _get_next_key(items, item_index)
+                ve = (key_info and key_info[1]) or items_len
+                sepa_value = (
+                    (ve > item_index) and '/'.join(items[item_index:ve])) or ''
+                sepa_dict[sepa_key] = sepa_value
             return sepa_dict
 
         def parse_type(field):