Merge lp://staging/~camptocamp/banking-addons/bank-statement-reconcile-7.0-add-cancel-line-lep into lp://staging/banking-addons/bank-statement-reconcile-70

After an internal review, I added french translations, and a migration file to set the state correctly on existing statements, and I also merged upstream.

I finished some fixes: an improved confirmation wizard, and a fix in the tests.

thanks!

I put this MP to 'work in progress' because I found a bug here. I'm doing a test and a fix.

I fixed the bug i'd found: if the last line in draft was confirmed and the statement confirmed, we were skipping the balance check.

I added two tests that show that.

LGTM, Thank you Leonardo

Thanks for considering account_banking for your naming! Having this module will certainly help in merging the projects.

I tried to test the module, and was confronted by the dependency on account_default_draft_move. This dependency enforces alternative accounting practises which may be required in your region, but not in ours and should not be part of this change IMHO. Would you consider making this module agnostic towards the cancelling draft moves? With this, I mean that when cancelling a bank statement line, you can simply attempt to cancel and remove the associated move line and do the constraints in the various modules do their work.

s/cancelling draft moves/cancelling *posted* moves/

obviously

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

OK. Abstain for now then.

oops, we found that that bug:

- create a statement with two lines
- confirm the first one
- confirm the whole statement

Then the first line gets two moves. At the moment, I pushed a yaml test to expose it.

The bug should now be fixed, and we are green again.

LGTM

Ok is fine.

Thanks leo

everything seems to work smoothly now.