← Back to team overview

banking-addons-team team mailing list archive

Re: lp:~akretion-team/banking-addons/bank-statement-reconcile-7.0-one-move into lp:banking-addons/bank-statement-reconcile-7.0

 

Review: Needs Fixing

Thanks for the changes.

Regarding the line "assert st.line_ids, "This statement does not contain any lines""
I just made a quick check: you are allowed to create a bank statement with 0 lines, then use the "Confirm" / "Cancel" buttons.
I couldn't test it with your module due to the error below but I think you would get an assert error.
assert should be used only for errors that *should never happen* or you *don't expect to ever happen* (this is also an efficient way to document what you expect not to happen).
I think in this case, you just want to skip the for loop when it has 0 lines, as it *can* happen (and we do not want to display an except_orm error neither here).

I got this error during my test
  File "/home/gbaconnier/code/instances/7.0/parts/bank-statement-reconcile/account_statement_one_move/statement.py", line 137, in button_confirm_bank
    move_id = context['move_id']
KeyError: 'move_id'

The module account_statement_one_move/statement.py also has a syntax error (closing bracket } missing)
-- 
https://code.launchpad.net/~akretion-team/banking-addons/bank-statement-reconcile-7.0-one-move/+merge/197769
Your team Banking Addons Core Editors is subscribed to branch lp:banking-addons/bank-statement-reconcile-7.0.