Merge lp:~akretion-team/openerp-connector-magento/7-fix-invoice-binding into lp:~openerp-connector-core-editors/openerp-connector-magento/7.0-next-release

Proposed by David BEAL (ak) on 2014-04-29
Status: Merged
Approved by: Guewen Baconnier @ Camptocamp on 2014-05-06
Approved revision: 981
Merged at revision: 982
Proposed branch: lp:~akretion-team/openerp-connector-magento/7-fix-invoice-binding
Merge into: lp:~openerp-connector-core-editors/openerp-connector-magento/7.0-next-release
Diff against target: 13 lines (+3/-0)
1 file modified
magentoerpconnect/invoice.py (+3/-0)
To merge this branch: bzr merge lp:~akretion-team/openerp-connector-magento/7-fix-invoice-binding
Reviewer Review Type Date Requested Status
Romain Deheele - Camptocamp (community) code review Approve on 2014-05-02
Guewen Baconnier @ Camptocamp code review 2014-04-29 Approve on 2014-05-01
Review via email: mp+217605@code.launchpad.net

Description of the change

prevent create a second binding in invoice if invoice is cancel and yet validate (with account cancel)

To post a comment you must log in.

Hi,

Following the logic of the code, if several magento.account.invoice bindings could be created for an invoice (several sales orders or linked with several backends). Even if this situation is highly improbable, I think it is important to follow the same logic.

So the check should be done in the inner(-inner) loop.

Also, I prefer when a condition returns eagerly than when it pushes all a block of an indentation level (avoid long lines, improve readability, reduce diff size hence conflicts).

In that case, it would have been:

    if invoice.magento_bind_ids:
        return

Or in the inner loop

    if [condition]:
        continue

thanks

review: Needs Fixing
981. By David BEAL (ak) on 2014-04-29

[FIX] prevent create a second binding with the same backend

David BEAL (ak) (davidbeal) wrote :

done

Thanks!

review: Approve (code review)

LGTM,

Romain

review: Approve (code review)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'magentoerpconnect/invoice.py'
2--- magentoerpconnect/invoice.py 2014-03-19 10:34:00 +0000
3+++ magentoerpconnect/invoice.py 2014-04-29 14:12:17 +0000
4@@ -215,6 +215,9 @@
5 # we use the shop as many sale orders can be related to an invoice
6 for sale in invoice.sale_ids:
7 for magento_sale in sale.magento_bind_ids:
8+ for mag_inv in invoice.magento_bind_ids:
9+ if mag_inv.backend_id.id == magento_sale.backend_id.id:
10+ continue
11 # Check if invoice state matches configuration setting
12 # for when to export an invoice
13 magento_stores = magento_sale.shop_id.magento_bind_ids

Subscribers

People subscribed via source and target branches