3
0
mirror of https://github.com/snipe/snipe-it.git synced 2026-08-18 11:15:42 +00:00

More importer fixes and test fixes

This commit is contained in:
snipe
2026-08-05 22:46:24 +01:00
parent 3503dc7ee7
commit 94869caaee
8 changed files with 111 additions and 46 deletions

View File

@ -98,18 +98,22 @@ trait HandlesAdjustQuantity
// Only dedupe when there's a real order_number label to match
// on. A blank order_number is a distinct transaction each time
// (own timestamp, supplier, cost, currency), not a bucket to
// pool anonymous acquisitions into. created_by is set via
// property assignment rather than mass-fill because it's
// guarded on both Order and OrderItem to prevent forgery.
// pool anonymous acquisitions into. purchase_date is part of
// the dedup key because Snipe-IT has no partial-receipt concept
// — every Order is a completed receipt-in-hand, so "same
// order_number on a different receipt date" is a distinct
// event, not a staggered delivery of one order. created_by is
// set via property assignment rather than mass-fill because
// it's guarded on both Order and OrderItem to prevent forgery.
if ($payload['order_number'] !== null) {
$order = Order::firstOrNew(
[
'order_number' => $payload['order_number'],
'supplier_id' => $payload['supplier_id'],
'company_id' => $model->company_id ?? null,
'purchase_date' => $payload['purchase_date'],
],
[
'purchase_date' => $payload['purchase_date'],
'currency' => $payload['currency'],
'notes' => $payload['notes'],
],

View File

@ -74,6 +74,14 @@ class AccessoryImporter extends ItemImporter
}
}
// See ConsumableImporter::handle for the default_* mirror rationale.
if (array_key_exists('supplier_id', $this->item)) {
$this->item['default_supplier_id'] = $this->item['supplier_id'];
}
if (array_key_exists('purchase_cost', $this->item)) {
$this->item['default_purchase_cost'] = $this->item['purchase_cost'];
}
// Internal signals used by the checkout logic below; neither is
// fillable on Accessory so sanitize's fillable filter drops them.
$this->item['checkout_class'] = $this->findCsvMatch($row, 'checkout_class');

View File

@ -73,6 +73,14 @@ class ComponentImporter extends ItemImporter
}
}
// See ConsumableImporter::handle for the default_* mirror rationale.
if (array_key_exists('supplier_id', $this->item)) {
$this->item['default_supplier_id'] = $this->item['supplier_id'];
}
if (array_key_exists('purchase_cost', $this->item)) {
$this->item['default_purchase_cost'] = $this->item['purchase_cost'];
}
$this->item['created_by'] = $this->created_by;
$this->createComponentIfNotExists($row);

View File

@ -69,6 +69,20 @@ class ConsumableImporter extends ItemImporter
}
}
// Mirror supplier_id / purchase_cost into the parent's
// default_* template fields so future orders pre-populate from
// the last-known-good CSV import. See Consumable::$fillable and
// ItemImporter::recordOrderForImportedRow for the split — Order
// rows still receive supplier_id / purchase_cost via that helper
// (that's the per-acquisition record); default_* here is the
// per-parent template.
if (array_key_exists('supplier_id', $this->item)) {
$this->item['default_supplier_id'] = $this->item['supplier_id'];
}
if (array_key_exists('purchase_cost', $this->item)) {
$this->item['default_purchase_cost'] = $this->item['purchase_cost'];
}
// Internal signals for the checkout logic; neither is fillable on
// Consumable so sanitize's fillable filter drops them.
$this->item['checkout_class'] = $this->findCsvMatch($row, 'checkout_class');

View File

@ -230,8 +230,8 @@ class AdjustAccessoryQuantityApiTest extends TestCase
{
// Repeated qty adjusts referencing the same order_number should
// reuse the existing Order row (dedupe on order_number +
// supplier + company), while each adjust still produces its own
// OrderItem line (one line per event).
// supplier + company + purchase_date), while each adjust still
// produces its own OrderItem line (one line per event).
$accessory = Accessory::factory()->create(['qty' => 0]);
$supplier = \App\Models\Supplier::factory()->create();
$actor = User::factory()->editAccessories()->create();
@ -240,21 +240,52 @@ class AdjustAccessoryQuantityApiTest extends TestCase
$this->actingAsForApi($actor)
->postJson(route('api.accessories.adjust-quantity', $accessory), [
'amount' => $delta,
'note' => 'staggered receipt',
'order_number' => 'PO-DEDUPE',
'note' => 'same receipt, split line',
'order_number' => 'ORD-DEDUPE',
'supplier_id' => $supplier->id,
'purchase_date' => '2026-04-01',
])
->assertOk();
}
$orders = \App\Models\Order::where('order_number', 'PO-DEDUPE')->get();
$this->assertCount(1, $orders, 'Same order_number should reuse the existing Order row.');
$orders = \App\Models\Order::where('order_number', 'ORD-DEDUPE')->get();
$this->assertCount(1, $orders, 'Same order_number + supplier + date should reuse the existing Order row.');
$items = $orders->first()->orderItems;
$this->assertCount(2, $items, 'Each adjust event should have its own OrderItem line.');
$this->assertEqualsCanonicalizing([3, 4], $items->pluck('qty')->map(fn ($q) => (int) $q)->all());
}
public function test_same_order_number_on_different_purchase_dates_creates_distinct_orders()
{
// Snipe-IT has no partial-receipt concept — every Order is a
// completed receipt-in-hand. Same order_number appearing with
// two different purchase_dates therefore represents two
// distinct events, not staggered delivery, so each gets its
// own Order row.
$accessory = Accessory::factory()->create(['qty' => 0]);
$supplier = \App\Models\Supplier::factory()->create();
$actor = User::factory()->editAccessories()->create();
foreach (['2026-04-01', '2026-04-15'] as $date) {
$this->actingAsForApi($actor)
->postJson(route('api.accessories.adjust-quantity', $accessory), [
'amount' => 5,
'note' => 'later receipt on same order number',
'order_number' => 'ORD-SPLIT-DATE',
'supplier_id' => $supplier->id,
'purchase_date' => $date,
])
->assertOk();
}
$this->assertSame(
2,
\App\Models\Order::where('order_number', 'ORD-SPLIT-DATE')->count(),
'Different purchase_dates under the same order_number should be distinct Orders.',
);
}
public function test_blank_order_number_creates_a_distinct_order_per_event()
{
// A blank order_number is a distinct transaction each time, not

View File

@ -66,7 +66,7 @@ class ImportAccessoriesTest extends ImportDataTestCase implements TestsPermissio
]);
$newAccessory = Accessory::query()
->with(['location', 'category', 'manufacturer', 'supplier', 'company'])
->with(['location', 'category', 'manufacturer', 'defaultSupplier', 'company'])
->where('name', $row['itemName'])
->sole();
@ -308,17 +308,13 @@ class ImportAccessoriesTest extends ImportDataTestCase implements TestsPermissio
$this->assertEquals($row['itemName'], $updatedAccessory->name);
$this->assertEquals($row['companyName'], $updatedAccessory->company->name);
$this->assertEquals($row['quantity'], $updatedAccessory->qty);
// Acquisition metadata (order_number / purchase_date /
// purchase_cost / supplier) lives on the latest OrderItem's
// Order, not the parent. Update mode writes a fresh Order +
// OrderItem via recordOrderForImportedRow when the CSV carries
// acquisition columns. (When qty differs, the value also rides
// on the QuantityAdjust log — see the sibling test
// importer_qty_change_creates_quantity_adjust_log.)
$latestOrderItem = $updatedAccessory->orderItems()->latest('id')->firstOrFail();
$this->assertEquals($row['purchaseDate'], $latestOrderItem->order->purchase_date->toDateString());
$this->assertEquals((float) $row['purchaseCost'], (float) $latestOrderItem->price);
$this->assertEquals($row['supplierName'], $latestOrderItem->order->supplier->name);
// Update mode does NOT rewrite historical Orders — the CSV's
// purchase_cost / supplier map to the parent's default_*
// template fields; purchase_date has no forward-use equivalent
// and is silently dropped. When qty differs the value rides on
// the QuantityAdjust log (see importer_qty_change_creates_...).
$this->assertEquals((float) $row['purchaseCost'], (float) $updatedAccessory->default_purchase_cost);
$this->assertEquals($row['supplierName'], $updatedAccessory->defaultSupplier->name);
$this->assertEquals($row['notes'], $updatedAccessory->notes);
$this->assertEquals($row['category'], $updatedAccessory->category->name);
$this->assertEquals('accessory', $updatedAccessory->category->category_type);

View File

@ -97,7 +97,7 @@ class ImportComponentsTest extends ImportDataTestCase implements TestsPermission
$this->assertNull($newComponent->min_amt);
$this->assertEquals($row['serialNumber'], $newComponent->serial);
$this->assertNull($newComponent->image);
$this->assertNull($newComponent->notes);
$this->assertEquals($row['notes'], $newComponent->notes);
}
#[Test]
@ -244,18 +244,18 @@ class ImportComponentsTest extends ImportDataTestCase implements TestsPermission
$this->assertEquals($row['itemName'], $updatedComponent->name);
$this->assertEquals($row['category'], $updatedComponent->category->name);
$this->assertEquals($row['location'], $updatedComponent->location->name);
$this->assertEquals($component->default_supplier_id, $updatedComponent->default_supplier_id);
$this->assertEquals($row['quantity'], $updatedComponent->qty);
// Acquisition metadata lives on the latest OrderItem's Order —
// the update path writes a new Order + OrderItem when the CSV
// carries acquisition columns, per recordOrderForImportedRow.
$latestOrderItem = $updatedComponent->orderItems()->latest('id')->firstOrFail();
$this->assertEquals($row['purchaseDate'], $latestOrderItem->order->purchase_date->toDateString());
$this->assertEquals((float) $row['purchaseCost'], (float) $latestOrderItem->price);
// Update mode does NOT rewrite historical Orders. purchase_cost
// maps to the parent's default_purchase_cost template;
// purchase_date has no parent equivalent and is dropped on
// update.
$this->assertEquals((float) $row['purchaseCost'], (float) $updatedComponent->default_purchase_cost);
$this->assertEquals($component->min_amt, $updatedComponent->min_amt);
$this->assertEquals($row['serialNumber'], $updatedComponent->serial);
$this->assertEquals($component->image, $updatedComponent->image);
$this->assertEquals($component->notes, $updatedComponent->notes);
// notes IS present in the CSV (see ComponentsImportFileBuilder
// definition), so update mode overwrites the seeded value.
$this->assertEquals($row['notes'], $updatedComponent->notes);
}
#[Test]
@ -364,9 +364,10 @@ class ImportComponentsTest extends ImportDataTestCase implements TestsPermission
])->assertOk();
$component->refresh();
// purchase_cost moved to the OrderItem's price column.
$latestOrderItem = $component->orderItems()->latest('id')->firstOrFail();
$this->assertEquals((float) $updatedRow['purchaseCost'], (float) $latestOrderItem->price);
// Update path maps CSV purchase_cost to the parent's template
// field (see update_component_from_import); historical Orders
// aren't rewritten on update mode.
$this->assertEquals((float) $updatedRow['purchaseCost'], (float) $component->default_purchase_cost);
$updateLog = ActionLog::query()
->where('item_type', Component::class)

View File

@ -100,7 +100,7 @@ class ImportConsumablesTest extends ImportDataTestCase implements TestsPermissio
$this->assertEquals('', $newConsumable->model_number);
$this->assertNull($newConsumable->item_number);
$this->assertNull($newConsumable->manufacturer_id);
$this->assertNull($newConsumable->notes);
$this->assertEquals($row['notes'], $newConsumable->notes);
}
#[Test]
@ -237,20 +237,22 @@ class ImportConsumablesTest extends ImportDataTestCase implements TestsPermissio
$this->assertEquals($row['category'], $updatedConsumable->category->name);
$this->assertEquals($row['location'], $updatedConsumable->location->name);
$this->assertEquals($row['companyName'], $updatedConsumable->company->name);
// Acquisition metadata lives on the latest OrderItem's Order —
// the update path writes a new Order + OrderItem when the CSV
// carries acquisition columns, per recordOrderForImportedRow.
$latestOrderItem = $updatedConsumable->orderItems()->latest('id')->firstOrFail();
$this->assertEquals($row['purchaseDate'], $latestOrderItem->order->purchase_date->toDateString());
$this->assertEquals((float) $row['purchaseCost'], (float) $latestOrderItem->price);
$this->assertEquals($row['supplier'], $latestOrderItem->order->supplier->name);
// Update mode does NOT rewrite historical Orders a CSV
// "update" corrects the parent, it doesn't stamp a new purchase.
// purchase_cost / supplier on the CSV map to the parent's
// default_* template fields; purchase_date has no forward-use
// equivalent on the parent and is silently dropped on update.
$this->assertEquals((float) $row['purchaseCost'], (float) $updatedConsumable->default_purchase_cost);
$this->assertEquals($row['supplier'], $updatedConsumable->defaultSupplier->name);
$this->assertEquals($consumable->requestable, $updatedConsumable->requestable);
$this->assertEquals($consumable->min_amt, $updatedConsumable->min_amt);
$this->assertEquals($consumable->model_number, $updatedConsumable->model_number);
$this->assertEquals($consumable->item_number, $updatedConsumable->item_number);
$this->assertEquals($consumable->manufacturer_id, $updatedConsumable->manufacturer_id);
$this->assertEquals($consumable->notes, $updatedConsumable->notes);
// notes IS present in the CSV (see ConsumablesImportFileBuilder
// definition), so update mode overwrites the seeded value.
$this->assertEquals($row['notes'], $updatedConsumable->notes);
$this->assertEquals($consumable->item_number, $updatedConsumable->item_number);
}
@ -388,9 +390,10 @@ class ImportConsumablesTest extends ImportDataTestCase implements TestsPermissio
])->assertOk();
$consumable->refresh();
// purchase_cost moved to the OrderItem's price column.
$latestOrderItem = $consumable->orderItems()->latest('id')->firstOrFail();
$this->assertEquals((float) $updatedRow['purchaseCost'], (float) $latestOrderItem->price);
// Update path maps CSV purchase_cost to the parent's template
// field (see update_consumable_from_import); historical Orders
// aren't rewritten on update mode.
$this->assertEquals((float) $updatedRow['purchaseCost'], (float) $consumable->default_purchase_cost);
$updateLog = ActivityLog::query()
->where('item_type', Consumable::class)