Tryton - Issues

 

Issue6430

Title explode_bom does not delete existing inputs/outputs moves
Priority bug Status resolved
Superseder Missing delete in on_change keywords
View: 4140
Nosy List ced, nblock, pokoli, reviewbot, roundup-bot
Type behavior Components production_split
Assigned To pokoli Keywords review
Reviews 281031002
View: 281031002

Created on 2017-04-10.11:36:41 by nblock, last changed by roundup-bot.

Messages
New changeset 7c3362dbd03b by Sergi Almacellas Abellana in branch '5.4':
Remove production moves before spliting it
https://hg.tryton.org/modules/production_split/rev/7c3362dbd03b

New changeset 11ee0a591295 by Sergi Almacellas Abellana in branch '5.2':
Remove production moves before spliting it
https://hg.tryton.org/modules/production_split/rev/11ee0a591295

New changeset 54056e821c61 by Sergi Almacellas Abellana in branch '5.0':
Remove production moves before spliting it
https://hg.tryton.org/modules/production_split/rev/54056e821c61
msg57192 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-04-16.09:49:53
As inputs and outputs are One2Many, the existing moves should be deleted when running explode_bom. So I do not think there is anything else to do.
We could just backport review281031002 on older branches.
msg57191 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2020-04-16.09:30:54
issue4140 has been resolved, do I need to do anything else on this issue?
msg56303 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-03-17.13:03:04
On 2020-03-17 11:35, Sergi Almacellas Abellana wrote:
> I do not understand: issue4140 is about client on_change but our problem is on server side. So how an change to clients can fix some error on server side?

No issue4140 is both client and server side. It aims to unify the
behavior in a flexible way.

> If the clients are performing a delete on client side, we should do the same on server side, don't we?

Yes we should that the point of issue4140.

> Another question is: as issue4140 is a feature request with api change, it won't be backported to current released series. So how we fix the issue for current suported series? 

I think we can not fix it because it is a design issue.

> This bug causes a side effect of having wrong forecast quantities, so the whole stock_supply module are not creating the right requests, giving the feeling that the whole system is wrong.

Indeed so we could apply your review as a partial fix on current
releases to limit this behavior (but there could still be similar issue
with other usage of explode_bom).
But we must first fix the design issue.
msg56298 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2020-03-17.11:35:25
I do not understand: issue4140 is about client on_change but our problem is on server side. So how an change to clients can fix some error on server side?

If the clients are performing a delete on client side, we should do the same on server side, don't we?

Another question is: as issue4140 is a feature request with api change, it won't be backported to current released series. So how we fix the issue for current suported series? 

This bug causes a side effect of having wrong forecast quantities, so the whole stock_supply module are not creating the right requests, giving the feeling that the whole system is wrong.
msg56297 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-03-17.11:23:09
This is because clients interpret the 'remove' action as a delete while on the server side it is actually a remove.
So the scenario will not work if it is performed only on server-side (like the split does). This is why issue4140 is the solution because it synchronize the behavior with both sides.
msg56296 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2020-03-17.10:44:52
El 17/3/20 a les 10:36, C├ędric Krier ha escrit:
> You can create a production fill with stock move and save. Then select a bom which needs to remove current moves. The previously saved moves are still present in the database without production link instead of being deleted.

I can not reproduce this on 5.4 series. My steps:

1. Create a production for a product without bom, manually create an output move and save it.
2. Open the moves list and search for the new created product. Open it's form
3. Set the BOM and quantity of the production. The inputs and outputs are set with the BOM and the previous manual move is removed. 


When I reload the move on I've opened on step 2 I get an error that I'm tryting to read an existing move that does not exist. This demostrates that the move has been removed correctly.
msg56295 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-03-17.10:36:48
You can create a production fill with stock move and save. Then select a bom which needs to remove current moves. The previously saved moves are still present in the database without production link instead of being deleted.
msg56294 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2020-03-17.10:33:14
Which is the real problem? could you provide some steps to reproduce it?
msg56292 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-03-17.10:16:44
I do not agree. review281031002 only fix a specific use case but not the real problem. bom_explode should remove the former moves.
msg56285 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2020-03-17.09:48:23
Just to clarify: This is not related to on_change but only when spliting the production. 

I've given several testes changing the production quantities and using the explode bom button and everything works as expected. It only crashes when spliting the production. Fixing issue4140 will not solve this issue
msg56284 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2020-03-17.09:45:05
Here is review281031002 which fixes it.
msg33180 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2017-04-10.13:37:12
Indeed I just finally understood what is happening so I will describe it to let other understand also.
The method explode_bom just remove the existing inputs/outputs moves instead of deleting them. So we have draft move not linked to any production that are left.
So for me, it is not the production_split that must be fixed but the bom_explode. But unfortunately, we do not have a way to delete record via on_change see issue4140.
review36311002 updated at https://codereview.tryton.org/36311002/#ps30001
New review36311002 at https://codereview.tryton.org/36311002/#ps10003
msg33175 (view) Author: [hidden] (nblock) Date: 2017-04-10.11:36:41
The split of a production causes stale stock moves.

Steps to reproduce:
- Create products A, B, and C.
- Create a BOM with A, B as inputs and C as output (each with quantity: 1).
- Create a production of e.g. 12 pieces of C using the BOM.
- Open the forecast of the stock level of product A. You will see an expected
  stock amount of -12.
- Split the production, e.g. to 4x3.
- Reload the forecast, and you will see a number significantly different
  from -12, probably -36.
- Open the stock movements, and filter by product A. You will see two
  movements with 12 items from stock to production, and 4 movements with 3 items
  from stock to production.

Trytond: 4.2
production-split: 4.2.0
History
Date User Action Args
2020-05-01 00:18:21roundup-botsetstatus: testing -> resolved
nosy: + roundup-bot
messages: + msg57753
keyword: - backport
2020-04-16 09:49:53cedsetmessages: + msg57192
2020-04-16 09:30:55pokolisetstatus: in-progress -> testing
messages: + msg57191
keyword: + backport
2020-03-19 23:20:29cedsetstatus: testing -> in-progress
2020-03-17 13:03:04cedsetmessages: + msg56303
2020-03-17 11:35:26pokolisetmessages: + msg56298
2020-03-17 11:23:09cedsetmessages: + msg56297
2020-03-17 10:44:53pokolisetmessages: + msg56296
2020-03-17 10:36:48cedsetmessages: + msg56295
2020-03-17 10:33:15pokolisetmessages: + msg56294

Showing 10 items. Show all history (warning: this could be VERY long)