Merge lp:~manu-tiedra/openobject-addons/trunk into lp:openobject-addons

[Manu]

[IMP] Delivery module: support for carrier tracking when using the "Delivery Cost" button in a sale order

We can not work with the current delivery module as is because we need to track the delivery.carrier assigned to each picking in order to create a text file that integrates with the logistic platform of the carrier.
The standard delivery module has a field called carrier_id, displayed as Delivery Method that is used to track the carrier used but also has a second meaning: when the field is set, it means that the system should compute the shipping costs of the picking when creating the invoice for the picking.

Unfortunately there are companies that use the "Delivery Costs" button in the sales order to add the delivery costs to the sale order. When this is done, filling the Delivery Method field of the sale.order would cause the invoice to have another line added because that field is used to indicate the delivery.carrier and that the system should compute the shipping costs of the picking when creating the invoice for the picking. This led these kind of companies with the problem that they can not track the carrier used in the shipments. However, this can easily be fixed as I have done:

* I have added another field to the sale.order and to the stock.picking called invoice_shipping_after_picking (displayed as Auto-Invoice Delivery) that controls if the system needs to add the delivery line to the invoice.
* the carrier_id field (displayed as Delivery Method) will be used to indicate the carrier used to ship the order, and in the picking the carrier used to ship the picking. This way I have splitted the previous functionality of the carrier_id field in two fields as it was really doing two things (specifying the carrier and telling the system to invoice the delivery after picking).
* I have changed the wizard that adds the delivery costs to a sale.order. Now when the delivery costs are added to the sale order, the carrier_id is set to the carrier selected in the wizard and the invoice_shipping_after_picking field is set to false as the delivery line has been added. If the user never clicks on the Delivery Cost button, the system will behave as expected as the default value for invoice_shipping_after_picking is true.
* when creating the invoice from the picking, I check the invoice_shipping_after_picking field and if it is not set, it won't try to add the delivery line to the invoices.
* this branch also fixes bug 625235

With this solution you can know the carrier used if you add the delivery costs manually or if they get added later, so it seems more natural.

As I said before, this changes are critical for us now so please tell me if I need to change anything to get this merged ASAP.

Thank you very much.

[IMP] Delivery module: support for carrier tracking when using the "Delivery Cost" button in a sale order

We can not work with the current delivery module as is because we need to track the delivery.carrier assigned to each picking in order to create a text file that integrates with the logistic platform of the carrier.
The standard delivery module has a field called carrier_id, displayed as Delivery Method that is used to track the carrier used but also has a second meaning: when the field is set, it means that the system should compute the shipping costs of the picking when creating the invoice for the picking.

Unfortunately there are companies that use the "Delivery Costs" button in the sales order to add the delivery costs to the sale order. When this is done, filling the Delivery Method field of the sale.order would cause the invoice to have another line added because that field is used to indicate the delivery.carrier and that the system should compute the shipping costs of the picking when creating the invoice for the picking. This led these kind of companies with the problem that they can not track the carrier used in the shipments. However, this can easily be fixed as I have done:

* I have added another field to the sale.order and to the stock.picking called invoice_shipping_after_picking (displayed as Auto-Invoice Delivery) that controls if the system needs to add the delivery line to the invoice.
* the carrier_id field (displayed as Delivery Method) will be used to indicate the carrier used to ship the order, and in the picking the carrier used to ship the picking. This way I have splitted the previous functionality of the carrier_id field in two fields as it was really doing two things (specifying the carrier and telling the system to invoice the delivery after picking).
* I have changed the wizard that adds the delivery costs to a sale.order. Now when the delivery costs are added to the sale order, the carrier_id is set to the carrier selected in the wizard and the invoice_shipping_after_picking field is set to false as the delivery line has been added. If the user never clicks on the Delivery Cost button, the system will behave as expected as the default value for invoice_shipping_after_picking is true.
* when creating the invoice from the picking, I check the invoice_shipping_after_picking field and if it is not set, it won't try to add the delivery line to the invoices.
* this branch also fixes bug 625235

With this solution you can know the carrier used if you add the delivery costs manually or if they get added later, so it seems more natural.

As I said before, this changes are critical for us now so please tell me if I need to change anything to get this merged ASAP.

Thank you very much.

[Manu (manu-tiedra)]

Any news on this one? I got a "rejected" and "needs review" messages from Fabien last month but nothing new since then.

[Olivier Dony (Odoo) (odo-openerp)]

Hello Manu,

Sorry for the response delay, we're rather overloaded with merge proposals and the preparation of OpenERP 6.1.
Your merge proposal looks good on a technical level, but functionally we'd prefer to keep this feature in an extra module rather than modifying the logic of the original delivery module (currently the carrier_id field added by the delivery module on sale_order has only one meaning: the invoicing to be done).

I imagine it would have been quite dirty to do this in an extra module with the current state of the code, so I've just applied a quick refactoring to the delivery module, extracting a new '_prepare_shipping_invoice_line' method for preparing the creation of the shipping invoice line (see [2]). This should make it easier for you to implement what you need in an extra module.

Once your module is ready, don't hesitate to publish it on OpenERP Apps, it may be useful for other users!
See also our updated merge proposal policy and guidelines [3].

Thanks for your understanding and for your contributions!

PS: Concerning bug 625235, you'll be glad to see it was finally fixed along with bug 816138, so the patch is unnecessary now.

[1] addons rev.5395 revid:<email address hidden>
[2] http://bazaar.launchpad.net/~<email address hidden>
[3] http://bit.ly/openerp-contrib-mp

[Manu (manu-tiedra)]

Olivier,

thank you very much for looking into it.

I was afraid that you would reject my merge proposal. However, in that case, what I really wanted was the possibility to add the functionallity I needed to the delivery module (currently it was complex to add as the module wasn't extensible). So I'm really glad you made those changes.

I will add my changes on the top of that as a module.

Keep on your great work! 6.1 will be awesome.

Unmerged revisions

5290. By Manu on 2011-10-03

We can not work with the current delivery module as is because we need to track the delivery.carrier assigned to each picking in order to create a text file that integrates with the logistic platform of the carrier.
The standard delivery module has a field called carrier_id, displayed as Delivery Method that is used to track the carrier used but also has a second meaning: when the field is set, it means that the system should compute the shipping costs of the picking when creating the invoice for the picking.

Unfortunately there are companies that use the "Delivery Costs" button in the sales order to add the delivery costs to the sale order. When this is done, filling the Delivery Method field of the sale.order would cause the invoice to have another line added because that field is used to indicate the delivery.carrier and that the system should compute the shipping costs of the picking when creating the invoice for the picking. This led these kind of companies with the problem that they can not track the carrier used in the shipments. However, this can easily be fixed as I have done:

* I have added another field to the sale.order and to the stock.picking called invoice_shipping_after_picking (displayed as Auto-Invoice Delivery) that controls if the system needs to add the delivery line to the invoice.
* the carrier_id field (displayed as Delivery Method) will be used to indicate the carrier used to ship the order, and in the picking the carrier used to ship the picking. This way I have splitted the previous functionality of the carrier_id field in two fields as it was really doing two things (specifying the carrier and telling the system to invoice the delivery after picking).
* I have changed the wizard that adds the delivery costs to a sale.order. Now when the delivery costs are added to the sale order, the carrier_id is set to the carrier selected in the wizard and the invoice_shipping_after_picking field is set to false as the delivery line has been added. If the user never clicks on the Delivery Cost button, the system will behave as expected as the default value for invoice_shipping_after_picking is true.
* when creating the invoice from the picking, I check the invoice_shipping_after_picking field and if it is not set, it won't try to add the delivery line to the invoices.
* this branch also fixes bug 625235

With this solution you can know the carrier used if you add the delivery costs manually or if they get added later, so it seems more natural.

As I said before, this changes are critical for us now so please tell me if I need to change anything to get this merged ASAP.

Thank you very much.

=== modified file 'delivery/delivery_view.xml'
--- delivery/delivery_view.xml	2011-10-02 16:12:38 +0000
+++ delivery/delivery_view.xml	2011-10-03 14:34:22 +0000
@@ -179,6 +179,7 @@
                 <group name="logistics" position="inside">
                     <field name="id" invisible="True"/>
                     <field name="carrier_id" context="{'order_id':active_id or False}"/>
+                    <field name="invoice_shipping_after_picking"/>
                 </group>
             </field>
         </record>
@@ -193,6 +194,7 @@
                     <field name="carrier_id"/>
                     <field name="carrier_tracking_ref" groups="base.group_extended"/>
                     <field name="number_of_packages" groups="base.group_extended"/>
+                    <field name="invoice_shipping_after_picking"/>
                     <field name="weight"/>
                     <field name="weight_net"/>
                 </field>

=== modified file 'delivery/partner.py'
--- delivery/partner.py	2011-07-05 12:28:57 +0000
+++ delivery/partner.py	2011-10-03 14:34:22 +0000
@@ -30,7 +30,7 @@
           relation='delivery.carrier',
           string="Delivery Method",
           view_load=True,
-          help="This delivery method will be used when invoicing from picking."),
+          help="The default delivery method for this partner."),
     }
 res_partner()
 

=== modified file 'delivery/sale.py'
--- delivery/sale.py	2011-01-14 00:11:01 +0000
+++ delivery/sale.py	2011-10-03 14:34:22 +0000
@@ -25,10 +25,15 @@
 class sale_order(osv.osv):
     _inherit = 'sale.order'
     _columns = {
-        'carrier_id':fields.many2one("delivery.carrier", "Delivery Method", help="Complete this field if you plan to invoice the shipping based on picking."),
+        'carrier_id':fields.many2one("delivery.carrier", "Delivery Method", help="Carrier used for shipping"),
+        'invoice_shipping_after_picking': fields.boolean("Auto-Invoice Delivery", help="Check this field if you plan to invoice the shipping based on picking."),
         'id': fields.integer('ID', readonly=True,invisible=True),
     }
 
+    _defaults = {
+        'invoice_shipping_after_picking': lambda *a: 1,
+    }
+
     def onchange_partner_id(self, cr, uid, ids, part):
         result = super(sale_order, self).onchange_partner_id(cr, uid, ids, part)
         if part:
@@ -42,6 +47,7 @@
             pids = [ x.id for x in order.picking_ids]
             self.pool.get('stock.picking').write(cr, uid, pids, {
                 'carrier_id':order.carrier_id.id,
+                'invoice_shipping_after_picking': order.invoice_shipping_after_picking,
             })
         return result
 sale_order()

=== modified file 'delivery/stock.py'
--- delivery/stock.py	2011-07-01 23:41:24 +0000
+++ delivery/stock.py	2011-10-03 14:34:22 +0000
@@ -52,7 +52,8 @@
         return result.keys()
     
     _columns = {
-        'carrier_id':fields.many2one("delivery.carrier","Carrier"),
+        'carrier_id':fields.many2one("delivery.carrier","Carrier used for shipping"),
+        'invoice_shipping_after_picking': fields.boolean("Auto-Invoice Delivery", help="Check this field if you plan to invoice the shipping based on picking."),
         'volume': fields.float('Volume'),
         'weight': fields.function(_cal_weight, type='float', string='Weight', digits_compute= dp.get_precision('Stock Weight'), multi='_cal_weight',
                   store={
@@ -68,6 +69,10 @@
         'number_of_packages': fields.integer('Number of Packages'),
         }
 
+    _defaults = {
+        'invoice_shipping_after_picking': lambda *a: 1,
+    }
+
     def action_invoice_create(self, cursor, user, ids, journal_id=False,
             group=False, type='out_invoice', context=None):
         invoice_obj = self.pool.get('account.invoice')
@@ -90,6 +95,8 @@
 
         for picking in picking_obj.browse(cursor, user, picking_ids,
                 context=context):
+            if not picking.invoice_shipping_after_picking:
+                continue
             if not picking.carrier_id:
                 continue
             grid_id = carrier_obj.grid_get(cursor, user, [picking.carrier_id.id],
@@ -128,6 +135,9 @@
                 'quantity': 1,
                 'invoice_line_tax_id': [(6, 0,taxes_ids)],
             })
+
+        # recalculates taxes because a line may have been added to the invoices in the previous code
+        invoice_obj.button_compute(cursor, user,invoice_ids, context=context, set_total=True)
         return result
 
 stock_picking()

=== modified file 'delivery/wizard/delivery_sale_order.py'
--- delivery/wizard/delivery_sale_order.py	2011-05-19 12:13:36 +0000
+++ delivery/wizard/delivery_sale_order.py	2011-10-03 14:34:22 +0000
@@ -26,7 +26,7 @@
 
 class make_delivery(osv.osv_memory):
     _name = "delivery.sale.order"
-    _description = 'Make Delievery'
+    _description = 'Make Delivery'
 
     _columns = {
         'carrier_id': fields.many2one('delivery.carrier','Delivery Method', required=True),
@@ -111,6 +111,7 @@
                     'tax_id': [(6,0,taxes_ids)],
                     'type': 'make_to_stock'
                 })
+                order_obj.write(cr, uid, [order.id], {'carrier_id': grid.carrier_id.id, 'invoice_shipping_after_picking': False})
     
         return {'type': 'ir.actions.act_window_close'}