Code review comment for lp:~camptocamp/banking-addons/bank-statement-reconcile-7.0-add-cancel-line-lep

Revision history for this message
Leonardo Pistone (lepistone) wrote :

Thanks for your review Stefan.

The point you raise makes sense to me. Speaking as a developer and not an accountant, making this module agnostic on account_default_draft_move would be probably feasible, but the integration test would break. That makes sense to me, since the business logic changes a bit.

In fact I would not feel at ease adapting the test, because the two workflows (with or without default_draft) seem like two scenarios that I'd like to test separately to be somewhat confident.

To do that properly, probably I would need to remove the dependency here, write new tests for the non-default-draft case, and then write a new module that depends on account_statement_cancel_line and account_default_draft_move, with the tests I have now.

So your proposal makes sense to me, but I am not prepared to do that right now - it does not cover my use case anyway.

I would be happy to review and accept such a change if someone does it.

Leo

« Back to merge proposal