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
1=== modified file 'sale_dropshipping/__openerp__.py'
2--- sale_dropshipping/__openerp__.py 2014-03-21 16:32:11 +0000
3+++ sale_dropshipping/__openerp__.py 2014-06-26 12:35:37 +0000
4@@ -25,6 +25,7 @@
5 "website": "http://www.openerp.com",
6 "category": "Generic Modules/Purchase",
7 "depends": ["purchase",
8+ "purchase_group_hooks",
9 "sale_stock"],
10 "description": """
11 Makes it better to deal with purchases with known sale schemes, specially the following case:
12
13=== modified file 'sale_dropshipping/purchase.py'
14--- sale_dropshipping/purchase.py 2013-11-08 13:18:10 +0000
15+++ sale_dropshipping/purchase.py 2014-06-26 12:35:37 +0000
16@@ -110,3 +110,14 @@
17 'sale_id': purchase.sale_id and purchase.sale_id.id},
18 context=context)
19 return res
20+
21+ def _key_fields_for_grouping(self):
22+ """Do not merge orders that have different destination addresses."""
23+ field_list = super(purchase_order, self)._key_fields_for_grouping()
24+ return field_list + ('dest_address_id', )
25+
26+ def _initial_merged_order_data(self, order):
27+ """Populate the destination address in the merged order."""
28+ res = super(purchase_order, self)._initial_merged_order_data(order)
29+ res['dest_address_id'] = order.dest_address_id and order.dest_address_id.id or False
30+ return res
31
32=== added directory 'sale_dropshipping/tests'
33=== added file 'sale_dropshipping/tests/__init__.py'
34--- sale_dropshipping/tests/__init__.py 1970-01-01 00:00:00 +0000
35+++ sale_dropshipping/tests/__init__.py 2014-06-26 12:35:37 +0000
36@@ -0,0 +1,26 @@
37+# -*- coding: utf-8 -*-
38+##############################################################################
39+#
40+# Author: Leonardo Pistone
41+# Copyright 2014 Camptocamp SA
42+#
43+# This program is free software: you can redistribute it and/or modify
44+# it under the terms of the GNU Affero General Public License as
45+# published by the Free Software Foundation, either version 3 of the
46+# License, or (at your option) any later version.
47+#
48+# This program is distributed in the hope that it will be useful,
49+# but WITHOUT ANY WARRANTY; without even the implied warranty of
50+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
51+# GNU Affero General Public License for more details.
52+#
53+# You should have received a copy of the GNU Affero General Public License
54+# along with this program. If not, see <http://www.gnu.org/licenses/>.
55+#
56+##############################################################################
57+
58+from . import test_merge_po
59+
60+checks = [
61+ test_merge_po,
62+]
63
64=== added file 'sale_dropshipping/tests/test_merge_po.py'
65--- sale_dropshipping/tests/test_merge_po.py 1970-01-01 00:00:00 +0000
66+++ sale_dropshipping/tests/test_merge_po.py 2014-06-26 12:35:37 +0000
67@@ -0,0 +1,35 @@
68+from mock import Mock
69+
70+from openerp.tests.common import BaseCase
71+from openerp.osv.orm import browse_record
72+
73+
74+class TestGroupOrders(BaseCase):
75+
76+ def setUp(self):
77+ super(TestGroupOrders, self).setUp()
78+ self.order1 = Mock()
79+ self.order2 = Mock()
80+ # I have to use the registry to get an instance of a model. I cannot
81+ # use the class constructor because that is modified to return nothing.
82+ self.po = self.registry('purchase.order')
83+
84+ def test_two_orders_different_destination_addresses(self):
85+ """Orders with the same partner, location, pricelist, but different
86+ destination addresses should not be merged.
87+
88+ We do not care about the order lines here.
89+ """
90+ self.order1.partner_id = self.order2.partner_id = Mock(
91+ spec=browse_record, id=1)
92+ self.order1.location_id = self.order2.location_id = Mock(
93+ spec=browse_record, id=2)
94+ self.order1.pricelist_id = self.order2.pricelist_id = Mock(
95+ spec=browse_record, id=3)
96+ self.order1.order_line = self.order2.order_line = []
97+
98+ self.order1.dest_address_id = Mock(spec=browse_record, id=5)
99+ self.order2.dest_address_id = Mock(spec=browse_record, id=6)
100+
101+ grouped = self.po._group_orders([self.order1, self.order2])
102+ self.assertEqual(grouped, {}, u'These orders should not be merged')

Subscribers

People subscribed via source and target branches