Merge lp:~camptocamp/sale-wkfl/7.0-dropshipping-merge-po-lep into lp:~sale-core-editors/sale-wkfl/7.0

Proposed by Leonardo Pistone
Status: Rejected
Rejected by: Leonardo Pistone
Proposed branch: lp:~camptocamp/sale-wkfl/7.0-dropshipping-merge-po-lep
Merge into: lp:~sale-core-editors/sale-wkfl/7.0
Diff against target: 102 lines (+73/-0)
4 files modified
sale_dropshipping/__openerp__.py (+1/-0)
sale_dropshipping/purchase.py (+11/-0)
sale_dropshipping/tests/__init__.py (+26/-0)
sale_dropshipping/tests/test_merge_po.py (+35/-0)
To merge this branch: bzr merge lp:~camptocamp/sale-wkfl/7.0-dropshipping-merge-po-lep
Reviewer Review Type Date Requested Status
Pedro Manuel Baeza Needs Resubmitting
Leonardo Pistone Approve
Romain Deheele - Camptocamp (community) Abstain
Alexandre Fayolle - camptocamp code review, no test Approve
Review via email: mp+216756@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Leonardo Pistone (lepistone) wrote :
Revision history for this message
Romain Deheele - Camptocamp (romaindeheele) wrote :

Hi Leonardo,

LGTM,

Romain

review: Approve (code review, no test)
Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

LGTM

review: Approve (code review, no test)
Revision history for this message
Romain Deheele - Camptocamp (romaindeheele) wrote :

Hi Leonardo,

In l.23
the call of super _key_fields_for_grouping returns a tuple object.
To add the element 'dest_adress_id' in l.24,
we need : return field_list + ('dest_address_id', )

Regards,

Romain

review: Needs Fixing (test)
45. By Romain Deheele - Camptocamp

[FIX] dest_address_id must be a tuple to be added in field_list

46. By Romain Deheele - Camptocamp

[FIX] to populate the merged order, provide id of dest_address instead of browse_record

Revision history for this message
Romain Deheele - Camptocamp (romaindeheele) wrote :

Hi,

I commited needed changes about my last comment.
I added too : " to populate the merged order, provide id of dest_address instead of browse_record"

Regards,

Romain

Revision history for this message
Romain Deheele - Camptocamp (romaindeheele) wrote :

As I commited too, I change my review to : Abstain

review: Abstain
Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

LGTM

Regards.

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

Romain,
I approve your changes, thanks

review: Approve
Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

This project is now hosted on https://github.com/OCA/sale-workflow. Please move your proposal there. This guide may help you https://github.com/OCA/maintainers-tools/wiki/How-to-move-a-Merge-Proposal-to-GitHub

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

Unmerged revisions

46. By Romain Deheele - Camptocamp

[FIX] to populate the merged order, provide id of dest_address instead of browse_record

45. By Romain Deheele - Camptocamp

[FIX] dest_address_id must be a tuple to be added in field_list

44. By Leonardo Pistone

[imp] dropshipping: populate the destination address when merging two purchase orders

43. By Leonardo Pistone

[add] sale dropshipping: do not merge orders with different destination addresses

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'sale_dropshipping/__openerp__.py'
--- sale_dropshipping/__openerp__.py 2014-03-21 16:32:11 +0000
+++ sale_dropshipping/__openerp__.py 2014-06-26 12:35:37 +0000
@@ -25,6 +25,7 @@
25 "website": "http://www.openerp.com",25 "website": "http://www.openerp.com",
26 "category": "Generic Modules/Purchase",26 "category": "Generic Modules/Purchase",
27 "depends": ["purchase",27 "depends": ["purchase",
28 "purchase_group_hooks",
28 "sale_stock"],29 "sale_stock"],
29 "description": """30 "description": """
30Makes it better to deal with purchases with known sale schemes, specially the following case:31Makes it better to deal with purchases with known sale schemes, specially the following case:
3132
=== modified file 'sale_dropshipping/purchase.py'
--- sale_dropshipping/purchase.py 2013-11-08 13:18:10 +0000
+++ sale_dropshipping/purchase.py 2014-06-26 12:35:37 +0000
@@ -110,3 +110,14 @@
110 'sale_id': purchase.sale_id and purchase.sale_id.id},110 'sale_id': purchase.sale_id and purchase.sale_id.id},
111 context=context)111 context=context)
112 return res112 return res
113
114 def _key_fields_for_grouping(self):
115 """Do not merge orders that have different destination addresses."""
116 field_list = super(purchase_order, self)._key_fields_for_grouping()
117 return field_list + ('dest_address_id', )
118
119 def _initial_merged_order_data(self, order):
120 """Populate the destination address in the merged order."""
121 res = super(purchase_order, self)._initial_merged_order_data(order)
122 res['dest_address_id'] = order.dest_address_id and order.dest_address_id.id or False
123 return res
113124
=== added directory 'sale_dropshipping/tests'
=== added file 'sale_dropshipping/tests/__init__.py'
--- sale_dropshipping/tests/__init__.py 1970-01-01 00:00:00 +0000
+++ sale_dropshipping/tests/__init__.py 2014-06-26 12:35:37 +0000
@@ -0,0 +1,26 @@
1# -*- coding: utf-8 -*-
2##############################################################################
3#
4# Author: Leonardo Pistone
5# Copyright 2014 Camptocamp SA
6#
7# This program is free software: you can redistribute it and/or modify
8# it under the terms of the GNU Affero General Public License as
9# published by the Free Software Foundation, either version 3 of the
10# License, or (at your option) any later version.
11#
12# This program is distributed in the hope that it will be useful,
13# but WITHOUT ANY WARRANTY; without even the implied warranty of
14# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15# GNU Affero General Public License for more details.
16#
17# You should have received a copy of the GNU Affero General Public License
18# along with this program. If not, see <http://www.gnu.org/licenses/>.
19#
20##############################################################################
21
22from . import test_merge_po
23
24checks = [
25 test_merge_po,
26]
027
=== added file 'sale_dropshipping/tests/test_merge_po.py'
--- sale_dropshipping/tests/test_merge_po.py 1970-01-01 00:00:00 +0000
+++ sale_dropshipping/tests/test_merge_po.py 2014-06-26 12:35:37 +0000
@@ -0,0 +1,35 @@
1from mock import Mock
2
3from openerp.tests.common import BaseCase
4from openerp.osv.orm import browse_record
5
6
7class TestGroupOrders(BaseCase):
8
9 def setUp(self):
10 super(TestGroupOrders, self).setUp()
11 self.order1 = Mock()
12 self.order2 = Mock()
13 # I have to use the registry to get an instance of a model. I cannot
14 # use the class constructor because that is modified to return nothing.
15 self.po = self.registry('purchase.order')
16
17 def test_two_orders_different_destination_addresses(self):
18 """Orders with the same partner, location, pricelist, but different
19 destination addresses should not be merged.
20
21 We do not care about the order lines here.
22 """
23 self.order1.partner_id = self.order2.partner_id = Mock(
24 spec=browse_record, id=1)
25 self.order1.location_id = self.order2.location_id = Mock(
26 spec=browse_record, id=2)
27 self.order1.pricelist_id = self.order2.pricelist_id = Mock(
28 spec=browse_record, id=3)
29 self.order1.order_line = self.order2.order_line = []
30
31 self.order1.dest_address_id = Mock(spec=browse_record, id=5)
32 self.order2.dest_address_id = Mock(spec=browse_record, id=6)
33
34 grouped = self.po._group_orders([self.order1, self.order2])
35 self.assertEqual(grouped, {}, u'These orders should not be merged')

Subscribers

People subscribed via source and target branches