[6.0 / 6.1 / 7.0 ] Sale Order marked as delivered when the OUT move has not yet been processed

[Alexandre Fayolle - camptocamp]

Hello,

My customer is facing a critical bug, which results in a wrong state for the sale orders, and notifications of shipments sent to the customers before the orders have been shipped. I'll be submitting an OPW shortly.

Steps to reproduce:
---------------------------

* install a database with the demo data, and the modules sales and warehouse
* configure the Ouput stock.location of the warehouse with a manually chained move to the customer location (instead of an automatic move). This is required because my customer has a 2 step procedure where picking and shipping are two very differents steps. Moreover, sometimes the carrier cannot take all the goods and has to come back later, but the orders are still prepared and waiting in a specific place in the real world warehouse
* create a Sale Order for 1 CPU1 sold to Camptocamp
* Confirm the sale order, run the scheduler. In the History tab of the sale order, you should have 2 pickings : one INT picking containing one move from Stock to Output, and one OUT picking with one move from Output to Customer.
* Process the INT picking

Refresh the view of the Sale Order. The shipped field of the Sale Order is True, when it should still be False, as the OUT move has not been processed.

Cause of the bug
------------------------

This bug is caused by the workflow of sale.order which checks for the single procurement.order linked to each sale.order.line of the sale.order. When the sale.order.line for the product is created, the procurement is linked to the Stock -> Output picking.

When the Output -> Customer move is created by the chaining code, it should either grab the procurement order of the Stock -> Output picking or create a new procurement.order and link it to the sale.order.line.

Being able to link several procurement.orders to a single sale.order.line would be nice. Probably not possible in 6.1, but maybe in 7.0 ?

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

I've checked this issue against http://trunk_25216.runbot.openerp.com and the bug is present on 7.0 too. It is quite possible that 6.0 is also concerned.

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

I've reproduced the issue on 6.0 on runbot too.

[Joël Grand-Guillaume @ camptocamp (jgrandguillaume-c2c)]

Dear OpenERP,

I think the best way would be to forward the procurement on the new created move to respect the push philosophy. This is already the way "automatic move" is working. So, if we have :

SO 001 -> Move from : Stock -> Output ; Output -> Place 1 ; Place 1 -> Customer

The procurement generated by SO 001 should be linked to the Place 1 -> Customer move. So the workflow of the SO will respect the reality, setting it to "shipped" when reaching the customer.

My2Cents,

Joël

[Sebastien LANGE - http://www.Syleam.fr (alnslang)]

Dear Joël,

If you move the procurement in picking out, the compute procument will not check picking internal and this picking will not be available without check manually.

For me, the field must verify all picking out and not take only procurement.

My2Cents,
Sebastien

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

Hello,

Sebastien is right, with such a patch, the internal pickings always stays to "exception" when they should normally be done.

Here are 2 scenarios before and after the application of a patch which links the sale order line to the last procurement:

Before
product: 03/0584GM
so: SO045
confirmation => generate
  picking: INT/00592
           INT/00593
           OUT/00422
  procurements: Stock > Stock2 (INT/00593)
                stock2 > Output (INT/00592)
Run Scheduler => exception =>
  PO: PO00469 => confirm & process the incoming shipment => purchase procurement done
Run Scheduler =>
  procurement Stock > Stock2 ready
  reception INT/00593 (Stock > Stock2) =>
    so SO045 sale order line is done and sale is shipped, wrong
Run Scheduler =>
  procurement Stock2 Output ready
  reception INT/00593 =>
    all procurements are done
    pick OUT/00422 ready

After
product: 03/0584GM
so: SO046
confirmation =>
  picking: INT/00595
        INT/00594
        OUT/00423
  procurement: Stock > Stock2 (INT/00595)
          stock2 > Output (INT/00594)
Run Scheduler => exception "Not enough stock" on the 2 procurements
  One procurement generated for the purchase
  PO: PO00173 => confirm & process the incoming shipment => purchase procurement done
Run Scheduler => procurements in exception
Run Scheduler => procurements in exception
Run Scheduler => procurements in exception
... both procurements always stay in "Not enough stock." exception

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

A bit of context:
The goal of the procurement orders driven by the SO flow is to deliver the products to the "output" location of the warehouse. The procurement logic is correct in this respect, and when these procurement orders are "done", the goods are indeed delivered to the "output" location. This should not be changed.
However it also happens that with the default one-step-delivery mode, delivering to "output" equals delivering to "customers", and sale orders abuse this and indicate as "shipped" any order whose lines are all "delivered to output" (which often means shipped, but not always)

Resolution exploration (not sure yet what to do):
Changing the above behavior to have the "shipped" flag really mean that all products are delivered would be possible if "shipped" was replaced by a stored function field. However that would make "shipped" possibly desynchronized from the SO workflow, which could thus reach "ship_end" (or even "done") without having been fully delivered. On the other hand, making these 2 things properly synchronous with the "fully delivered logic" would require deeper changes in the workflow, possibly too much for stable versions -> this needs more investigation. In any case, I'm pretty sure that the procurement logic is fine here and must not be altered.

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

Hello Olivier,

thanks for looking into this.

Your analysis matches ours, regarding the procurement logic.

Regarding the resolution, this is really an important topic for my current customer running 6.1 (hence the OPW I opened). The current situation for the sales team is a total mess of sales marked shipped, some of which are indeed shipped and others which are not, which makes giving information on the phone to the customers inquiring about the status of their orders quite painful (and with Christmas approaching people are indeed getting touchy on that topic, and the Sales period will start shortly after that in January). The invoicing situation is also awkward, as the invoicing policy is to send the invoice when the order is shipped, which results in invoices being sent before the goods are really shipped...

For the record, the Magento connector uses the following SQL query to find out which sale orders have been completely shipped (crossing fingers in hope the formatting will be preserved)

class sale_shop(osv.osv):
    _inherit = 'sale.shop'
    def _export_shipping_query(self, cr, uid, shop, context=None):
        query = """
        SELECT stock_picking.id AS picking_id,
               sale_order.id AS order_id,
               count(pickings.id) AS picking_number
        FROM stock_picking
        LEFT JOIN sale_order
                  ON sale_order.id = stock_picking.sale_id
        LEFT JOIN stock_picking as pickings
                  ON (sale_order.id = pickings.sale_id
                      AND pickings.type='out'
                      AND pickings.state != 'cancel')
        LEFT JOIN ir_model_data
                  ON stock_picking.id = ir_model_data.res_id
                  AND ir_model_data.model = 'stock.picking'
        LEFT JOIN delivery_carrier
                  ON delivery_carrier.id = stock_picking.carrier_id
        WHERE shop_id = %(shop_id)s
              AND ir_model_data.res_id ISNULL
              AND stock_picking.state = 'done'
              AND stock_picking.type = 'out'
              AND NOT stock_picking.do_not_export
              AND (NOT delivery_carrier.export_needs_tracking
                   OR stock_picking.carrier_tracking_ref IS NOT NULL)
        GROUP BY stock_picking.id,
                 sale_order.id,
                 delivery_carrier.export_needs_tracking,
                 stock_picking.carrier_tracking_ref,
                 stock_picking.backorder_id
        ORDER BY sale_order.id ASC,
                 COALESCE(stock_picking.backorder_id, NULL, 0) ASC"""
        params = {'shop_id': shop.id}
        return query, params

    def export_shipping(self, cr, uid, ids, context):
        picking_obj = self.pool.get('stock.picking')
        for shop in self.browse(cr, uid, ids):
            cr.execute(*self._export_shipping_query(
                            cr, uid, shop, context=context))
            results = cr.dictfetchall()
            if not results:
                _logger.info("There is no shipping to export for the shop '%s' to the external referential", shop.name)
                continue
            context['conn_obj'] = shop.referential_id.external_connection()

            picking_cr = ...

[qdp (OpenERP) (qdp)]

i'll be looking into it... and so far i've only one thing to say: the formatting of the big query has been preserved :-p

[qdp (OpenERP) (qdp)]

hello there,

after few investigations, i must say that i totally agree with Olivier. Procurements flow shouldn't be touched. I think also that the SO workflow is just fine like this...

So here comes my suggestion:
1) we need to add a new functional field that will compute if the sale order has been totally delivered or not, based on picking (must have all picking in [done,cancel] and at least one picking 'done').
2) This new field will take place in the view(s) of the SO of the current 'shipped' field.
3) the shipped field is not removed, even if it's not shown in the view it can be used for internal purpose. It will receive a new label that reflects more its real nature (like 'fully procured', or 'True if all procurements have been set to done', or whatever...)
4) this also mean that we don't touch the procurement, or the SO workflow at all.

if you clearly followed me: the way i propose to fix this issue is acceptable for stable versions as well as for trunk.

I'm confirming this bug and we should soon have a merge proposal for that.

Thanks everyone for your contribution,
Quentin

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

Thanks for following up on this bug.

While you are at it, can you make sure that the field can be used in filters (it means that the function field must be stored or that you have to provide a search function for this).

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

@qdp any news?

[Javier Fuentes (javier-codeback)]

Would it be possible that solving this issue would solve this other bug (https://bugs.launchpad.net/openobject-addons/+bug/1088412) as well?. The root of the problem seems to be the same.

BTW, we have found this problem in 7.0.

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

@javier-codeback: I have an OPW opened on this, and the proposed fix does not involve fixing your issue. On 6.1, the proposed fix involves hiding part of the "wrong" information and computing a separate state for display, and not touching the sale order workflow at all. :-(

[Javier Fuentes (javier-codeback)]

@alexandre-fayolle-c2c: Thank you for the information Alexandre. Definitely, the proposed fix you mention does not seem to be the optimal solution to this problem. Let's hope it is properly handled for future versions.

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

The problem is still there in 7.0 :-(

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

Opening a new OPW in 7.0...

[Michael Karrer (michaelkarrer81)]

Any news on this one?

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

@michalkarrer81 : there are two branches with a complicated workaround for this issue, which is hackish at best and will certainly not be merged in stable.

Trunk handles things in a different way (trunk-wms that is) and the bug is not there by design.