Merge lp:~rvalyi/openobject-addons/extensible-mrp into lp:openobject-addons

[Raphaël Valyi - http://www.akretion.com]

[Raphaël Valyi - http://www.akretion.com (rvalyi)]

Hello,

This an almost backward compatible refactoring. But I grepped, no addons module is broken by it at least.

mrp action_produce_assign_product and action_po_assign methods where respectively creating several production orders and purchase orders while not judging it useful to link them to their original procurement, and passing only the last id at the end of the method. As a consequence, no method could override them properly, for instance to pass some custom options from a procurement to a production order or a purchase order.
A concrete illustration is to be found in the mrp_bom_configuration module (stable extra addons 5.0) we made when I was still at Smile: action_produce_assign_product couldn't be overriden properly and has thus been copied/pasted/hacked. So if two modules do that, then they are incompatible which is bad.

Design consideration:
There are basically two options to give the required info to potential overriders: those hopefully smart hooks such as in https://code.launchpad.net/~akretion-team/openobject-addons/extensible-inventory/+merge/20293 or return 'res' hash of associated objects, such as sale_order#action_invoice_create. I'm not sure which method is best (I the method extension point offers more performance because it optimize database access when overriden, at least when it's done thge way I did). I here opted for the {} thing because returning the id of the last created resource only was just to so plain wrong, so at least we had to remove that and the {} was the only coherent choice here. But I'm very open if somebody suggests to also add some hook like in https://code.launchpad.net/~akretion-team/openobject-addons/extensible-inventory/+merge/20293 or add it and just return True.

Still I wonder if ideally that wouldn't be better to have only one style instead of both as it is currently the case. May be a longer term reflexion for OpenERP SA for future version (though it's sad to let develop modules on a non homogeneous API meanwhile).

Don't hesitate to ask if you have any question.

[Harry (OpenERP) (hmo-tinyerp)]

Hello,

these two methods methods call from workflow.
I think, need to split them like :

=== modified file 'mrp/mrp.py'
--- mrp/mrp.py 2010-03-17 08:19:51 +0000
+++ mrp/mrp.py 2010-03-17 11:07:53 +0000
@@ -1107,7 +1107,20 @@
         return True

     def action_produce_assign_product(self, cr, uid, ids, context={}):
- produce_id = False
+ """
+ @summary : This is action which call from workflow to assign production order for procurements
+ @return : True
+ """
+ res = self.make_mo(cr, uid, ids, context=context)
+ return 1
+
+ def make_mo(self, cr, uid, ids, context={}):
+ """
+ @summary : Make Manufacturing(production) order from procurement
+
+ @return : New create Production Orders procurement wise
+ """
+ res = {}
         company = self.pool.get('res.users').browse(cr, uid, uid, context).company_id
         for procurement in self.browse(cr, uid, ids):
             res_id = procurement.move_id.id
@@ -1128,6 +1141,7 @@
                 'move_prod_id': res_id,
                 'company_id': procurement.company_id.id,
             })
+ res[procurement.id] = produce_id
             self.write(cr, uid, [procurement.id], {'state':'running'})
             bom_result = self.pool.get('mrp.production').action_compute(cr, uid,
                     [produce_id], properties=[x.id for x in procurement.property_ids])
@@ -1135,10 +1149,23 @@
             wf_service.trg_validate(uid, 'mrp.production', produce_id, 'button_confirm', cr)
             self.pool.get('stock.move').write(cr, uid, [res_id],
                     {'location_id':procurement.location_id.id})
- return produce_id
-
+ return res
+
     def action_po_assign(self, cr, uid, ids, context={}):
- purchase_id = False
+ """
+ @summary : This is action which call from workflow to assign purchase order for procurements
+ @return : True
+ """
+ res = self.make_po(cr, uid, ids, context=context)
+ return 1
+
+ def make_po(self, cr, uid, ids, context={}):
+ """
+ @summary : Make purchase order from procurement
+
+ @return : New create Purchase Orders
+ """
+ res = {}
         company = self.pool.get('res.users').browse(cr, uid, uid, context).company_id
         for procurement in self.browse(cr, uid, ids):
             res_id = procurement.move_id.id
@@ -1190,8 +1217,9 @@
                 'company_id': procurement.company_id.id,
                 'fiscal_position': partner.property_account_position and partner.property_account_position.id or False
             })
+ res[procurement.id] = purchase_id
             self.write(cr, uid, [procurement.id], {'state':'running', 'purchase_id':purchase_id})
- return purchase_id
+ return res

     def action_cancel(self, cr, uid, ids):
         todo = []

[Harry (OpenERP) (hmo-tinyerp)]

#problem: workflow is generated error if return not integer value. procurement has subflow and workflow engine has need new resource id to call subflow.
worflow should support dict or list of new created resources to call subflow.

here problem occured from workflow engine if return non integer value from workflow action: ( when I try to return list of new created resources)
-------------
[2010-03-17 17:11:27,586] ERROR:web-services:[36]: File "/media/disk/hmo/Office/Projects/OpenERP/openobject-server/bin/workflow/workitem.py", line 53, in process
[2010-03-17 17:11:27,586] ERROR:web-services:[37]: result = _execute(cr, workitem, activity, ident, stack)
[2010-03-17 17:11:27,586] ERROR:web-services:[38]: File "/media/disk/hmo/Office/Projects/OpenERP/openobject-server/bin/workflow/workitem.py", line 130, in _execute
[2010-03-17 17:11:27,586] ERROR:web-services:[39]: assert type(id_new)==type(1) or type(id_new)==type(1L), 'Wrong return value: '+str(id_new)+' '+str(type(id_new))
[2010-03-17 17:11:27,586] ERROR:web-services:[40]: AssertionError: Wrong return value: [7L] <type 'list'>
-----------------------
code in workflow/workitem.py:

....
...
...
elif activity['kind']=='subflow':
        if workitem['state']=='active':
            _state_set(cr, workitem, activity, 'running', ident)
            if activity.get('action', False):
                id_new = wkf_expr.execute(cr, ident, workitem, activity)
                if not (id_new):
                    cr.execute('delete from wkf_workitem where id=%s', (workitem['id'],))
                    return False
                assert type(id_new)==type(1) or type(id_new)==type(1L), 'Wrong return value: '+str(id_new)+' '+str(type(id_new))
                cr.execute('select id from wkf_instance where res_id=%s and wkf_id=%s', (id_new,activity['subflow_id']))
                id_new = cr.fetchone()[0]
            else:
                id_new = instance.create(cr, ident, activity['subflow_id'])
            cr.execute('update wkf_workitem set subflow_id=%s where id=%s', (id_new, workitem['id']))
            workitem['subflow_id'] = id_new
        if workitem['state']=='running':
            cr.execute("select state from wkf_instance where id=%s", (workitem['subflow_id'],))
            state= cr.fetchone()[0]
            if state=='complete':
                _state_set(cr, workitem, activity, 'complete', ident)

....
....
...

sorry I'm not expert in workflow engine but I'm think to support list or dict in this case.

Thanks to listen me.

=== modified file 'mrp/mrp.py'
--- mrp/mrp.py	2010-02-24 10:55:29 +0000
+++ mrp/mrp.py	2010-02-28 04:30:31 +0000
@@ -1115,7 +1115,7 @@
         return True
 
     def action_produce_assign_product(self, cr, uid, ids, context={}):
-        produce_id = False
+        res = {}
         company = self.pool.get('res.users').browse(cr, uid, uid, context).company_id
         for procurement in self.browse(cr, uid, ids):
             res_id = procurement.move_id.id
@@ -1136,6 +1136,7 @@
                 'move_prod_id': res_id,
                 'company_id': procurement.company_id.id,
             })
+            res[procurement.id] = produce_id
             self.write(cr, uid, [procurement.id], {'state':'running'})
             bom_result = self.pool.get('mrp.production').action_compute(cr, uid,
                     [produce_id], properties=[x.id for x in procurement.property_ids])
@@ -1143,10 +1144,10 @@
             wf_service.trg_validate(uid, 'mrp.production', produce_id, 'button_confirm', cr)
             self.pool.get('stock.move').write(cr, uid, [res_id],
                     {'location_id':procurement.location_id.id})
-        return produce_id
+        return res
 
     def action_po_assign(self, cr, uid, ids, context={}):
-        purchase_id = False
+        res = {}
         company = self.pool.get('res.users').browse(cr, uid, uid, context).company_id
         for procurement in self.browse(cr, uid, ids):
             res_id = procurement.move_id.id
@@ -1198,8 +1199,9 @@
                 'company_id': procurement.company_id.id,
                 'fiscal_position': partner.property_account_position and partner.property_account_position.id or False
             })
+            res[procurement.id] = purchase_id
             self.write(cr, uid, [procurement.id], {'state':'running', 'purchase_id':purchase_id})
-        return purchase_id
+        return res
 
     def action_cancel(self, cr, uid, ids):
         todo = []