From d850075397fcdcf2a3e5846b2cdecb8f8ebb5c82 Mon Sep 17 00:00:00 2001
From: Jeppe B <2jepp9350@gmail.com>
Date: Mon, 10 Aug 2026 20:16:04 +0200
Subject: [PATCH] fix(api): order order_items so primary precedes addons in
 getOrderItems (#361)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## Summary

orders_o::getOrderItems() selected order items without an explicit ORDER
BY clause, so MySQL was free to return rows in any order. On the POS
Fuldfør click and the superuser invoice tree, addons (related_item_id !=
NULL) were sometimes returned before their primary item, which broke the
FE tree-builder and the OrderContentTable render.

Add a stable ordering: primary items first (related_item_id IS NULL
DESC), addons grouped by their parent (related_item_id ASC), and
insertion order as the final tiebreaker (id ASC).

## Commits

- 7ec64ec8 fix(api): order order_items so primary precedes addons in
getOrderItems
- 68e19bee test(api): pin order_items listing ordering in getOrderItems

## Test plan

- Wiring unit test asserts the SELECT inside getOrderItems still carries
ORDER BY (related_item_id IS NULL) DESC, related_item_id ASC, id ASC.
- Verified locally with php -l on the modified file.
- Existing OrderItemReasonPolicyTest, CustomerOrderProductPolicyTest,
OrdersIncludeInInvoiceOverrideTest continue to pass in the worktree
setup (no DB fixtures touched).

---------

Co-authored-by: Truck Wash Agent <agent@copenhagentruckwash.local>
---
 services/nginx/app/objects/orders_o.php       |  7 ++-
 .../Orders/OrderItemsListingOrderingTest.php  | 60 +++++++++++++++++++
 2 files changed, 66 insertions(+), 1 deletion(-)
 create mode 100644 services/nginx/app/tests/Unit/Orders/OrderItemsListingOrderingTest.php

diff --git a/services/nginx/app/objects/orders_o.php b/services/nginx/app/objects/orders_o.php
index f37b49c8..a039d3dd 100644
--- a/services/nginx/app/objects/orders_o.php
+++ b/services/nginx/app/objects/orders_o.php
@@ -615,7 +615,12 @@ class orders_o extends db
     public function getOrderItems(int $order_id): array
     {
         global $db;
-        $sql = "SELECT * FROM order_items WHERE order_id = $order_id";
+        // Order primary items first (related_item_id IS NULL), then addons grouped by
+        // their parent (related_item_id ASC), and finally fall back to insertion order
+        // (id ASC). Without an explicit ORDER BY, MySQL is free to return rows in any
+        // order, which causes the FE tree-builder to render addons before their
+        // primary on the invoice and POS displays.
+        $sql = "SELECT * FROM order_items WHERE order_id = $order_id ORDER BY (related_item_id IS NULL) DESC, related_item_id ASC, id ASC";
         $result = $db->query($sql);
         $order_items = [];
         if ($result->num_rows > 0 && $result) {
diff --git a/services/nginx/app/tests/Unit/Orders/OrderItemsListingOrderingTest.php b/services/nginx/app/tests/Unit/Orders/OrderItemsListingOrderingTest.php
new file mode 100644
index 00000000..ec7d4f22
--- /dev/null
+++ b/services/nginx/app/tests/Unit/Orders/OrderItemsListingOrderingTest.php
@@ -0,0 +1,60 @@
+<?php
+
+declare(strict_types=1);
+
+it('orders the SELECT in getOrderItems so primary items precede their addons', function (): void {
+    $ordersObjectFile = app_path('objects/orders_o.php');
+    $content = file_get_contents($ordersObjectFile);
+
+    expect($content)->not->toBeFalse();
+
+    // Locate the getOrderItems method body.
+    $start = strpos($content, 'public function getOrderItems(int $order_id): array');
+    expect($start)->not->toBeFalse();
+
+    // Bound the search so we don't accidentally match unrelated SQL further down
+    // the file (the helper around line 645 in orders_o.php also uses ORDER BY id DESC).
+    $end = strpos($content, "\n    }\n", $start);
+    expect($end)->not->toBeFalse();
+
+    $methodBody = substr($content, (int)$start, (int)$end - (int)$start);
+
+    // The SELECT against order_items must include an explicit ORDER BY so MySQL
+    // does not return rows in undefined order (which has been observed to put
+    // primary order items after their addons, breaking the FE tree-builder and
+    // the invoice line listing).
+    expect($methodBody)
+        ->toContain("FROM order_items WHERE order_id = \$order_id")
+        ->and($methodBody)
+        ->toContain('ORDER BY');
+
+    // The ORDER BY must place primary items first (related_item_id IS NULL DESC),
+    // group addons by their parent (related_item_id ASC), and fall back to
+    // insertion order (id ASC).
+    expect($methodBody)
+        ->toContain('(related_item_id IS NULL) DESC')
+        ->and($methodBody)
+        ->toContain('related_item_id ASC')
+        ->and($methodBody)
+        ->toContain('id ASC');
+});
+
+it('does not leave the legacy unordered SELECT in getOrderItems', function (): void {
+    $ordersObjectFile = app_path('objects/orders_o.php');
+    $content = file_get_contents($ordersObjectFile);
+
+    expect($content)->not->toBeFalse();
+
+    $start = strpos($content, 'public function getOrderItems(int $order_id): array');
+    $end = strpos($content, "\n    }\n", $start);
+
+    expect($start)->not->toBeFalse()
+        ->and($end)->not->toBeFalse();
+
+    $methodBody = substr($content, (int)$start, (int)$end - (int)$start);
+
+    // The buggy SQL must be gone: previously this returned rows in whatever
+    // order MySQL felt like, leading to addons being listed before their primary.
+    expect($methodBody)
+        ->not->toContain("FROM order_items WHERE order_id = \$order_id\";\n        \$result");
+});