Merge lp:~camptocamp/purchase-wkfl/7.0-browse_record_error-1287159 into lp:~purchase-core-editors/purchase-wkfl/7.0

Proposed by Guewen Baconnier @ Camptocamp on 2014-03-03
Status: Merged
Approved by: Yannick Vaucher @ Camptocamp on 2014-03-12
Approved revision: 30
Merged at revision: 29
Proposed branch: lp:~camptocamp/purchase-wkfl/7.0-browse_record_error-1287159
Merge into: lp:~purchase-core-editors/purchase-wkfl/7.0
Diff against target: 21 lines (+2/-2)
1 file modified
purchase_landed_costs/purchase.py (+2/-2)
To merge this branch: bzr merge lp:~camptocamp/purchase-wkfl/7.0-browse_record_error-1287159
Reviewer Review Type Date Requested Status
Yannick Vaucher @ Camptocamp code review, no tests Approve on 2014-03-07
Pedro Manuel Baeza code review 2014-03-03 Approve on 2014-03-03
Review via email: mp+209056@code.launchpad.net

Commit message

[FIX] browse_record given to create() instead of the integer id

Description of the change

To post a comment you must log in.
29. By Guewen Baconnier @ Camptocamp on 2014-03-03

browse_record given to create() instead of the integer id

Pedro Manuel Baeza (pedro.baeza) wrote :

There is another MP resolving the same issue and more complete:

https://code.launchpad.net/~elicoidal/purchase-wkfl/purchase-wkfl-fix-0001/+merge/208758

Please review it.

Regards.

review: Disapprove

> There is another MP resolving the same issue and more complete:
>
> https://code.launchpad.net/~elicoidal/purchase-wkfl/purchase-wkfl-
> fix-0001/+merge/208758
>
> Please review it.
>
> Regards.

Please see my answer and reconsider your vote: https://code.launchpad.net/~elicoidal/purchase-wkfl/purchase-wkfl-fix-0001/+merge/208758/comments/491532

Could fiscal_position be renamed fiscal_position_id?

BTW
fiscal_position_id = po.fiscal_position.id

Would also be valid as browse_null.id would return False

review: Needs Fixing (code review, no tests)
Pedro Manuel Baeza (pedro.baeza) wrote :

I revert my review based on comments.

Regards.

review: Approve (code review)
30. By Guewen Baconnier @ Camptocamp on 2014-03-03

rename fiscal_position_id so it is clear that it refers to an ID and not to a browse_record

I renamed the field, Yannick can you review again please?

Thanks for the change.

review: Approve (code review, no tests)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'purchase_landed_costs/purchase.py'
2--- purchase_landed_costs/purchase.py 2013-12-12 15:34:53 +0000
3+++ purchase_landed_costs/purchase.py 2014-03-03 13:49:35 +0000
4@@ -550,7 +550,7 @@
5 po = (landed_cost.purchase_order_id or
6 landed_cost.purchase_order_line_id.order_id)
7 currency_id = landed_cost.purchase_order_id.pricelist_id.currency_id.id
8- fiscal_position = po.fiscal_position or False
9+ fiscal_position_id = po.fiscal_position.id if po.fiscal_position else False
10 journal_obj = self.pool.get('account.journal')
11 journal_ids = journal_obj.search(
12 cr, uid,
13@@ -568,7 +568,7 @@
14 'account_id': landed_cost.partner_id.property_account_payable.id,
15 'type': 'in_invoice',
16 'origin': po.name,
17- 'fiscal_position': fiscal_position,
18+ 'fiscal_position': fiscal_position_id,
19 'company_id': po.company_id.id,
20 'journal_id': len(journal_ids) and journal_ids[0] or False,
21 }

Subscribers

People subscribed via source and target branches