fix(api): order order_items so primary precedes addons in getOrderItems (#361)

## 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>
This commit is contained in:
Jeppe B
2026-08-10 20:16:04 +02:00
committed by GitHub
co-authored by Truck Wash Agent
parent 43df3e4dca
commit d850075397
2 changed files with 66 additions and 1 deletions
+6 -1
View File
@@ -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) {
@@ -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");
});