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

Proposed by Guewen Baconnier @ Camptocamp
Status: Merged
Approved by: Yannick Vaucher @ Camptocamp
Approved revision: 30
Merged at revision: 29
Proposed branch: lp://staging/~camptocamp/purchase-wkfl/7.0-browse_record_error-1287159
Merge into: lp://staging/~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://staging/~camptocamp/purchase-wkfl/7.0-browse_record_error-1287159
Reviewer Review Type Date Requested Status
Yannick Vaucher @ Camptocamp code review, no tests Approve
Pedro Manuel Baeza code review Approve
Review via email: mp+209056@code.staging.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

browse_record given to create() instead of the integer id

Revision history for this message
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
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) 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.

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

Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote :

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)
Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

I revert my review based on comments.

Regards.

review: Approve (code review)
30. By Guewen Baconnier @ Camptocamp

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

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

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

Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote :

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
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches