search

partkeepr / PartKeepr

Open Source Inventory Management
http://www.partkeepr.org
GNU General Public License v3.0
1.38k stars 400 forks source link

Negative stock levels are possible #1113

Closed christianlupus closed 3 years ago

christianlupus commented 4 years ago

Bug description

There is no logic that prevents a user from reducing the stock level beyond zero.

Steps to reproduce

  1. Go to part list and select a part with stock > 0.
  2. Click in the part details on Remove stock
  3. Remove more parts than currently present

Expected behavior

A warning appears and the stock is not removed.

Observed behavior

The stock is removed causing the total stock after the operation to drop below zero.

System Information

JavaScript errors

no errors found as it is a business logic problem.

christianlupus commented 4 years ago

The solution provided in #1126 solves this issue only partly. At least two parts are missing:

  1. The backend should plainly refuse to store negative numbers. This will cover all sorts of synchronization problems (like multiple users, non-updated browser tab,...) in a central place. How this is handled in detail can be discussed, here I only put the statement that we should/can not rely solely on the frontend.
  2. The user can also put a decrement in the large table. When a user clicks to edit the stock level in the parts table and inserts a negative number, this is interpreted as a decrease in the stock level (see this part). Here a test should be inserted as well into the frontend.
mchateauvert commented 3 years ago

@christianlupus - I started to look through this as it seemed a easy first contribution, but came across some code that gave me pause.

In PartKeepr/PartBundle/Entity/Part.php -> recomputeStockLevels(), negative stock levels are explicitly handled when looping through associated StockLevel objects.

Is is possible that allowing negative stock levels is intended behavior? One potential scenario is that someone has added stock to a location (physically) without logging the change in Partkeepr. The next person to remove some pieces might then leave the system tracking a negative stock. I think potentially the "right" way to handle this is to allow the second person to accurately log their transaction, and track a temporary negative stock level with a prompt or flag somehow to indicate that a correction to a previous stock transaction is required.

Any thoughts? Perhaps @Drachenkaetzchen can clarify the intended behavior?

Drachenkaetzchen commented 3 years ago

Is is possible that allowing negative stock levels is intended behavior? One potential scenario is that someone has added stock to a location (physically) without logging the change in Partkeepr. The next person to remove some pieces might then leave the system tracking a negative stock. I think potentially the "right" way to handle this is to allow the second person to accurately log their transaction, and track a temporary negative stock level with a prompt or flag somehow to indicate that a correction to a previous stock transaction is required.

Any thoughts? Perhaps @Drachenkaetzchen can clarify the intended behavior?

It's exactly what you are writing. If I go to a bin and remove 2 items, I should be able to reflect that in PartKeepr as well. For regular manual removals, you most likely would notice it and maybe correct the existing stock. Example: The box has 2 items, PartKeepr knows about 1, you want to remove 2 but remove 1 instead to fix the incorrect stock.

But that's not always the case. If you have a barcode reader and use the scenario from above, you would not know that PartKeepr has a stock of 1 where in reality the box has 2. If you then would use a barcode reader to remove 2 items, this would fail with confusion on the user side.

If you use project reports to automatically remove stock, for example, if you want to populate 50 boards, but you only have 49 of 50 resistors in stock, you still should be able to remove the parts for all 50 boards, so you can at least populate 49 boards and you'd probably immediately order the missing part. Also by having a stock of -1 for the resistors, PartKeepr shows you the missing stock when ordering from a distributor if you use the "Stock < Min Stock" feature, even if you have a minimum stock of 0.

I'm pretty sure that there are more use cases, but making it configurable or giving a warning would probably be best. Negative stock levels are common in like every application where you track stock.

mchateauvert commented 3 years ago

Thanks! Glad I checked first. Should we consider walking back merge #1126 then? Or I can work on a new commit that rolls that change out and adds a warning dialog as well.