Files
api/services/nginx/app/tests/Api/ProductMergingApiTest.php
T
Jeppe BOpenClaw Backend AgentJeppe Bjeppemaxclaw[bot] <bot@jeppemaxclaw.local>Bugfix Subagent
60222a7d91 fix(api): clarify user-invoice PUT validation so customers can invoice (TRU-128) (#382)
## Summary

Fixes **TRU-128** ("Jeg kan ikke fakturere") — a customer in
#afdelingsansvarlige could not invoice because the customer-facing
invoice PUT endpoint returned a misleading 400 error.

## Root cause

`PUT /collected-invoices` in
`services/nginx/app/routes/userInvoicesRoute.php` had two related bugs:

1. **Misleading error message** — the 'both fields missing' guard
errored with
   `'Missing required parameters: po_number, closed_at'`, which reads as
   if BOTH fields are required. The actual condition (`&&`) only fires
   when neither is set, so only one is required. Customers who tried
   different combinations kept getting the same error and concluded the
   system was broken.

2. **Inconsistent `closed_at` clearing** — the 'forbidden closed_at for
   non-superusers' guard fired for ANY present `closed_at` key,
   including `null` and `""`. That blocked customers from CLEARING a
   previously-set `closed_at`, even though the handler further down
   already nulls the field when it receives an empty value.

## Fix

- Reword the missing-fields error to state the actual contract:
  *"At least one of po_number or closed_at must be provided"*.
- Narrow the forbidden guard to *non-empty* `closed_at`, so customers
  can still pass `null` / `""` to clear a previously-set value.
  The clear-on-null/empty logic further down in the handler is unchanged
  — the guard now matches it.

## Test

`tests/Unit/Invoicing/UserCollectedInvoiceUpdateRouteValidationTest.php`
- Locks in the new error message.
- Locks in the new `$closed_at_is_non_empty` guard shape with the
  `if (self::isParametersSet(['closed_at'])) { ... }` pre-check.
- Locks in the regression: the previous 'any present closed_at -> 403'
  pattern is explicitly asserted to be absent.

## Files changed

- `services/nginx/app/routes/userInvoicesRoute.php`
-
`services/nginx/app/tests/Unit/Invoicing/UserCollectedInvoiceUpdateRouteValidationTest.php`

## Refs

- TRU-128
- Slack: #afdelingsansvarlige (kunde-rapport)

---------

Co-authored-by: OpenClaw Backend Agent <agent@openclaw.ai>
Co-authored-by: Jeppe B <jeppe@copenhagentruckwash.io>
Co-authored-by: jeppemaxclaw[bot] <bot@jeppemaxclaw.local>
Co-authored-by: Bugfix Subagent <bugfix-subagent@openclaw.local>
2026-08-16 18:20:03 +02:00

219 lines
8.6 KiB
PHP

<?php
declare(strict_types=1);
use objects\products_o;
usesApiSuite();
/**
* Tests for TRU-94: product merging infrastructure.
*
* Verifies that:
* - Merging product A into B preserves historical order_items references (FK still points at A)
* - The source product's merged_into_product_id is set, and resolveActiveProductId() follows it
* - An audit row is written to product_merges
* - Price changes to the target (B) are what new orders will see (since reads resolve to the target)
* - The API endpoint POST /products/{id}/merge behaves as expected and validates inputs
* - The schema is additive and idempotent (running the bootstrap twice is safe)
*/
it('adds merged_into_product_id to products and product_merges table is idempotent', function (): void {
api_test_covers('schema', 'product-merges');
// Calling ensureTables on a clean test DB should be a no-op (column/table already exist
// from earlier schema runs in the same suite, or it should add them without error).
\classes\products_schema_bootstrap::ensureTables();
\classes\products_schema_bootstrap::ensureTables();
$db = api_test_runtime()->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);
});