ForgeFlow / stock-rma

Stock RMA
GNU Affero General Public License v3.0
38 stars 41 forks source link

[16][IMP] rma : propagate cancelation #539

Open florian-dacosta opened 1 month ago

florian-dacosta commented 1 month ago

Replace https://github.com/ForgeFlow/stock-rma/pull/429

With current code, in the case you create a RMA group with 2 rma lines, if you cancel one rma line, the whole picking related to the rma line is canceled, so the second rma line can't be done.

The goal of this PR is to avoid this, and also to manage the propagation of the cancelation on all origin stock moves related to the rma line. For destination moves, it should be configurable on the stock rules.

Like commented in https://github.com/ForgeFlow/stock-rma/pull/429 I don't understand the use case covered with the current code. A example on how to reproduce a process where it is usefull would certainly help me!

I'll set the PR in draft for now, since the goal for now is more to discuss on how can we avoid the problematic of the rma group use while continuing covering the current use case (which I don't get)

@AaronHForgeFlow @DavidFIB

DavidJForgeFlow commented 1 month ago

Hi @florian-dacosta, IMO this seems to be a good option, as you say this should fix the behavior of the RMA Groups while maintining the base functionality for the RMA Lines. I would appreciate some tests to make sure the features works as expected, as it wasn't before tested. Thank you.

Also the user @DavidFIB is an outdated profile, so you can tag @DavidJForgeFlow instead!

florian-dacosta commented 1 month ago

Thanks for the feedback. Well, like I said, I did not observe (while testing) the cancelation in chain in case of multi step picking, with the current code. So I am not sure I "maintain" the base functionality... But the cancelation in chain in case of multi step picking seems to be what is expected so I hope it is fine.

I will add some tests about it, and once ok, will remove the WIP mark of the PR.

florian-dacosta commented 2 weeks ago

@DavidJForgeFlow @AaronHForgeFlow Here, I have added a test. My main goal here is to avoid canceling the whole picking when a single rma line is canceled, in the case of rma.order with multiple lines.

If you play the new test without the changes on action_rma_cancel (on present v16), you will see it will fail on a test where the picking is canceled, where it should not. This is what I want to prevent with this PR.

Let me know what you think.