banking-addons-team team mailing list archive

["Stefan Rijnhart \(Therp\)" <stefan@xxxxxxxx>]

Review: Needs Fixing

I must apologize for the elaborate 'type' discovery in _get_move_info() but what it attempts is to set match type to 'invoice' only if all candidate move lines are linked to an invoice. Your fix does not honour this admittedly implicit logic.

Of course, if match_type comes out as 'invoice' while not all move lines actually have an invoice then the code is broken and should be fixed. The loop where it checks for invoice on the move lines and a couple of other lines after it can probably be replaced by something like

if move_lines:
    if all(line.invoice for line in move_lines):
        retval['match_type'] = 'invoice'
        retval['type'] = type_map[move_lines[0].invoice.type]
    else:
        retval['match_type'] = 'move'

The method's signature allows for an apriori match_type argument but it does not seem to be used anywhere and could probably be refactored out.

Does this in fact fix your case or am I missing something?

-- 
https://code.launchpad.net/~therp-nl/banking-addons/6.1_lp1176783/+merge/162566
Your team Banking Addons Team is subscribed to branch lp:banking-addons.