From 9138127a67286d4d362e6ea6e17c81012e20e456 Mon Sep 17 00:00:00 2001
From: objecttothis <17935339+objecttothis@users.noreply.github.com>
Date: Fri, 21 Aug 2026 12:49:51 +0400
Subject: [PATCH] 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>
---
 app/Models/Item.php             | 20 +++++++++++++++-----
 tests/Models/ItemSearchTest.php | 27 +++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/app/Models/Item.php b/app/Models/Item.php
index 1fac2aa73..c413f3b1c 100644
--- a/app/Models/Item.php
+++ b/app/Models/Item.php
@@ -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);
         }
diff --git a/tests/Models/ItemSearchTest.php b/tests/Models/ItemSearchTest.php
index 9287ac757..0fb2f18ea 100644
--- a/tests/Models/ItemSearchTest.php
+++ b/tests/Models/ItemSearchTest.php
@@ -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);
+    }
 }