## Problem `POST /superuser/invoicing/period/tree-actions/preview` could return `409 Invoice-period snapshot is missing or expired` during normal UI flows when a snapshot binding aged out before the user triggered the action. ## Fix - introduce a dedicated snapshot cache TTL (`SNAPSHOT_BINDING_TTL_SECONDS`) - keep preview cache TTL unchanged (`PREVIEW_TTL_SECONDS`) - use the longer snapshot TTL for actor/customer snapshot binding writes This preserves existing safety because snapshot bindings are still revalidated against fresh revision data before use. ## Tests - `vendor/bin/pest tests/Unit/Invoicing/InvoiceCollectionBulkActionSafetyTest.php --colors=never` - `vendor/bin/pest tests/Api/CollectedInvoiceBulkActionsApiTest.php --colors=never` (suite present; skipped without `RUN_API_TESTS=1`) Co-authored-by: Jeppe Bundgaard <jb@truckwash.dk> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
212 lines
11 KiB
PHP
212 lines
11 KiB
PHP
<?php
|
|
|
|
app_require('classes/invoice_collection_bulk_action_service.php');
|
|
|
|
use classes\invoice_collection_bulk_action_service;
|
|
|
|
it('rejects stale bulk-action previews with a content digest and conflict response', function (): void {
|
|
$service = (string)file_get_contents(app_path('classes/invoice_collection_bulk_action_service.php'));
|
|
$route = (string)file_get_contents(app_path('routes/orderInvoicesRoute.php'));
|
|
|
|
expect($service)
|
|
->toContain('$preview[\'content_digest\'] = $this->previewContentDigest($preview);')
|
|
->toContain('hash_equals((string)($cached[\'preview\'][\'content_digest\'] ?? \'\'), $freshDigest)')
|
|
->toContain('throw new invoice_collection_bulk_action_conflict(')
|
|
->and($route)
|
|
->toContain('catch (invoice_collection_bulk_action_conflict $e)')
|
|
->toContain('$response->error($e->getMessage(), 409);');
|
|
});
|
|
|
|
it('binds invoice-period tree bulk previews to the actor and cached preview action', function (): void {
|
|
$service = (string)file_get_contents(app_path('classes/invoice_collection_bulk_action_service.php'));
|
|
$route = (string)file_get_contents(app_path('routes/orderInvoicesRoute.php'));
|
|
|
|
expect($service)
|
|
->toContain("'actor_user_id' => \$actorUserId")
|
|
->toContain('validateSnapshotBinding(')
|
|
->toContain('public function cachedPreviewAction(string $previewId): string')
|
|
->toContain('Preview belongs to another user. Create a new preview.')
|
|
->and($route)
|
|
->toContain("\$service->cachedPreviewAction((string)self::getParameter('preview_id'))")
|
|
->toContain("'customer_number'")
|
|
->toContain("'snapshot_revision'");
|
|
});
|
|
|
|
it('keeps snapshot bindings alive longer than preview caches for invoice-period tree actions', function (): void {
|
|
$service = (string)file_get_contents(app_path('classes/invoice_collection_bulk_action_service.php'));
|
|
$reflection = new ReflectionClass(invoice_collection_bulk_action_service::class);
|
|
$previewTtl = (int)$reflection->getConstant('PREVIEW_TTL_SECONDS');
|
|
$snapshotTtl = (int)$reflection->getConstant('SNAPSHOT_BINDING_TTL_SECONDS');
|
|
|
|
expect($snapshotTtl)->toBeGreaterThan($previewTtl)
|
|
->and($service)->toContain('private const SNAPSHOT_BINDING_TTL_SECONDS = 7200;')
|
|
->toContain('self::SNAPSHOT_BINDING_TTL_SECONDS')
|
|
->toContain("setEx(\n \$this->snapshotCacheKey(\$actorUserId, \$revision),");
|
|
});
|
|
|
|
it('previews whole-tree cleanup and merge supersession semantics', function (): void {
|
|
$service = (string)file_get_contents(app_path('classes/invoice_collection_bulk_action_service.php'));
|
|
$object = (string)file_get_contents(app_path('objects/collected_order_invoices_o.php'));
|
|
|
|
expect($service)
|
|
->toContain('$this->orderItemRows((int)$collection->id, true)')
|
|
->toContain('private function expandCleanupRows(array $rows, array $violationsByItemId): array')
|
|
->toContain('foreach ($this->allCollectionOrderRows((int)$collection->id) as $orderIdRow)')
|
|
->toContain("'supersession' => array_map")
|
|
->toContain('$source->markSupersededBy($targetId, $actorUserId)')
|
|
->and($object)
|
|
->toContain('public function markSupersededBy(int $targetInvoiceCollectionId, int $actorUserId): void')
|
|
->toContain('SUPERSESSION_MARKER_PATTERN')
|
|
->toContain('public function getSupersessionMetadata(): ?array')
|
|
->toContain('Collections superseded before the additive columns existed')
|
|
->toMatch('/Collections superseded before the additive columns existed.*?}\s*if \(!preg_match\(self::SUPERSESSION_MARKER_PATTERN/s')
|
|
->toContain('The source invoice collection has already been superseded.')
|
|
->toContain('The target invoice collection has already been superseded.')
|
|
->toContain('if ($this->getSupersessionMetadata() !== null)');
|
|
});
|
|
|
|
it('expands hidden cleanup descendants recursively and remains cycle safe', function (): void {
|
|
$service = (new ReflectionClass(invoice_collection_bulk_action_service::class))
|
|
->newInstanceWithoutConstructor();
|
|
$method = (new ReflectionClass(invoice_collection_bulk_action_service::class))
|
|
->getMethod('expandCleanupRows');
|
|
$rows = [
|
|
['order_item_id' => 10, 'order_id' => 1, 'related_item_id' => 13],
|
|
['order_item_id' => 11, 'order_id' => 1, 'related_item_id' => 10, 'include_in_invoice' => 0],
|
|
['order_item_id' => 12, 'order_id' => 1, 'related_item_id' => 11, 'include_in_invoice' => 0],
|
|
['order_item_id' => 13, 'order_id' => 1, 'related_item_id' => 12, 'include_in_invoice' => 0],
|
|
['order_item_id' => 14, 'order_id' => 2, 'related_item_id' => 10, 'include_in_invoice' => 0],
|
|
['order_item_id' => 99, 'order_id' => 1, 'related_item_id' => null],
|
|
];
|
|
|
|
$expanded = $method->invoke($service, $rows, [10 => 'customer_product_restricted']);
|
|
|
|
expect(array_column(array_column($expanded, 'row'), 'order_item_id'))->toBe([10, 11, 12, 13])
|
|
->and(array_column($expanded, 'rule'))->toBe([
|
|
'customer_product_restricted',
|
|
'related_to_removed_item',
|
|
'related_to_removed_item',
|
|
'related_to_removed_item',
|
|
]);
|
|
});
|
|
|
|
it('keeps legacy full-payload apply and exposes cached tree-action aliases', function (): void {
|
|
$route = (string)file_get_contents(app_path('routes/orderInvoicesRoute.php'));
|
|
|
|
expect($route)
|
|
->toMatch("/\\/collected-invoices\\/bulk-actions\\/apply'.*?requireParameters\\(\\['preview_id', 'action', 'invoice_collection_ids', 'confirmation_text'\\]\\)/s")
|
|
->toContain("\$this->post('/superuser/invoicing/period/tree-actions/preview'")
|
|
->toContain("\$this->post('/superuser/invoicing/period/tree-actions/apply'")
|
|
->toContain('InvoicingPeriodRoute::isInvoicePeriodObjectTreeV2Enabled((int)$user->id)')
|
|
->toContain('$service->applyCachedPreview(')
|
|
->toContain('$response->error($e->getMessage(), 409);');
|
|
});
|
|
|
|
it('changes the stale-preview digest when an export-relevant line changes', function (): void {
|
|
global $db;
|
|
|
|
$previousDb = $GLOBALS['db'] ?? null;
|
|
$hadDb = array_key_exists('db', $GLOBALS);
|
|
$db = new class {
|
|
public array $rows = [];
|
|
|
|
public function query(string $sql): object
|
|
{
|
|
expect($sql)
|
|
->toContain('oi.price')
|
|
->toContain('oi.quantity')
|
|
->toContain('p.economic_product_id')
|
|
->toContain('o.reference AS order_reference');
|
|
return new class($this->rows) {
|
|
private int $index = 0;
|
|
|
|
public function __construct(private readonly array $rows)
|
|
{
|
|
}
|
|
|
|
public function fetch_assoc(): ?array
|
|
{
|
|
return $this->rows[$this->index++] ?? null;
|
|
}
|
|
};
|
|
}
|
|
};
|
|
|
|
try {
|
|
$service = (new ReflectionClass(invoice_collection_bulk_action_service::class))->newInstanceWithoutConstructor();
|
|
$method = (new ReflectionClass(invoice_collection_bulk_action_service::class))
|
|
->getMethod('collectionExportContentDigest');
|
|
|
|
$db->rows = [[
|
|
'order_id' => 91,
|
|
'order_item_id' => 191,
|
|
'product_id' => 7,
|
|
'price' => 100,
|
|
'quantity' => 1,
|
|
'product_name' => 'Wash',
|
|
'economic_product_id' => 700,
|
|
]];
|
|
$before = $method->invoke($service, 41);
|
|
$db->rows[0]['price'] = 125;
|
|
$after = $method->invoke($service, 41);
|
|
|
|
expect($before)->toMatch('/^[a-f0-9]{64}$/')
|
|
->and($after)->toMatch('/^[a-f0-9]{64}$/')
|
|
->and($after)->not->toBe($before);
|
|
} finally {
|
|
if ($hadDb) {
|
|
$db = $previousDb;
|
|
} else {
|
|
unset($GLOBALS['db']);
|
|
}
|
|
}
|
|
});
|
|
|
|
it('enqueues bulk E-conomic transfers durably and deduplicates active jobs by global target', function (): void {
|
|
$service = (string)file_get_contents(app_path('classes/invoice_collection_bulk_action_service.php'));
|
|
$queue = (string)file_get_contents(app_path('classes/economic_transfer_queue.php'));
|
|
|
|
expect($service)
|
|
->toContain('self::ACTION_QUEUE_ECONOMIC => $this->applyQueueEconomic(')
|
|
->toContain('economic_transfer_queue::TYPE_COLLECTED_INVOICE_EXPORT')
|
|
->toContain("'queue_job_ids'")
|
|
->toContain('public static function assertCollectionCanQueueEconomic(')
|
|
->toContain('(new economic())->assertCustomerNumberIsNotDraft(')
|
|
->and($queue)
|
|
->toContain('Active work is unique by transfer type and business target across all requesting users.')
|
|
->toContain('CAST(JSON_UNQUOTE(JSON_EXTRACT(payload_json, \'$json_path\')) AS UNSIGNED) = ?')
|
|
->not->toContain("CAST(JSON_UNQUOTE(JSON_EXTRACT(payload_json, '$" . "json_path')) AS UNSIGNED) = ?\n AND created_by = ?");
|
|
});
|
|
|
|
it('serializes collected-invoice enqueue and worker export with collection mutations', function (): void {
|
|
$queue = (string)file_get_contents(app_path('classes/economic_transfer_queue.php'));
|
|
$route = (string)file_get_contents(app_path('routes/orderInvoicesRoute.php'));
|
|
$service = (string)file_get_contents(app_path('classes/invoice_collection_bulk_action_service.php'));
|
|
|
|
$enqueueStart = strpos($queue, 'public function enqueue(');
|
|
$activeJobCheck = strpos($queue, '$active_job = $this->findActiveJobByTarget', (int)$enqueueStart);
|
|
$enqueueLock = strpos($queue, '$this->acquireCollectedInvoiceExportLock($transfer_type, $payload)', (int)$enqueueStart);
|
|
$workerStart = strpos($queue, 'private function executeCollectedInvoiceExport(');
|
|
$workerLock = strpos($queue, '$this->acquireCollectedInvoiceExportLock(', (int)$workerStart);
|
|
$workerRevalidation = strpos($queue, '$this->assertCollectedInvoiceExportIsStillEligible($payload)', (int)$workerStart);
|
|
$workerExport = strpos($queue, '$this->executor->exportCollectedInvoice(', (int)$workerStart);
|
|
$routeStart = strpos($route, "\$this->post('/collected-invoices/economic'");
|
|
$routeEnd = strpos($route, "\$this->get('/collected-invoices/economic/queue'", (int)$routeStart);
|
|
$routeBlock = substr($route, (int)$routeStart, (int)$routeEnd - (int)$routeStart);
|
|
$routeLock = strpos($routeBlock, '$collection_export_lock = order_payment_lock::tryAcquireInvoiceCollection(');
|
|
$routeEligibility = strpos($routeBlock, 'invoice_collection_bulk_action_service::assertCollectionCanQueueEconomic(');
|
|
|
|
expect($enqueueStart)->not->toBeFalse()
|
|
->and($enqueueLock)->not->toBeFalse()->toBeLessThan($activeJobCheck)
|
|
->and($workerStart)->not->toBeFalse()
|
|
->and($workerLock)->not->toBeFalse()->toBeLessThan($workerRevalidation)
|
|
->and($workerRevalidation)->not->toBeFalse()->toBeLessThan($workerExport)
|
|
->and($queue)->toContain('order_payment_lock::tryAcquireInvoiceCollection($collected_invoice_id)')
|
|
->toContain('invoice_collection_bulk_action_service::assertCollectionCanQueueEconomic($collection)')
|
|
->and($route)->toContain('use classes\\order_payment_lock;')
|
|
->and($routeStart)->not->toBeFalse()
|
|
->and($routeEnd)->not->toBeFalse()
|
|
->and($routeLock)->not->toBeFalse()->toBeLessThan($routeEligibility)
|
|
->and($service)->toContain("Invoice collection has been superseded and cannot be exported.");
|
|
});
|