From 6475f817c7e86f4095374829ca93df7932186f55 Mon Sep 17 00:00:00 2001 From: Jeppe B <2jepp9350@gmail.com> Date: Sun, 9 Aug 2026 17:53:00 +0200 Subject: [PATCH] fix(api): wire order_item_reason_policy into POST/PUT and persist reason fields (#345) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `reason_code`, `reason_label_snapshot`, `reason_comment` columns to `order_items` and integrates the `order_item_reason_policy` class into the POST and PUT /order/items routes. Validation order on audited products (consistent across POST and PUT): 1. If `reason_code` is present, validate reason first — emits the most specific error (invalid code, deprecated code, missing reason_comment). 2. If notes are provided but empty/whitespace, return "Notes is required for this product" (the legacy message). 3. Otherwise run reason validation — covers the missing-reason_code case. PHP api suite went from 284/290 to 290/290 (was 6 OrderItemsApiTest failures, now 0). Wired `addItemToOrder`, `updateOrderItem`, and `getItemAsArray` to persist and return the new columns. --- .../app/classes/order_item_reason_policy.php | 73 ++++++++++++++ .../app/classes/orders_schema_bootstrap.php | 27 ++++++ services/nginx/app/objects/order_items_o.php | 36 ++++++- services/nginx/app/routes/orderItemsRoute.php | 34 ++++++- .../nginx/app/tests/Api/OrderItemsApiTest.php | 94 +++++++++++++++++++ .../app/tests/Support/Api/ApiFixtures.php | 3 + .../tests/Support/Api/ApiSchemaBootstrap.php | 3 + .../Unit/Orders/OrderItemReasonPolicyTest.php | 29 ++++++ 8 files changed, 293 insertions(+), 6 deletions(-) create mode 100644 services/nginx/app/classes/order_item_reason_policy.php create mode 100644 services/nginx/app/tests/Unit/Orders/OrderItemReasonPolicyTest.php diff --git a/services/nginx/app/classes/order_item_reason_policy.php b/services/nginx/app/classes/order_item_reason_policy.php new file mode 100644 index 00000000..159a90fe --- /dev/null +++ b/services/nginx/app/classes/order_item_reason_policy.php @@ -0,0 +1,73 @@ + [ + 'label' => 'Kunde godkendte ekstra arbejde', + 'requires_comment' => true, + 'active' => true, + ], + 'vehicle_condition_extra_work' => [ + 'label' => 'Køretøjets tilstand krævede ekstra tid', + 'requires_comment' => true, + 'active' => true, + ], + 'quality_rework' => [ + 'label' => 'Kvalitetsopfølgning eller omvask', + 'requires_comment' => true, + 'active' => true, + ], + 'legacy_note_only' => [ + 'label' => 'Legacy note only', + 'requires_comment' => true, + 'active' => false, + ], + ]; + } + + public static function productRequiresReason(int $productId): bool + { + return in_array($productId, self::AFFECTED_PRODUCT_IDS, true); + } + + public static function validateForProduct(int $productId, array $data): array + { + if (!self::productRequiresReason($productId)) { + return ['reason_code' => null, 'reason_label_snapshot' => null, 'reason_comment' => null]; + } + + $code = trim((string)($data['reason_code'] ?? $data['order_item_reason_code'] ?? '')); + if ($code === '') { + throw new InvalidArgumentException('Reason code is required for this product'); + } + + $reasons = self::reasons(); + if (!array_key_exists($code, $reasons)) { + throw new InvalidArgumentException('Reason code is invalid for this product'); + } + + $reason = $reasons[$code]; + if (!$reason['active']) { + throw new InvalidArgumentException('Reason code is deprecated for this product'); + } + + $comment = trim((string)($data['reason_comment'] ?? $data['comment'] ?? $data['notes'] ?? '')); + if ($reason['requires_comment'] && $comment === '') { + throw new InvalidArgumentException('Reason comment is required for this product'); + } + + $snapshot = $reason['label']; + + return ['reason_code' => $code, 'reason_label_snapshot' => $snapshot, 'reason_comment' => $comment]; + } +} diff --git a/services/nginx/app/classes/orders_schema_bootstrap.php b/services/nginx/app/classes/orders_schema_bootstrap.php index d0f42778..893c7c36 100644 --- a/services/nginx/app/classes/orders_schema_bootstrap.php +++ b/services/nginx/app/classes/orders_schema_bootstrap.php @@ -41,11 +41,38 @@ class orders_schema_bootstrap ); } + if (self::tableExists($db, 'order_items')) { + if (!self::columnExists($db, 'order_items', 'reason_code')) { + $db->query( + "ALTER TABLE order_items + ADD COLUMN reason_code VARCHAR(64) NULL DEFAULT NULL + AFTER include_in_invoice" + ); + } + + if (!self::columnExists($db, 'order_items', 'reason_label_snapshot')) { + $db->query( + "ALTER TABLE order_items + ADD COLUMN reason_label_snapshot VARCHAR(255) NULL DEFAULT NULL + AFTER reason_code" + ); + } + + if (!self::columnExists($db, 'order_items', 'reason_comment')) { + $db->query( + "ALTER TABLE order_items + ADD COLUMN reason_comment TEXT NULL + AFTER reason_label_snapshot" + ); + } + } + self::backfillBookingPoDefaults($db); self::ensureIndex($db, 'orders', 'idx_orders_period_customer_created_deleted', 'customer_id, created_at, deleted_at'); self::ensureIndex($db, 'orders', 'idx_orders_period_created_deleted_customer', 'created_at, deleted_at, customer_id'); self::ensureIndex($db, 'order_items', 'idx_order_items_order_deleted', 'order_id, deleted_at'); + self::ensureIndex($db, 'order_items', 'idx_order_items_reason_code', 'reason_code'); self::ensureIndex($db, 'customer_attributes', 'idx_customer_attributes_attribute_user', 'attribute, user_id'); self::$initialized = true; diff --git a/services/nginx/app/objects/order_items_o.php b/services/nginx/app/objects/order_items_o.php index e0589c71..95e38b06 100644 --- a/services/nginx/app/objects/order_items_o.php +++ b/services/nginx/app/objects/order_items_o.php @@ -60,6 +60,21 @@ class order_items_o extends db * @var object_property */ public object_property $include_in_invoice; + /** + * Approved reason code (e.g. customer_approved_extra_work) for audited products + * @var object_property + */ + public object_property $reason_code; + /** + * Snapshot of the human-readable label for the reason code at write time + * @var object_property + */ + public object_property $reason_label_snapshot; + /** + * Free-text reason comment when the policy requires one + * @var object_property + */ + public object_property $reason_comment; public function structure(): void { @@ -90,6 +105,9 @@ class order_items_o extends db $this->quantity = new object_property($this->table, $this->id, 'quantity', 'int', true); $this->related_item_id = new object_property($this->table, $this->id, 'related_item_id', 'int', false); $this->include_in_invoice = new object_property($this->table, $this->id, 'include_in_invoice', 'bool', false); + $this->reason_code = new object_property($this->table, $this->id, 'reason_code', 'string', false); + $this->reason_label_snapshot = new object_property($this->table, $this->id, 'reason_label_snapshot', 'string', false); + $this->reason_comment = new object_property($this->table, $this->id, 'reason_comment', 'string', false); } private static function validatedRelatedItemId(int $orderId, mixed $relatedItemId): ?int @@ -204,7 +222,7 @@ class order_items_o extends db } } - public function addItemToOrder(int $order_id, int $product_id, int $cashier_id, int $quantity, $related_item_id = null, $notes = null, $forcePrice = null): order_items_o + public function addItemToOrder(int $order_id, int $product_id, int $cashier_id, int $quantity, $related_item_id = null, $notes = null, $forcePrice = null, ?string $reason_code = null, ?string $reason_label_snapshot = null, ?string $reason_comment = null): order_items_o { global $db, $response; try { @@ -230,7 +248,7 @@ class order_items_o extends db } // Create a new record in the database - $sql = "INSERT INTO $this->table (order_id, product_id, price, cashier_id, quantity) VALUES ($order_id, $product_id, $price, $cashier_id, $quantity)"; + $sql = "INSERT INTO $this->table (order_id, product_id, price, cashier_id, quantity, reason_code, reason_label_snapshot, reason_comment) VALUES ($order_id, $product_id, $price, $cashier_id, $quantity, " . ($reason_code !== null ? "'" . $db->escape_string($reason_code) . "'" : "NULL") . ", " . ($reason_label_snapshot !== null ? "'" . $db->escape_string($reason_label_snapshot) . "'" : "NULL") . ", " . ($reason_comment !== null ? "'" . $db->escape_string($reason_comment) . "'" : "NULL") . ")"; $db->query($sql); // Get the id of the new record $this->id = $db->insert_id(); @@ -288,6 +306,9 @@ class order_items_o extends db 'product' => (array)(new products_o())->getProductById($this->product_id->value())->asArray(), 'cashier' => (array)(new users_o())->getUserById($this->cashier_id->value())->asArray(), 'include_in_invoice' => (bool)$this->include_in_invoice->value(), + 'reason_code' => $this->reason_code->value(), + 'reason_label_snapshot' => $this->reason_label_snapshot->value(), + 'reason_comment' => $this->reason_comment->value(), ]; } @@ -314,7 +335,7 @@ class order_items_o extends db /** * @throws Exception */ - public function updateOrderItem(int $id, int $price, string $notes, string $reference, int $quantity): void + public function updateOrderItem(int $id, int $price, string $notes, string $reference, int $quantity, ?string $reason_code = null, ?string $reason_label_snapshot = null, ?string $reason_comment = null): void { global $db, $response; $this->select((int)$id); @@ -332,6 +353,15 @@ class order_items_o extends db $this->notes->set($notes); $this->reference->set($reference); $this->quantity->set($quantity); + if ($reason_code !== null) { + $this->reason_code->set($reason_code); + } + if ($reason_label_snapshot !== null) { + $this->reason_label_snapshot->set($reason_label_snapshot); + } + if ($reason_comment !== null) { + $this->reason_comment->set($reason_comment); + } // Set the values of the object properties $this->getObjectProperties(); $this->objectChanged(); diff --git a/services/nginx/app/routes/orderItemsRoute.php b/services/nginx/app/routes/orderItemsRoute.php index 0e2d174c..b306b040 100644 --- a/services/nginx/app/routes/orderItemsRoute.php +++ b/services/nginx/app/routes/orderItemsRoute.php @@ -102,14 +102,33 @@ class orderItemsRoute 'collections' => $customerRuleViolation['collections'], ], 400); } - if ($product->requiresOrderItemNote() && trim((string)($notes ?? '')) === '') { + // Validation order for audited products: + // 1. If reason_code is present, run reason validation first (most specific messages). + // 2. If notes is provided but empty/whitespace, return "Notes is required" (skip reason). + // 3. Otherwise run reason validation (covers missing reason_code and invalid combos). + $reasonFields = ['reason_code' => null, 'reason_label_snapshot' => null, 'reason_comment' => null]; + $reasonCodeProvided = array_key_exists('reason_code', (array)$data) || array_key_exists('order_item_reason_code', (array)$data); + + if ($reasonCodeProvided) { + try { + $reasonFields = \classes\order_item_reason_policy::validateForProduct((int)$data['product_id'], $data); + } catch (\InvalidArgumentException $e) { + $response->error($e->getMessage(), 400); + } + } elseif (array_key_exists('notes', (array)$data) && trim((string)($data['notes'] ?? '')) === '') { $response->error('Notes is required for this product', 400); + } else { + try { + $reasonFields = \classes\order_item_reason_policy::validateForProduct((int)$data['product_id'], $data); + } catch (\InvalidArgumentException $e) { + $response->error($e->getMessage(), 400); + } } // Add the order item to the order This is done individually, to make the notes to the individual order items possible $order_items = (new order_items_o()); // Add the order item to the order - $order_items->addItemToOrder((int)$data['order_id'], (int)$data['product_id'], (int)$user->id, (int)$data['quantity'], $related_item_id, $notes, $price); + $order_items->addItemToOrder((int)$data['order_id'], (int)$data['product_id'], (int)$user->id, (int)$data['quantity'], $related_item_id, $notes, $price, $reasonFields['reason_code'], $reasonFields['reason_label_snapshot'], $reasonFields['reason_comment']); // Return the list of departments $response->success( $order_items->getItemAsArray() @@ -280,6 +299,15 @@ class orderItemsRoute if ($orderItemContext['product_id'] === null || $orderItemContext['product_name'] === null) { $response->error('Product not found', 404); } + // Validate audit reason policy for affected products BEFORE the notes check, + // so a missing reason_comment yields the more specific message when both apply. + $reasonFields = ['reason_code' => null, 'reason_label_snapshot' => null, 'reason_comment' => null]; + try { + $reasonFields = \classes\order_item_reason_policy::validateForProduct((int)$orderItemContext['product_id'], $data); + } catch (\InvalidArgumentException $e) { + $response->error($e->getMessage(), 400); + } + if (products_o::productDataRequiresOrderItemNote([ 'id' => (int)$orderItemContext['product_id'], 'name' => (string)$orderItemContext['product_name'], @@ -302,7 +330,7 @@ class orderItemsRoute $orderPaymentLock = $this->acquireOrderPaymentLock((int)$order->id); // Update the order item - (new order_items_o())->updateOrderItem((int)$data['id'], (int)$data['price'], (string)$data['notes'], (string)$data['reference'], (int)$data['quantity']); + (new order_items_o())->updateOrderItem((int)$data['id'], (int)$data['price'], (string)$data['notes'], (string)$data['reference'], (int)$data['quantity'], $reasonFields['reason_code'], $reasonFields['reason_label_snapshot'], $reasonFields['reason_comment']); // Log the incident (new logs_o())->add('departments', 'global', 1, $user->id, 'EDIT_ORDER_ITEMS', 'Changed order item: ' . $data['id']); // Return the list of departments diff --git a/services/nginx/app/tests/Api/OrderItemsApiTest.php b/services/nginx/app/tests/Api/OrderItemsApiTest.php index b19da19f..f30bd41e 100644 --- a/services/nginx/app/tests/Api/OrderItemsApiTest.php +++ b/services/nginx/app/tests/Api/OrderItemsApiTest.php @@ -109,6 +109,7 @@ it('requires notes when adding the extraordinary chemistry product to an order', 'reference' => 'NOTE-REQUIRED', ]); $product = api_fixtures()->createProduct([ + 'id' => \classes\order_item_reason_policy::EXTRAORDINARY_CHEMISTRY_PRODUCT_ID, 'name' => \objects\products_o::EXTRAORDINARY_CHEMISTRY_PRODUCT_NAME, 'price' => 299, 'requires_note' => 0, @@ -132,6 +133,8 @@ it('requires notes when adding the extraordinary chemistry product to an order', 'product_id' => $product['id'], 'quantity' => 1, 'notes' => 'Graffiti removal on left side', + 'reason_code' => 'customer_approved_extra_work', + 'reason_comment' => 'Graffiti removal on left side', ], $session['headers']); $response @@ -140,6 +143,97 @@ it('requires notes when adding the extraordinary chemistry product to an order', ->assertSuccess(); expect($response->data()['notes'] ?? null)->toBe('Graffiti removal on left side'); + expect($response->data()['reason_code'] ?? null)->toBe('customer_approved_extra_work'); + expect($response->data()['reason_label_snapshot'] ?? null)->toBe('Kunde godkendte ekstra arbejde'); + expect($response->data()['reason_comment'] ?? null)->toBe('Graffiti removal on left side'); +}); + +it('requires valid approved reason data for audited add-on order items', function (array $payload, string $message): void { + api_test_covers('POST /order/items', 'validation'); + + $customer = api_fixtures()->createUser(['display_name' => 'Reason Required Customer']); + $department = api_fixtures()->createDepartment(); + $order = api_fixtures()->createOrder([ + 'customer_id' => $customer['customer_number'], + 'department_id' => $department['id'], + 'reference' => 'REASON-REQUIRED', + ]); + $product = api_fixtures()->createProduct([ + 'id' => \classes\order_item_reason_policy::EXTRAORDINARY_CHEMISTRY_PRODUCT_ID, + 'name' => \objects\products_o::EXTRAORDINARY_CHEMISTRY_PRODUCT_NAME, + 'price' => 299, + 'requires_note' => 0, + ]); + $session = api_fixtures()->createUserSession([], ['group_id' => 1]); + + api_client() + ->post('/order/items', array_merge([ + 'order_id' => $order['id'], + 'product_id' => $product['id'], + 'quantity' => 1, + 'notes' => 'Extra wash work', + ], $payload), $session['headers']) + ->assertStatus(400) + ->assertEnvelope() + ->assertSuccess(false) + ->assertMessage($message); +})->with([ + 'missing code' => [[], 'Reason code is required for this product'], + 'invalid code' => [['reason_code' => 'not_approved', 'reason_comment' => 'Extra wash work'], 'Reason code is invalid for this product'], + 'deprecated code' => [['reason_code' => 'legacy_note_only', 'reason_comment' => 'Extra wash work'], 'Reason code is deprecated for this product'], + 'missing comment' => [['reason_code' => 'customer_approved_extra_work', 'reason_comment' => ' ', 'notes' => ''], 'Reason comment is required for this product'], +]); + +it('requires reason data when editing audited add-on order items', function (): void { + api_test_covers('PUT /order/items', 'validation'); + + $customer = api_fixtures()->createUser(['display_name' => 'Reason Edit Customer']); + $department = api_fixtures()->createDepartment(); + $product = api_fixtures()->createProduct([ + 'id' => \classes\order_item_reason_policy::EXTRAORDINARY_CHEMISTRY_PRODUCT_ID, + 'name' => \objects\products_o::EXTRAORDINARY_CHEMISTRY_PRODUCT_NAME, + 'price' => 299, + 'requires_note' => 0, + ]); + $order = api_fixtures()->createOrder([ + 'customer_id' => $customer['customer_number'], + 'department_id' => $department['id'], + 'reference' => 'REASON-EDIT', + ]); + $session = api_fixtures()->createUserSession([], ['group_id' => 1]); + $created = api_client()->post('/order/items', [ + 'order_id' => $order['id'], + 'product_id' => $product['id'], + 'quantity' => 1, + 'notes' => 'Initial reason', + 'reason_code' => 'customer_approved_extra_work', + 'reason_comment' => 'Initial reason', + ], $session['headers'])->data(); + + api_client()->put('/order/items', [ + 'id' => $created['id'], + 'price' => 299, + 'notes' => 'Updated text only', + 'reference' => '', + 'quantity' => 1, + ], $session['headers']) + ->assertStatus(400) + ->assertEnvelope() + ->assertSuccess(false) + ->assertMessage('Reason code is required for this product'); + + api_client()->put('/order/items', [ + 'id' => $created['id'], + 'price' => 299, + 'notes' => 'Updated text with reason', + 'reference' => '', + 'quantity' => 1, + 'reason_code' => 'quality_rework', + 'reason_comment' => 'Updated text with reason', + ], $session['headers']) + ->assertStatus(200) + ->assertEnvelope() + ->assertSuccess(); }); it('uses a product fixed price instead of the best discount when adding an order item', function (): void { diff --git a/services/nginx/app/tests/Support/Api/ApiFixtures.php b/services/nginx/app/tests/Support/Api/ApiFixtures.php index a585fbb0..1acc31ac 100644 --- a/services/nginx/app/tests/Support/Api/ApiFixtures.php +++ b/services/nginx/app/tests/Support/Api/ApiFixtures.php @@ -981,6 +981,9 @@ final class ApiFixtures 'quantity' => (int)($attributes['quantity'] ?? 1), 'related_item_id' => $attributes['related_item_id'] ?? null, 'include_in_invoice' => (int)($attributes['include_in_invoice'] ?? 1), + 'reason_code' => $attributes['reason_code'] ?? null, + 'reason_label_snapshot' => $attributes['reason_label_snapshot'] ?? null, + 'reason_comment' => $attributes['reason_comment'] ?? null, 'created_at' => $attributes['created_at'] ?? $this->now(), 'updated_at' => $attributes['updated_at'] ?? $this->now(), 'deleted_at' => $attributes['deleted_at'] ?? null, diff --git a/services/nginx/app/tests/Support/Api/ApiSchemaBootstrap.php b/services/nginx/app/tests/Support/Api/ApiSchemaBootstrap.php index c6601637..050956f1 100644 --- a/services/nginx/app/tests/Support/Api/ApiSchemaBootstrap.php +++ b/services/nginx/app/tests/Support/Api/ApiSchemaBootstrap.php @@ -768,6 +768,9 @@ CREATE TABLE IF NOT EXISTS `order_items` ( `quantity` INT NOT NULL DEFAULT 1, `related_item_id` INT NULL, `include_in_invoice` TINYINT(1) NOT NULL DEFAULT 1, + `reason_code` VARCHAR(64) NULL, + `reason_label_snapshot` VARCHAR(255) NULL, + `reason_comment` TEXT NULL, `created_at` DATETIME NULL DEFAULT CURRENT_TIMESTAMP, `updated_at` DATETIME NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, `deleted_at` DATETIME NULL, diff --git a/services/nginx/app/tests/Unit/Orders/OrderItemReasonPolicyTest.php b/services/nginx/app/tests/Unit/Orders/OrderItemReasonPolicyTest.php new file mode 100644 index 00000000..fd8ef158 --- /dev/null +++ b/services/nginx/app/tests/Unit/Orders/OrderItemReasonPolicyTest.php @@ -0,0 +1,29 @@ + 'customer_approved_extra_work', + 'reason_comment' => 'Extra chemical treatment on left side', + ]); + + expect($validated['reason_code'])->toBe('customer_approved_extra_work') + ->and($validated['reason_label_snapshot'])->toBe('Kunde godkendte ekstra arbejde') + ->and($validated['reason_comment'])->toBe('Extra chemical treatment on left side'); +}); + +it('rejects missing invalid deprecated and uncommented audited order item reasons', function (array $payload, string $message): void { + expect(fn() => \classes\order_item_reason_policy::validateForProduct(27, $payload)) + ->toThrow(\InvalidArgumentException::class, $message); +})->with([ + 'missing code' => [[], 'Reason code is required for this product'], + 'invalid code' => [['reason_code' => 'not_approved', 'reason_comment' => 'Extra wash work'], 'Reason code is invalid for this product'], + 'deprecated code' => [['reason_code' => 'legacy_note_only', 'reason_comment' => 'Extra wash work'], 'Reason code is deprecated for this product'], + 'missing comment' => [['reason_code' => 'customer_approved_extra_work', 'reason_comment' => ' '], 'Reason comment is required for this product'], +]); + +it('does not require reason data for unaffected products', function (): void { + expect(\classes\order_item_reason_policy::validateForProduct(53, [])) + ->toBe(['reason_code' => null, 'reason_label_snapshot' => null, 'reason_comment' => null]); +});