Tryton Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(433)

Issue 38931002: stock: Complete Inventory button creates lines for inactive products

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 weeks ago by albert
Modified:
1 week, 1 day ago
Reviewers:
pokoli, ced, reviewbot
Visibility:
Public.

Description

Patch Set 1 #

Patch Set 2 : Remove active check for products #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -2 lines) Patch
M inventory.py View 1 1 chunk +1 line, -2 lines 2 comments Download

Messages

Total messages: 7
albert
2 weeks ago (2017-11-10 23:57:37 UTC) #1
reviewbot
flake8 OK URL: https://codereview.tryton.org/38931002
2 weeks ago (2017-11-10 23:58:55 UTC) #2
albert
2 weeks ago (2017-11-11 00:01:40 UTC) #3
reviewbot
flake8 OK URL: https://codereview.tryton.org/38931002
2 weeks ago (2017-11-11 00:31:53 UTC) #4
pokoli
https://codereview.tryton.org/38931002/diff/20001/inventory.py File inventory.py (right): https://codereview.tryton.org/38931002/diff/20001/inventory.py#newcode230 inventory.py:230: if not (line.product.type == 'goods' Indeed, I think it's ...
1 week, 4 days ago (2017-11-13 08:22:12 UTC) #5
pokoli
Missing issue_id on description
1 week, 4 days ago (2017-11-13 08:23:05 UTC) #6
ced
1 week, 1 day ago (2017-11-16 10:06:18 UTC) #7
I find the title not descriptive.
I would go with something like: "Do not delete line of in active product"
And a description which explains that the inventory fill first with inactive
product.

https://codereview.tryton.org/38931002/diff/20001/inventory.py
File inventory.py (right):

https://codereview.tryton.org/38931002/diff/20001/inventory.py#newcode230
inventory.py:230: if not (line.product.type == 'goods'
On 2017/11/13 08:22:12, pokoli wrote:
> Indeed, I think it's easier to read without not conditions and using or
> operator. 

This is a fix. So I would prefer to keep it minimal to ease the backport. Later
you can prose a rewrite of the code if you want.
 
> But as there is a domain that ensures that the product is of type good and not
> consumable, I think that we can remove this code.

The idea is to not raise an exception if the product type has changed between
two steps of the inventory. This can happen if the product has no move in the
stock but was manually set for inventory (by mistake).
So I'm in favor of keeping it.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld bc8b922