fix(api): wire order_item_reason_policy into POST/PUT and persist reason fields (#345)

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.
This commit is contained in:
Jeppe B
2026-08-09 17:53:00 +02:00
committed by GitHub
parent b107ba649c
commit 6475f817c7
8 changed files with 293 additions and 6 deletions
@@ -0,0 +1,73 @@
<?php
namespace classes;
use InvalidArgumentException;
class order_item_reason_policy
{
public const EXTRAORDINARY_CHEMISTRY_PRODUCT_ID = 27;
public const AFFECTED_PRODUCT_IDS = [21, 22, 24, 25, 26, 27];
public static function reasons(): array
{
return [
'customer_approved_extra_work' => [
'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];
}
}
@@ -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::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_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, '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_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::ensureIndex($db, 'customer_attributes', 'idx_customer_attributes_attribute_user', 'attribute, user_id');
self::$initialized = true; self::$initialized = true;
+33 -3
View File
@@ -60,6 +60,21 @@ class order_items_o extends db
* @var object_property * @var object_property
*/ */
public object_property $include_in_invoice; 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 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->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->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->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 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; global $db, $response;
try { try {
@@ -230,7 +248,7 @@ class order_items_o extends db
} }
// Create a new record in the database // 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); $db->query($sql);
// Get the id of the new record // Get the id of the new record
$this->id = $db->insert_id(); $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(), 'product' => (array)(new products_o())->getProductById($this->product_id->value())->asArray(),
'cashier' => (array)(new users_o())->getUserById($this->cashier_id->value())->asArray(), 'cashier' => (array)(new users_o())->getUserById($this->cashier_id->value())->asArray(),
'include_in_invoice' => (bool)$this->include_in_invoice->value(), '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 * @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; global $db, $response;
$this->select((int)$id); $this->select((int)$id);
@@ -332,6 +353,15 @@ class order_items_o extends db
$this->notes->set($notes); $this->notes->set($notes);
$this->reference->set($reference); $this->reference->set($reference);
$this->quantity->set($quantity); $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 // Set the values of the object properties
$this->getObjectProperties(); $this->getObjectProperties();
$this->objectChanged(); $this->objectChanged();
+31 -3
View File
@@ -102,14 +102,33 @@ class orderItemsRoute
'collections' => $customerRuleViolation['collections'], 'collections' => $customerRuleViolation['collections'],
], 400); ], 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); $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 // 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()); $order_items = (new order_items_o());
// Add the order item to the order // 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 // Return the list of departments
$response->success( $response->success(
$order_items->getItemAsArray() $order_items->getItemAsArray()
@@ -280,6 +299,15 @@ class orderItemsRoute
if ($orderItemContext['product_id'] === null || $orderItemContext['product_name'] === null) { if ($orderItemContext['product_id'] === null || $orderItemContext['product_name'] === null) {
$response->error('Product not found', 404); $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([ if (products_o::productDataRequiresOrderItemNote([
'id' => (int)$orderItemContext['product_id'], 'id' => (int)$orderItemContext['product_id'],
'name' => (string)$orderItemContext['product_name'], 'name' => (string)$orderItemContext['product_name'],
@@ -302,7 +330,7 @@ class orderItemsRoute
$orderPaymentLock = $this->acquireOrderPaymentLock((int)$order->id); $orderPaymentLock = $this->acquireOrderPaymentLock((int)$order->id);
// Update the order item // 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 // Log the incident
(new logs_o())->add('departments', 'global', 1, $user->id, 'EDIT_ORDER_ITEMS', 'Changed order item: ' . $data['id']); (new logs_o())->add('departments', 'global', 1, $user->id, 'EDIT_ORDER_ITEMS', 'Changed order item: ' . $data['id']);
// Return the list of departments // Return the list of departments
@@ -109,6 +109,7 @@ it('requires notes when adding the extraordinary chemistry product to an order',
'reference' => 'NOTE-REQUIRED', 'reference' => 'NOTE-REQUIRED',
]); ]);
$product = api_fixtures()->createProduct([ $product = api_fixtures()->createProduct([
'id' => \classes\order_item_reason_policy::EXTRAORDINARY_CHEMISTRY_PRODUCT_ID,
'name' => \objects\products_o::EXTRAORDINARY_CHEMISTRY_PRODUCT_NAME, 'name' => \objects\products_o::EXTRAORDINARY_CHEMISTRY_PRODUCT_NAME,
'price' => 299, 'price' => 299,
'requires_note' => 0, 'requires_note' => 0,
@@ -132,6 +133,8 @@ it('requires notes when adding the extraordinary chemistry product to an order',
'product_id' => $product['id'], 'product_id' => $product['id'],
'quantity' => 1, 'quantity' => 1,
'notes' => 'Graffiti removal on left side', 'notes' => 'Graffiti removal on left side',
'reason_code' => 'customer_approved_extra_work',
'reason_comment' => 'Graffiti removal on left side',
], $session['headers']); ], $session['headers']);
$response $response
@@ -140,6 +143,97 @@ it('requires notes when adding the extraordinary chemistry product to an order',
->assertSuccess(); ->assertSuccess();
expect($response->data()['notes'] ?? null)->toBe('Graffiti removal on left side'); 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 { it('uses a product fixed price instead of the best discount when adding an order item', function (): void {
@@ -981,6 +981,9 @@ final class ApiFixtures
'quantity' => (int)($attributes['quantity'] ?? 1), 'quantity' => (int)($attributes['quantity'] ?? 1),
'related_item_id' => $attributes['related_item_id'] ?? null, 'related_item_id' => $attributes['related_item_id'] ?? null,
'include_in_invoice' => (int)($attributes['include_in_invoice'] ?? 1), '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(), 'created_at' => $attributes['created_at'] ?? $this->now(),
'updated_at' => $attributes['updated_at'] ?? $this->now(), 'updated_at' => $attributes['updated_at'] ?? $this->now(),
'deleted_at' => $attributes['deleted_at'] ?? null, 'deleted_at' => $attributes['deleted_at'] ?? null,
@@ -768,6 +768,9 @@ CREATE TABLE IF NOT EXISTS `order_items` (
`quantity` INT NOT NULL DEFAULT 1, `quantity` INT NOT NULL DEFAULT 1,
`related_item_id` INT NULL, `related_item_id` INT NULL,
`include_in_invoice` TINYINT(1) NOT NULL DEFAULT 1, `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, `created_at` DATETIME NULL DEFAULT CURRENT_TIMESTAMP,
`updated_at` DATETIME NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, `updated_at` DATETIME NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
`deleted_at` DATETIME NULL, `deleted_at` DATETIME NULL,
@@ -0,0 +1,29 @@
<?php
app_require('classes/order_item_reason_policy.php');
it('validates active reason codes and immutable labels for audited order item products', function (): void {
$validated = \classes\order_item_reason_policy::validateForProduct(27, [
'reason_code' => '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]);
});