Review: Needs Fixing code review, no test
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" ?
--
https://code.launchpad.net/~camptocamp-reviewer/banking-addons/fix-type-and-account-in-bk-st-line/+merge/160591
Your team Banking Addons Team is subscribed to branch
lp:banking-addons/bank-statement-reconcile-70.
--
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
