From 92b3fdc79235630afa06a40f56d57a93066f1f08 Mon Sep 17 00:00:00 2001 From: JanKolo04 Date: Fri, 7 Aug 2026 00:18:26 +0200 Subject: [PATCH 1/4] feat(warehouse): deduct consumption from the workshop location it came off MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Allocation already moved materials.stock_quantity and the picked lot, but nothing said where the material physically was, so a plant running several stores could not tell which one had emptied. Consumption now moves the per-location balance. The location resolves most-specific-first — the picked lot's warehouse, then the line's own stock location (new lines.warehouse_id), then the default raw-material warehouse — and is frozen on the allocation once anything has been deducted, so a correction always credits back the location that actually gave the material up. Deductions move by the difference, not the total, so an operator entry, a correction and batch completion never double-count; cancelling a batch returns what it took. Each deduction writes a stock_movements row carrying the warehouse. The plant-wide quantity is deliberately not moved again. Consumption beyond the location balance is refused when the system-wide block_negative_stock setting is on, and otherwise flags the shortfall on the movement instead of stopping production. The race-safe balance upsert moves into a shared WarehouseStockService, which the stock-document posting path now uses too. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 7 + .../Web/Admin/LineManagementController.php | 23 +- backend/app/Models/Line.php | 8 + backend/app/Models/MaterialAllocation.php | 11 + .../Material/ConsumptionLocationService.php | 245 +++++++++++ .../Material/MaterialAllocationService.php | 35 +- .../Material/StockMovementService.php | 20 +- .../Warehouse/StockDocumentService.php | 72 +--- .../Warehouse/WarehouseStockService.php | 106 +++++ backend/app/Sync/ShapeRegistry.php | 2 +- ...26_08_06_100000_add_warehouse_to_lines.php | 35 ++ ...tion_deduction_to_material_allocations.php | 42 ++ backend/lang/en.json | 5 +- backend/lang/pl.json | 5 +- .../resources/js/Pages/admin/lines/Create.jsx | 6 +- .../resources/js/Pages/admin/lines/Edit.jsx | 5 +- .../resources/js/Pages/admin/lines/fields.js | 10 +- .../Warehouse/ConsumptionLocationTest.php | 403 ++++++++++++++++++ .../Warehouse/LineStockLocationTest.php | 107 +++++ 19 files changed, 1067 insertions(+), 80 deletions(-) create mode 100644 backend/app/Services/Material/ConsumptionLocationService.php create mode 100644 backend/app/Services/Warehouse/WarehouseStockService.php create mode 100644 backend/database/migrations/2026_08_06_100000_add_warehouse_to_lines.php create mode 100644 backend/database/migrations/2026_08_06_100001_add_location_deduction_to_material_allocations.php create mode 100644 backend/tests/Feature/Warehouse/ConsumptionLocationTest.php create mode 100644 backend/tests/Feature/Warehouse/LineStockLocationTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index c194ca0c3..6905d5738 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,13 @@ Format based on [Keep a Changelog](https://keepachangelog.com/). ## [Unreleased] ### Added +- **Consumption is deducted from the workshop location it came off** — stock levels per storage location now reflect what production actually used. Allocation already moved the plant-wide quantity and the picked lot, but nothing said *where* the material physically was, so a plant running several stores could not tell which one had emptied. + - **Each line names its stock location.** A production line gains a **Stock location** (Admin → Lines), picked from the raw-material warehouses. Optional: a plant that doesn't track stock per location leaves it unset and nothing changes. + - **The location is resolved most-specific-first.** The **picked lot's** warehouse wins (it knows exactly where it sits), then the **line's** stock location, then the plant's **default raw-material** warehouse. Once a deduction has been made the location is **frozen on the allocation**, so a later correction always credits back the location that actually gave the material up — even if the lot has since been moved or the line re-pointed. + - **Booked by difference, never twice.** Consumption is recorded more than once for the same allocation (an operator's entry, a correction, then batch completion finalising the rest), so the balance moves by the **difference** each time. A downward correction credits the location back, and cancelling a batch returns everything it had taken. + - **Auditable per deduction.** Every deduction writes a `stock_movements` row carrying the **warehouse**, the batch/step it came from and the quantity — the plant-wide quantity is deliberately *not* moved again, since allocation already booked it. + - **Stock cannot silently go negative.** Consumption exceeding the location's balance is **refused** when the system-wide **"block negative stock"** setting is on (the same switch warehouse documents respect) — and when it's off, production is not stopped but the movement records the **shortfall** explicitly, so an overdraw stays findable. + - New `lines.warehouse_id` and `material_allocations.consumption_warehouse_id` / `location_deducted_qty` columns, `App\Services\Material\ConsumptionLocationService`, and a shared `App\Services\Warehouse\WarehouseStockService` that the stock-document posting path now uses too, so both routes into a location balance share one race-safe implementation. - **Engineering CAD documents** *([#179](https://github.com/Mes-Open/OpenMes/issues/179))* — attach engineering drawings and CAD files to the parts, products and processes they belong to, so the shop floor always works from the right revision. - **Attach where it matters.** Link files to a **material, product type, product revision, subassembly, or process template/step**. Supported formats: native & neutral CAD (**STEP / IGES**), **eDrawings** (`.eprt` / `.easm` / `.edrw`), **PDF** drawings, **images**, and self-contained **interactive-HTML** packages. Every file is stored on private storage with a **SHA-256 checksum**; the 100 MB size cap and the allowed formats are configurable. - **Revisions with real traceability.** Documents move through a **Draft → Released → Obsolete** lifecycle, and a released document is **immutable**. When a work order is created, the documents released at that moment are **frozen onto it** — publishing a newer revision later never rewrites what a past order was built against. diff --git a/backend/app/Http/Controllers/Web/Admin/LineManagementController.php b/backend/app/Http/Controllers/Web/Admin/LineManagementController.php index ab73ebe8c..a0485990e 100644 --- a/backend/app/Http/Controllers/Web/Admin/LineManagementController.php +++ b/backend/app/Http/Controllers/Web/Admin/LineManagementController.php @@ -39,10 +39,26 @@ public function create() { return Inertia::render('admin/lines/Create', [ 'areas' => $this->areaOptions(), + 'warehouses' => $this->warehouseOptions(), 'customFields' => app(CustomFieldService::class)->clientConfig('line'), ]); } + /** + * Raw-material locations a line can consume from. Only those, because a line + * draws components, never finished goods. + * + * @return \Illuminate\Support\Collection> + */ + private function warehouseOptions(): \Illuminate\Support\Collection + { + return \App\Models\Warehouse::forMaterials() + ->where('is_active', true) + ->orderBy('name') + ->get(['id', 'code', 'name']) + ->map(fn ($w) => ['id' => $w->id, 'name' => "{$w->name} ({$w->code})"]); + } + /** Areas as {id, name (with site)} options for the line form. */ private function areaOptions(): \Illuminate\Support\Collection { @@ -61,6 +77,8 @@ public function store(Request $request) 'name' => 'required|string|max:255', 'description' => 'nullable|string', 'area_id' => 'nullable|exists:areas,id', + // The stock location this line's consumption comes off. + 'warehouse_id' => 'nullable|exists:warehouses,id', 'is_active' => 'boolean', ], $cf->rules('line')), [], $cf->attributeNames('line')); @@ -156,8 +174,9 @@ public function show(Line $line) public function edit(Line $line) { return Inertia::render('admin/lines/Edit', [ - 'line' => $line->only('id', 'code', 'name', 'description', 'area_id', 'is_active', 'custom_fields'), + 'line' => $line->only('id', 'code', 'name', 'description', 'area_id', 'warehouse_id', 'is_active', 'custom_fields'), 'areas' => $this->areaOptions(), + 'warehouses' => $this->warehouseOptions(), 'customFields' => app(CustomFieldService::class)->clientConfig('line'), ]); } @@ -173,6 +192,8 @@ public function update(Request $request, Line $line) 'name' => 'required|string|max:255', 'description' => 'nullable|string', 'area_id' => 'nullable|exists:areas,id', + // The stock location this line's consumption comes off. + 'warehouse_id' => 'nullable|exists:warehouses,id', 'is_active' => 'boolean', ], $cf->rules('line')), [], $cf->attributeNames('line')); diff --git a/backend/app/Models/Line.php b/backend/app/Models/Line.php index b98851270..24fee9ee0 100644 --- a/backend/app/Models/Line.php +++ b/backend/app/Models/Line.php @@ -20,6 +20,8 @@ class Line extends Model protected $fillable = [ 'area_id', 'division_id', + // Stock location this line consumes from; null when stock is not tracked per location. + 'warehouse_id', 'code', 'name', 'description', @@ -44,6 +46,12 @@ public function division(): BelongsTo return $this->belongsTo(Division::class); } + /** The stock location this line's consumption comes off. */ + public function warehouse(): BelongsTo + { + return $this->belongsTo(Warehouse::class); + } + /** * Get the ISA-95 area this line belongs to. */ diff --git a/backend/app/Models/MaterialAllocation.php b/backend/app/Models/MaterialAllocation.php index 7833dd53e..7f0fea3ea 100644 --- a/backend/app/Models/MaterialAllocation.php +++ b/backend/app/Models/MaterialAllocation.php @@ -25,10 +25,14 @@ class MaterialAllocation extends Model 'batch_step_id', 'material_id', 'work_order_id', + // The location this allocation is consumed from, frozen on first deduction. + 'consumption_warehouse_id', 'allocated_qty', 'expected_qty', 'returned_qty', 'consumed_qty', + // How much of it has already been taken off that location's balance. + 'location_deducted_qty', 'adjustment_qty', 'scrap_qty', 'status', @@ -47,6 +51,7 @@ protected function casts(): array 'expected_qty' => 'decimal:4', 'returned_qty' => 'decimal:4', 'consumed_qty' => 'decimal:4', + 'location_deducted_qty' => 'decimal:4', 'adjustment_qty' => 'decimal:4', 'scrap_qty' => 'decimal:4', 'allocated_at' => 'datetime', @@ -84,6 +89,12 @@ public function allocatedBy(): BelongsTo return $this->belongsTo(User::class, 'allocated_by'); } + /** The location this allocation's consumption is booked against. */ + public function consumptionWarehouse(): BelongsTo + { + return $this->belongsTo(Warehouse::class, 'consumption_warehouse_id'); + } + public function lotPicks(): \Illuminate\Database\Eloquent\Relations\HasMany { return $this->hasMany(AllocationLotPick::class, 'material_allocation_id'); diff --git a/backend/app/Services/Material/ConsumptionLocationService.php b/backend/app/Services/Material/ConsumptionLocationService.php new file mode 100644 index 000000000..9e5a5e639 --- /dev/null +++ b/backend/app/Services/Material/ConsumptionLocationService.php @@ -0,0 +1,245 @@ +getKey())->lockForUpdate()->first(); + + if (! $locked) { + return null; + } + + $warehouse = $this->resolveWarehouse($locked); + + if ($warehouse === null) { + // No location to attribute this to — a plant that does not track stock + // per location. Global stock and the lot already moved at allocation, + // so there is simply nothing further to do. + return null; + } + + $delta = round($consumedTotal - (float) $locked->location_deducted_qty, 4); + + if (abs($delta) < 0.00005) { + return null; + } + + $keys = [ + 'warehouse_id' => $warehouse->id, + 'material_id' => $locked->material_id, + ]; + + // Read the balance once, before anything moves: it decides both whether the + // deduction is refused and — when it is not — how big a shortfall to flag. + $available = $this->warehouseStock->available($keys); + + $this->assertSufficient($locked, $warehouse, $delta, $available); + + // The lot-level balance and the material total for that location are kept + // separately (#212), so a per-material view does not have to sum lots. + foreach ($this->lotShares($locked, $delta) as $lotId => $lotDelta) { + $this->warehouseStock->adjust([...$keys, 'material_lot_id' => $lotId], -$lotDelta); + } + + $this->warehouseStock->adjust([...$keys, 'material_lot_id' => null], -$delta); + + $locked->update([ + 'consumption_warehouse_id' => $warehouse->id, + 'location_deducted_qty' => round((float) $locked->location_deducted_qty + $delta, 4), + ]); + + // Audit: one ledger row per deduction, carrying the location. `adjustGlobal` + // is off because allocation already moved the plant-wide quantity — this + // row exists to say which location gave the material up. + return $this->stockMovements->record( + material: $locked->material, + movementType: $delta > 0 ? StockMovement::TYPE_CONSUME : StockMovement::TYPE_RETURN, + signedQuantity: -$delta, + user: $user, + sourceType: $locked->batch_step_id + ? StockMovement::SOURCE_BATCH_STEP + : StockMovement::SOURCE_BATCH, + sourceId: $locked->batch_step_id ?: $locked->batch_id, + reason: $this->reason($locked, $delta, $available), + warehouseId: $warehouse->id, + adjustGlobal: false, + ); + }); + } + + /** + * Give back everything this allocation took off its location — used when a batch + * is cancelled after consumption had already been booked. + */ + public function reverse(MaterialAllocation $allocation, ?User $user = null): ?StockMovement + { + return $this->deduct($allocation, 0, $user); + } + + /** + * Which location this allocation's material comes off. + * + * Once something has been deducted the answer is frozen on the allocation: a + * correction must credit back the location that actually gave the material up, + * even if the lot has since moved or the line has been re-pointed elsewhere. + * + * Otherwise, most specific first — the picked lot knows exactly where it sits, the + * line knows its own workshop store, and the plant default is the last resort. + */ + public function resolveWarehouse(MaterialAllocation $allocation): ?Warehouse + { + if ($allocation->consumption_warehouse_id !== null) { + return Warehouse::find($allocation->consumption_warehouse_id); + } + + $fromLot = $allocation->lotPicks + ->map(fn ($pick) => $pick->lot?->warehouse_id) + ->filter() + ->first(); + + if ($fromLot) { + return Warehouse::find($fromLot); + } + + $fromLine = $allocation->batch?->workOrder?->line?->warehouse_id; + + if ($fromLine) { + return Warehouse::find($fromLine); + } + + return Warehouse::resolveDefault(Warehouse::KIND_RAW_MATERIAL); + } + + /** + * Split a deduction across the lots this allocation picked, proportionally to what + * was picked from each, so a lot-level balance never drifts from the material + * total above it. Returns [lot id => quantity] and is empty when nothing was + * picked by lot. + * + * @return array + */ + private function lotShares(MaterialAllocation $allocation, float $delta): array + { + $picks = $allocation->lotPicks->filter(fn ($pick) => (float) $pick->picked_qty > 0); + $total = (float) $picks->sum('picked_qty'); + + if ($total <= 0) { + return []; + } + + $shares = []; + $assigned = 0.0; + + foreach ($picks->values() as $index => $pick) { + // The last share takes the remainder, so rounding can never leave the lot + // rows summing to something other than the material total. + $share = $index === $picks->count() - 1 + ? round($delta - $assigned, 4) + : round($delta * ((float) $pick->picked_qty / $total), 4); + + $assigned += $share; + $lotId = (int) $pick->material_lot_id; + $shares[$lotId] = round(($shares[$lotId] ?? 0) + $share, 4); + } + + return $shares; + } + + /** + * Refuse a deduction the location cannot cover, when the plant says so. + * + * When it does not, the deduction goes through and the balance is allowed below + * zero — the movement's reason records the shortfall so it can be investigated + * without production having been stopped. + * + * @throws \DomainException + */ + private function assertSufficient( + MaterialAllocation $allocation, + Warehouse $warehouse, + float $delta, + float $available, + ): void { + if ($delta <= 0 || ! $this->warehouseStock->blocksNegativeStock()) { + return; + } + + if ($available + 0.00005 < $delta) { + throw new \DomainException(__( + 'Location :warehouse does not hold enough :material to consume :needed (:available available).', + [ + 'warehouse' => $warehouse->code, + 'material' => $allocation->material?->code ?? $allocation->material_id, + 'needed' => $delta, + 'available' => $available, + ], + )); + } + } + + /** + * Ledger text for the deduction. + * + * When the plant does not block negative balances, an overdraw still has to be + * findable afterwards — so the shortfall is spelled out in the movement rather + * than left to be inferred from a balance that is now negative for other reasons + * too. `$available` is the balance from before the move. + */ + private function reason(MaterialAllocation $allocation, float $delta, float $available): string + { + $base = $delta > 0 + ? 'Consumed on batch #'.$allocation->batch_id + : 'Consumption corrected down on batch #'.$allocation->batch_id; + + if ($delta > 0 && $available + 0.00005 < $delta) { + return $base.' — SHORTFALL: location held '.round($available, 4).' of '.round($delta, 4); + } + + return $base; + } +} diff --git a/backend/app/Services/Material/MaterialAllocationService.php b/backend/app/Services/Material/MaterialAllocationService.php index 1219a0f27..27d0b8f28 100644 --- a/backend/app/Services/Material/MaterialAllocationService.php +++ b/backend/app/Services/Material/MaterialAllocationService.php @@ -18,6 +18,7 @@ class MaterialAllocationService public function __construct( protected StockMovementService $stockMovements, protected LotPickingService $lotPicking, + protected ConsumptionLocationService $consumptionLocation, ) {} /** @@ -254,6 +255,12 @@ public function consumeForBatch(Batch $batch): void ); } + // Finalise the location balance against the quantity that actually + // stands. Deducts only the part not already booked by the operator's + // own entries, so a batch whose consumption was recorded step by step + // is not deducted twice. + $this->consumptionLocation->deduct($allocation, $actualConsumed); + $allocation->update([ 'status' => MaterialAllocation::STATUS_CONSUMED, 'consumed_qty' => $actualConsumed, @@ -288,6 +295,11 @@ public function returnForBatch(Batch $batch): void $this->releaseReservation($allocation->material, (float) $allocation->allocated_qty); } + // Give back anything already taken off the location: a cancelled batch + // consumed nothing, so a location that was debited by an operator's + // entry must be made whole again. + $this->consumptionLocation->reverse($allocation); + // Lot tracking: return picked qty back to each lot. $this->lotPicking->returnPicksForAllocation($allocation); @@ -316,15 +328,22 @@ public function recordConsumption( throw new \InvalidArgumentException('Consumed and scrap quantities must be non-negative.'); } - $allocation->update([ - 'consumed_qty' => $actualConsumed, - 'scrap_qty' => $scrap, - // Snapshot the price so historical cost reports stay stable. - 'unit_price_snapshot' => $actualConsumed > 0 ? $allocation->material?->unit_price : null, - 'price_currency_snapshot' => $actualConsumed > 0 ? $allocation->material?->price_currency : null, - ]); + return DB::transaction(function () use ($allocation, $actualConsumed, $scrap) { + $allocation->update([ + 'consumed_qty' => $actualConsumed, + 'scrap_qty' => $scrap, + // Snapshot the price so historical cost reports stay stable. + 'unit_price_snapshot' => $actualConsumed > 0 ? $allocation->material?->unit_price : null, + 'price_currency_snapshot' => $actualConsumed > 0 ? $allocation->material?->price_currency : null, + ]); - return $allocation->fresh(); + // Take it off the location it came from. Same transaction as the quantity + // itself, so a refused deduction (location short, plant blocks negatives) + // leaves no consumed_qty claiming material that never moved. + $this->consumptionLocation->deduct($allocation->fresh(), $actualConsumed); + + return $allocation->fresh(); + }); } /** diff --git a/backend/app/Services/Material/StockMovementService.php b/backend/app/Services/Material/StockMovementService.php index f36bf3560..39fc48925 100644 --- a/backend/app/Services/Material/StockMovementService.php +++ b/backend/app/Services/Material/StockMovementService.php @@ -28,18 +28,26 @@ public function record( ?int $sourceId = null, ?string $reason = null, ?int $warehouseId = null, + bool $adjustGlobal = true, ): StockMovement { - return DB::transaction(function () use ($material, $movementType, $signedQuantity, $user, $sourceType, $sourceId, $reason, $warehouseId) { + return DB::transaction(function () use ($material, $movementType, $signedQuantity, $user, $sourceType, $sourceId, $reason, $warehouseId, $adjustGlobal) { // Lock + re-read so the balance_after we record is the real // post-mutation value, even under concurrency. $locked = Material::where('id', $material->id)->lockForUpdate()->first(); - if ($signedQuantity >= 0) { - $locked->increment('stock_quantity', $signedQuantity); - } else { - $locked->decrement('stock_quantity', abs($signedQuantity)); + // Location-only movements pass adjustGlobal: false. Shop-floor consumption + // is the case: the plant-wide quantity already went down when the material + // was allocated, and moving it again here would count the same material + // twice. The ledger row is still written — it is what makes the per-location + // deduction auditable. + if ($adjustGlobal) { + if ($signedQuantity >= 0) { + $locked->increment('stock_quantity', $signedQuantity); + } else { + $locked->decrement('stock_quantity', abs($signedQuantity)); + } + \App\Sync\CollectionBroadcaster::flush($locked); // increment/decrement bypass model events } - \App\Sync\CollectionBroadcaster::flush($locked); // increment/decrement bypass model events $locked->refresh(); diff --git a/backend/app/Services/Warehouse/StockDocumentService.php b/backend/app/Services/Warehouse/StockDocumentService.php index 5562ae082..b186a6d97 100644 --- a/backend/app/Services/Warehouse/StockDocumentService.php +++ b/backend/app/Services/Warehouse/StockDocumentService.php @@ -8,7 +8,6 @@ use App\Models\StockMovement; use App\Models\User; use App\Models\Warehouse; -use App\Models\WarehouseStock; use App\Services\Material\StockMovementService; use Illuminate\Database\UniqueConstraintViolationException; use Illuminate\Support\Facades\DB; @@ -35,7 +34,10 @@ class StockDocumentService /** How many times a generated document number is retried on a collision. */ private const NUMBER_ATTEMPTS = 5; - public function __construct(private StockMovementService $stockMovements) {} + public function __construct( + private StockMovementService $stockMovements, + private WarehouseStockService $warehouseStock, + ) {} /** * Create a draft document with its lines. @@ -284,54 +286,23 @@ private function adjustWarehouseStock(StockDocument $document, $line, float $sig 'product_type_id' => $isMaterial ? null : $line->product_type_id, ]; - if ($isMaterial && $line->material_lot_id !== null) { - $this->incrementBalance( - [...$keys, 'material_lot_id' => $line->material_lot_id], - $signed, - $line->unit_of_measure, - ); - } - - $this->incrementBalance([...$keys, 'material_lot_id' => null], $signed, $line->unit_of_measure); - } - - /** @param array $keys */ - private function incrementBalance(array $keys, float $signed, ?string $unit): void - { - $stock = WarehouseStock::query() - ->where($keys) - ->lockForUpdate() - ->first(); - - if (! $stock) { - // lockForUpdate() can only lock rows that exist, so two concurrent - // posts can both miss and then race to insert. The partial unique - // index decides the winner; the loser re-reads the row it lost to - // (now committed) instead of failing the whole posting. - try { - $stock = WarehouseStock::create([ - ...$keys, - 'quantity' => 0, - 'unit_of_measure' => $unit, - ]); - } catch (UniqueConstraintViolationException) { - $stock = WarehouseStock::query()->where($keys)->lockForUpdate()->first(); + try { + if ($isMaterial && $line->material_lot_id !== null) { + $this->warehouseStock->adjust( + [...$keys, 'material_lot_id' => $line->material_lot_id], + $signed, + $line->unit_of_measure, + ); } - } - if (! $stock) { + $this->warehouseStock->adjust([...$keys, 'material_lot_id' => null], $signed, $line->unit_of_measure); + } catch (\RuntimeException) { + // The shared service is used by non-HTTP callers too, so it reports a + // plain runtime failure; posting turns it back into a form error. throw ValidationException::withMessages([ 'lines' => __('Could not read the stock balance to update. Try again.'), ]); } - - $stock->quantity = round((float) $stock->quantity + $signed, 3); - - if ($unit && ! $stock->unit_of_measure) { - $stock->unit_of_measure = $unit; - } - - $stock->save(); } /** Keep the lot's remaining quantity in step with what was issued/returned. */ @@ -365,7 +336,7 @@ private function adjustLot(int $lotId, float $signed, int $materialId): void */ private function guardNegativeStock(Material $material, float $signed): void { - if ($signed >= 0 || ! $this->blockNegativeStockEnabled()) { + if ($signed >= 0 || ! $this->warehouseStock->blocksNegativeStock()) { return; } @@ -379,17 +350,6 @@ private function guardNegativeStock(Material $material, float $signed): void } } - private function blockNegativeStockEnabled(): bool - { - try { - $row = DB::table('system_settings')->where('key', 'block_negative_stock')->value('value'); - - return (bool) json_decode($row ?? 'false', true); - } catch (\Throwable) { - return false; - } - } - /** * Coerce a submitted line into the columns its document type uses, so a * product line can never smuggle in a material id (and vice versa). diff --git a/backend/app/Services/Warehouse/WarehouseStockService.php b/backend/app/Services/Warehouse/WarehouseStockService.php new file mode 100644 index 000000000..3bcc7a9d3 --- /dev/null +++ b/backend/app/Services/Warehouse/WarehouseStockService.php @@ -0,0 +1,106 @@ + $keys warehouse_id, material_id, product_type_id, material_lot_id + */ + public function adjust(array $keys, float $signed, ?string $unit = null): WarehouseStock + { + $keys = $this->normalizeKeys($keys); + + $stock = WarehouseStock::query()->where($keys)->lockForUpdate()->first(); + + if (! $stock) { + // lockForUpdate() can only lock rows that exist, so two concurrent callers + // can both miss and then race to insert. The partial unique index decides + // the winner; the loser re-reads the row it lost to instead of failing. + try { + $stock = WarehouseStock::create([...$keys, 'quantity' => 0, 'unit_of_measure' => $unit]); + } catch (UniqueConstraintViolationException) { + $stock = WarehouseStock::query()->where($keys)->lockForUpdate()->first(); + } + } + + if (! $stock) { + throw new \RuntimeException('Could not read the stock balance to update.'); + } + + $stock->quantity = round((float) $stock->quantity + $signed, 3); + + if ($unit && ! $stock->unit_of_measure) { + $stock->unit_of_measure = $unit; + } + + $stock->save(); + + return $stock; + } + + /** + * Current balance for a slot, 0 when the row does not exist yet — an untouched + * location holds nothing, which is the same answer as an emptied one. + * + * @param array $keys + */ + public function available(array $keys): float + { + return (float) (WarehouseStock::query() + ->where($this->normalizeKeys($keys)) + ->value('quantity') ?? 0); + } + + /** + * Whether the plant refuses to let a balance go below zero. + * + * Read straight from the settings table rather than through a cache, because the + * answer decides whether production is allowed to continue and a stale yes/no + * here is worse than the query. Missing or unreadable settings mean "not blocked": + * this switch exists to tighten the default, never to halt a plant that never + * turned it on. + */ + public function blocksNegativeStock(): bool + { + try { + $row = DB::table('system_settings')->where('key', 'block_negative_stock')->value('value'); + + return (bool) json_decode($row ?? 'false', true); + } catch (\Throwable) { + return false; + } + } + + /** + * @param array $keys + * @return array + */ + private function normalizeKeys(array $keys): array + { + return [ + 'warehouse_id' => $keys['warehouse_id'] ?? null, + 'material_id' => $keys['material_id'] ?? null, + 'product_type_id' => $keys['product_type_id'] ?? null, + 'material_lot_id' => $keys['material_lot_id'] ?? null, + ]; + } +} diff --git a/backend/app/Sync/ShapeRegistry.php b/backend/app/Sync/ShapeRegistry.php index d7d29b658..79f72fdc2 100644 --- a/backend/app/Sync/ShapeRegistry.php +++ b/backend/app/Sync/ShapeRegistry.php @@ -203,7 +203,7 @@ class ShapeRegistry // All lines (incl. inactive) for the admin list — lines_active is active-only. 'lines_all' => [ 'table' => 'lines', - 'columns' => ['id', 'code', 'name', 'description', 'is_active', 'area_id', 'division_id', 'view_template_id', 'default_operator_view', 'custom_fields', 'created_at', 'updated_at'], + 'columns' => ['id', 'code', 'name', 'description', 'is_active', 'area_id', 'division_id', 'warehouse_id', 'view_template_id', 'default_operator_view', 'custom_fields', 'created_at', 'updated_at'], ], 'maintenance_events' => [ 'table' => 'maintenance_events', diff --git a/backend/database/migrations/2026_08_06_100000_add_warehouse_to_lines.php b/backend/database/migrations/2026_08_06_100000_add_warehouse_to_lines.php new file mode 100644 index 000000000..bdf54b62c --- /dev/null +++ b/backend/database/migrations/2026_08_06_100000_add_warehouse_to_lines.php @@ -0,0 +1,35 @@ +foreignId('warehouse_id')->nullable()->after('division_id') + ->constrained()->nullOnDelete() + ->comment('Stock location this line consumes from'); + }); + } + + public function down(): void + { + Schema::table('lines', function (Blueprint $table) { + $table->dropConstrainedForeignId('warehouse_id'); + }); + } +}; diff --git a/backend/database/migrations/2026_08_06_100001_add_location_deduction_to_material_allocations.php b/backend/database/migrations/2026_08_06_100001_add_location_deduction_to_material_allocations.php new file mode 100644 index 000000000..8e43f7935 --- /dev/null +++ b/backend/database/migrations/2026_08_06_100001_add_location_deduction_to_material_allocations.php @@ -0,0 +1,42 @@ +foreignId('consumption_warehouse_id')->nullable()->after('work_order_id') + ->constrained('warehouses')->nullOnDelete() + ->comment('Location this allocation is consumed from'); + + $table->decimal('location_deducted_qty', 12, 4)->default(0)->after('consumed_qty') + ->comment('How much has already been taken off the location balance'); + }); + } + + public function down(): void + { + Schema::table('material_allocations', function (Blueprint $table) { + $table->dropConstrainedForeignId('consumption_warehouse_id'); + $table->dropColumn('location_deducted_qty'); + }); + } +}; diff --git a/backend/lang/en.json b/backend/lang/en.json index d32e5afbb..ea950d7c8 100644 --- a/backend/lang/en.json +++ b/backend/lang/en.json @@ -5073,5 +5073,8 @@ "This document moves products, not materials.": "This document moves products, not materials.", "A product line cannot carry a material lot.": "A product line cannot carry a material lot.", "That lot belongs to a different material.": "That lot belongs to a different material.", - "Lot ':lot' already belongs to material ':code'": "Lot ':lot' already belongs to material ':code'" + "Lot ':lot' already belongs to material ':code'": "Lot ':lot' already belongs to material ':code'", + "Stock location": "Stock location", + "Consumption booked on this line is deducted from this location.": "Consumption booked on this line is deducted from this location.", + "Location :warehouse does not hold enough :material to consume :needed (:available available).": "Location :warehouse does not hold enough :material to consume :needed (:available available)." } diff --git a/backend/lang/pl.json b/backend/lang/pl.json index 3cfec0cf4..ea7bee0f6 100644 --- a/backend/lang/pl.json +++ b/backend/lang/pl.json @@ -5073,5 +5073,8 @@ "This document moves products, not materials.": "Ten dokument obraca wyrobami, nie materiałami.", "A product line cannot carry a material lot.": "Pozycja wyrobu nie może mieć partii materiału.", "That lot belongs to a different material.": "Ta partia należy do innego materiału.", - "Lot ':lot' already belongs to material ':code'": "Partia ':lot' należy już do materiału ':code'" + "Lot ':lot' already belongs to material ':code'": "Partia ':lot' należy już do materiału ':code'", + "Stock location": "Lokalizacja magazynowa", + "Consumption booked on this line is deducted from this location.": "Zużycie zarejestrowane na tej linii jest odejmowane z tej lokalizacji.", + "Location :warehouse does not hold enough :material to consume :needed (:available available).": "Lokalizacja :warehouse nie ma wystarczającej ilości :material, aby zużyć :needed (dostępne: :available)." } diff --git a/backend/resources/js/Pages/admin/lines/Create.jsx b/backend/resources/js/Pages/admin/lines/Create.jsx index c41d98586..3a6ba3231 100644 --- a/backend/resources/js/Pages/admin/lines/Create.jsx +++ b/backend/resources/js/Pages/admin/lines/Create.jsx @@ -5,7 +5,7 @@ import ResourceForm from '../../../components/ResourceForm'; import { lineFields } from './fields'; export default function LineCreate() { - const { areas = [] } = usePage().props; + const { areas = [], warehouses = [] } = usePage().props; return (
@@ -13,8 +13,8 @@ export default function LineCreate() { diff --git a/backend/resources/js/Pages/admin/lines/Edit.jsx b/backend/resources/js/Pages/admin/lines/Edit.jsx index 2d6feeb87..54da107f4 100644 --- a/backend/resources/js/Pages/admin/lines/Edit.jsx +++ b/backend/resources/js/Pages/admin/lines/Edit.jsx @@ -5,7 +5,7 @@ import ResourceForm from '../../../components/ResourceForm'; import { lineFields } from './fields'; export default function LineEdit() { - const { line, areas = [] } = usePage().props; + const { line, areas = [], warehouses = [] } = usePage().props; return (
@@ -13,11 +13,12 @@ export default function LineEdit() { ({ value: String(a.id), label: a.name }))], }, + { + name: 'warehouse_id', label: __('Stock location'), type: 'select', + help: __('Consumption booked on this line is deducted from this location.'), + options: [ + { value: '', label: __('— None —') }, + ...warehouses.map((w) => ({ value: String(w.id), label: w.name })), + ], + }, { name: 'description', label: __('Description'), type: 'textarea' }, { name: 'is_active', label: __('Active'), type: 'checkbox' }, ]; diff --git a/backend/tests/Feature/Warehouse/ConsumptionLocationTest.php b/backend/tests/Feature/Warehouse/ConsumptionLocationTest.php new file mode 100644 index 000000000..2ec612c39 --- /dev/null +++ b/backend/tests/Feature/Warehouse/ConsumptionLocationTest.php @@ -0,0 +1,403 @@ +service = app(ConsumptionLocationService::class); + } + + private function warehouse(string $code, bool $default = false): Warehouse + { + $factory = Warehouse::factory()->rawMaterial(); + + if ($default) { + $factory = $factory->isDefault(); + } + + return $factory->create(['code' => $code]); + } + + private function stockAt(Warehouse $warehouse, Material $material, float $qty, ?MaterialLot $lot = null): WarehouseStock + { + return WarehouseStock::factory()->create([ + 'warehouse_id' => $warehouse->id, + 'material_id' => $material->id, + 'material_lot_id' => $lot?->id, + 'quantity' => $qty, + ]); + } + + private function balance(Warehouse $warehouse, Material $material, ?MaterialLot $lot = null): float + { + return (float) WarehouseStock::where([ + 'warehouse_id' => $warehouse->id, + 'material_id' => $material->id, + 'material_lot_id' => $lot?->id, + ])->value('quantity'); + } + + private function blockNegativeStock(bool $on): void + { + DB::table('system_settings')->updateOrInsert( + ['key' => 'block_negative_stock'], + ['value' => json_encode($on)], + ); + } + + /** An allocation on a line pointed at the given warehouse. */ + private function allocationOnLine(Warehouse $warehouse, Material $material, float $allocated = 100): MaterialAllocation + { + $line = Line::factory()->create(['warehouse_id' => $warehouse->id]); + $workOrder = WorkOrder::factory()->create(['line_id' => $line->id]); + + return MaterialAllocation::factory()->create([ + 'material_id' => $material->id, + 'work_order_id' => $workOrder->id, + 'batch_id' => \App\Models\Batch::factory()->create(['work_order_id' => $workOrder->id])->id, + 'allocated_qty' => $allocated, + ]); + } + + // ── Deduction ───────────────────────────────────────────────────────────── + + public function test_recording_consumption_deducts_from_the_line_location(): void + { + $warehouse = $this->warehouse('WS-1'); + $material = Material::factory()->create(['code' => 'FLOUR-01']); + $this->stockAt($warehouse, $material, 500); + + $allocation = $this->allocationOnLine($warehouse, $material); + + $this->service->deduct($allocation, 120.5); + + $this->assertEquals(379.5, $this->balance($warehouse, $material)); + $this->assertEquals(120.5, (float) $allocation->fresh()->location_deducted_qty); + $this->assertSame($warehouse->id, $allocation->fresh()->consumption_warehouse_id); + } + + public function test_the_deduction_is_auditable_as_a_movement_carrying_the_location(): void + { + $warehouse = $this->warehouse('WS-1'); + $material = Material::factory()->create(); + $this->stockAt($warehouse, $material, 500); + + $allocation = $this->allocationOnLine($warehouse, $material); + $this->service->deduct($allocation, 40); + + $movement = StockMovement::where('material_id', $material->id) + ->where('movement_type', StockMovement::TYPE_CONSUME) + ->latest('id') + ->firstOrFail(); + + $this->assertSame($warehouse->id, $movement->warehouse_id); + $this->assertEquals(-40, (float) $movement->quantity); + $this->assertStringContainsString('Consumed on batch', $movement->reason); + } + + /** + * The plant-wide quantity already went down at allocation; moving it again here + * would count the same material twice. + */ + public function test_the_deduction_does_not_move_the_plant_wide_quantity_again(): void + { + $warehouse = $this->warehouse('WS-1'); + $material = Material::factory()->create(['stock_quantity' => 500]); + $this->stockAt($warehouse, $material, 500); + + $allocation = $this->allocationOnLine($warehouse, $material); + $this->service->deduct($allocation, 40); + + $this->assertEquals(500, (float) $material->fresh()->stock_quantity); + } + + public function test_repeated_recording_moves_only_the_difference(): void + { + $warehouse = $this->warehouse('WS-1'); + $material = Material::factory()->create(); + $this->stockAt($warehouse, $material, 500); + + $allocation = $this->allocationOnLine($warehouse, $material); + + $this->service->deduct($allocation, 30); + $this->service->deduct($allocation, 50); + + // 50 consumed in total, not 80. + $this->assertEquals(450.0, $this->balance($warehouse, $material)); + $this->assertEquals(50.0, (float) $allocation->fresh()->location_deducted_qty); + } + + public function test_correcting_consumption_down_credits_the_location_back(): void + { + $warehouse = $this->warehouse('WS-1'); + $material = Material::factory()->create(); + $this->stockAt($warehouse, $material, 500); + + $allocation = $this->allocationOnLine($warehouse, $material); + + $this->service->deduct($allocation, 80); + $this->service->deduct($allocation, 30); + + $this->assertEquals(470.0, $this->balance($warehouse, $material)); + + $credit = StockMovement::where('material_id', $material->id) + ->where('movement_type', StockMovement::TYPE_RETURN) + ->latest('id') + ->firstOrFail(); + $this->assertEquals(50, (float) $credit->quantity); + } + + public function test_an_unchanged_quantity_writes_nothing(): void + { + $warehouse = $this->warehouse('WS-1'); + $material = Material::factory()->create(); + $this->stockAt($warehouse, $material, 500); + + $allocation = $this->allocationOnLine($warehouse, $material); + $this->service->deduct($allocation, 25); + $movements = StockMovement::count(); + + $this->assertNull($this->service->deduct($allocation, 25)); + $this->assertSame($movements, StockMovement::count()); + } + + // ── Location selection ──────────────────────────────────────────────────── + + public function test_a_picked_lot_decides_the_location_over_the_line(): void + { + $lineWarehouse = $this->warehouse('WS-LINE'); + $lotWarehouse = $this->warehouse('WS-LOT'); + $material = Material::factory()->create(); + + $lot = MaterialLot::factory()->create([ + 'material_id' => $material->id, + 'warehouse_id' => $lotWarehouse->id, + ]); + + $this->stockAt($lineWarehouse, $material, 500); + $this->stockAt($lotWarehouse, $material, 500); + $this->stockAt($lotWarehouse, $material, 500, $lot); + + $allocation = $this->allocationOnLine($lineWarehouse, $material); + $allocation->lotPicks()->create(['material_lot_id' => $lot->id, 'picked_qty' => 100]); + + $this->service->deduct($allocation->fresh(), 60); + + // The lot knows exactly where it sits; the line is only the fallback. + $this->assertEquals(440.0, $this->balance($lotWarehouse, $material)); + $this->assertEquals(500.0, $this->balance($lineWarehouse, $material)); + // The lot-level balance follows the material total for that location. + $this->assertEquals(440.0, $this->balance($lotWarehouse, $material, $lot)); + } + + public function test_the_line_location_is_used_when_no_lot_was_picked(): void + { + $lineWarehouse = $this->warehouse('WS-LINE'); + $this->warehouse('WS-DEFAULT', default: true); + $material = Material::factory()->create(); + $this->stockAt($lineWarehouse, $material, 500); + + $allocation = $this->allocationOnLine($lineWarehouse, $material); + $this->service->deduct($allocation, 70); + + $this->assertEquals(430.0, $this->balance($lineWarehouse, $material)); + } + + public function test_the_default_location_is_the_last_resort(): void + { + $default = $this->warehouse('WS-DEFAULT', default: true); + $material = Material::factory()->create(); + $this->stockAt($default, $material, 500); + + // A line with no warehouse of its own. + $line = Line::factory()->create(['warehouse_id' => null]); + $workOrder = WorkOrder::factory()->create(['line_id' => $line->id]); + $allocation = MaterialAllocation::factory()->create([ + 'material_id' => $material->id, + 'work_order_id' => $workOrder->id, + 'batch_id' => \App\Models\Batch::factory()->create(['work_order_id' => $workOrder->id])->id, + ]); + + $this->service->deduct($allocation, 25); + + $this->assertEquals(475.0, $this->balance($default, $material)); + } + + public function test_a_plant_with_no_locations_at_all_is_left_alone(): void + { + $material = Material::factory()->create(['stock_quantity' => 100]); + $line = Line::factory()->create(['warehouse_id' => null]); + $workOrder = WorkOrder::factory()->create(['line_id' => $line->id]); + $allocation = MaterialAllocation::factory()->create([ + 'material_id' => $material->id, + 'work_order_id' => $workOrder->id, + 'batch_id' => \App\Models\Batch::factory()->create(['work_order_id' => $workOrder->id])->id, + ]); + + $this->assertNull($this->service->deduct($allocation, 25)); + $this->assertSame(0, WarehouseStock::count()); + $this->assertEquals(0.0, (float) $allocation->fresh()->location_deducted_qty); + } + + /** + * A correction must credit back the location the material actually left, even + * after the lot has been moved somewhere else. + */ + public function test_the_location_is_frozen_once_something_has_been_deducted(): void + { + $original = $this->warehouse('WS-1'); + $moved = $this->warehouse('WS-2'); + $material = Material::factory()->create(); + $this->stockAt($original, $material, 500); + $this->stockAt($moved, $material, 500); + + $line = Line::factory()->create(['warehouse_id' => $original->id]); + $workOrder = WorkOrder::factory()->create(['line_id' => $line->id]); + $allocation = MaterialAllocation::factory()->create([ + 'material_id' => $material->id, + 'work_order_id' => $workOrder->id, + 'batch_id' => \App\Models\Batch::factory()->create(['work_order_id' => $workOrder->id])->id, + ]); + + $this->service->deduct($allocation, 100); + + // The line is re-pointed at another store after the fact. + $line->update(['warehouse_id' => $moved->id]); + $this->service->deduct($allocation->fresh(), 60); + + $this->assertEquals(440.0, $this->balance($original, $material)); + $this->assertEquals(500.0, $this->balance($moved, $material)); + } + + // ── Insufficient stock ──────────────────────────────────────────────────── + + public function test_consumption_beyond_the_location_balance_is_refused_when_the_plant_blocks_it(): void + { + $this->blockNegativeStock(true); + + $warehouse = $this->warehouse('WS-1'); + $material = Material::factory()->create(['code' => 'FLOUR-01']); + $this->stockAt($warehouse, $material, 10); + + $allocation = $this->allocationOnLine($warehouse, $material); + + $this->expectException(\DomainException::class); + + try { + $this->service->deduct($allocation, 40); + } finally { + // Nothing moved and nothing was booked as deducted. + $this->assertEquals(10.0, $this->balance($warehouse, $material)); + $this->assertEquals(0.0, (float) $allocation->fresh()->location_deducted_qty); + } + } + + public function test_the_same_consumption_is_allowed_and_flagged_when_the_plant_does_not_block_it(): void + { + $this->blockNegativeStock(false); + + $warehouse = $this->warehouse('WS-1'); + $material = Material::factory()->create(); + $this->stockAt($warehouse, $material, 10); + + $allocation = $this->allocationOnLine($warehouse, $material); + $movement = $this->service->deduct($allocation, 40); + + // Production is not stopped, but the overdraw is on the record. + $this->assertEquals(-30.0, $this->balance($warehouse, $material)); + $this->assertStringContainsString('SHORTFALL', $movement->reason); + $this->assertStringContainsString('10 of 40', $movement->reason); + } + + public function test_a_location_that_holds_exactly_enough_is_not_a_shortfall(): void + { + $this->blockNegativeStock(true); + + $warehouse = $this->warehouse('WS-1'); + $material = Material::factory()->create(); + $this->stockAt($warehouse, $material, 40); + + $allocation = $this->allocationOnLine($warehouse, $material); + $movement = $this->service->deduct($allocation, 40); + + $this->assertEquals(0.0, $this->balance($warehouse, $material)); + $this->assertStringNotContainsString('SHORTFALL', $movement->reason); + } + + // ── Through the allocation service ──────────────────────────────────────── + + public function test_recording_consumption_through_the_allocation_service_deducts_once(): void + { + $warehouse = $this->warehouse('WS-1'); + $material = Material::factory()->create(); + $this->stockAt($warehouse, $material, 500); + + $allocation = $this->allocationOnLine($warehouse, $material); + $allocations = app(MaterialAllocationService::class); + + $allocations->recordConsumption($allocation, 60); + $this->assertEquals(440.0, $this->balance($warehouse, $material)); + + // Batch completion finalises the same quantity — it must not deduct again. + $allocations->consumeForBatch($allocation->batch); + $this->assertEquals(440.0, $this->balance($warehouse, $material)); + } + + public function test_batch_completion_without_an_operator_entry_deducts_the_allocated_quantity(): void + { + $warehouse = $this->warehouse('WS-1'); + $material = Material::factory()->create(); + $this->stockAt($warehouse, $material, 500); + + $allocation = $this->allocationOnLine($warehouse, $material, allocated: 75); + + app(MaterialAllocationService::class)->consumeForBatch($allocation->batch); + + $this->assertEquals(425.0, $this->balance($warehouse, $material)); + } + + public function test_cancelling_a_batch_gives_the_location_its_material_back(): void + { + $warehouse = $this->warehouse('WS-1'); + $material = Material::factory()->create(); + $this->stockAt($warehouse, $material, 500); + + $allocation = $this->allocationOnLine($warehouse, $material); + $allocations = app(MaterialAllocationService::class); + + $allocations->recordConsumption($allocation, 60); + $this->assertEquals(440.0, $this->balance($warehouse, $material)); + + $allocations->returnForBatch($allocation->batch); + + $this->assertEquals(500.0, $this->balance($warehouse, $material)); + $this->assertEquals(0.0, (float) $allocation->fresh()->location_deducted_qty); + } +} diff --git a/backend/tests/Feature/Warehouse/LineStockLocationTest.php b/backend/tests/Feature/Warehouse/LineStockLocationTest.php new file mode 100644 index 000000000..b160b7883 --- /dev/null +++ b/backend/tests/Feature/Warehouse/LineStockLocationTest.php @@ -0,0 +1,107 @@ +seed(\Database\Seeders\RolesAndPermissionsSeeder::class); + + $this->admin = User::factory()->create(); + $this->admin->assignRole('Admin'); + } + + /** The picker must offer material-holding locations, and only those. */ + public function test_the_line_form_offers_raw_material_locations(): void + { + $raw = Warehouse::factory()->rawMaterial()->create(['code' => 'RAW-1', 'name' => 'Raw store']); + $mixed = Warehouse::factory()->create(['code' => 'MIX-1', 'name' => 'Mixed store', 'kind' => Warehouse::KIND_MIXED]); + Warehouse::factory()->create(['code' => 'FG-1', 'name' => 'Finished goods', 'kind' => Warehouse::KIND_FINISHED_GOODS]); + + $this->actingAs($this->admin) + ->get('/admin/lines/create') + ->assertOk() + ->assertInertia(fn (AssertableInertia $page) => $page + ->component('admin/lines/Create') + ->has('warehouses', 2) + ->etc() + ); + + $codes = collect($this->actingAs($this->admin)->get('/admin/lines/create')->inertiaProps()['warehouses'] ?? []) + ->pluck('id') + ->all(); + + $this->assertEqualsCanonicalizing([$raw->id, $mixed->id], $codes); + } + + public function test_a_line_can_be_created_with_a_stock_location(): void + { + $warehouse = Warehouse::factory()->rawMaterial()->create(); + + $this->actingAs($this->admin) + ->post('/admin/lines', [ + 'code' => 'L-1', + 'name' => 'Assembly 1', + 'warehouse_id' => $warehouse->id, + 'is_active' => true, + ]) + ->assertRedirect(); + + $this->assertSame($warehouse->id, Line::where('code', 'L-1')->value('warehouse_id')); + } + + public function test_a_line_can_be_repointed_at_another_location(): void + { + $first = Warehouse::factory()->rawMaterial()->create(); + $second = Warehouse::factory()->rawMaterial()->create(); + $line = Line::factory()->create(['warehouse_id' => $first->id]); + + $this->actingAs($this->admin) + ->put("/admin/lines/{$line->id}", [ + 'code' => $line->code, + 'name' => $line->name, + 'warehouse_id' => $second->id, + 'is_active' => true, + ]) + ->assertRedirect(); + + $this->assertSame($second->id, $line->fresh()->warehouse_id); + } + + public function test_an_unknown_location_is_rejected(): void + { + $this->actingAs($this->admin) + ->post('/admin/lines', [ + 'code' => 'L-2', + 'name' => 'Assembly 2', + 'warehouse_id' => 999999, + ]) + ->assertSessionHasErrors('warehouse_id'); + } + + /** Stock location stays optional — a plant that doesn't track it is unaffected. */ + public function test_a_line_without_a_stock_location_is_still_valid(): void + { + $this->actingAs($this->admin) + ->post('/admin/lines', ['code' => 'L-3', 'name' => 'Assembly 3']) + ->assertRedirect(); + + $this->assertDatabaseHas('lines', ['code' => 'L-3', 'warehouse_id' => null]); + } +} From 5396c899da92ebaef4532aa655d219ef62d46a2f Mon Sep 17 00:00:00 2001 From: JanKolo04 Date: Sun, 30 Aug 2026 15:49:29 +0200 Subject: [PATCH 2/4] fix(warehouse): gate consumption deduction on the Warehouses module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per-location balances belong to the optional Warehouses module (#212), but the consumption deduction ran regardless. A plant that had warehouses once and then switched the module off would still have production booked against — and, with "block negative stock" on, refused by — balances nobody maintains any more. The deduction now returns early when the module is off, the same gate the work-order document listener uses. Rollout runbook documents the new step (pointing a line at its stock location) and both new failure modes. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 1 + .../Material/ConsumptionLocationService.php | 11 ++++++++++- .../Warehouse/ConsumptionLocationTest.php | 18 ++++++++++++++++++ docs/warehouse-erp-rollout.md | 15 +++++++++++++-- 4 files changed, 42 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6b84e1c4..a1f0721f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ Format based on [Keep a Changelog](https://keepachangelog.com/). - **Booked by difference, never twice.** Consumption is recorded more than once for the same allocation (an operator's entry, a correction, then batch completion finalising the rest), so the balance moves by the **difference** each time. A downward correction credits the location back, and cancelling a batch returns everything it had taken. - **Auditable per deduction.** Every deduction writes a `stock_movements` row carrying the **warehouse**, the batch/step it came from and the quantity — the plant-wide quantity is deliberately *not* moved again, since allocation already booked it. - **Stock cannot silently go negative.** Consumption exceeding the location's balance is **refused** when the system-wide **"block negative stock"** setting is on (the same switch warehouse documents respect) — and when it's off, production is not stopped but the movement records the **shortfall** explicitly, so an overdraw stays findable. + - **Part of the optional Warehouses module.** With the module off, consumption moves no location balance and refuses nothing — a plant that does not run per-location stock is unaffected. Rollout steps are in [`docs/warehouse-erp-rollout.md`](docs/warehouse-erp-rollout.md). - New `lines.warehouse_id` and `material_allocations.consumption_warehouse_id` / `location_deducted_qty` columns, `App\Services\Material\ConsumptionLocationService`, and a shared `App\Services\Warehouse\WarehouseStockService` that the stock-document posting path now uses too, so both routes into a location balance share one race-safe implementation. - **Product types as Bill-of-Materials components** *(admin)* — a BOM line can now be a manufactured **product type** (a sub-assembly), not only a material. In the BOM editor a Material / Product type switch picks the component kind; product-type lines carry the same quantity-per-unit, step, scrap %, consumption timing and notes as materials. A product type can't be a component of itself, and each appears once per template. Lines are captured in the work-order snapshot as sub-assembly references; they're a simple component reference (they don't explode into their own BOM) and are skipped by the material stock/consumption engine. Additive — existing material BOMs are unaffected. diff --git a/backend/app/Services/Material/ConsumptionLocationService.php b/backend/app/Services/Material/ConsumptionLocationService.php index 9e5a5e639..a5384c4af 100644 --- a/backend/app/Services/Material/ConsumptionLocationService.php +++ b/backend/app/Services/Material/ConsumptionLocationService.php @@ -7,6 +7,7 @@ use App\Models\User; use App\Models\Warehouse; use App\Services\Warehouse\WarehouseStockService; +use App\Support\ModuleRegistry; use Illuminate\Support\Facades\DB; /** @@ -38,12 +39,20 @@ public function __construct( * downward correction produces a positive delta and credits the location back. * * Returns the movement it wrote, or null when there was nothing to move (no - * location resolvable, or the quantity did not change). + * location resolvable, the Warehouses module is off, or the quantity did not + * change). * * @throws \DomainException When the location lacks the stock and the plant blocks negative balances. */ public function deduct(MaterialAllocation $allocation, float $consumedTotal, ?User $user = null): ?StockMovement { + // Per-location balances belong to the Warehouses module (#212). With it off, + // a plant that once had warehouses must not have production refused — or + // silently booked — against balances nobody is maintaining any more. + if (! ModuleRegistry::isModuleEnabled('warehouse')) { + return null; + } + return DB::transaction(function () use ($allocation, $consumedTotal, $user) { // Re-read under a lock: `location_deducted_qty` is a read-modify-write, and // two operators booking on the same allocation would otherwise each deduct diff --git a/backend/tests/Feature/Warehouse/ConsumptionLocationTest.php b/backend/tests/Feature/Warehouse/ConsumptionLocationTest.php index 2ec612c39..4fc44f8d4 100644 --- a/backend/tests/Feature/Warehouse/ConsumptionLocationTest.php +++ b/backend/tests/Feature/Warehouse/ConsumptionLocationTest.php @@ -12,6 +12,7 @@ use App\Models\WorkOrder; use App\Services\Material\ConsumptionLocationService; use App\Services\Material\MaterialAllocationService; +use App\Support\ModuleRegistry; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\DB; use Tests\TestCase; @@ -266,6 +267,23 @@ public function test_a_plant_with_no_locations_at_all_is_left_alone(): void $this->assertEquals(0.0, (float) $allocation->fresh()->location_deducted_qty); } + public function test_the_deduction_stays_out_of_the_way_when_the_module_is_off(): void + { + ModuleRegistry::save(array_values(array_diff(ModuleRegistry::enabled(), ['warehouse']))); + + $warehouse = $this->warehouse('WS-1', default: true); + $material = Material::factory()->create(); + $stock = $this->stockAt($warehouse, $material, 500); + + $allocation = $this->allocationOnLine($warehouse, $material); + + // Balances nobody maintains any more must neither move nor refuse production. + $this->assertNull($this->service->deduct($allocation, 120)); + $this->assertEquals(500.0, (float) $stock->fresh()->quantity); + $this->assertEquals(0.0, (float) $allocation->fresh()->location_deducted_qty); + $this->assertNull($allocation->fresh()->consumption_warehouse_id); + } + /** * A correction must credit back the location the material actually left, even * after the lot has been moved somewhere else. diff --git a/docs/warehouse-erp-rollout.md b/docs/warehouse-erp-rollout.md index 976c9a2a6..f469ad705 100644 --- a/docs/warehouse-erp-rollout.md +++ b/docs/warehouse-erp-rollout.md @@ -154,9 +154,18 @@ until the module is enabled — that is expected. Leave it on unless the ERP is going to create that paperwork itself. Generated documents are **drafts** — they move no stock until posted. -5. **Check the negative-stock policy.** Settings → System → *block negative stock*. +5. **Point each line at its stock location.** Admin → Lines → *Stock location*. + Consumption booked on a line is deducted from the location the material actually + came off: the **picked lot's** warehouse if the pick knows one, otherwise the + line's stock location, otherwise the default raw-material warehouse. Every + deduction writes a `stock_movements` row carrying that warehouse. A line left + without a stock location still works — it falls back to the default — and with the + Warehouses module off, consumption moves no location balance at all. +6. **Check the negative-stock policy.** Settings → System → *block negative stock*. With it on, posting a release that would drive a material below zero is refused — - which is what you want once opening stock is loaded, and painful before that. + and so is **shop-floor consumption a location cannot cover** (step 5) — which is + what you want once opening stock is loaded, and painful before that. Load the + opening stock first, or leave the setting off until cutover is done. At this point the UI works and nothing is synced yet. @@ -357,4 +366,6 @@ reversible without one. | Balances look halved or doubled | A per-material total was added to its own per-lot rows | Report the lot-less row as the total; lot rows are the breakdown | | Global stock disagrees with the warehouses | A balance was written by a path that skipped reconciliation | Reconciliation query in step 5, then re-run the stock import | | Posting refused: "would drive below zero" | `block negative stock` is on and opening stock is short | Load the opening stock, or clear the setting during cutover | +| Consumption refused: "location does not hold enough" | `block negative stock` is on and that location's balance is short | Load its opening stock, move stock to it, or clear the setting during cutover | +| Consumption deducted from the wrong store | The location is frozen on the allocation at its first deduction | Check the picked lot's warehouse and the line's stock location; later corrections credit back the store that gave the material up | | ERP sync keeps inflating stock | Treating the import as a delta | It is a snapshot — send the current quantity, not the change | From a8b7dadf51f8889212a3093d871890b4c7215c71 Mon Sep 17 00:00:00 2001 From: JanKolo04 Date: Mon, 31 Aug 2026 00:05:56 +0200 Subject: [PATCH 3/4] fix(warehouse): address CodeRabbit review on consumption-by-location MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Line validation moves into StoreLineRequest / UpdateLineRequest (hard rule 3). The stock location must now be a live, active, material-holding warehouse of the caller's own tenant — matching what the picker offers, instead of any row in `warehouses` by id. - Picks spanning two stores are split per lot warehouse. Lot picking is FEFO across lots and ignores warehouses, so the old "first lot's warehouse wins" charged one store for what another gave up. - Scrap is deducted with the consumed quantity at both call sites: it left the store too — only the returned leftover stayed behind. - The balance row is locked before it is read (WarehouseStockService:: lockOrCreate), closing the window where two bookings both passed the sufficiency check and overdrew the location together. - Document posting checks the issuing warehouse's balance as well as the plant-wide quantity, and checks before moving anything rather than relying on the rollback. - The race-losing INSERT runs in a nested transaction: on PostgreSQL a failed statement poisons the whole transaction, so catching the unique violation without a savepoint left the caller's transaction aborted. - Deductions quantise to the balance column's 3 decimals, so the allocation's running total and the balance cannot drift and re-deduct the residue. Tests: scrap (booking + cancellation), picks split across two locations and credited back per store, a warehouse-short posting refused while the plant is long, plus guest / operator authorization, a 422 for JSON clients and rejection of finished-goods and archived locations on the line form. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 3 +- .../Web/Admin/LineManagementController.php | 32 +-- .../Concerns/ValidatesLineStockLocation.php | 42 ++++ .../Requests/Web/Admin/StoreLineRequest.php | 43 ++++ .../Requests/Web/Admin/UpdateLineRequest.php | 47 ++++ .../Material/ConsumptionLocationService.php | 200 ++++++++++++------ .../Material/MaterialAllocationService.php | 17 +- .../Warehouse/StockDocumentService.php | 42 +++- .../Warehouse/WarehouseStockService.php | 45 +++- backend/lang/en.json | 3 +- backend/lang/pl.json | 3 +- .../Warehouse/ConsumptionLocationTest.php | 110 +++++++++- .../Warehouse/LineStockLocationTest.php | 74 +++++++ .../Warehouse/StockDocumentServiceTest.php | 44 ++++ 14 files changed, 582 insertions(+), 123 deletions(-) create mode 100644 backend/app/Http/Requests/Concerns/ValidatesLineStockLocation.php create mode 100644 backend/app/Http/Requests/Web/Admin/StoreLineRequest.php create mode 100644 backend/app/Http/Requests/Web/Admin/UpdateLineRequest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 480dcddb3..4eb65d95b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,9 +11,10 @@ Format based on [Keep a Changelog](https://keepachangelog.com/). - **Consumption is deducted from the workshop location it came off** — stock levels per storage location now reflect what production actually used. Allocation already moved the plant-wide quantity and the picked lot, but nothing said *where* the material physically was, so a plant running several stores could not tell which one had emptied. - **Each line names its stock location.** A production line gains a **Stock location** (Admin → Lines), picked from the raw-material warehouses. Optional: a plant that doesn't track stock per location leaves it unset and nothing changes. - **The location is resolved most-specific-first.** The **picked lot's** warehouse wins (it knows exactly where it sits), then the **line's** stock location, then the plant's **default raw-material** warehouse. Once a deduction has been made the location is **frozen on the allocation**, so a later correction always credits back the location that actually gave the material up — even if the lot has since been moved or the line re-pointed. + - **Split across the stores it really came from.** Lot picking is FEFO across the material's lots and knows nothing about stores, so one allocation can draw from two — each pick's share is booked against its own lot's warehouse, and a later correction credits every store back exactly what it gave. **Scrap counts as consumed** for this: it left the store too, unlike the leftover that is returned. - **Booked by difference, never twice.** Consumption is recorded more than once for the same allocation (an operator's entry, a correction, then batch completion finalising the rest), so the balance moves by the **difference** each time. A downward correction credits the location back, and cancelling a batch returns everything it had taken. - **Auditable per deduction.** Every deduction writes a `stock_movements` row carrying the **warehouse**, the batch/step it came from and the quantity — the plant-wide quantity is deliberately *not* moved again, since allocation already booked it. - - **Stock cannot silently go negative.** Consumption exceeding the location's balance is **refused** when the system-wide **"block negative stock"** setting is on (the same switch warehouse documents respect) — and when it's off, production is not stopped but the movement records the **shortfall** explicitly, so an overdraw stays findable. + - **Stock cannot silently go negative.** The balance row is **locked before it is read**, so two concurrent bookings cannot both pass the same check; posting a warehouse document now honours the **location's** balance as well as the plant-wide one. Consumption exceeding the location's balance is **refused** when the system-wide **"block negative stock"** setting is on (the same switch warehouse documents respect) — and when it's off, production is not stopped but the movement records the **shortfall** explicitly, so an overdraw stays findable. - **Part of the optional Warehouses module.** With the module off, consumption moves no location balance and refuses nothing — a plant that does not run per-location stock is unaffected. Rollout steps are in [`docs/warehouse-erp-rollout.md`](docs/warehouse-erp-rollout.md). - New `lines.warehouse_id` and `material_allocations.consumption_warehouse_id` / `location_deducted_qty` columns, `App\Services\Material\ConsumptionLocationService`, and a shared `App\Services\Warehouse\WarehouseStockService` that the stock-document posting path now uses too, so both routes into a location balance share one race-safe implementation. - **Traceability when editing an in-use process template** *(admin)* — editing a template's steps (add / rename / delete / reorder) still mutates the current template in place, and running work orders correctly keep their frozen snapshot — but that used to happen silently. Now the template page shows a **warning banner** when the template backs active (non-finished) work orders, destructive step edits ask for confirmation while it's in use, and **every step change is written to the immutable audit log** (before/after shape) so the previous version is never lost. No change to how orders resolve their steps. diff --git a/backend/app/Http/Controllers/Web/Admin/LineManagementController.php b/backend/app/Http/Controllers/Web/Admin/LineManagementController.php index a0485990e..5f00e62c3 100644 --- a/backend/app/Http/Controllers/Web/Admin/LineManagementController.php +++ b/backend/app/Http/Controllers/Web/Admin/LineManagementController.php @@ -3,6 +3,8 @@ namespace App\Http\Controllers\Web\Admin; use App\Http\Controllers\Controller; +use App\Http\Requests\Web\Admin\StoreLineRequest; +use App\Http\Requests\Web\Admin\UpdateLineRequest; use App\Models\Area; use App\Models\Line; use App\Models\LineStatus; @@ -69,20 +71,11 @@ private function areaOptions(): \Illuminate\Support\Collection /** * Store a newly created line */ - public function store(Request $request) + public function store(StoreLineRequest $request) { $cf = app(CustomFieldService::class); - $validated = $request->validate(array_merge([ - 'code' => 'required|string|max:50|unique:lines', - 'name' => 'required|string|max:255', - 'description' => 'nullable|string', - 'area_id' => 'nullable|exists:areas,id', - // The stock location this line's consumption comes off. - 'warehouse_id' => 'nullable|exists:warehouses,id', - 'is_active' => 'boolean', - ], $cf->rules('line')), [], $cf->attributeNames('line')); - - $validated['is_active'] = $request->boolean('is_active', true); + $validated = $request->validated(); + unset($validated['custom_field_files']); if ($cf->touched($request)) { $validated['custom_fields'] = $cf->fromRequest($request, 'line') ?: null; @@ -184,20 +177,11 @@ public function edit(Line $line) /** * Update the specified line */ - public function update(Request $request, Line $line) + public function update(UpdateLineRequest $request, Line $line) { $cf = app(CustomFieldService::class); - $validated = $request->validate(array_merge([ - 'code' => 'required|string|max:50|unique:lines,code,'.$line->id, - 'name' => 'required|string|max:255', - 'description' => 'nullable|string', - 'area_id' => 'nullable|exists:areas,id', - // The stock location this line's consumption comes off. - 'warehouse_id' => 'nullable|exists:warehouses,id', - 'is_active' => 'boolean', - ], $cf->rules('line')), [], $cf->attributeNames('line')); - - $validated['is_active'] = $request->boolean('is_active'); + $validated = $request->validated(); + unset($validated['custom_field_files']); if ($cf->touched($request)) { $validated['custom_fields'] = $cf->fromRequest($request, 'line', $line->custom_fields) ?: null; diff --git a/backend/app/Http/Requests/Concerns/ValidatesLineStockLocation.php b/backend/app/Http/Requests/Concerns/ValidatesLineStockLocation.php new file mode 100644 index 000000000..b1b5b6c3d --- /dev/null +++ b/backend/app/Http/Requests/Concerns/ValidatesLineStockLocation.php @@ -0,0 +1,42 @@ + */ + protected function stockLocationRules(): array + { + return [ + 'nullable', + 'integer', + Rule::exists('warehouses', 'id') + ->whereNull('deleted_at') + ->where('is_active', true) + ->whereIn('kind', [Warehouse::KIND_RAW_MATERIAL, Warehouse::KIND_MIXED]) + ->where(function ($query) { + $tenantId = $this->user()?->tenant_id; + + if ($tenantId) { + $query->where('tenant_id', $tenantId); + } + }), + ]; + } +} diff --git a/backend/app/Http/Requests/Web/Admin/StoreLineRequest.php b/backend/app/Http/Requests/Web/Admin/StoreLineRequest.php new file mode 100644 index 000000000..792d0eb9a --- /dev/null +++ b/backend/app/Http/Requests/Web/Admin/StoreLineRequest.php @@ -0,0 +1,43 @@ +merge(['is_active' => $this->boolean('is_active', true)]); + } + + public function rules(): array + { + return array_merge([ + 'code' => ['required', 'string', 'max:50', 'unique:lines,code'], + 'name' => ['required', 'string', 'max:255'], + 'description' => ['nullable', 'string'], + 'area_id' => ['nullable', 'exists:areas,id'], + // The stock location this line's consumption comes off. + 'warehouse_id' => $this->stockLocationRules(), + 'is_active' => ['boolean'], + ], $this->customFieldRules()); + } +} diff --git a/backend/app/Http/Requests/Web/Admin/UpdateLineRequest.php b/backend/app/Http/Requests/Web/Admin/UpdateLineRequest.php new file mode 100644 index 000000000..9135bf861 --- /dev/null +++ b/backend/app/Http/Requests/Web/Admin/UpdateLineRequest.php @@ -0,0 +1,47 @@ +merge(['is_active' => $this->boolean('is_active')]); + } + + public function rules(): array + { + return array_merge([ + 'code' => [ + 'required', 'string', 'max:50', + Rule::unique('lines', 'code')->ignore($this->route('line')?->id), + ], + 'name' => ['required', 'string', 'max:255'], + 'description' => ['nullable', 'string'], + 'area_id' => ['nullable', 'exists:areas,id'], + // The stock location this line's consumption comes off. + 'warehouse_id' => $this->stockLocationRules(), + 'is_active' => ['boolean'], + ], $this->customFieldRules()); + } +} diff --git a/backend/app/Services/Material/ConsumptionLocationService.php b/backend/app/Services/Material/ConsumptionLocationService.php index a5384c4af..45f259f1a 100644 --- a/backend/app/Services/Material/ConsumptionLocationService.php +++ b/backend/app/Services/Material/ConsumptionLocationService.php @@ -31,26 +31,28 @@ public function __construct( ) {} /** - * Book consumption for an allocation against its location, moving the balance by - * the difference between what is now consumed and what was already deducted. + * Book consumption for an allocation against its location(s), moving the balance + * by the difference between what is now consumed and what was already deducted. * * Called every time a consumed quantity is written — an operator's entry, a * correction, batch completion — so it must be the delta, not the total. A - * downward correction produces a positive delta and credits the location back. + * downward correction produces a negative delta and credits the location back. * - * Returns the movement it wrote, or null when there was nothing to move (no - * location resolvable, the Warehouses module is off, or the quantity did not - * change). + * Returns the movements it wrote, one per location touched, and an empty array + * when there was nothing to move (no location resolvable, the Warehouses module + * is off, or the quantity did not change). * - * @throws \DomainException When the location lacks the stock and the plant blocks negative balances. + * @return array + * + * @throws \DomainException When a location lacks the stock and the plant blocks negative balances. */ - public function deduct(MaterialAllocation $allocation, float $consumedTotal, ?User $user = null): ?StockMovement + public function deduct(MaterialAllocation $allocation, float $consumedTotal, ?User $user = null): array { // Per-location balances belong to the Warehouses module (#212). With it off, // a plant that once had warehouses must not have production refused — or // silently booked — against balances nobody is maintaining any more. if (! ModuleRegistry::isModuleEnabled('warehouse')) { - return null; + return []; } return DB::transaction(function () use ($allocation, $consumedTotal, $user) { @@ -60,72 +62,120 @@ public function deduct(MaterialAllocation $allocation, float $consumedTotal, ?Us $locked = MaterialAllocation::where('id', $allocation->getKey())->lockForUpdate()->first(); if (! $locked) { - return null; + return []; } - $warehouse = $this->resolveWarehouse($locked); + $fallback = $this->resolveWarehouse($locked); - if ($warehouse === null) { + if ($fallback === null) { // No location to attribute this to — a plant that does not track stock // per location. Global stock and the lot already moved at allocation, // so there is simply nothing further to do. - return null; + return []; } - $delta = round($consumedTotal - (float) $locked->location_deducted_qty, 4); + // Quantise to the precision `warehouse_stocks.quantity` actually stores + // (decimal(14,3)). Booking a 4-decimal quantity against a 3-decimal column + // would leave the allocation's running total and the balance disagreeing by + // the rounding, and the residue would be re-deducted on every later call. + $delta = round(round($consumedTotal, 3) - (float) $locked->location_deducted_qty, 3); - if (abs($delta) < 0.00005) { - return null; + if (abs($delta) < 0.0005) { + return []; } - $keys = [ - 'warehouse_id' => $warehouse->id, - 'material_id' => $locked->material_id, - ]; - - // Read the balance once, before anything moves: it decides both whether the - // deduction is refused and — when it is not — how big a shortfall to flag. - $available = $this->warehouseStock->available($keys); + $movements = []; - $this->assertSufficient($locked, $warehouse, $delta, $available); + foreach ($this->splitByLocation($locked, $delta, $fallback) as $warehouseId => $split) { + $movement = $this->applyAtLocation($locked, (int) $warehouseId, $split, $user); - // The lot-level balance and the material total for that location are kept - // separately (#212), so a per-material view does not have to sum lots. - foreach ($this->lotShares($locked, $delta) as $lotId => $lotDelta) { - $this->warehouseStock->adjust([...$keys, 'material_lot_id' => $lotId], -$lotDelta); + if ($movement) { + $movements[] = $movement; + } } - $this->warehouseStock->adjust([...$keys, 'material_lot_id' => null], -$delta); - $locked->update([ - 'consumption_warehouse_id' => $warehouse->id, - 'location_deducted_qty' => round((float) $locked->location_deducted_qty + $delta, 4), + // The allocation's own location: the one resolved for it, kept for the + // lot-less path and frozen so a correction credits back the same store. + 'consumption_warehouse_id' => $locked->consumption_warehouse_id ?? $fallback->id, + 'location_deducted_qty' => round((float) $locked->location_deducted_qty + $delta, 3), ]); - // Audit: one ledger row per deduction, carrying the location. `adjustGlobal` - // is off because allocation already moved the plant-wide quantity — this - // row exists to say which location gave the material up. - return $this->stockMovements->record( - material: $locked->material, - movementType: $delta > 0 ? StockMovement::TYPE_CONSUME : StockMovement::TYPE_RETURN, - signedQuantity: -$delta, - user: $user, - sourceType: $locked->batch_step_id - ? StockMovement::SOURCE_BATCH_STEP - : StockMovement::SOURCE_BATCH, - sourceId: $locked->batch_step_id ?: $locked->batch_id, - reason: $this->reason($locked, $delta, $available), - warehouseId: $warehouse->id, - adjustGlobal: false, - ); + return $movements; }); } /** - * Give back everything this allocation took off its location — used when a batch - * is cancelled after consumption had already been booked. + * Move one location's share of a deduction and write its ledger row. + * + * @param array{total: float, lots: array} $split + */ + private function applyAtLocation( + MaterialAllocation $allocation, + int $warehouseId, + array $split, + ?User $user, + ): ?StockMovement { + $delta = $split['total']; + + if (abs($delta) < 0.0005) { + return null; + } + + $keys = [ + 'warehouse_id' => $warehouseId, + 'material_id' => $allocation->material_id, + ]; + + // Lock the material total for this location before reading it: the balance + // decides both whether the deduction is refused and how big a shortfall to + // flag, and an unlocked read would let two bookings pass the same check and + // overdraw together. The lock is held for the rest of this transaction, so the + // adjust() below re-locks nothing. + $balance = $this->warehouseStock->lockOrCreate([...$keys, 'material_lot_id' => null]); + $available = (float) $balance->quantity; + + $warehouse = Warehouse::find($warehouseId); + + if ($warehouse === null) { + return null; + } + + $this->assertSufficient($allocation, $warehouse, $delta, $available); + + // The lot-level balance and the material total for that location are kept + // separately (#212), so a per-material view does not have to sum lots. + foreach ($split['lots'] as $lotId => $lotDelta) { + $this->warehouseStock->adjust([...$keys, 'material_lot_id' => $lotId], -$lotDelta); + } + + $this->warehouseStock->adjust([...$keys, 'material_lot_id' => null], -$delta); + + // Audit: one ledger row per location per deduction, carrying the warehouse. + // `adjustGlobal` is off because allocation already moved the plant-wide + // quantity — this row exists to say which location gave the material up. + return $this->stockMovements->record( + material: $allocation->material, + movementType: $delta > 0 ? StockMovement::TYPE_CONSUME : StockMovement::TYPE_RETURN, + signedQuantity: -$delta, + user: $user, + sourceType: $allocation->batch_step_id + ? StockMovement::SOURCE_BATCH_STEP + : StockMovement::SOURCE_BATCH, + sourceId: $allocation->batch_step_id ?: $allocation->batch_id, + reason: $this->reason($allocation, $delta, $available), + warehouseId: $warehouseId, + adjustGlobal: false, + ); + } + + /** + * Give back everything this allocation took off its location(s) — used when a + * batch is cancelled after consumption had already been booked. + * + * @return array */ - public function reverse(MaterialAllocation $allocation, ?User $user = null): ?StockMovement + public function reverse(MaterialAllocation $allocation, ?User $user = null): array { return $this->deduct($allocation, 0, $user); } @@ -165,38 +215,54 @@ public function resolveWarehouse(MaterialAllocation $allocation): ?Warehouse } /** - * Split a deduction across the lots this allocation picked, proportionally to what - * was picked from each, so a lot-level balance never drifts from the material - * total above it. Returns [lot id => quantity] and is empty when nothing was - * picked by lot. + * Split a deduction across the locations it actually comes off. + * + * Lot picks are not guaranteed to sit in one store — lot selection is FEFO across + * the material's lots, so a single allocation can legitimately draw from two. + * Each pick's share therefore goes to its own lot's warehouse (falling back to the + * allocation's location for a lot that names none), proportionally to what was + * picked from it, so no store is ever charged for material another one gave up. + * With nothing picked by lot, the whole delta goes to the allocation's location. * - * @return array + * The shares are proportional to `picked_qty`, which does not change after the + * fact — so a later correction splits exactly the way the deduction did and every + * store is credited back precisely what it gave. + * + * @return array}> */ - private function lotShares(MaterialAllocation $allocation, float $delta): array + private function splitByLocation(MaterialAllocation $allocation, float $delta, Warehouse $fallback): array { $picks = $allocation->lotPicks->filter(fn ($pick) => (float) $pick->picked_qty > 0); $total = (float) $picks->sum('picked_qty'); if ($total <= 0) { - return []; + return [$fallback->id => ['total' => $delta, 'lots' => []]]; } - $shares = []; + $split = []; $assigned = 0.0; foreach ($picks->values() as $index => $pick) { - // The last share takes the remainder, so rounding can never leave the lot - // rows summing to something other than the material total. + // The last share takes the remainder, so rounding can never leave the + // per-location rows summing to something other than the delta. $share = $index === $picks->count() - 1 - ? round($delta - $assigned, 4) - : round($delta * ((float) $pick->picked_qty / $total), 4); + ? round($delta - $assigned, 3) + : round($delta * ((float) $pick->picked_qty / $total), 3); - $assigned += $share; + $assigned = round($assigned + $share, 3); + + if (abs($share) < 0.0005) { + continue; + } + + $warehouseId = (int) ($pick->lot?->warehouse_id ?: $fallback->id); $lotId = (int) $pick->material_lot_id; - $shares[$lotId] = round(($shares[$lotId] ?? 0) + $share, 4); + + $split[$warehouseId]['total'] = round(($split[$warehouseId]['total'] ?? 0) + $share, 3); + $split[$warehouseId]['lots'][$lotId] = round(($split[$warehouseId]['lots'][$lotId] ?? 0) + $share, 3); } - return $shares; + return $split; } /** diff --git a/backend/app/Services/Material/MaterialAllocationService.php b/backend/app/Services/Material/MaterialAllocationService.php index 6c37f995b..06cf53c04 100644 --- a/backend/app/Services/Material/MaterialAllocationService.php +++ b/backend/app/Services/Material/MaterialAllocationService.php @@ -260,8 +260,12 @@ public function consumeForBatch(Batch $batch): void // Finalise the location balance against the quantity that actually // stands. Deducts only the part not already booked by the operator's // own entries, so a batch whose consumption was recorded step by step - // is not deducted twice. - $this->consumptionLocation->deduct($allocation, $actualConsumed); + // is not deducted twice. Scrap counts: it physically left the store + // too — only the leftover being returned above stayed behind. + $this->consumptionLocation->deduct( + $allocation, + $actualConsumed + (float) $allocation->scrap_qty, + ); $allocation->update([ 'status' => MaterialAllocation::STATUS_CONSUMED, @@ -340,10 +344,11 @@ public function recordConsumption( 'price_currency_snapshot' => $actualConsumed > 0 ? $allocation->material?->price_currency : null, ]); - // Take it off the location it came from. Same transaction as the quantity - // itself, so a refused deduction (location short, plant blocks negatives) - // leaves no consumed_qty claiming material that never moved. - $this->consumptionLocation->deduct($allocation->fresh(), $actualConsumed); + // Take it off the location it came from — consumed plus scrap, since both + // physically left the store. Same transaction as the quantity itself, so a + // refused deduction (location short, plant blocks negatives) leaves no + // consumed_qty claiming material that never moved. + $this->consumptionLocation->deduct($allocation->fresh(), $actualConsumed + $scrap); return $allocation->fresh(); }); diff --git a/backend/app/Services/Warehouse/StockDocumentService.php b/backend/app/Services/Warehouse/StockDocumentService.php index b186a6d97..f54429ec9 100644 --- a/backend/app/Services/Warehouse/StockDocumentService.php +++ b/backend/app/Services/Warehouse/StockDocumentService.php @@ -238,20 +238,22 @@ private function applyLine(StockDocument $document, $line, bool $reverse, ?User { $signed = $document->direction() * (float) $line->quantity * ($reverse ? -1 : 1); - $this->adjustWarehouseStock($document, $line, $signed); - - if (! $document->isMaterialDocument() || $line->material_id === null) { - return; + $material = $document->isMaterialDocument() && $line->material_id !== null + ? Material::find($line->material_id) + : null; + + // Before anything moves: an issue the stock cannot cover is refused here, not + // rolled back after the balances have already been written. + if ($material) { + $this->guardNegativeStock($document, $material, $signed); } - $material = Material::find($line->material_id); + $this->adjustWarehouseStock($document, $line, $signed); if (! $material) { return; } - $this->guardNegativeStock($material, $signed); - // materials.stock_quantity + the stock_movements ledger. The movement // type mirrors the direction so the ledger reads the same as a shop-floor // consumption or an inbound receipt. @@ -332,14 +334,36 @@ private function adjustLot(int $lotId, float $signed, int $materialId): void /** * Honour the system-wide "block negative stock" setting, the same switch the - * material allocation path respects. + * material allocation and shop-floor consumption paths respect. + * + * Both views of the stock are checked, because either can be the short one: the + * plant may hold plenty of a material while the warehouse this document issues + * from holds none of it. */ - private function guardNegativeStock(Material $material, float $signed): void + private function guardNegativeStock(StockDocument $document, Material $material, float $signed): void { if ($signed >= 0 || ! $this->warehouseStock->blocksNegativeStock()) { return; } + // Locked, not just read: the lock is held for the rest of this transaction, so + // the balance cannot be spent by a concurrent posting between the check here + // and the move that follows it. + $balance = $this->warehouseStock->lockOrCreate([ + 'warehouse_id' => $document->warehouse_id, + 'material_id' => $material->id, + ]); + + if ((float) $balance->quantity + $signed < 0) { + throw ValidationException::withMessages([ + 'lines' => __('Posting would drive :material below zero at :warehouse (:available available).', [ + 'material' => $material->code, + 'warehouse' => $document->warehouse?->code ?? $document->warehouse_id, + 'available' => (float) $balance->quantity, + ]), + ]); + } + if ((float) $material->stock_quantity + $signed < 0) { throw ValidationException::withMessages([ 'lines' => __('Posting would drive :material below zero stock (:available available).', [ diff --git a/backend/app/Services/Warehouse/WarehouseStockService.php b/backend/app/Services/Warehouse/WarehouseStockService.php index 3bcc7a9d3..cad4b89ac 100644 --- a/backend/app/Services/Warehouse/WarehouseStockService.php +++ b/backend/app/Services/Warehouse/WarehouseStockService.php @@ -26,6 +26,32 @@ class WarehouseStockService * @param array $keys warehouse_id, material_id, product_type_id, material_lot_id */ public function adjust(array $keys, float $signed, ?string $unit = null): WarehouseStock + { + $stock = $this->lockOrCreate($keys, $unit); + + $stock->quantity = round((float) $stock->quantity + $signed, 3); + + if ($unit && ! $stock->unit_of_measure) { + $stock->unit_of_measure = $unit; + } + + $stock->save(); + + return $stock; + } + + /** + * The balance row for a slot, locked for the rest of the caller's transaction and + * created at zero if it does not exist yet. + * + * Callers that decide something from the balance (refusing a deduction the + * location cannot cover) must read it through this rather than {@see available()}: + * an unlocked read lets two concurrent bookings both pass the same check and + * overdraw the location together. + * + * @param array $keys warehouse_id, material_id, product_type_id, material_lot_id + */ + public function lockOrCreate(array $keys, ?string $unit = null): WarehouseStock { $keys = $this->normalizeKeys($keys); @@ -35,8 +61,17 @@ public function adjust(array $keys, float $signed, ?string $unit = null): Wareho // lockForUpdate() can only lock rows that exist, so two concurrent callers // can both miss and then race to insert. The partial unique index decides // the winner; the loser re-reads the row it lost to instead of failing. + // + // The insert runs in a nested transaction (a SAVEPOINT) because on + // PostgreSQL a failed statement poisons the whole transaction: catching the + // violation without a savepoint to roll back to would leave the caller's + // transaction aborted, and every later statement in it would fail too. try { - $stock = WarehouseStock::create([...$keys, 'quantity' => 0, 'unit_of_measure' => $unit]); + $stock = DB::transaction(fn () => WarehouseStock::create([ + ...$keys, + 'quantity' => 0, + 'unit_of_measure' => $unit, + ])); } catch (UniqueConstraintViolationException) { $stock = WarehouseStock::query()->where($keys)->lockForUpdate()->first(); } @@ -46,14 +81,6 @@ public function adjust(array $keys, float $signed, ?string $unit = null): Wareho throw new \RuntimeException('Could not read the stock balance to update.'); } - $stock->quantity = round((float) $stock->quantity + $signed, 3); - - if ($unit && ! $stock->unit_of_measure) { - $stock->unit_of_measure = $unit; - } - - $stock->save(); - return $stock; } diff --git a/backend/lang/en.json b/backend/lang/en.json index ed29edfe9..46c13327d 100644 --- a/backend/lang/en.json +++ b/backend/lang/en.json @@ -5676,5 +5676,6 @@ "This template backs :count active work order(s).": "This template backs :count active work order(s).", "Editing steps here does not affect those running orders — they keep the frozen snapshot taken when they were created. Changes are not versioned; the previous shape is kept only in the audit log. For a controlled change to a specific order, use its change request instead.": "Editing steps here does not affect those running orders — they keep the frozen snapshot taken when they were created. Changes are not versioned; the previous shape is kept only in the audit log. For a controlled change to a specific order, use its change request instead.", "Edit a template that is in use?": "Edit a template that is in use?", - "This template backs :count active work order(s). They keep their frozen snapshot and are unaffected, but this change is not versioned — only the audit log keeps the previous shape.": "This template backs :count active work order(s). They keep their frozen snapshot and are unaffected, but this change is not versioned — only the audit log keeps the previous shape." + "This template backs :count active work order(s). They keep their frozen snapshot and are unaffected, but this change is not versioned — only the audit log keeps the previous shape.": "This template backs :count active work order(s). They keep their frozen snapshot and are unaffected, but this change is not versioned — only the audit log keeps the previous shape.", + "Posting would drive :material below zero at :warehouse (:available available).": "Posting would drive :material below zero at :warehouse (:available available)." } diff --git a/backend/lang/pl.json b/backend/lang/pl.json index fd8c8d57c..cd21d842c 100644 --- a/backend/lang/pl.json +++ b/backend/lang/pl.json @@ -5676,5 +5676,6 @@ "This template backs :count active work order(s).": "Ten szablon obsługuje :count aktywnych zleceń.", "Editing steps here does not affect those running orders — they keep the frozen snapshot taken when they were created. Changes are not versioned; the previous shape is kept only in the audit log. For a controlled change to a specific order, use its change request instead.": "Edycja kroków tutaj nie wpływa na trwające zlecenia — zachowują one zamrożony snapshot z chwili utworzenia. Zmiany nie są wersjonowane; poprzedni kształt pozostaje wyłącznie w dzienniku audytu. Dla kontrolowanej zmiany konkretnego zlecenia użyj jego wniosku o zmianę.", "Edit a template that is in use?": "Edytować szablon będący w użyciu?", - "This template backs :count active work order(s). They keep their frozen snapshot and are unaffected, but this change is not versioned — only the audit log keeps the previous shape.": "Ten szablon obsługuje :count aktywnych zleceń. Zachowują one zamrożony snapshot i pozostają nienaruszone, ale ta zmiana nie jest wersjonowana — poprzedni kształt zachowuje tylko dziennik audytu." + "This template backs :count active work order(s). They keep their frozen snapshot and are unaffected, but this change is not versioned — only the audit log keeps the previous shape.": "Ten szablon obsługuje :count aktywnych zleceń. Zachowują one zamrożony snapshot i pozostają nienaruszone, ale ta zmiana nie jest wersjonowana — poprzedni kształt zachowuje tylko dziennik audytu.", + "Posting would drive :material below zero at :warehouse (:available available).": "Księgowanie zeszłoby poniżej zera dla :material w lokalizacji :warehouse (dostępne: :available)." } diff --git a/backend/tests/Feature/Warehouse/ConsumptionLocationTest.php b/backend/tests/Feature/Warehouse/ConsumptionLocationTest.php index 4fc44f8d4..3acf83da5 100644 --- a/backend/tests/Feature/Warehouse/ConsumptionLocationTest.php +++ b/backend/tests/Feature/Warehouse/ConsumptionLocationTest.php @@ -185,7 +185,7 @@ public function test_an_unchanged_quantity_writes_nothing(): void $this->service->deduct($allocation, 25); $movements = StockMovement::count(); - $this->assertNull($this->service->deduct($allocation, 25)); + $this->assertSame([], $this->service->deduct($allocation, 25)); $this->assertSame($movements, StockMovement::count()); } @@ -262,7 +262,7 @@ public function test_a_plant_with_no_locations_at_all_is_left_alone(): void 'batch_id' => \App\Models\Batch::factory()->create(['work_order_id' => $workOrder->id])->id, ]); - $this->assertNull($this->service->deduct($allocation, 25)); + $this->assertSame([], $this->service->deduct($allocation, 25)); $this->assertSame(0, WarehouseStock::count()); $this->assertEquals(0.0, (float) $allocation->fresh()->location_deducted_qty); } @@ -278,7 +278,7 @@ public function test_the_deduction_stays_out_of_the_way_when_the_module_is_off() $allocation = $this->allocationOnLine($warehouse, $material); // Balances nobody maintains any more must neither move nor refuse production. - $this->assertNull($this->service->deduct($allocation, 120)); + $this->assertSame([], $this->service->deduct($allocation, 120)); $this->assertEquals(500.0, (float) $stock->fresh()->quantity); $this->assertEquals(0.0, (float) $allocation->fresh()->location_deducted_qty); $this->assertNull($allocation->fresh()->consumption_warehouse_id); @@ -346,7 +346,7 @@ public function test_the_same_consumption_is_allowed_and_flagged_when_the_plant_ $this->stockAt($warehouse, $material, 10); $allocation = $this->allocationOnLine($warehouse, $material); - $movement = $this->service->deduct($allocation, 40); + $movement = $this->service->deduct($allocation, 40)[0]; // Production is not stopped, but the overdraw is on the record. $this->assertEquals(-30.0, $this->balance($warehouse, $material)); @@ -363,7 +363,7 @@ public function test_a_location_that_holds_exactly_enough_is_not_a_shortfall(): $this->stockAt($warehouse, $material, 40); $allocation = $this->allocationOnLine($warehouse, $material); - $movement = $this->service->deduct($allocation, 40); + $movement = $this->service->deduct($allocation, 40)[0]; $this->assertEquals(0.0, $this->balance($warehouse, $material)); $this->assertStringNotContainsString('SHORTFALL', $movement->reason); @@ -401,6 +401,106 @@ public function test_batch_completion_without_an_operator_entry_deducts_the_allo $this->assertEquals(425.0, $this->balance($warehouse, $material)); } + public function test_scrap_is_taken_off_the_location_together_with_what_was_used(): void + { + $warehouse = $this->warehouse('WS-1'); + $material = Material::factory()->create(); + $this->stockAt($warehouse, $material, 500); + + $allocation = $this->allocationOnLine($warehouse, $material); + $allocations = app(MaterialAllocationService::class); + + // 60 used + 5 spoiled: both left the store, only the rest stayed behind. + $allocations->recordConsumption($allocation, 60, scrap: 5); + + $this->assertEquals(435.0, $this->balance($warehouse, $material)); + $this->assertEquals(65.0, (float) $allocation->fresh()->location_deducted_qty); + + // Batch completion finalises the same consumed + scrap pair: no second bite. + $allocations->consumeForBatch($allocation->batch); + + $this->assertEquals(435.0, $this->balance($warehouse, $material)); + } + + public function test_cancelling_a_batch_gives_back_the_scrap_it_took_too(): void + { + $warehouse = $this->warehouse('WS-1'); + $material = Material::factory()->create(); + $this->stockAt($warehouse, $material, 500); + + $allocation = $this->allocationOnLine($warehouse, $material); + $allocations = app(MaterialAllocationService::class); + + $allocations->recordConsumption($allocation, 60, scrap: 5); + $allocations->returnForBatch($allocation->batch); + + $this->assertEquals(500.0, $this->balance($warehouse, $material)); + $this->assertEquals(0.0, (float) $allocation->fresh()->location_deducted_qty); + } + + /** + * Lot picking is FEFO across the material's lots and knows nothing about stores, + * so one allocation can legitimately draw from two — and neither may be charged + * for what the other gave up. + */ + public function test_picks_spanning_two_locations_are_split_between_them(): void + { + $lineWarehouse = $this->warehouse('WS-LINE'); + $first = $this->warehouse('WS-A'); + $second = $this->warehouse('WS-B'); + $material = Material::factory()->create(); + + $lotA = MaterialLot::factory()->create(['material_id' => $material->id, 'warehouse_id' => $first->id]); + $lotB = MaterialLot::factory()->create(['material_id' => $material->id, 'warehouse_id' => $second->id]); + + $this->stockAt($first, $material, 500); + $this->stockAt($first, $material, 500, $lotA); + $this->stockAt($second, $material, 500); + $this->stockAt($second, $material, 500, $lotB); + + $allocation = $this->allocationOnLine($lineWarehouse, $material); + $allocation->lotPicks()->create(['material_lot_id' => $lotA->id, 'picked_qty' => 75]); + $allocation->lotPicks()->create(['material_lot_id' => $lotB->id, 'picked_qty' => 25]); + + $movements = $this->service->deduct($allocation->fresh(), 40); + + // 75/25 of the picked quantity, so 30/10 of the consumption. + $this->assertEquals(470.0, $this->balance($first, $material)); + $this->assertEquals(470.0, $this->balance($first, $material, $lotA)); + $this->assertEquals(490.0, $this->balance($second, $material)); + $this->assertEquals(490.0, $this->balance($second, $material, $lotB)); + + // One ledger row per location, each naming its own. + $this->assertCount(2, $movements); + $this->assertEqualsCanonicalizing( + [$first->id, $second->id], + collect($movements)->pluck('warehouse_id')->all(), + ); + } + + public function test_a_correction_credits_each_location_its_own_share_back(): void + { + $first = $this->warehouse('WS-A'); + $second = $this->warehouse('WS-B'); + $material = Material::factory()->create(); + + $lotA = MaterialLot::factory()->create(['material_id' => $material->id, 'warehouse_id' => $first->id]); + $lotB = MaterialLot::factory()->create(['material_id' => $material->id, 'warehouse_id' => $second->id]); + + $this->stockAt($first, $material, 500); + $this->stockAt($second, $material, 500); + + $allocation = $this->allocationOnLine($first, $material); + $allocation->lotPicks()->create(['material_lot_id' => $lotA->id, 'picked_qty' => 50]); + $allocation->lotPicks()->create(['material_lot_id' => $lotB->id, 'picked_qty' => 50]); + + $this->service->deduct($allocation->fresh(), 40); + $this->service->deduct($allocation->fresh(), 10); + + $this->assertEquals(495.0, $this->balance($first, $material)); + $this->assertEquals(495.0, $this->balance($second, $material)); + } + public function test_cancelling_a_batch_gives_the_location_its_material_back(): void { $warehouse = $this->warehouse('WS-1'); diff --git a/backend/tests/Feature/Warehouse/LineStockLocationTest.php b/backend/tests/Feature/Warehouse/LineStockLocationTest.php index b160b7883..8193c97b6 100644 --- a/backend/tests/Feature/Warehouse/LineStockLocationTest.php +++ b/backend/tests/Feature/Warehouse/LineStockLocationTest.php @@ -95,6 +95,80 @@ public function test_an_unknown_location_is_rejected(): void ->assertSessionHasErrors('warehouse_id'); } + public function test_a_finished_goods_location_is_rejected(): void + { + $finishedGoods = Warehouse::factory()->finishedGoods()->create(); + + // A line draws components, never finished product — the picker does not offer + // this warehouse, and a hand-made request must not get past that either. + $this->actingAs($this->admin) + ->post('/admin/lines', [ + 'code' => 'L-FG', + 'name' => 'Assembly FG', + 'warehouse_id' => $finishedGoods->id, + ]) + ->assertSessionHasErrors('warehouse_id'); + + $this->assertDatabaseMissing('lines', ['code' => 'L-FG']); + } + + public function test_an_archived_location_is_rejected(): void + { + $archived = Warehouse::factory()->rawMaterial()->create(['is_active' => false]); + + $this->actingAs($this->admin) + ->post('/admin/lines', [ + 'code' => 'L-OFF', + 'name' => 'Assembly Off', + 'warehouse_id' => $archived->id, + ]) + ->assertSessionHasErrors('warehouse_id'); + } + + /** The API shape of the same rejection: a JSON client gets 422, not a redirect. */ + public function test_the_rejection_is_a_422_for_a_json_client(): void + { + $this->actingAs($this->admin) + ->postJson('/admin/lines', [ + 'code' => 'L-4', + 'name' => 'Assembly 4', + 'warehouse_id' => 999999, + ]) + ->assertStatus(422) + ->assertJsonValidationErrors('warehouse_id'); + } + + public function test_a_guest_cannot_point_a_line_at_a_location(): void + { + $warehouse = Warehouse::factory()->rawMaterial()->create(); + + $this->post('/admin/lines', [ + 'code' => 'L-GUEST', + 'name' => 'Assembly Guest', + 'warehouse_id' => $warehouse->id, + ])->assertRedirect('/login'); + + $this->assertDatabaseMissing('lines', ['code' => 'L-GUEST']); + } + + public function test_an_operator_cannot_point_a_line_at_a_location(): void + { + $operator = User::factory()->create(); + $operator->assignRole('Operator'); + $warehouse = Warehouse::factory()->rawMaterial()->create(); + $line = Line::factory()->create(['warehouse_id' => null]); + + $this->actingAs($operator) + ->put("/admin/lines/{$line->id}", [ + 'code' => $line->code, + 'name' => $line->name, + 'warehouse_id' => $warehouse->id, + ]) + ->assertForbidden(); + + $this->assertNull($line->fresh()->warehouse_id); + } + /** Stock location stays optional — a plant that doesn't track it is unaffected. */ public function test_a_line_without_a_stock_location_is_still_valid(): void { diff --git a/backend/tests/Feature/Warehouse/StockDocumentServiceTest.php b/backend/tests/Feature/Warehouse/StockDocumentServiceTest.php index 3f3cfc348..cdce6fe67 100644 --- a/backend/tests/Feature/Warehouse/StockDocumentServiceTest.php +++ b/backend/tests/Feature/Warehouse/StockDocumentServiceTest.php @@ -283,6 +283,50 @@ public function test_posting_respects_the_block_negative_stock_setting(): void $this->assertTrue($document->fresh()->isDraft()); } + /** + * The plant can hold plenty of a material while the store this document issues + * from holds none of it — either view being short has to stop the posting. + */ + public function test_posting_is_blocked_when_the_warehouse_is_short_but_the_plant_is_not(): void + { + DB::table('system_settings')->updateOrInsert( + ['key' => 'block_negative_stock'], + ['value' => json_encode(true)], + ); + + $warehouse = $this->rawWarehouse(); + $material = Material::factory()->create(['code' => 'FLOUR-02', 'stock_quantity' => 1000]); + + // Plenty in the plant, 5 in this store. + \App\Models\WarehouseStock::factory()->create([ + 'warehouse_id' => $warehouse->id, + 'material_id' => $material->id, + 'quantity' => 5, + ]); + + $document = $this->service->createDraft([ + 'type' => StockDocument::TYPE_MATERIAL_ISSUE, + 'warehouse_id' => $warehouse->id, + 'lines' => [['material_id' => $material->id, 'quantity' => 50]], + ]); + + try { + $this->service->post($document); + $this->fail('Posting below the warehouse balance should have been blocked.'); + } catch (ValidationException $e) { + $this->assertStringContainsString('FLOUR-02', collect($e->errors())->flatten()->implode(' ')); + } + + // Nothing moved: not the location balance, not the plant-wide quantity. + $this->assertEquals(5.0, (float) \App\Models\WarehouseStock::where([ + 'warehouse_id' => $warehouse->id, + 'material_id' => $material->id, + 'material_lot_id' => null, + ])->value('quantity')); + $this->assertEquals(1000.0, (float) $material->fresh()->stock_quantity); + $this->assertTrue($document->fresh()->isDraft()); + } + public function test_document_numbers_are_sequential_per_type_and_year(): void { $this->rawWarehouse(); From d905000136f91c7841cd4dab21ce6823df03fa70 Mon Sep 17 00:00:00 2001 From: JanKolo04 Date: Mon, 31 Aug 2026 10:22:15 +0200 Subject: [PATCH 4/4] fix(warehouse): address second CodeRabbit round on consumption-by-location MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A picked lot the location cannot cover is refused on its own account. The material total having enough is a different question: it is every lot plus the untracked remainder, so consuming an empty lot used to leave a negative lot row behind a healthy-looking total. Every lot row is now locked and checked before any of them moves, in lot-id order so two bookings cannot deadlock. - Each pick freezes the location it was deducted from (allocation_lot_picks.consumption_warehouse_id), so a lot moved between the deduction and the correction still credits back the store that gave the material up — the same rule the allocation already followed, one level down. - The line's stock-location rule keeps mirroring TenantScope (scope when there is a tenant, no clause when there is not) rather than rejecting on a null tenant: tenancy is dormant on single-tenant installs, where users and warehouses both carry a null tenant_id and the picker offers all of them. Rejecting there would refuse every valid warehouse. A test now pins the cross-tenant rejection that the clause exists for. Tests: a moved lot credited back to the store it left, a picked lot the location cannot cover refused with nothing moved, and another tenant's warehouse rejected while the caller's own is accepted. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 2 +- .../Concerns/ValidatesLineStockLocation.php | 5 ++ backend/app/Models/AllocationLotPick.php | 8 +++ .../Material/ConsumptionLocationService.php | 56 ++++++++++++++- ...tion_warehouse_to_allocation_lot_picks.php | 36 ++++++++++ backend/lang/en.json | 3 +- backend/lang/pl.json | 3 +- .../Warehouse/ConsumptionLocationTest.php | 68 +++++++++++++++++++ .../Warehouse/LineStockLocationTest.php | 35 ++++++++++ 9 files changed, 211 insertions(+), 5 deletions(-) create mode 100644 backend/database/migrations/2026_08_31_100000_add_consumption_warehouse_to_allocation_lot_picks.php diff --git a/CHANGELOG.md b/CHANGELOG.md index de2fed31a..e7f1c7521 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ Format based on [Keep a Changelog](https://keepachangelog.com/). - **Consumption is deducted from the workshop location it came off** — stock levels per storage location now reflect what production actually used. Allocation already moved the plant-wide quantity and the picked lot, but nothing said *where* the material physically was, so a plant running several stores could not tell which one had emptied. - **Each line names its stock location.** A production line gains a **Stock location** (Admin → Lines), picked from the raw-material warehouses. Optional: a plant that doesn't track stock per location leaves it unset and nothing changes. - **The location is resolved most-specific-first.** The **picked lot's** warehouse wins (it knows exactly where it sits), then the **line's** stock location, then the plant's **default raw-material** warehouse. Once a deduction has been made the location is **frozen on the allocation**, so a later correction always credits back the location that actually gave the material up — even if the lot has since been moved or the line re-pointed. - - **Split across the stores it really came from.** Lot picking is FEFO across the material's lots and knows nothing about stores, so one allocation can draw from two — each pick's share is booked against its own lot's warehouse, and a later correction credits every store back exactly what it gave. **Scrap counts as consumed** for this: it left the store too, unlike the leftover that is returned. + - **Split across the stores it really came from.** Lot picking is FEFO across the material's lots and knows nothing about stores, so one allocation can draw from two — each pick's share is booked against its own lot's warehouse, **frozen on the pick at its first deduction** so a lot moved afterwards still credits back the store that gave the material up. A picked lot the location cannot cover is refused on its own account: a healthy material total is not the same answer as the lot being there. **Scrap counts as consumed** for this: it left the store too, unlike the leftover that is returned. - **Booked by difference, never twice.** Consumption is recorded more than once for the same allocation (an operator's entry, a correction, then batch completion finalising the rest), so the balance moves by the **difference** each time. A downward correction credits the location back, and cancelling a batch returns everything it had taken. - **Auditable per deduction.** Every deduction writes a `stock_movements` row carrying the **warehouse**, the batch/step it came from and the quantity — the plant-wide quantity is deliberately *not* moved again, since allocation already booked it. - **Stock cannot silently go negative.** The balance row is **locked before it is read**, so two concurrent bookings cannot both pass the same check; posting a warehouse document now honours the **location's** balance as well as the plant-wide one. Consumption exceeding the location's balance is **refused** when the system-wide **"block negative stock"** setting is on (the same switch warehouse documents respect) — and when it's off, production is not stopped but the movement records the **shortfall** explicitly, so an overdraw stays findable. diff --git a/backend/app/Http/Requests/Concerns/ValidatesLineStockLocation.php b/backend/app/Http/Requests/Concerns/ValidatesLineStockLocation.php index b1b5b6c3d..44f039348 100644 --- a/backend/app/Http/Requests/Concerns/ValidatesLineStockLocation.php +++ b/backend/app/Http/Requests/Concerns/ValidatesLineStockLocation.php @@ -31,6 +31,11 @@ protected function stockLocationRules(): array ->where('is_active', true) ->whereIn('kind', [Warehouse::KIND_RAW_MATERIAL, Warehouse::KIND_MIXED]) ->where(function ($query) { + // Mirrors TenantScope exactly: scope to the tenant when there is + // one, and to nothing when there is not. Rejecting outright on a + // null tenant would break every single-tenant install — tenancy is + // dormant there, so users and warehouses both carry a null + // tenant_id and the picker offers all of them. $tenantId = $this->user()?->tenant_id; if ($tenantId) { diff --git a/backend/app/Models/AllocationLotPick.php b/backend/app/Models/AllocationLotPick.php index d0c5bde47..2163ec7dc 100644 --- a/backend/app/Models/AllocationLotPick.php +++ b/backend/app/Models/AllocationLotPick.php @@ -23,6 +23,8 @@ class AllocationLotPick extends Model protected $fillable = [ 'material_allocation_id', 'material_lot_id', + // The location this pick was deducted from, frozen at the first deduction. + 'consumption_warehouse_id', 'tenant_id', 'picked_qty', 'picking_strategy', @@ -44,4 +46,10 @@ public function lot(): BelongsTo { return $this->belongsTo(MaterialLot::class, 'material_lot_id'); } + + /** The location this pick was consumed from, once anything has been deducted. */ + public function consumptionWarehouse(): BelongsTo + { + return $this->belongsTo(Warehouse::class, 'consumption_warehouse_id'); + } } diff --git a/backend/app/Services/Material/ConsumptionLocationService.php b/backend/app/Services/Material/ConsumptionLocationService.php index 45f259f1a..1a110255d 100644 --- a/backend/app/Services/Material/ConsumptionLocationService.php +++ b/backend/app/Services/Material/ConsumptionLocationService.php @@ -3,6 +3,7 @@ namespace App\Services\Material; use App\Models\MaterialAllocation; +use App\Models\MaterialLot; use App\Models\StockMovement; use App\Models\User; use App\Models\Warehouse; @@ -145,7 +146,23 @@ private function applyAtLocation( // The lot-level balance and the material total for that location are kept // separately (#212), so a per-material view does not have to sum lots. - foreach ($split['lots'] as $lotId => $lotDelta) { + // + // Every lot row is locked and checked BEFORE any of them moves: a location can + // hold enough of a material in total while the particular lot this allocation + // picked is empty, and a half-applied deduction would leave one lot negative + // and the material total already spent. + $lotDeltas = $split['lots']; + ksort($lotDeltas); // Deterministic lock order — two bookings cannot deadlock. + + $lotRows = []; + + foreach ($lotDeltas as $lotId => $lotDelta) { + $lotRows[$lotId] = $this->warehouseStock->lockOrCreate([...$keys, 'material_lot_id' => $lotId]); + + $this->assertLotSufficient($lotId, $warehouse, $lotDelta, (float) $lotRows[$lotId]->quantity); + } + + foreach ($lotDeltas as $lotId => $lotDelta) { $this->warehouseStock->adjust([...$keys, 'material_lot_id' => $lotId], -$lotDelta); } @@ -255,9 +272,17 @@ private function splitByLocation(MaterialAllocation $allocation, float $delta, W continue; } - $warehouseId = (int) ($pick->lot?->warehouse_id ?: $fallback->id); + // Frozen first: a lot moved after its first deduction must still credit + // back the store that actually gave the material up. + $warehouseId = (int) ($pick->consumption_warehouse_id ?: $pick->lot?->warehouse_id ?: $fallback->id); $lotId = (int) $pick->material_lot_id; + if ($pick->consumption_warehouse_id === null) { + // Freeze it on first use, inside the caller's transaction — a deduction + // refused further down rolls this back with it. + $pick->update(['consumption_warehouse_id' => $warehouseId]); + } + $split[$warehouseId]['total'] = round(($split[$warehouseId]['total'] ?? 0) + $share, 3); $split[$warehouseId]['lots'][$lotId] = round(($split[$warehouseId]['lots'][$lotId] ?? 0) + $share, 3); } @@ -265,6 +290,33 @@ private function splitByLocation(MaterialAllocation $allocation, float $delta, W return $split; } + /** + * Refuse a deduction a picked lot cannot cover at that location, when the plant + * says so. The material total having enough is not the same answer: the total is + * every lot plus the untracked remainder, and consuming a lot that is not there + * would leave a negative lot row behind a healthy-looking total. + * + * @throws \DomainException + */ + private function assertLotSufficient(int $lotId, Warehouse $warehouse, float $delta, float $available): void + { + if ($delta <= 0 || ! $this->warehouseStock->blocksNegativeStock()) { + return; + } + + if ($available + 0.0005 < $delta) { + throw new \DomainException(__( + 'Lot :lot at :warehouse holds :available, less than the :needed being consumed.', + [ + 'lot' => MaterialLot::find($lotId)?->lot_number ?? $lotId, + 'warehouse' => $warehouse->code, + 'available' => round($available, 3), + 'needed' => round($delta, 3), + ], + )); + } + } + /** * Refuse a deduction the location cannot cover, when the plant says so. * diff --git a/backend/database/migrations/2026_08_31_100000_add_consumption_warehouse_to_allocation_lot_picks.php b/backend/database/migrations/2026_08_31_100000_add_consumption_warehouse_to_allocation_lot_picks.php new file mode 100644 index 000000000..359afd6c5 --- /dev/null +++ b/backend/database/migrations/2026_08_31_100000_add_consumption_warehouse_to_allocation_lot_picks.php @@ -0,0 +1,36 @@ +foreignId('consumption_warehouse_id')->nullable()->after('material_lot_id') + ->constrained('warehouses')->nullOnDelete() + ->comment('Location this pick was deducted from, frozen at the first deduction'); + }); + } + + public function down(): void + { + Schema::table('allocation_lot_picks', function (Blueprint $table) { + $table->dropConstrainedForeignId('consumption_warehouse_id'); + }); + } +}; diff --git a/backend/lang/en.json b/backend/lang/en.json index 68a56fe91..55338cc9a 100644 --- a/backend/lang/en.json +++ b/backend/lang/en.json @@ -5699,5 +5699,6 @@ "Could not add the maintenance.": "Could not add the maintenance.", "Pick a maintenance for this slot": "Pick a maintenance for this slot", "Search maintenance": "Search maintenance", - "No maintenance schedules.": "No maintenance schedules." + "No maintenance schedules.": "No maintenance schedules.", + "Lot :lot at :warehouse holds :available, less than the :needed being consumed.": "Lot :lot at :warehouse holds :available, less than the :needed being consumed." } diff --git a/backend/lang/pl.json b/backend/lang/pl.json index ca56ee7bc..3384002c7 100644 --- a/backend/lang/pl.json +++ b/backend/lang/pl.json @@ -5699,5 +5699,6 @@ "Could not add the maintenance.": "Nie udało się dodać przeglądu.", "Pick a maintenance for this slot": "Wybierz przegląd dla tego slotu", "Search maintenance": "Szukaj przeglądu", - "No maintenance schedules.": "Brak harmonogramów przeglądów." + "No maintenance schedules.": "Brak harmonogramów przeglądów.", + "Lot :lot at :warehouse holds :available, less than the :needed being consumed.": "Partia :lot w :warehouse ma :available, mniej niż zużywane :needed." } diff --git a/backend/tests/Feature/Warehouse/ConsumptionLocationTest.php b/backend/tests/Feature/Warehouse/ConsumptionLocationTest.php index 3acf83da5..01969d91b 100644 --- a/backend/tests/Feature/Warehouse/ConsumptionLocationTest.php +++ b/backend/tests/Feature/Warehouse/ConsumptionLocationTest.php @@ -478,6 +478,74 @@ public function test_picks_spanning_two_locations_are_split_between_them(): void ); } + /** + * A lot that has been moved between the deduction and the correction must still + * credit back the store that gave the material up — the pick freezes its location + * the first time it is deducted, exactly as the allocation does. + */ + public function test_a_moved_lot_still_credits_back_the_store_it_left(): void + { + $original = $this->warehouse('WS-A'); + $moved = $this->warehouse('WS-B'); + $material = Material::factory()->create(); + + $lot = MaterialLot::factory()->create([ + 'material_id' => $material->id, + 'warehouse_id' => $original->id, + ]); + + $this->stockAt($original, $material, 500); + $this->stockAt($original, $material, 500, $lot); + $this->stockAt($moved, $material, 500); + + $allocation = $this->allocationOnLine($original, $material); + $allocation->lotPicks()->create(['material_lot_id' => $lot->id, 'picked_qty' => 100]); + + $this->service->deduct($allocation->fresh(), 40); + $this->assertEquals(460.0, $this->balance($original, $material)); + + // The lot is transferred to another store, then the entry is corrected down. + $lot->update(['warehouse_id' => $moved->id]); + $this->service->deduct($allocation->fresh(), 10); + + $this->assertEquals(490.0, $this->balance($original, $material)); + $this->assertEquals(490.0, $this->balance($original, $material, $lot)); + // The store the lot moved to never gave anything up, so it is left alone. + $this->assertEquals(500.0, $this->balance($moved, $material)); + } + + public function test_a_picked_lot_the_location_cannot_cover_is_refused(): void + { + $this->blockNegativeStock(true); + + $warehouse = $this->warehouse('WS-1'); + $material = Material::factory()->create(); + $lot = MaterialLot::factory()->create([ + 'material_id' => $material->id, + 'warehouse_id' => $warehouse->id, + 'lot_number' => 'LOT-EMPTY', + ]); + + // The store holds plenty of the material overall, but almost none of this lot. + $this->stockAt($warehouse, $material, 500); + $this->stockAt($warehouse, $material, 2, $lot); + + $allocation = $this->allocationOnLine($warehouse, $material); + $allocation->lotPicks()->create(['material_lot_id' => $lot->id, 'picked_qty' => 100]); + + try { + $this->service->deduct($allocation->fresh(), 40); + $this->fail('Consuming a lot the location cannot cover should have been refused.'); + } catch (\DomainException $e) { + $this->assertStringContainsString('LOT-EMPTY', $e->getMessage()); + } + + // Neither the lot row nor the material total moved. + $this->assertEquals(2.0, $this->balance($warehouse, $material, $lot)); + $this->assertEquals(500.0, $this->balance($warehouse, $material)); + $this->assertEquals(0.0, (float) $allocation->fresh()->location_deducted_qty); + } + public function test_a_correction_credits_each_location_its_own_share_back(): void { $first = $this->warehouse('WS-A'); diff --git a/backend/tests/Feature/Warehouse/LineStockLocationTest.php b/backend/tests/Feature/Warehouse/LineStockLocationTest.php index 8193c97b6..2d20b91b8 100644 --- a/backend/tests/Feature/Warehouse/LineStockLocationTest.php +++ b/backend/tests/Feature/Warehouse/LineStockLocationTest.php @@ -125,6 +125,41 @@ public function test_an_archived_location_is_rejected(): void ->assertSessionHasErrors('warehouse_id'); } + /** + * `Rule::exists` queries the table directly and so bypasses the model's global + * TenantScope — without the tenant clause, one tenant could name another's + * warehouse by id even though the picker never offers it. + */ + public function test_another_tenants_location_is_rejected(): void + { + $ours = \App\Models\Tenant::factory()->create(); + $theirs = \App\Models\Tenant::factory()->create(); + + $this->admin->update(['tenant_id' => $ours->id]); + + $foreign = Warehouse::factory()->rawMaterial()->create(['tenant_id' => $theirs->id]); + $own = Warehouse::factory()->rawMaterial()->create(['tenant_id' => $ours->id]); + + $this->actingAs($this->admin) + ->post('/admin/lines', [ + 'code' => 'L-TEN', + 'name' => 'Assembly Tenant', + 'warehouse_id' => $foreign->id, + ]) + ->assertSessionHasErrors('warehouse_id'); + + $this->assertDatabaseMissing('lines', ['code' => 'L-TEN']); + + // The tenant's own store is still accepted. + $this->actingAs($this->admin) + ->post('/admin/lines', [ + 'code' => 'L-TEN-OK', + 'name' => 'Assembly Tenant OK', + 'warehouse_id' => $own->id, + ]) + ->assertRedirect(); + } + /** The API shape of the same rejection: a JSON client gets 422, not a redirect. */ public function test_the_rejection_is_a_422_for_a_json_client(): void {