diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml new file mode 100644 index 00000000..524b7bd7 --- /dev/null +++ b/.github/workflows/deploy.yml @@ -0,0 +1,136 @@ +name: Deploy to Hetzner (staging) + +on: + push: + branches: [master] + workflow_dispatch: + inputs: + reason: + description: 'Reason for manual deploy' + required: false + default: 'manual' + +concurrency: + group: deploy-${{ github.repository }} + cancel-in-progress: false + +env: + DEPLOY_HOST: ${{ secrets.DEPLOY_HOST }} + DEPLOY_USER: ${{ secrets.DEPLOY_USER }} + +jobs: + test-and-deploy: + name: CI + Deploy + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 1 + + - name: Show commit info + run: | + echo "Repo: ${{ github.repository }}" + echo "Branch: ${{ github.ref }}" + echo "Commit: ${{ github.sha }}" + echo "Actor: ${{ github.actor }}" + + # === CI (phpunit / vitest) runs here via repo's existing CI config === + # (Most of our repos already have a "Required CI" check; this section + # would invoke that. If your repo doesn't have a CI workflow, the + # required-check on the branch will block this workflow's deploy step.) + + - name: Setup SSH + uses: webfactory/ssh-agent@v0.9.0 + with: + ssh-private-key: ${{ secrets.DEPLOY_SSH_KEY }} + + - name: Add host key + run: | + mkdir -p ~/.ssh + ssh-keyscan -H "$DEPLOY_HOST" >> ~/.ssh/known_hosts 2>/dev/null + + - name: Pre-deploy snapshot + id: pre + run: | + ssh "$DEPLOY_USER@$DEPLOY_HOST" ' + set -e + cd /opt/${{ github.event.repository.name }} + git rev-parse HEAD > /tmp/last_deploy_sha + echo "PRE_SHA=$(cat /tmp/last_deploy_sha)" + echo "pre_sha=$(cat /tmp/last_deploy_sha)" >> $GITHUB_OUTPUT + ' + + - name: Deploy + id: deploy + run: | + ssh "$DEPLOY_USER@$DEPLOY_HOST" ' + set -e + cd /opt/${{ github.event.repository.name }} + git fetch origin master + git reset --hard origin/master + # PHP repos: composer install + clear cache + if [ -f composer.json ]; then + composer install --no-dev --optimize-autoloader --no-interaction + php artisan cache:clear || true + php artisan config:cache || true + # Restart php-fpm if used + sudo systemctl reload php8.2-fpm || true + fi + # Node repos: npm ci + build + if [ -f package.json ]; then + npm ci --ignore-scripts + npm run build + # Restart node service + sudo systemctl reload pleno-vue || sudo systemctl reload nginx || true + fi + # Restart generic services + sudo systemctl reload nginx || true + echo "Deploy complete: $(git rev-parse --short HEAD)" + ' + + - name: Smoke test + id: smoke + continue-on-error: true + run: | + chmod +x scripts/smoke-test.sh + ./scripts/smoke-test.sh + + - name: Auto-rollback on smoke failure + if: steps.smoke.outcome == 'failure' + run: | + echo "::error::Smoke test failed — rolling back to ${{ steps.pre.outputs.pre_sha }}" + ssh "$DEPLOY_USER@$DEPLOY_HOST" ' + set -e + cd /opt/${{ github.event.repository.name }} + git reset --hard ${{ steps.pre.outputs.pre_sha }} + if [ -f composer.json ]; then + composer install --no-dev --optimize-autoloader --no-interaction + sudo systemctl reload php8.2-fpm || true + fi + if [ -f package.json ]; then + npm ci --ignore-scripts + npm run build + sudo systemctl reload nginx || true + fi + ' + + - name: Post Slack status + if: always() + uses: slackapi/slack-github-action@v1.27.0 + with: + channel-id: ${{ secrets.AI_DAILY_CHANNEL }} + payload: | + { + "text": "${{ job.status == 'success' && '✅' || '❌' }} Deploy *${{ github.repository }}@${{ github.sha[0:7] }}* — ${{ job.status }}\n${{ steps.smoke.outcome == 'failure' && '⚠️ Auto-rolled back' || '✓ Smoke test passed' }}" + } + env: + SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} + + - name: Update Linear issue + if: success() && steps.deploy.outcome == 'success' + run: | + # Find Linear issues in this commit's history and post a comment + # (uses GitHub's auto-link: if PR body contains "TRU-123" it auto-links) + # We skip this here; the OpenClaw cron `f26dfd83` handles Linear updates. + echo "Deploy notification will be picked up by OpenClaw cron." diff --git a/scripts/smoke-test.sh b/scripts/smoke-test.sh new file mode 100644 index 00000000..a0370a71 --- /dev/null +++ b/scripts/smoke-test.sh @@ -0,0 +1,80 @@ +#!/usr/bin/env bash +# Generic smoke test for any deployed app. +# +# Usage: ./scripts/smoke-test.sh [base_url] +# Default: https://staging.truckwash.io +# +# Required env vars (set by GitHub Action): +# SMOKE_BASE_URL - base URL to test (default: https://staging.truckwash.io) +# +# Optional env vars: +# SMOKE_TOKEN - bearer token for authenticated checks +# SMOKE_TIMEOUT - curl timeout in seconds (default: 10) +# +# Exits 0 on all-pass, 1 on any failure. + +set -euo pipefail + +BASE_URL="${SMOKE_BASE_URL:-${1:-https://staging.truckwash.io}}" +TIMEOUT="${SMOKE_TIMEOUT:-10}" + +# Color codes +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +NC='\033[0m' + +FAIL=0 + +check() { + local name="$1" + local url="$2" + local expected="${3:-200}" + local method="${4:-GET}" + + local status + status=$(curl -s -o /dev/null -w "%{http_code}" -X "$method" --max-time "$TIMEOUT" "$url" || echo "000") + + if [[ "$status" =~ ^($expected)$ ]] || [[ "$expected" == "2xx" && "$status" =~ ^2 ]]; then + echo -e " ${GREEN}✓${NC} $name ($status) — $url" + else + echo -e " ${RED}✗${NC} $name (expected $expected, got $status) — $url" + FAIL=1 + fi +} + +echo "Smoke test against $BASE_URL" +echo " (timeout ${TIMEOUT}s per check)" +echo + +# === Health endpoints (universal) === +check "health check" "$BASE_URL/healthz" "2xx" +check "ping" "$BASE_URL/api/ping" "2xx" + +# === Authentication (should NOT 500) === +check "login page" "$BASE_URL/login" "2xx" + +# === Public endpoints (api repo) === +check "customer list (public schema)" "$BASE_URL/api/customer" "2xx" +check "kundeoprettelse form" "$BASE_URL/kundeoprettelse" "2xx" + +# === Public endpoints (pleno-vue) === +check "self-serve program picker" "$BASE_URL/self-serve/program" "2xx" +check "vehicle step" "$BASE_URL/self-serve/vehicle" "2xx" + +# === Custom 404 should not 500 === +check "404 page" "$BASE_URL/this-route-does-not-exist" "404" + +# === Optional authenticated check === +if [ -n "${SMOKE_TOKEN:-}" ]; then + check "auth check" "$BASE_URL/api/me" "2xx" +fi + +echo +if [ "$FAIL" -eq 0 ]; then + echo -e "${GREEN}✓ All smoke tests passed${NC}" + exit 0 +else + echo -e "${RED}✗ Some smoke tests failed${NC}" + exit 1 +fi diff --git a/services/nginx/app/classes/products_schema_bootstrap.php b/services/nginx/app/classes/products_schema_bootstrap.php index 3963f1d9..a12b6166 100644 --- a/services/nginx/app/classes/products_schema_bootstrap.php +++ b/services/nginx/app/classes/products_schema_bootstrap.php @@ -33,6 +33,31 @@ class products_schema_bootstrap ); } + if (!self::columnExists($db, 'products', 'merged_into_product_id')) { + $db->query( + "ALTER TABLE products + ADD COLUMN merged_into_product_id INT NULL DEFAULT NULL + AFTER max_quantity_per_order, + ADD KEY idx_products_merged_into (merged_into_product_id)" + ); + } + + if (!self::tableExists($db, 'product_merges')) { + $db->query( + "CREATE TABLE IF NOT EXISTS product_merges ( + id INT AUTO_INCREMENT PRIMARY KEY, + source_product_id INT NOT NULL, + target_product_id INT NOT NULL, + merged_by_user_id INT NULL, + reason VARCHAR(500) NULL, + merged_at DATETIME NOT NULL, + KEY idx_product_merges_source (source_product_id), + KEY idx_product_merges_target (target_product_id), + UNIQUE KEY uq_product_merges_source (source_product_id) + ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci" + ); + } + self::$initialized = true; } diff --git a/services/nginx/app/objects/products_o.php b/services/nginx/app/objects/products_o.php index d289c286..cd3343d8 100644 --- a/services/nginx/app/objects/products_o.php +++ b/services/nginx/app/objects/products_o.php @@ -84,6 +84,12 @@ class products_o extends db * @var object_property $max_quantity_per_order */ public object_property $max_quantity_per_order; + /** + * If non-null, this product has been merged into the product with the given id. + * All read paths should resolve to the target product (see resolveActiveProductId()). + * @var object_property $merged_into_product_id + */ + public object_property $merged_into_product_id; /** * The timestamp of when the object was created * @var object_property @@ -134,6 +140,7 @@ class products_o extends db $this->display_in_booking_form = new object_property($this->table, $this->id, 'display_in_booking_form', 'bool', false); $this->order_priority = new object_property($this->table, $this->id, 'order_priority', 'int', false); $this->max_quantity_per_order = new object_property($this->table, $this->id, 'max_quantity_per_order', 'int', false); + $this->merged_into_product_id = new object_property($this->table, $this->id, 'merged_into_product_id', 'int', false); $this->created_at = new object_property($this->table, $this->id, 'created_at', 'string', false); $this->updated_at = new object_property($this->table, $this->id, 'updated_at', 'string', false); } @@ -227,6 +234,7 @@ class products_o extends db 'display_in_booking_form' => (bool)$this->display_in_booking_form->value(), 'order_priority' => (int)$this->order_priority->value(), 'max_quantity_per_order' => $this->max_quantity_per_order->value() === null ? null : (int)$this->max_quantity_per_order->value(), + 'merged_into_product_id' => $this->merged_into_product_id->value() === null ? null : (int)$this->merged_into_product_id->value(), 'created_at' => (string)$this->created_at->value(), 'updated_at' => (string)$this->updated_at->value(), ]; @@ -370,4 +378,128 @@ class products_o extends db self::requireSelected(); return $this->id === 41; } + + /** + * Returns the product id that should be used for new orders and pricing. + * If this product has been merged into another (merged_into_product_id is set), + * the target id is returned. The merge chain is followed transitively with a + * safety cap to avoid infinite loops. + */ + public function resolveActiveProductId(): int + { + self::requireSelected(); + products_schema_bootstrap::ensureTables(); + + $currentId = (int)$this->id; + $visited = [$currentId => true]; + $maxHops = 16; + + for ($i = 0; $i < $maxHops; $i++) { + $next = self::fetchMergedInto($currentId); + if ($next === null) { + return $currentId; + } + if (isset($visited[$next])) { + // Cycle detected: stop at the current node rather than spinning. + return $currentId; + } + $visited[$next] = true; + $currentId = $next; + } + return $currentId; + } + + /** + * Static helper: given a product id, return the product id it is merged into, + * or null if it is not merged. Performs a single hop (no chain following). + */ + public static function fetchMergedInto(int $productId): ?int + { + global $db; + if (!isset($db) || $productId <= 0) { + return null; + } + $productId = (int)$db->escape_string((string)$productId); + $result = $db->query("SELECT merged_into_product_id FROM products WHERE id = {$productId}"); + if ($result === false || !is_object($result) || (int)$result->num_rows === 0) { + return null; + } + $row = $db->fetch_assoc($result); + $merged = $row['merged_into_product_id'] ?? null; + if ($merged === null || $merged === '' || (int)$merged === 0) { + return null; + } + return (int)$merged; + } + + /** + * Merge this product into another. The source product keeps its id (and therefore + * its historical order_items references), but reads and new orders will resolve to + * the target product. An audit row is written to product_merges. + * + * Throws \RuntimeException on validation failure. + */ + public function mergeInto(int $targetProductId, ?int $mergedByUserId = null, ?string $reason = null): void + { + self::requireSelected(); + products_schema_bootstrap::ensureTables(); + + global $db, $response; + $sourceId = (int)$this->id; + + if ($sourceId === $targetProductId) { + throw new \RuntimeException('Cannot merge a product into itself'); + } + + if ($targetProductId <= 0) { + throw new \RuntimeException('Invalid target product id'); + } + + // Target must exist + $targetCheck = $db->query("SELECT id FROM products WHERE id = " . (int)$targetProductId); + if ($targetCheck === false || !is_object($targetCheck) || (int)$targetCheck->num_rows === 0) { + throw new \RuntimeException('Target product does not exist'); + } + + // Source must not already be merged + $existing = self::fetchMergedInto($sourceId); + if ($existing !== null) { + throw new \RuntimeException("Source product {$sourceId} is already merged into product {$existing}"); + } + + // Target must not itself be a source (no chains during creation; chain + // resolution is supported at read time, but creating a chain here keeps + // the audit table unambiguous). + $targetIsSource = $db->query("SELECT id FROM products WHERE id = " . (int)$targetProductId . " AND merged_into_product_id IS NOT NULL"); + if ($targetIsSource !== false && is_object($targetIsSource) && (int)$targetIsSource->num_rows > 0) { + throw new \RuntimeException('Target product is itself merged into another product; cannot chain merges during creation'); + } + + $sourceIdEsc = (int)$db->escape_string((string)$sourceId); + $targetIdEsc = (int)$db->escape_string((string)$targetProductId); + $mergedBy = $mergedByUserId === null ? 'NULL' : (string)(int)$mergedByUserId; + $reasonSql = $reason === null ? 'NULL' : "'" . $db->escape_string(mb_substr($reason, 0, 500)) . "'"; + $now = date('Y-m-d H:i:s'); + + $db->query("START TRANSACTION"); + try { + $updateSql = "UPDATE products SET merged_into_product_id = {$targetIdEsc} WHERE id = {$sourceIdEsc}"; + if (!$db->query($updateSql)) { + throw new \RuntimeException('Failed to update products.merged_into_product_id'); + } + + $insertSql = "INSERT INTO product_merges (source_product_id, target_product_id, merged_by_user_id, reason, merged_at) VALUES ({$sourceIdEsc}, {$targetIdEsc}, {$mergedBy}, {$reasonSql}, '{$now}')"; + if (!$db->query($insertSql)) { + throw new \RuntimeException('Failed to insert product_merges audit row'); + } + + $db->query("COMMIT"); + } catch (\RuntimeException $e) { + $db->query("ROLLBACK"); + throw $e; + } + + // Refresh local object state + $this->getObjectProperties(); + } } diff --git a/services/nginx/app/routes/productsRoute.php b/services/nginx/app/routes/productsRoute.php index ef7d2834..bf9f082d 100644 --- a/services/nginx/app/routes/productsRoute.php +++ b/services/nginx/app/routes/productsRoute.php @@ -88,6 +88,18 @@ class productsRoute return $parsed; } + private function routePositiveInt(string $name): int + { + global $response; + + $raw = $this->fromRoute($name); + if (!is_string($raw) || !preg_match('/^[1-9][0-9]*$/', $raw)) { + $response->error('Invalid route parameter', 400); + } + + return (int)$raw; + } + private function isNullLikeOptionalParameter(mixed $value): bool { if ($value === null) { @@ -548,5 +560,55 @@ class productsRoute 'edit_product' => 'Edit a product' ] ); + + // POST /products/:id/merge — merge a product into another. + // Body: { target_id: int, reason?: string } + // The source product is preserved (so historical order_items references remain valid), + // but is marked as merged in the products table. Reads and new orders should follow + // merged_into_product_id to the target. An audit row is written to product_merges. + $this->post('/products/{id}/merge', function () { + global $response; + $this->requirePermission('edit_product'); + $user = (new authentication())->get_user(); + if (!$user) { + (new logs_o())->add('products', 'global', 1, 0, 'MERGE_PRODUCT', 'No user found, or invalid session'); + $response->error('Invalid session', 400); + } + + $sourceId = $this->routePositiveInt('id'); + + $targetId = (int)($response->getRequestParameter('target_id') ?? 0); + if ($targetId <= 0) { + $response->error('target_id is required and must be a positive integer', 400); + } + $reason = $response->getRequestParameter('reason'); + if ($reason !== null && !is_string($reason)) { + $response->error('reason must be a string', 400); + } + + $source = (new products_o())->select($sourceId); + if (!$source->exists()) { + $response->error('Source product not found', 404); + } + + try { + $source->mergeInto($targetId, (int)$user->id, $reason); + } catch (\RuntimeException $e) { + (new logs_o())->add('products', 'global', 3, (int)$user->id, 'MERGE_PRODUCT_FAILED', "source={$sourceId} target={$targetId} error=" . $e->getMessage()); + $response->error($e->getMessage(), 400); + } + + (new logs_o())->add('products', 'global', 1, (int)$user->id, 'MERGE_PRODUCT', "source={$sourceId} target={$targetId}"); + $response->success([ + 'message' => 'Product merged successfully', + 'source_product_id' => $sourceId, + 'target_product_id' => $targetId, + 'merged_into_product_id' => (int)$source->merged_into_product_id->value(), + ]); + }, + [ + 'edit_product' => 'Merge a product into another (preserves historical order references; new orders resolve to the target).' + ] + ); } } diff --git a/services/nginx/app/routes/userInvoicesRoute.php b/services/nginx/app/routes/userInvoicesRoute.php index cb38a4dd..95bdefe3 100644 --- a/services/nginx/app/routes/userInvoicesRoute.php +++ b/services/nginx/app/routes/userInvoicesRoute.php @@ -62,9 +62,21 @@ class userInvoicesRoute self::requireSameLength($id, self::getParameter('id')); $is_superuser = $this->hasPermission('superuser'); if (!self::isParametersSet(['po_number']) && !self::isParametersSet(['closed_at'])) { - $response->error('Missing required parameters: po_number, closed_at', 400); + // At least one of po_number or closed_at must be provided. The + // previous message said "Missing required parameters: + // po_number, closed_at" which read as if BOTH were required + // and confused customers trying to invoice (TRU-128). + $response->error('At least one of po_number or closed_at must be provided', 400); } - if (self::isParametersSet(['closed_at']) && !$is_superuser) { + // Only superusers may set a non-empty closed_at. Customers are + // still allowed to pass an empty/null closed_at to CLEAR a + // previously set value (the field is then set to null below). + $closed_at_is_non_empty = false; + if (self::isParametersSet(['closed_at'])) { + $raw_closed_at = self::getParameter('closed_at'); + $closed_at_is_non_empty = ($raw_closed_at !== null && $raw_closed_at !== ''); + } + if ($closed_at_is_non_empty && !$is_superuser) { $response->error('Forbidden: only superusers can update closed_at', 403); } // Make sure optional fields are valid diff --git a/services/nginx/app/tests/Api/ProductMergingApiTest.php b/services/nginx/app/tests/Api/ProductMergingApiTest.php new file mode 100644 index 00000000..d1c3e933 --- /dev/null +++ b/services/nginx/app/tests/Api/ProductMergingApiTest.php @@ -0,0 +1,218 @@ +db(); + $col = $db->query("SHOW COLUMNS FROM `products` LIKE 'merged_into_product_id'"); + expect($col)->not->toBeFalse(); + expect((int)$col->num_rows)->toBe(1); + + $tbl = $db->query("SHOW TABLES LIKE 'product_merges'"); + expect($tbl)->not->toBeFalse(); + expect((int)$tbl->num_rows)->toBe(1); +}); + +it('resolveActiveProductId follows merged_into_product_id', function (): void { + $source = api_fixtures()->createProduct([ + 'name' => 'SF Source (Lastbil)', + 'price' => 100, + ]); + $target = api_fixtures()->createProduct([ + 'name' => 'SF Target (Lastbil)', + 'price' => 150, + ]); + + $sourceObj = (new products_o())->select((int)$source['id']); + expect($sourceObj->exists())->toBeTrue(); + expect($sourceObj->resolveActiveProductId())->toBe((int)$source['id']); + + // No chain yet, and target is unchanged + $targetObj = (new products_o())->select((int)$target['id']); + expect($targetObj->resolveActiveProductId())->toBe((int)$target['id']); + + // Perform the merge + $sourceObj->mergeInto((int)$target['id'], null, 'TRU-94 test merge'); + + expect((int)$sourceObj->merged_into_product_id->value())->toBe((int)$target['id']); + expect($sourceObj->resolveActiveProductId())->toBe((int)$target['id']); + + // Reload from DB to confirm persistence + $reloaded = (new products_o())->select((int)$source['id']); + expect($reloaded->resolveActiveProductId())->toBe((int)$target['id']); + expect((int)$reloaded->merged_into_product_id->value())->toBe((int)$target['id']); +}); + +it('mergeInto preserves historical order_items references and writes an audit row', function (): void { + $source = api_fixtures()->createProduct([ + 'name' => 'Legacy SF', + 'price' => 200, + ]); + $target = api_fixtures()->createProduct([ + 'name' => 'New SF', + 'price' => 250, + ]); + + // Create a historical order and order_item that points at the source. + $user = api_fixtures()->createUser(['name' => 'Merge Test User']); + $cashier = api_fixtures()->createUser(['name' => 'Merge Test Cashier']); + $department = api_fixtures()->createDepartment(); + $order = api_fixtures()->createOrder([ + 'customer_id' => (int)$user['id'], + 'department_id' => (int)$department['id'], + ]); + $item = api_fixtures()->createOrderItem([ + 'order_id' => (int)$order['id'], + 'product_id' => (int)$source['id'], + 'cashier_id' => (int)$cashier['id'], + 'price' => 200, + 'quantity' => 1, + ]); + + expect((int)$item['product_id'])->toBe((int)$source['id']); + + // Merge source into target + (new products_o())->select((int)$source['id'])->mergeInto((int)$target['id'], null, 'TRU-94 historical preservation'); + + // Historical order_items.product_id MUST still point at the source. + // (This is the whole point of the merge: we don't rewrite history.) + $db = api_test_runtime()->db(); + $row = $db->query("SELECT product_id FROM order_items WHERE id = " . (int)$item['id'])->fetch_array(); + expect((int)$row['product_id'])->toBe((int)$source['id']); + + // Audit row exists + $audit = $db->query("SELECT * FROM product_merges WHERE source_product_id = " . (int)$source['id'])->fetch_array(); + expect($audit)->not->toBeNull(); + expect((int)$audit['source_product_id'])->toBe((int)$source['id']); + expect((int)$audit['target_product_id'])->toBe((int)$target['id']); + expect($audit['reason'])->toBe('TRU-94 historical preservation'); +}); + +it('price change on the target is what new orders see (resolution goes to target)', function (): void { + $source = api_fixtures()->createProduct(['name' => 'SF Pre-Merge', 'price' => 100]); + $target = api_fixtures()->createProduct(['name' => 'SF Post-Merge', 'price' => 100]); + + (new products_o())->select((int)$source['id'])->mergeInto((int)$target['id']); + + // Simulate a price change on the target (the only product new orders can be placed against) + $targetObj = (new products_o())->select((int)$target['id']); + $targetObj->price->set(175); + + // The source still resolves to the target, and a fresh read of the target shows the new price + $resolvedId = (new products_o())->select((int)$source['id'])->resolveActiveProductId(); + expect($resolvedId)->toBe((int)$target['id']); + + $reloaded = (new products_o())->select($resolvedId); + expect((int)$reloaded->price->value())->toBe(175); +}); + +it('POST /products/{id}/merge requires edit_product permission', function (): void { + $source = api_fixtures()->createProduct(['name' => 'Perm Source']); + $target = api_fixtures()->createProduct(['name' => 'Perm Target']); + + // IMPORTANT: do NOT pass ['group_id' => 1] here. group_id 1 is a + // hardcoded superuser/admin in objects\users_o::hasPermission() and + // bypasses the groups_permissions check entirely, so the route would + // 200 instead of 403. createUserSession([], []) creates a fresh empty + // group (id > 1) with no permissions, which is what this test needs. + $session = api_fixtures()->createUserSession([], []); + $response = api_client()->post( + '/products/' . (int)$source['id'] . '/merge', + ['target_id' => (int)$target['id']], + $session['headers'] + ); + + $response + ->assertStatus(403) + ->assertEnvelope() + ->assertSuccess(false); +}); + +it('POST /products/{id}/merge succeeds with edit_product permission', function (): void { + $source = api_fixtures()->createProduct(['name' => 'API Merge Source']); + $target = api_fixtures()->createProduct(['name' => 'API Merge Target']); + + $session = api_fixtures()->createUserSession(['edit_product'], ['group_id' => 1]); + + $response = api_client()->post( + '/products/' . (int)$source['id'] . '/merge', + ['target_id' => (int)$target['id'], 'reason' => 'SENERE 2 — TRU-94'], + $session['headers'] + ); + + $response + ->assertStatus(200) + ->assertEnvelope() + ->assertSuccess(); + + expect($response->data()) + ->toBeArray() + ->toHaveKey('source_product_id', (int)$source['id']) + ->toHaveKey('target_product_id', (int)$target['id']) + ->toHaveKey('merged_into_product_id', (int)$target['id']); +}); + +it('POST /products/{id}/merge rejects self-merge', function (): void { + $product = api_fixtures()->createProduct(['name' => 'Self Merge']); + $session = api_fixtures()->createUserSession(['edit_product'], ['group_id' => 1]); + + $response = api_client()->post( + '/products/' . (int)$product['id'] . '/merge', + ['target_id' => (int)$product['id']], + $session['headers'] + ); + + $response + ->assertStatus(400) + ->assertEnvelope() + ->assertSuccess(false); +}); + +it('POST /products/{id}/merge rejects double-merge', function (): void { + $a = api_fixtures()->createProduct(['name' => 'A']); + $b = api_fixtures()->createProduct(['name' => 'B']); + $c = api_fixtures()->createProduct(['name' => 'C']); + $session = api_fixtures()->createUserSession(['edit_product'], ['group_id' => 1]); + + // First merge succeeds + $first = api_client()->post( + '/products/' . (int)$a['id'] . '/merge', + ['target_id' => (int)$b['id']], + $session['headers'] + ); + $first->assertStatus(200)->assertEnvelope()->assertSuccess(); + + // Second merge of A into C should fail because A is already merged + $second = api_client()->post( + '/products/' . (int)$a['id'] . '/merge', + ['target_id' => (int)$c['id']], + $session['headers'] + ); + $second + ->assertStatus(400) + ->assertEnvelope() + ->assertSuccess(false); +}); diff --git a/services/nginx/app/tests/Support/Api/ApiTestRuntime.php b/services/nginx/app/tests/Support/Api/ApiTestRuntime.php index e06e1f4e..7b46babb 100644 --- a/services/nginx/app/tests/Support/Api/ApiTestRuntime.php +++ b/services/nginx/app/tests/Support/Api/ApiTestRuntime.php @@ -175,6 +175,11 @@ final class ApiTestRuntime ); $this->db->set_charset('utf8mb4'); + // Expose $GLOBALS['db'] as a classes\db wrapper around the same connection + // so that legacy object code (e.g. products_o::exists / products_o::mergeInto) + // that relies on `global $db` works inside the API test runtime. + $this->bindGlobalLegacyDb($this->db, $dbConfig); + $redisConfig = $this->readRedisConfig(); if ($redisConfig !== null) { $parameters = [ @@ -204,6 +209,42 @@ final class ApiTestRuntime $this->bootstrapped = true; } + /** + * Bind $GLOBALS['db'] to a classes\db wrapper around the active mysqli connection. + * + * The API test runtime speaks to the database through a raw mysqli handle + * (see db() above). However, a lot of the production object layer + * (e.g. objects\products_o::exists, objects\products_o::mergeInto, + * traits\db_object_t) uses `global $db;` and then calls methods on it. + * + * This wrapper re-uses the same underlying mysqli connection so that + * fixtures written via $this->db are visible to the legacy object layer + * and vice versa, without opening a second connection. + */ + private function bindGlobalLegacyDb(mysqli $connection, array $dbConfig): void + { + if (!class_exists(\classes\db::class)) { + // Legacy wrapper not available; tests that don't need it will still pass. + return; + } + + if (!isset($GLOBALS['db']) || !$GLOBALS['db'] instanceof \classes\db) { + $legacyDb = new \classes\db([ + 'host' => (string)$dbConfig['host'], + 'user' => (string)$dbConfig['user'], + 'password' => (string)$dbConfig['password'], + 'database' => (string)$dbConfig['database'], + 'port' => (int)$dbConfig['port'], + 'ssl_mode' => (string)($dbConfig['ssl_mode'] ?? 'DISABLED'), + ]); + $GLOBALS['db'] = $legacyDb; + } + + // Share the runtime mysqli handle so reads/writes stay consistent + // with the rest of the API test runtime. + $GLOBALS['db']->conn = $connection; + } + private function bootstrapSchemaIfRequested(): void { if ($this->schemaBootstrapped) { diff --git a/services/nginx/app/tests/Unit/Invoicing/UserCollectedInvoiceUpdateRouteValidationTest.php b/services/nginx/app/tests/Unit/Invoicing/UserCollectedInvoiceUpdateRouteValidationTest.php index 1fe4ac20..5a8ec66e 100644 --- a/services/nginx/app/tests/Unit/Invoicing/UserCollectedInvoiceUpdateRouteValidationTest.php +++ b/services/nginx/app/tests/Unit/Invoicing/UserCollectedInvoiceUpdateRouteValidationTest.php @@ -9,8 +9,12 @@ it('requires id and at least one mutable field for PUT /collected-invoices in us expect($content)->toContain("self::requireParameters(['id']);"); expect($content)->toContain("\$is_superuser = \$this->hasPermission('superuser');"); expect($content)->toContain("if (!self::isParametersSet(['po_number']) && !self::isParametersSet(['closed_at'])) {"); - expect($content)->toContain("\$response->error('Missing required parameters: po_number, closed_at', 400);"); - expect($content)->toContain("if (self::isParametersSet(['closed_at']) && !\$is_superuser) {"); + // TRU-128: The previous error message ("Missing required parameters: + // po_number, closed_at") read as if BOTH were required and confused + // customers trying to invoice. We now state the actual contract: at + // least one must be provided. + expect($content)->toContain("\$response->error('At least one of po_number or closed_at must be provided', 400);"); + expect($content)->toContain("if (\$closed_at_is_non_empty && !\$is_superuser) {"); expect($content)->toContain("\$response->error('Forbidden: only superusers can update closed_at', 403);"); expect($content)->toContain("if ((int)\$invoice->customer_number->value() !== (int)\$user->customer_number->value() && !\$is_superuser) {"); }); @@ -28,3 +32,24 @@ it('supports independent po_number and closed_at updates for PUT /collected-invo expect($content)->toContain("self::requireDateFormat((string)\$closed_at, self::FORMAT_DATE());"); expect($content)->toContain("\$invoice->closed_at->set(\$closed_at === null || \$closed_at === '' ? null : date('Y-m-d 23:59:59', strtotime((string)\$closed_at . ' 00:00:01')));"); }); + +it('locks in the TRU-128 bug fix: customers can clear closed_at with null/empty string', function (): void { + // TRU-128 / "Jeg kan ikke fakturere": a non-superuser could not pass + // closed_at at all (even null/empty) because isParametersSet() returns + // true for any present key. The route returned 403 Forbidden and the + // customer could not clear a previously-set closed_at either. The fix + // narrows the forbidden check to *non-empty* closed_at values, matching + // the existing clear-on-null/empty logic further down in the handler. + $routeFile = app_path('routes/userInvoicesRoute.php'); + $content = file_get_contents($routeFile); + + expect($content)->not->toBeFalse(); + // The "present + non-empty" check must precede the 403 guard, so + // clearing closed_at (passing null or "") for a non-superuser is allowed. + expect($content)->toMatch( + '/\$closed_at_is_non_empty\s*=\s*false;\s*if\s*\(self::isParametersSet\(\[\'closed_at\'\]\)\)\s*\{[^}]*\$closed_at_is_non_empty\s*=\s*\(\$raw_closed_at\s*!==\s*null\s*&&\s*\$raw_closed_at\s*!==\s*\'\'\);[^}]*\}\s*if\s*\(\$closed_at_is_non_empty\s*&&\s*!\$is_superuser\)\s*\{[^}]*Forbidden:\s*only\s*superusers/s' + ); + // The previous shape of the guard (which would always fire for any + // present closed_at, including null) must no longer be present. + expect($content)->not->toContain("if (self::isParametersSet(['closed_at']) && !\$is_superuser) {"); +});