[5.0][sale] Cancelling an out picking sets the sale order as shipped

Hi,

I confirm the bug is present !! This is quite unfortunate, cause with the "Invoice on Order After Delivery" in SO, the invoiceis also generated if picking is canceled !!!

This is really unacceptable ! A cancel picking should never ever generate an invoice in that case !

The patch fix the problem, please include this in next release.

Regards,

Joël

Hi Jay,

I assigned the bug to you, cause we really need this fix ASAP. Could you please make something for us ?

Regards,

Joël

+1

Hello Joel and Omar,

I have a little disagreement with this patch.

Have you tried with a partial picking where one picking is done and another is cancelled?

I would like to request you to apply this patch.

Thanks.

Hi Jay,

First thank you for your review ! I tested the patch and it doesn't seems to work for me:

- Making an SO with 1 product, confirm order, cancel shipping => the "shipped" checkbox is ticked and the invoice generated when "Invoice on Order After Delivery"

- Making an SO with 2 product, confirm order, accept one product and cancel the second one => the "shipped" product is ticked

With the last patch, both were working as "shipped" was not ticked.

What's your opinion ?

Regards,

Joël

Hi Joël,

Both the cases seem working fine.

I wonder how is that!

My patch simply deals with SHIPPED if you cancelled the packing and the relevant Sale order has more than one pickings.

You talk about canceling moves I guess.

Please correct me.

Thanks.

Hi Jay,

In your patch, you only wrote a condition for picking_ids isn't empty [], I didn't see any other difference, but, this issue continues because:

In action_cancel() inheritance of sale module (ids = pickings_to_cancel) first, it calls to his base (super) method, therefore, 'ids' was set to state = 'cancel', then, in the inheritance it calls to action_ship_end() (method that set shipped = 'True' in sale order) if all sale pickings state are in ['done','cancel'], therefore if sale has one unique picking and his pick state is done it resets shipped = True or if picking is cancelled it sets shipped = True in sale (it's the behavior because you just cancelled a picking (action_cancel())).

The issue is that in cancel state, you shouldn't set shipped = True in sale. My patch reverses this if statement ('in' instead of 'not in'). Then if all pickings was in cancel or/and done states, it doesn't do anything, because if all or some pickings are done, sale order already was shipped = True, and if all pickings is cancelled it doesn't set shipped=True in sale order.

In your use case (partial pickings), when you cancel any of pickings, it doesn't set to shipped the sale, patch works for me, if the other picking was done, and, then when you cancel current picking, you want to set to shipped=True, you should manage it in this case, but is only in this case when the original source or your patch works correctly. But, really, the shipment isn't full.

Thanks for review.

Thanks for the study Omar.

Long story short, 'Shipped' should not be made true when you cancel the picking.

I used these cases:
1. SO has One picking, you cancel it, SHIPPED is not true now. Earlier it was getting true.
2. SO has one picking, I did partial picking. So there are 2 pickings on SO now. One is available, one is done. I cancel the available one. So the code was earlier setting shipped=True, and now too it can set shipped =True (with my patch). I guess that with your patch,the flag will be set False and ship will not be ticked. You say that it should not be set shipped?

Thanks again.

The bug which made current code available : https://bugs.launchpad.net/openobject-addons/+bug/399817.
Thanks.

Dear all,

You get our point I think ! I vote for your patch.

Complete shipping cancel => Shipped = True
Partial shipping done, partial shipping cancel => Shipped =True

Thanks for the fix.

Regards,

Joël

Hi Jay,

Other case, with your patch, if I confirm the sale order, it creates one picking, then I cancel this picking, shipped is not true, it works, but, I return to confirm this sale order and it creates one new picking, therefore if I cancel the current picking it sets shipped to True.

Can we manage the partial picking case checking backorder_id field on stock.picking?

Like:

             call_ship_end = True
             if pick.sale_id:
                 for picks in pick.sale_id.picking_ids:
- if picks.state not in ('done','cancel'):
+ if picks.state in ('done','cancel') and (not picks.backorder_id or picks.backorder_id.state != 'done'):
                         call_ship_end = False
                         break
                 if call_ship_end:

I attach the solution which I wrote in last comment, I can't test it now. Please review it.

Thanks

Hi Jay,

I finish to test my last patch and it doesn't work, but I redesigned it, I'm considering your case, therefore, I reattach the patch.

I tested:

Finish to done one picking. SO shipped.
Cancel one picking. SO not shipped.
Partial picking: I finished with partial picking button, the first picking, then, I cancelled the other picking. SO shipped.

Please test it when you can.

I will check and test asap.
Thanks.

Omar,

I have checked both the patches and they seem to solve issue.

But My view is,

Patch from Jay(OpenERP) considers all pickings related to the Sale order of to-be-cancelled picking.

Patch from you considers all pickings(backorder which is in turn the relevant picking of the to-be-cancelled picking.)

However, I have seen your patch not behaving as per expectations where there are mer than one partial pickings made.

Go through this example:
Create a sale order of Qty 10.
Confirm it.
Look at the picking attached.
Pick it partially for qty 5.
There are 2 picking now, 1 is done, another is available.
Go to the available one, pick it partially for qty 2.
Now the picking with qty 2 is done,qty 5 is done.
BUT the picking with qty 3 is available, don't wait anymore and cancel that one.

Logically, it should make the SO PICKED,but it does not do with your patch Omar as you worked on backorder.

Let me know your views.

Thanks.

Hi DHS,

Yes, your view is correct. But, test this with Jay patch:

Confirm one sale order
It creates one picking, then, cancel this picking, shipped field is not True.
Return to confirm this sale order then it creates one new picking,
Then return to cancel the current picking, it sets shipped to True.

When I test this case with Jay patch, this case fails. Maybe we have to write another patch...

Thanks DHS and Omar,

I see one problem here that how would a user be able to return a packing which is cancelled?

I am checking in that direction.

Thanks.

Updates...............

As it is checked in Picking and partial picking wizard, return picking wizard also should check about the state of the picking.

Thanks.

Please refer to the bug 686597.

Hi Omar,

With the fix of bug 686597, The patch from Comment #5 works as per expectations.

Let me know if you have more queries regarding this, so we can proceed to close this one.

Thanks for your participation and the follow-up.

Hello Omar,

Can we have an update?

Thanks.

Hi Jay and Dhruti,

I was finally able to test it today, if you do single cancellation of picking it works, if you do partial picking, it works but if you cancel two times the picking or more it doesn't work, then, if you cancel sale order, the shipped field sets to False but while you don't cancel sale order, it is shipped with its two pickings canceled.

Sorry for the delay.

Hello Omar,

The patch seems to work as expected.
It solves the issues.

However, I am not able to follow your last comment.
Would you please elaborate the latter part of your comment "but if you cancel two times the picking or more it doesn't work, ... " ?

Awaiting your response so we can come with a fix soon.

Regards,
JMA.

JMA,Jay and Omar,

I confirm the issue.

This happens as soon as we cancel the SO itself and reconfirm it.

Thanks Omar for the peer review.

Hello Omar,

Will you please check with the latest patch and let us know if anything is wrong with the patch?

Thanks.

Hi all,

It works, with last patch attached for Dhruti, it works well, thanks you.

Thanks for the patch Dhruti,thanks for the feedback Omar.
Fix has been applied to stable 5.0 by revision 2901 <email address hidden>.