Merge lp:~camptocamp/purchase-wkfl/7.0-merge-po-hooks-lep into lp:~purchase-core-editors/purchase-wkfl/7.0

[Leonardo Pistone]

[Pedro Manuel Baeza (pedro.baeza)]

Hi, Leonardo, very good workaround, but I think it deserves to propose this change to 7.0/trunk official branches instead of a module, so that we don't have to update module with each change/fix made in the replaced part. It doesn't imply changes in DB layout, so it can be merged in stable 7.0, but at least if we get this on trunk, it's a good win.

Regards.

review: Needs Information

[Leonardo Pistone (lepistone)]

Hi Pedro.

For trunk, I am actually going to propose it in the official branches tomorrow. See lp:~camptocamp/openobject-addons/trunk-refactor-po-merge-lep

For 7.0, I thought it would be easier to do an extra module.

Thanks!

[Leonardo Pistone (lepistone)]

Pedro, I proposed this to core trunk:

https://code.launchpad.net/~camptocamp/openobject-addons/trunk-refactor-po-merge-lep/+merge/216841

[Romain Deheele - Camptocamp (romaindeheele)]

I asked myself about the way to merge origins.
A case seems not managed:
If a PO has origins "A B" and an other PO has "B C", merge result won't be "A B C".
With standard OE processes, it's probably impossible, it's a bit a far-fetched case, so I abstain me.

review: Abstain

[Leonardo Pistone (lepistone)]

Your point makes sense, Romain.

I would rather leave it as it is, since:

1. This is just a refactor with a few tests added. It is probably better to keep the existing functionality for now.

2. I'm not sure that can happen often in practice, and the implementation could use string manipulation (brittle) or new field (complex).

Thanks!

[martin (riess82)]

I would like to suggest the following changes to purchase_group_hooks.py:

- Change _key_fields_for_grouping_lines to return a list like _key_fields_for_grouping and not a tuple
- function _update_merged_order_data exists but is not used? This might be the reason why the addition of field "origin" is not working.

[Leonardo Pistone (lepistone)]

You are right Martin, thanks.

You can set review: Needs fixing and press "save comment" (no comment is OK because you already commented).

[martin (riess82)]

review: Needs Fixing

[Leonardo Pistone (lepistone)]

I fixed martin's remarks, and added a new unit test. We're back into "needs review".

[Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c)]

LGTM

review: Approve (code review, no test)

[Leonardo Pistone (lepistone)]

Martin, can you please reconsider your review? your points should be fixed now.

[Pedro Manuel Baeza (pedro.baeza)]

I abstain for the moment until I can review the MP.

review: Abstain

[Leonardo Pistone (lepistone)]

highlights from the similar MP on trunk: Olivier Dony said they will merge on the core in trunk but not in 7.0, so this extra addon for 7.0 makes sense. Olivier made some remarks I agree with, they'll be fixed here as well ASAP.

thanks!

[martin (riess82)]

I would suggest that both _key_fields_for_grouping and _key_fields_for_grouping_lines do a return['a','b']. At the moment I get the message that the functions return tuples. If they return lists, a simple res.append('c') is sufficient to add custom fields.

review: Needs Fixing

[Leonardo Pistone (lepistone)]

Martin,

I am not convinced we need mutability here. Couldn't we still do "return res + ('c',)"?

[martin (riess82)]

Leonardo, you are right. Both methods will work.

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

martin, can you change your status here ? Or is it intended to let it in Needs Fixing ?

[martin (riess82)]

review: Approve

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

LGTM

review: Approve (code review, no test)

=== added directory 'purchase_group_hooks'
=== added file 'purchase_group_hooks/__init__.py'
--- purchase_group_hooks/__init__.py	1970-01-01 00:00:00 +0000
+++ purchase_group_hooks/__init__.py	2014-04-29 08:56:20 +0000
@@ -0,0 +1,22 @@
+# -*- coding: utf-8 -*-
+##############################################################################
+#
+#    Author: Leonardo Pistone
+#    Copyright 2014 Camptocamp SA
+#
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU Affero General Public License as
+#    published by the Free Software Foundation, either version 3 of the
+#    License, or (at your option) any later version.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU Affero General Public License for more details.
+#
+#    You should have received a copy of the GNU Affero General Public License
+#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+##############################################################################
+
+from . import purchase_group_hooks  # noqa

=== added file 'purchase_group_hooks/__openerp__.py'
--- purchase_group_hooks/__openerp__.py	1970-01-01 00:00:00 +0000
+++ purchase_group_hooks/__openerp__.py	2014-04-29 08:56:20 +0000
@@ -0,0 +1,47 @@
+# -*- coding: utf-8 -*-
+##############################################################################
+#
+#    Author: Leonardo Pistone
+#    Copyright 2014 Camptocamp SA
+#
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU Affero General Public License as
+#    published by the Free Software Foundation, either version 3 of the
+#    License, or (at your option) any later version.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU Affero General Public License for more details.
+#
+#    You should have received a copy of the GNU Affero General Public License
+#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+##############################################################################
+{'name': 'Add hooks to the merge PO feature.',
+ 'version': '0.1',
+ 'author': 'Camptocamp',
+ 'maintainer': 'Camptocamp',
+ 'category': 'Purchase Management',
+ 'complexity': "normal",
+ 'depends': ['purchase'],
+ 'description': """
+ In the core OpenERP purchase module, there is a wizard to merge purchase
+ orders. That feature is convenient, but as soon as a field is added to the
+ purchase order, it does not work anymore and needs to be patched.
+ The original implementation does not provide any hooks for extension, and
+ modules can only reimplement a method completely. This required a lot of copy
+ and paste, and worse, it breaks if two modules attempt to do that.
+
+ Therefore, this module reimplements the feature, with the same basic result
+ in the standard case. Hooks are provided for extra modules that add fields
+ or change the logic.
+ """,
+ 'website': 'http://www.camptocamp.com/',
+ 'data': [],
+ 'installable': True,
+ 'auto_install': False,
+ 'license': 'AGPL-3',
+ 'application': False,
+ 'test': ['test/merge_order.yml'],
+ }

=== added file 'purchase_group_hooks/purchase_group_hooks.py'
--- purchase_group_hooks/purchase_group_hooks.py	1970-01-01 00:00:00 +0000
+++ purchase_group_hooks/purchase_group_hooks.py	2014-04-29 08:56:20 +0000
@@ -0,0 +1,236 @@
+# -*- coding: utf-8 -*-
+##############################################################################
+#
+#    Author: Leonardo Pistone
+#    Copyright 2014 Camptocamp SA
+#
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU Affero General Public License as
+#    published by the Free Software Foundation, either version 3 of the
+#    License, or (at your option) any later version.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU Affero General Public License for more details.
+#
+#    You should have received a copy of the GNU Affero General Public License
+#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+##############################################################################
+
+from openerp.osv.orm import Model
+from openerp import netsvc
+from openerp.osv.orm import browse_record, browse_null
+
+
+class PurchaseOrder(Model):
+    _inherit = 'purchase.order'
+
+    def _key_fields_for_grouping(self):
+        """Return a list of fields used to identify orders that can be merged.
+
+        Orders that have this fields equal can be merged.
+
+        This function can be extended by other modules to modify the list.
+        """
+        return ('partner_id', 'location_id', 'pricelist_id')
+
+    def _key_fields_for_grouping_lines(self):
+        """Return a list of fields used to identify order lines that can be
+        merged.
+
+        Lines that have this fields equal can be merged.
+
+        This function can be extended by other modules to modify the list.
+        """
+        return ('name', 'date_planned', 'taxes_id', 'price_unit', 'product_id',
+                'move_dest_id', 'account_analytic_id')
+
+    def _make_key_for_grouping(self, order, fields):
+        """From an order, return a tuple to be used as a key.
+
+        If two orders have the same key, they can be merged.
+        """
+        key_list = []
+        for field in fields:
+            field_value = getattr(order, field)
+
+            if isinstance(field_value, browse_record):
+                field_value = field_value.id
+            elif isinstance(field_value, browse_null):
+                field_value = False
+            elif isinstance(field_value, list):
+                field_value = ((6, 0, tuple([v.id for v in field_value])),)
+            key_list.append((field, field_value))
+        key_list.sort()
+        return tuple(key_list)
+
+    def _can_merge(self, order):
+        """Can the order be considered for merging with others?
+
+        This method can be surcharged in other modules.
+        """
+        return order.state == 'draft'
+
+    def _initial_merged_order_data(self, order):
+        """Build the initial values of a merged order."""
+        return {
+            'origin': order.origin,
+            'date_order': order.date_order,
+            'partner_id': order.partner_id.id,
+            'dest_address_id': order.dest_address_id.id,
+            'warehouse_id': order.warehouse_id.id,
+            'location_id': order.location_id.id,
+            'pricelist_id': order.pricelist_id.id,
+            'state': 'draft',
+            'order_line': {},
+            'notes': '%s' % (order.notes or '',),
+            'fiscal_position': (
+                order.fiscal_position and order.fiscal_position.id or False
+            ),
+        }
+
+    def _update_merged_order_data(self, merged_data, order):
+        if order.date_order < merged_data['date_order']:
+            merged_data['date_order'] = order.date_order
+        if order.notes:
+            merged_data['notes'] = (
+                (merged_data['notes'] or '') + ('\n%s' % (order.notes,))
+            )
+        if order.origin:
+            if (
+                order.origin not in merged_data['origin']
+                and merged_data['origin'] not in order.origin
+            ):
+                merged_data['origin'] = (
+                    (merged_data['origin'] or '') + ' ' + order.origin
+                )
+        return merged_data
+
+    def _group_orders(self, input_orders):
+        """Return a dictionary where each element is in the form:
+
+        tuple_key: (dict_of_new_order_data, list_of_old_order_ids)
+
+        """
+        key_fields = self._key_fields_for_grouping()
+        grouped_orders = {}
+
+        if len(input_orders) < 2:
+            return {}
+
+        for input_order in input_orders:
+            key = self._make_key_for_grouping(input_order, key_fields)
+            if key in grouped_orders:
+                grouped_orders[key] = (
+                    self._update_merged_order_data(
+                        grouped_orders[key][0],
+                        input_order
+                    ),
+                    grouped_orders[key][1] + [input_order.id]
+                )
+            else:
+                grouped_orders[key] = (
+                    self._initial_merged_order_data(input_order),
+                    [input_order.id]
+                )
+            grouped_order_data = grouped_orders[key][0]
+
+            for input_line in input_order.order_line:
+                line_key = self._make_key_for_grouping(
+                    input_line,
+                    self._key_fields_for_grouping_lines()
+                )
+                o_line = grouped_order_data['order_line'].setdefault(
+                    line_key, {}
+                )
+                if o_line:
+                    # merge the line with an existing line
+                    o_line['product_qty'] += (
+                        input_line.product_qty
+                        * input_line.product_uom.factor
+                        / o_line['uom_factor']
+                    )
+                else:
+                    # append a new "standalone" line
+                    for field in ('product_qty', 'product_uom'):
+                        field_val = getattr(input_line, field)
+                        if isinstance(field_val, browse_record):
+                            field_val = field_val.id
+                        o_line[field] = field_val
+                    o_line['uom_factor'] = (
+                        input_line.product_uom
+                        and input_line.product_uom.factor
+                        or 1.0)
+
+        return self._cleanup_merged_line_data(grouped_orders)
+
+    def _cleanup_merged_line_data(self, grouped_orders):
+        """Remove keys from merged lines, and merges of 1 order."""
+        result = {}
+        for order_key, (order_data, old_ids) in grouped_orders.iteritems():
+            if len(old_ids) > 1:
+                for key, value in order_data['order_line'].iteritems():
+                    del value['uom_factor']
+                    value.update(dict(key))
+                order_data['order_line'] = [
+                    (0, 0, value)
+                    for value in order_data['order_line'].itervalues()
+                ]
+                result[order_key] = (order_data, old_ids)
+        return result
+
+    def _create_new_orders(self, cr, uid, grouped_orders, context=None):
+        """Create the new merged orders in the database.
+
+        Return a dictionary that puts the created order ids in relation to the
+        original ones, in the form
+
+        new_order_id: [old_order_1_id, old_order_2_id]
+
+        """
+        new_old_rel = {}
+        for key in grouped_orders:
+            new_order_data, old_order_ids = grouped_orders[key]
+            new_id = self.create(cr, uid, new_order_data, context=context)
+            new_old_rel[new_id] = old_order_ids
+        return new_old_rel
+
+    def _fix_workflow(self, cr, uid, new_old_rel):
+        """Fix the workflow of the old and new orders.
+
+        Specifically, cancel the old ones and assign workflows to the new ones.
+
+        """
+        wf_service = netsvc.LocalService("workflow")
+        for new_order_id in new_old_rel:
+            old_order_ids = new_old_rel[new_order_id]
+            for old_id in old_order_ids:
+                wf_service.trg_redirect(uid, 'purchase.order', old_id,
+                                        new_order_id, cr)
+                wf_service.trg_validate(uid, 'purchase.order', old_id,
+                                        'purchase_cancel', cr)
+
+    def do_merge(self, cr, uid, input_order_ids, context=None):
+        """Merge Purchase Orders.
+
+        This method replaces the original one in the purchase module because
+        it did not provide any hooks for customization.
+
+        Receive a list of order ids, and return a dictionary where each
+        element is in the form:
+
+        new_order_id: [old_order_1_id, old_order_2_id]
+
+        New orders are created, and old orders are deleted.
+
+        """
+        input_orders = self.browse(cr, uid, input_order_ids, context=context)
+        mergeable_orders = filter(self._can_merge, input_orders)
+        grouped_orders = self._group_orders(mergeable_orders)
+
+        new_old_rel = self._create_new_orders(cr, uid, grouped_orders,
+                                              context=context)
+        self._fix_workflow(cr, uid, new_old_rel)
+        return new_old_rel

=== added directory 'purchase_group_hooks/test'
=== added file 'purchase_group_hooks/test/merge_order.yml'
--- purchase_group_hooks/test/merge_order.yml	1970-01-01 00:00:00 +0000
+++ purchase_group_hooks/test/merge_order.yml	2014-04-29 08:56:20 +0000
@@ -0,0 +1,41 @@
+-
+  In order to merge RFQ, I merge two RFQ which has same supplier and check new merged order.
+-
+  !python {model: purchase.order}: |
+    new_id = self.do_merge(cr, uid, [ref('purchase.purchase_order_4'), ref('purchase.purchase_order_7')])
+    order4 = self.browse(cr, uid, ref('purchase.purchase_order_4'))
+    order7 = self.browse(cr, uid, ref('purchase.purchase_order_7'))
+    total_qty = sum([x.product_qty for x in order4.order_line] + [x.product_qty for x in order7.order_line])
+
+    assert order4.state == 'cancel', "Merged order should be canceled"
+    assert order7.state == 'cancel', "Merged order should be canceled"
+
+    def merged_data(lines):
+        product_id =[]
+        product_uom = []
+        res = {}
+        for line in lines:
+          product_id.append(line.product_id.id) 
+          product_uom.append(line.product_uom.id) 
+        res.update({'product_ids': product_id,'product_uom':product_uom})
+        return res
+
+    for order in self.browse(cr, uid, new_id.keys()):
+       total_new_qty = [x.product_qty for x in order.order_line]
+       total_new_qty = sum(total_new_qty)
+       
+       assert total_new_qty == total_qty,"product quantities are not correspond: {} != {}".format(total_new_qty, total_qty)
+       assert order.partner_id == order4.partner_id ,"partner is not correspond"
+       assert order.warehouse_id == order4.warehouse_id or order7.warehouse_id,"Warehouse is not correspond"
+       assert order.state == 'draft',"New created order state should be in draft"
+       assert order.pricelist_id == order4.pricelist_id,"Price list is not correspond"
+       assert order.date_order == order4.date_order ,"Date of order is not correspond"
+       assert order.location_id == order4.location_id ,"Location is not correspond"
+       n_product_data = merged_data(order.order_line)
+       o_product_data= merged_data(order4.order_line)
+       o_pro_data = merged_data(order7.order_line)
+       
+       assert n_product_data == o_product_data or o_pro_data,"product data are not correspond"
+       
+
+

=== added directory 'purchase_group_hooks/tests'
=== added file 'purchase_group_hooks/tests/__init__.py'
--- purchase_group_hooks/tests/__init__.py	1970-01-01 00:00:00 +0000
+++ purchase_group_hooks/tests/__init__.py	2014-04-29 08:56:20 +0000
@@ -0,0 +1,26 @@
+# -*- coding: utf-8 -*-
+##############################################################################
+#
+#    Author: Leonardo Pistone
+#    Copyright 2014 Camptocamp SA
+#
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU Affero General Public License as
+#    published by the Free Software Foundation, either version 3 of the
+#    License, or (at your option) any later version.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU Affero General Public License for more details.
+#
+#    You should have received a copy of the GNU Affero General Public License
+#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+##############################################################################
+
+from . import test_merge_po
+
+checks = [
+    test_merge_po,
+]

=== added file 'purchase_group_hooks/tests/test_merge_po.py'
--- purchase_group_hooks/tests/test_merge_po.py	1970-01-01 00:00:00 +0000
+++ purchase_group_hooks/tests/test_merge_po.py	2014-04-29 08:56:20 +0000
@@ -0,0 +1,76 @@
+from mock import Mock
+
+from openerp.tests.common import BaseCase
+from openerp.osv.orm import browse_record
+
+
+class TestGroupOrders(BaseCase):
+
+    def setUp(self):
+        super(TestGroupOrders, self).setUp()
+        self.order1 = Mock()
+        self.order2 = Mock()
+
+        self.order1.order_line = self.order2.order_line = []
+        self.order1.origin = self.order2.origin = ''
+        self.order1.notes = self.order2.notes = ''
+
+        # I have to use the registry to get an instance of a model. I cannot
+        # use the class constructor because that is modified to return nothing.
+        self.po = self.registry('purchase.order')
+
+    def test_no_orders(self):
+        """Group an empty list of orders as an empty dictionary."""
+
+        grouped = self.po._group_orders([])
+        self.assertEquals(grouped, {})
+
+    def test_one_order(self):
+        """A single order will not be grouped."""
+        grouped = self.po._group_orders([self.order1])
+        self.assertEquals(grouped, {})
+
+    def test_two_similar_orders(self):
+        """Two orders with the right conditions can be merged.
+
+        We do not care about the order lines here.
+        """
+        self.order1.partner_id = self.order2.partner_id = Mock(
+            spec=browse_record, id=1)
+        self.order1.location_id = self.order2.location_id = Mock(
+            spec=browse_record, id=2)
+        self.order1.pricelist_id = self.order2.pricelist_id = Mock(
+            spec=browse_record, id=3)
+
+        self.order1.id = 51
+        self.order2.id = 52
+
+        grouped = self.po._group_orders([self.order1, self.order2])
+        expected_key = (('location_id', 2), ('partner_id', 1),
+                        ('pricelist_id', 3))
+        self.assertEquals(grouped.keys(), [expected_key])
+        self.assertEquals(grouped[expected_key][1], [51, 52])
+
+    def test_merge_origin_and_notes(self):
+        self.order1.origin = 'ORIGIN1'
+        self.order2.origin = 'ORIGIN2'
+
+        self.order1.notes = 'Notes1'
+        self.order2.notes = 'Notes2'
+
+        self.order1.partner_id = self.order2.partner_id = Mock(
+            spec=browse_record, id=1)
+        self.order1.location_id = self.order2.location_id = Mock(
+            spec=browse_record, id=2)
+        self.order1.pricelist_id = self.order2.pricelist_id = Mock(
+            spec=browse_record, id=3)
+
+        grouped = self.po._group_orders([self.order1, self.order2])
+
+        expected_key = (('location_id', 2), ('partner_id', 1),
+                        ('pricelist_id', 3))
+
+        merged_data = grouped[expected_key][0]
+
+        self.assertEquals(merged_data['origin'], 'ORIGIN1 ORIGIN2')
+        self.assertEquals(merged_data['notes'], 'Notes1\nNotes2')