Merge lp:~akretion-team/carriers-deliveries/7-change-generate_shipping_labels-arg-dbl into lp:~camptocamp/carriers-deliveries/7.0-pending-merges-20140331

[David BEAL (ak)]

set picking in arg of generate_ship_label method
it's easier to inherit of this method

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

Not sure it is a good idea.

At least method shouldn't be public anymore as openerp standard method should use ids.

How is it supposed to be easier ?

review: Needs Information

[David BEAL (ak) (davidbeal)]

I override generate_shipping_labels().

I use picking directly without doing again browse(cr, uid, ids) to have my picking ever defined one line before the method call.

Ok to have _generate_shipping_labels()

[Leonardo Pistone (lepistone)]

I agree with Yannick there is a convention of sorts of passing lists of integers.

Since we will soon switch to the new API, I do not have a strong opinion now.

In the future (new API?), I would prefer passing some form of instance (and not ids) because in an isolated test you can easily build a test double. On the other hand, with the list of ids you would need to mock self.pool out, which is probably a pain.

review: Abstain

[Yannick Vaucher @ Camptocamp (yvaucher-c2c)]

Well self.browse(cr, uid, ids, context=context) is not that much a pain to write.

This change doesn't make it possible to keep the simple call pick.generate_shipping_labels(tracking_ids=tracking_ids)

I set this as disapprove unless you tell me there is a real gain with it, I think this is much a stylistic question.

review: Disapprove

Unmerged revisions

61. By David BEAL (ak) on 2014-03-18

[IMP] change signature generate_shipping_labels

=== modified file 'base_delivery_carrier_label/stock.py'
--- base_delivery_carrier_label/stock.py	2014-03-17 12:41:18 +0000
+++ base_delivery_carrier_label/stock.py	2014-03-18 16:16:31 +0000
@@ -53,7 +53,7 @@
                                        string='Options'),
     }
 
-    def generate_default_label(self, cr, uid, ids, tracking_ids=None,
+    def generate_default_label(self, cr, uid, picking, tracking_ids=None,
                                context=None):
         """ Abstract method
 
@@ -68,7 +68,7 @@
                 'Error',
                 'No label is configured for selected delivery method.')
 
-    def generate_shipping_labels(self, cr, uid, ids, tracking_ids=None,
+    def generate_shipping_labels(self, cr, uid, picking, tracking_ids=None,
                                  context=None):
         """Generate a shipping label by default
 
@@ -90,7 +90,7 @@
                         pack
 
         """
-        default_label = self.generate_default_label(cr, uid, ids,
+        default_label = self.generate_default_label(cr, uid, picking,
                                                     tracking_ids=tracking_ids,
                                                     context=None)
         if not tracking_ids:
@@ -114,7 +114,8 @@
         pickings = self.browse(cr, uid, ids, context=context)
 
         for pick in pickings:
-            shipping_labels = pick.generate_shipping_labels(tracking_ids=tracking_ids)
+            shipping_labels = self.generate_shipping_labels(
+                cr, uid, pick, tracking_ids=tracking_ids, context=context)
             for label in shipping_labels:
                 # map types with models
                 types = {'in': 'stock.picking.in',