mirror of
https://github.com/opensourcepos/opensourcepos.git
synced 2026-09-17 07:50:19 -04:00
fix(items): optimize quantity sum handling and add test to prevent over-counting
- Refactor quantity sum calculation by introducing a subquery to handle SUM without over-counting due to joins. - Update search logic and related query clauses for consistent sorting by total quantities across locations. - Add test to verify correct quantity aggregation without duplication. Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
This commit is contained in:
1 parent
c25c372175
commit
9138127a67
2 files changed
+42
-5
No files matched your search
+15
-5
@@ -258,8 +258,13 @@ class Item extends Model
|
||||
|
||||
// Order by name of item by default
|
||||
if ($sort === 'quantity' && $filters['stock_location_id'] <= -1) {
|
||||
$idBuilder->join('item_quantities AS item_quantities', 'item_quantities.item_id = items.item_id', 'left');
|
||||
$idBuilder->orderBy('SUM(item_quantities.quantity)', $order);
|
||||
$itemQuantitiesTable = $this->db->prefixTable('item_quantities');
|
||||
$idBuilder->join(
|
||||
"(SELECT item_id, SUM(quantity) AS total_quantity FROM $itemQuantitiesTable GROUP BY item_id) AS item_quantity_totals",
|
||||
'item_quantity_totals.item_id = items.item_id',
|
||||
'left'
|
||||
);
|
||||
$idBuilder->orderBy('item_quantity_totals.total_quantity', $order);
|
||||
} else {
|
||||
$idBuilder->orderBy($sort, $order);
|
||||
}
|
||||
@@ -314,7 +319,7 @@ class Item extends Model
|
||||
$builder->select('MAX(item_quantities.location_id) AS location_id');
|
||||
$builder->select('MAX(item_quantities.quantity) AS quantity');
|
||||
} elseif ($sortByQuantityAllLocations) {
|
||||
$builder->select('SUM(item_quantities.quantity) AS quantity');
|
||||
$builder->select('item_quantity_totals.total_quantity AS quantity');
|
||||
}
|
||||
|
||||
$builder->join('suppliers AS suppliers', 'suppliers.person_id = items.supplier_id', 'left');
|
||||
@@ -324,7 +329,12 @@ class Item extends Model
|
||||
$builder->join('item_quantities AS item_quantities', 'item_quantities.item_id = items.item_id');
|
||||
$builder->where('location_id', $filters['stock_location_id']);
|
||||
} elseif ($sortByQuantityAllLocations) {
|
||||
$builder->join('item_quantities AS item_quantities', 'item_quantities.item_id = items.item_id', 'left');
|
||||
$itemQuantitiesTable = $this->db->prefixTable('item_quantities');
|
||||
$builder->join(
|
||||
"(SELECT item_id, SUM(quantity) AS total_quantity FROM $itemQuantitiesTable GROUP BY item_id) AS item_quantity_totals",
|
||||
'item_quantity_totals.item_id = items.item_id',
|
||||
'left'
|
||||
);
|
||||
}
|
||||
|
||||
$applyTransDateRange($builder);
|
||||
@@ -344,7 +354,7 @@ class Item extends Model
|
||||
|
||||
// Re-apply order: WHERE...IN + GROUP BY do not preserve Phase A's row order
|
||||
if ($sortByQuantityAllLocations) {
|
||||
$builder->orderBy('SUM(item_quantities.quantity)', $order);
|
||||
$builder->orderBy('item_quantity_totals.total_quantity', $order);
|
||||
} else {
|
||||
$builder->orderBy($sort, $order);
|
||||
}
|
||||
|
||||
@@ -190,4 +190,31 @@ class ItemSearchTest extends CIUnitTestCase
|
||||
$this->assertCount(1, $resultsWhenTemporary);
|
||||
$this->assertCount(0, $resultsWhenNotTemporary);
|
||||
}
|
||||
|
||||
public function testSearchSortByQuantitySumsAcrossLocationsWithoutMultiplication(): void
|
||||
{
|
||||
$itemId = $this->createSearchableItem(['name' => 'Multi Location Item ' . uniqid()]);
|
||||
|
||||
// createSearchableItem already inserts one inventory row; add two more
|
||||
// inventory transactions so a naive SUM-after-join (which multiplies
|
||||
// item_quantities rows by the inventory join) would triple-count.
|
||||
$this->addInventoryRecord($itemId, 1);
|
||||
$this->addInventoryRecord($itemId, 1);
|
||||
|
||||
$this->addItemQuantity($itemId, 1, 10);
|
||||
|
||||
$results = $this->item->search(
|
||||
'',
|
||||
$this->defaultSearchFilters(),
|
||||
0,
|
||||
0,
|
||||
'quantity',
|
||||
'desc'
|
||||
)->getResult();
|
||||
|
||||
$match = array_values(array_filter($results, static fn ($r) => (int) $r->item_id === $itemId));
|
||||
|
||||
$this->assertCount(1, $match);
|
||||
$this->assertEquals(10, (float) $match[0]->quantity);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user