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.

-- 
Mailing list: https://launchpad.net/~banking-addons-team
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~banking-addons-team
More help   : https://help.launchpad.net/ListHelp

Reply via email to