Tryton - Issues

 

Issue6977

Title Zero quantity is updated with expected_quantity when saving new inventory line
Priority bug Status resolved
Superseder Nosy List ced, mrichez, pokoli, reviewbot, roundup-bot, tbruyere, yangoon
Type behavior Components stock
Assigned To pokoli Keywords review
Reviews 42981002, 44571002
View: 42981002, 44571002

Created on 2017-11-30.10:41:41 by mrichez, last changed by roundup-bot.

Messages
New changeset 7166c43434f7 by Sergi Almacellas Abellana in branch 'default':
Do not fill inventory quantity automatically
http://hg.tryton.org/modules/stock/rev/7166c43434f7
review44571002 updated at https://codereview.tryton.org/44571002/#ps190001
review44571002 updated at https://codereview.tryton.org/44571002/#ps170001
review44571002 updated at https://codereview.tryton.org/44571002/#ps150001
review44571002 updated at https://codereview.tryton.org/44571002/#ps130001
review44571002 updated at https://codereview.tryton.org/44571002/#ps110001
review44571002 updated at https://codereview.tryton.org/44571002/#ps90001
review44571002 updated at https://codereview.tryton.org/44571002/#ps70001
review44571002 updated at https://codereview.tryton.org/44571002/#ps50001
review44571002 updated at https://codereview.tryton.org/44571002/#ps40001
msg37596 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-01-12.13:46:04
For me, the UX is not good at all. Some users will want to consider non filled lines as zero and others as not changed.
The confirmation should take care of that.
New review44571002 at https://codereview.tryton.org/44571002/#ps1
msg37593 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-01-12.13:22:00
Here is review44571002 that makes quantity not required until done.
msg37588 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-01-12.12:58:53
Yes we must never change a value entered by the user. That's why we can not rely on the equality between quantity and expected quantity. And so we must make quantity not required and never fill it automatically. This leads to having a mechanism to decide what to do when the quantity is empty and the inventory is validated.
msg37587 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-01-12.12:55:41
So from your comments I understand that we should not update the quantity when updating values and only update the expected quantity so the user entered value is kept. Am I right?
msg37585 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-01-12.12:43:21
OK it seems there is some side effect, probably because complete_lines is called on write.
Any way, the test is against previous value or new values. So you can have the case:

- create inventory with:
   - 1 line with:
       - expected quantity: 10
       - quantity: 10
- count in the warehouse the quantity is 9
- run update inventory:
    - expected quantity is changed to: 9 (because a move has been done)
    - quantity stays to: 9
- make another move:
- run update inventory:
    - expected quantity is changed to: 8
    - quantity is changed to: 8 (but we have counted 9)
msg37584 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-01-12.12:33:33
Indeed it's tested on the scenario. See: 

http://hg.tryton.org/modules/stock/rev/b29afee7a6c7#l2.6
msg37583 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-01-12.12:29:48
I do not agree. It is not what the code does. It check quantity == expected_quantity before any changes. So it will be 10 == 10.
msg37582 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-01-12.12:13:43
With the patch of this issue your example becomes: 

- create inventory with:
   - 1 line with:
       - expected quantity: 10
       - quantity: 10
- count in the warehouse the quantity is 10
- run update inventory:
    - expected quantity is changed to: 9 (because a move has been done)
    - quantity is still 10 

So when confirming the inventory a new move of 1 unit will be created to make it 10.
msg37581 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-01-12.12:06:48
Wrong because it is not changed because it was correct initially.

I guess it require a numeral example:

- create inventory with:
   - 1 line with:
       - expected quantity: 10
       - quantity: 10
- count in the warehouse the quantity is 10
- run update inventory:
    - expected quantity is changed to: 9 (because a move has been done)
    - quantity is changed also to: 9 (but we have counted 10 in the warehouse)
msg37580 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-01-12.11:56:42
Indeed current behaviour is to not update the quantity if expected quantity is changed so msg37579 is fixed.
msg37579 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-01-12.11:16:11
Your fix is just about correct one particular case for quantity 0 but it does not fix the design issue which is to rely on the quantity and expected quantity being the same.
The case where it does not work is when you have counted the expected quantity so you do not change the quantity but on the update the expected quantity is changed so the quantity will. And this is wrong because you have counted the right quantity.
msg37578 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-01-12.11:09:37
I do not see any benefit on the proposal, just more complexity by having to manage the None values.
msg37577 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-01-12.10:48:31
As I said on IRC, we should remove the required on quantity and have an option that defines what to do with None quantity.
msg37576 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-01-12.10:34:02
> Not it is not. Your fix is just a workaround on the problem which is to detect if the quantity was set by the user or not.

So what do you propose in order to fix it?
msg37575 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-01-12.10:24:42
Not it is not. Your fix is just a workaround on the problem which is to detect if the quantity was set by the user or not.
msg37574 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2018-01-12.10:10:34
For me this discussion is a separate issue: Ensure that all the products are in the inventory. 

This issue is about adding lines with zero quantity which are wrongly overriden with expected quantity.
msg37573 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2018-01-12.10:04:21
For me this is not a good solution and this was discussed here: http://www.tryton.org/~irclog/2018-01-05.log.html#t2018-01-05%2010:32
New changeset b29afee7a6c7 by Sergi Almacellas Abellana in branch 'default':
Preserve user values when completing inventory lines
http://hg.tryton.org/modules/stock/rev/b29afee7a6c7
review42981002 updated at https://codereview.tryton.org/42981002/#ps20001
New review42981002 at https://codereview.tryton.org/42981002/#ps1
msg37366 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2017-12-19.16:30:34
Here is review42981002 that should fix it.
msg37105 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2017-11-30.15:03:49
It is a side effect of the test in update_values4complete. Maybe the test should take care of the special case when quantities are zero.
msg37102 (view) Author: [hidden] (mrichez) Date: 2017-11-30.11:10:07
This is happening only with quantity = 0, otherwise it's ok.
msg37101 (view) Author: [hidden] (yangoon) Date: 2017-11-30.11:00:47
JFTR: this works in 4.0 as expected, i.e. quantity is kept on saving. Looks like a regression was introduced (client side?).
msg37099 (view) Author: [hidden] (mrichez) Date: 2017-11-30.10:41:41
We use inventory for correcting stock errors. Sometimes a product has still a stock quantity in Tryton, but actually warehouse stock is empty. So we create an inventory, and then a new inventory line with missing product and quantity = 0. When saving, quantity is updated with expected quantity. Quantity has to be updated again with 0 and then it's ok.
History
Date User Action Args
2018-03-19 09:57:11roundup-botsetstatus: testing -> resolved
messages: + msg39063
2018-03-14 18:13:51reviewbotsetmessages: + msg38988
2018-03-09 17:49:00reviewbotsetmessages: + msg38903
2018-03-01 13:19:03reviewbotsetmessages: + msg38730
2018-02-27 17:45:17reviewbotsetmessages: + msg38650
2018-02-19 11:21:05reviewbotsetmessages: + msg38428
2018-02-12 12:53:25reviewbotsetmessages: + msg38312
2018-01-29 16:46:07reviewbotsetmessages: + msg38104
2018-01-23 10:35:09reviewbotsetmessages: + msg37866
2018-01-12 14:34:04reviewbotsetmessages: + msg37599

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