[stock] picking cancels done moves (5.0)

[Dukai Gábor]

Hi!
5.0 latest bzr.
When a picking is cancelled, it cancels all moves, even if the moves are in done state. 'done' moves should never be altered.

Test case:
A picking with 3 moves. 2 of them are assigned, 1 is only confirmed because it doesn't have enough stock. Make the picking done and write the originally requested amount for move 3 instead of 0 in the partial picking wizard. Force the picking.
Now there are 2 done lines and an assigned line. The stock worker changes his mind and cancels the picking. All 3 lines become cancelled.

Fix is provided in the attached patch.

[gpa(OpenERP) (gpa-openerp)]

Hello Dukai,

Sorry for the interruption.

Working on the situations provided by you:
Situation 1:

Make the picking done and let the 0 qty move be 0.
2 packings come into picture. If you cancel x picking, its correspondent moves are cancelled, which seems quite natural.

Situation 2:
Make the picking done and write the originally requested amount for move 3 instead of 0 in the partial picking wizard. Here its only 1 packing in the picture. If you cancel that picking, undoubtedly all the related moves have to be cancelled in order to affect the real/virtual stock.

I would like to ask you for more clarifications.

Thanks.

[Dukai Gábor (gdukai)]

There must be some misunderstanding, I wrote only one test case.
Here is a detailed description:
-start:
  a picking (state: assigned),
    move1: qty 1, state: assigned,
    move2: qty 1, state: assigned,
    move3: qty 1, state: confirmed (out of stock),
-partial picking wizard:
  move1: qty 1
  move2: qty 1
  move3: qty 1
-results in:
  move1: done
  move2: done
  move3: confirmed
-force button on the picking
results in:
  move1: done
  move2: done
  move3: assigned
-cancel button on the picking
  move1: cancelled ->BUG
  move2: cancelled ->BUG
  move3: cancelled

'done' moves can never be changed.

[gpa(OpenERP) (gpa-openerp)]

Hello Dukai,

I completely agree with what you have illustrated.
The same situation occurs at my end too.

But what I see is, if the Picking is cancelled,why the concerned moves stay done? this is where I am stuck.

Can we ask this question in expert list?

Thanks.

[Jan Verlaan (jan-verlaan)]

From request in the expert list;
Moves in "Done" state should NEVER be cancelled. Those materials are shipped already to the next stage (that could be the customer) and invoices are created and perhaps already paid!
The flow to follow for Moves in "Done" state that should be shipped back to the original location is via Return Goods functionality.

Only Moves in state <> Done can be cancelled.

[Jan Verlaan (jan-verlaan)]

I think it is even more worse then that.
If the order is created with "invoice from order" parameter, the invoice could already be sent for the whole order. Should we then allow a canceling on the picking without any action to the invoice as it is now? We have a serious issue here.

If the order is created with "invoice from picking" parameter the invoices are created at picking time, so then a canceling is possible before a picking is done, afterwards only a return goods will generate a credit note that can be sent to customer if the original invoice was sent already, otherwise it can be reconciled internally!

[Ferdinand (office-chricar)]

To be precise (good accouting practice requires this)
"Cancel" of a "done" picking must generate a new picking and all dependant documents (except payments of course) with "negative" qty and value of the original document(s).
For this to happen OpenER absolutely needs to know ALL relations between SO/PO - Picking - Invoice (which currently is not the case)
I know negative values are not allowed everywhere .... hence devit/credit must be reversed ....

These new documents follow the workflow of the original documents and can be reconciled automatically.

A problem persists - how to handle price differences of stock moves after the original move (only if average price is selected)

[Jan Verlaan (jan-verlaan)]

@Ferdinand Price differences after move are handeld in this bug tracker. That is also a issue in open state.
https://bugs.launchpad.net/openobject-addons/+bug/527120

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

Comment about the proposed patch: functionally it seems correct to me, but technically why not use the existing test_cancel() method that exists for this purpose, instead of adding one more method with the same code?
Even better, why not wrap the existing test_cancel() method in a check_cancel() method that calls it and raises an error when it can't be cancelled. So instead of having a button that does nothing users would see a meaningful message?

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

Now to answer other comments: in OpenERP rather than accepting false "cancel" actions on document that can't be cancelled (typically invoices and pickings), we currently provide wizards to create the reversed documents. For example you can use the return picking wizard on a picking, or the refund invoice wizard on an invoice.

As stock moves are usually less strict than invoices in legal terms, and often more error-prone (missing traceability info, packs, etc.), in v6 we will attempt to make the pickings/moves partially editable even after being processed, to be able fix these kinds of errors (except some main fields that won't be modifiable: products, quantities, locations, etc.). In terms of completely cancelling the document however, we still won't allow doing it, a reversed one must be created instead (and traceability will include the return accordingly)

[Samantha (samantha-z-mathews)]

if i understnd correctly my vote for Dukai Gábor.

[Dukai Gábor (gdukai)]

Olivier Dony:

The patch does exactly what you said is needed: prevent pickings to be cancelled if some lines were already done.

allow_cancel() is added because it does a totally different thing than test_cancel():
--allow_cancel() checks if any line is in done state --> cancel should be disallowed
--test_cancel() checks if all lines are in cancel state --> picking will be moved to cancel state automatically by the workflow

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

@Dukai Gábor:

You are of course right, I totally misread the proposed patch, sorry! :-)

I will ask our team to merge it, but I think it would still be great to have allow_cancel raise an exception with a clear message instead, so people pressing the Cancel button understand why it does not do anything (it should perhaps be named check_cancel if it does not return a boolean)

[Ferdinand (office-chricar)]

https://bugs.launchpad.net/openobject-addons/+bug/528320/comments/10

Quote "As stock moves are usually less strict...."
this is NOT the case if "immediate" posting is used, this creates accounting moves instantly and hence these must be reverted too as these can definitely not be modified without braking basic accounting rules.

Naming "Return picking wizard"
IMHO we need to distinguish between
* revert picking - this should be used to undo the workflow because of an error and allow to process the picking again.
* return picking - this should be used if a shipment was done in reality and the goods are returned by the partner.

[Kirti Savalia(OpenERP) (ksa-openerp)]

Hello Dukai,

 it has been fixed in branch lp:~~openerp/openobject-addons/trunk
 Revision ID: <email address hidden>

Thanks.