Merge lp:~camptocamp/banking-addons/already_delete_reconcile_id_resubmit into lp:banking-addons/bank-statement-reconcile-70

Proposed by Vincent Renaville@camptocamp
Status: Merged
Approved by: Guewen Baconnier @ Camptocamp
Approved revision: 103
Merged at revision: 108
Proposed branch: lp:~camptocamp/banking-addons/already_delete_reconcile_id_resubmit
Merge into: lp:banking-addons/bank-statement-reconcile-70
Diff against target: 16 lines (+4/-1)
1 file modified
account_statement_ext/account.py (+4/-1)
To merge this branch: bzr merge lp:~camptocamp/banking-addons/already_delete_reconcile_id_resubmit
Reviewer Review Type Date Requested Status
Guewen Baconnier @ Camptocamp code review Approve
Omar (Pexego) Approve
Leonardo Pistone Approve
Joël Grand-Guillaume @ camptocamp code review, no tests Needs Fixing
Review via email: mp+194679@code.launchpad.net

This proposal supersedes a proposal from 2013-07-26.

Description of the change

[FIX] remove account.reconcile before deleting a account.move

To post a comment you must log in.
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote : Posted in a previous version of this proposal

I think a more efficient and straightforward way would be something along those lines:

    [...]
    reconcile_obj = self.pool.get('account.move.reconcile')
    reconcile_ids = set()
    for move_line in move.line_id:
        if move_line.reconcile_id:
            reconcile_ids.add(move_line.reconcile_id.id)
    reconcile_obj.unlink(cr, uid, list(reconcile_ids), context=context)
    return [...]

So you'll do only 1 DELETE at the end of the loop.

review: Needs Fixing
Revision history for this message
Joël Grand-Guillaume @ camptocamp (jgrandguillaume-c2c) wrote : Posted in a previous version of this proposal

@Vincent, any update here please ?

Revision history for this message
Joël Grand-Guillaume @ camptocamp (jgrandguillaume-c2c) wrote :

LGTM

review: Approve (code review, no tests)
Revision history for this message
Omar (Pexego) (omar7r) wrote :

PEP8 compliant, please, spaces after comma.

Thanks

review: Needs Fixing (code review)
Revision history for this message
Leonardo Pistone (lepistone) wrote :

There are some tabs that should be spaces. With that and the commas fixed, I approve.

Thanks Vincent!

review: Needs Fixing (code review)
Revision history for this message
Joël Grand-Guillaume @ camptocamp (jgrandguillaume-c2c) wrote :

You're right, I miss those ones...

review: Needs Fixing (code review, no tests)
103. By Vincent Renaville@camptocamp

[FIX] PEP8

Revision history for this message
Vincent Renaville@camptocamp (vrenaville-c2c) wrote :

Hello,

Thanks for the review !
PEP8 seems to be ok now.

Vincent

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

Thanks Vincent!

review: Approve
Revision history for this message
Omar (Pexego) (omar7r) wrote :

Approved now, thanks

review: Approve
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

Thanks!

review: Approve (code review)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'account_statement_ext/account.py'
2--- account_statement_ext/account.py 2012-12-20 13:37:01 +0000
3+++ account_statement_ext/account.py 2013-12-13 14:07:06 +0000
4@@ -31,8 +31,11 @@
5 Delete the reconciliation when we delete the moves. This
6 allow an easier way of cancelling the bank statement.
7 """
8+ reconcile_to_delete = []
9+ reconcile_obj = self.pool.get('account.move.reconcile')
10 for move in self.browse(cr, uid, ids, context=context):
11 for move_line in move.line_id:
12 if move_line.reconcile_id:
13- move_line.reconcile_id.unlink(context=context)
14+ reconcile_to_delete.append(move_line.reconcile_id.id)
15+ reconcile_obj.unlink(cr, uid, reconcile_to_delete, context=context)
16 return super(account_move, self).unlink(cr, uid, ids, context=context)

Subscribers

People subscribed via source and target branches