diff --git a/_build/elements/settings.php b/_build/elements/settings.php index 282c7918..07aceb03 100644 --- a/_build/elements/settings.php +++ b/_build/elements/settings.php @@ -321,6 +321,11 @@ 'xtype' => 'numberfield', 'area' => 'ms3_statuses', ], + 'ms3_order_status_transitions' => [ + 'value' => '', + 'xtype' => 'textfield', + 'area' => 'ms3_statuses', + ], 'ms3_customer_cancel_allowed_statuses' => [ 'value' => '2,3', 'xtype' => 'textfield', diff --git a/core/components/minishop3/lexicon/en/default.inc.php b/core/components/minishop3/lexicon/en/default.inc.php index 30f266a0..96bc9a6d 100644 --- a/core/components/minishop3/lexicon/en/default.inc.php +++ b/core/components/minishop3/lexicon/en/default.inc.php @@ -214,6 +214,8 @@ $_lang['ms3_err_status_fixed'] = 'Fixed status is set. You cannot change it to earlier one.'; $_lang['ms3_err_status_wrong'] = 'Invalid order status.'; $_lang['ms3_err_status_same'] = 'This status is already set.'; +$_lang['ms3_err_status_transition'] = 'This status transition is not allowed.'; +$_lang['ms3_err_status_transitions_invalid'] = 'Order status transition allow-list is invalid.'; $_lang['ms3_err_register_globals'] = 'Error: php parameter register_globals must be disabled.'; $_lang['ms3_err_link_equal'] = 'You are trying to add product link to itself'; $_lang['ms3_err_no_link'] = 'Link type not found'; diff --git a/core/components/minishop3/lexicon/en/setting.inc.php b/core/components/minishop3/lexicon/en/setting.inc.php index 316a5254..e36d0db9 100644 --- a/core/components/minishop3/lexicon/en/setting.inc.php +++ b/core/components/minishop3/lexicon/en/setting.inc.php @@ -142,13 +142,15 @@ $_lang['setting_ms3_status_canceled'] = 'Canceled order status ID'; $_lang['setting_ms3_status_canceled_desc'] = 'What status to set when canceling order'; $_lang['setting_ms3_status_sent'] = 'Sent order status ID'; -$_lang['setting_ms3_status_sent_desc'] = 'Order status to set when a shipment becomes shipped (if the transition is allowed).'; +$_lang['setting_ms3_status_sent_desc'] = 'Order status to set when a shipment becomes shipped (if the transition is allowed). Also used as the shipped status for order lifecycle ports (default seed: 4).'; $_lang['setting_ms3_shipment_enabled'] = 'Enable shipment lifecycle'; $_lang['setting_ms3_shipment_enabled_desc'] = 'Off (default): checkout and order statuses are unchanged. On: shipment shipped maps to ms3_status_sent via OrderStatusService, cancelled/failed maps to ms3_status_canceled. Create/setTracking still work when off. Webhook is 404 when off. Replace ms3_shipment_lifecycle to use an external WMS.'; $_lang['setting_ms3_shipment_on_delivered_status'] = 'Order status ID on delivered shipment'; $_lang['setting_ms3_shipment_on_delivered_status_desc'] = 'Optional. 0 (default) keeps order status unchanged when the shipment becomes delivered. Seed sent is final, so leave 0 unless you use a non-final sent status.'; $_lang['setting_ms3_shipment_on_in_transit_status'] = 'Order status ID on in-transit shipment'; $_lang['setting_ms3_shipment_on_in_transit_status_desc'] = 'Optional. 0 (default) keeps order status unchanged when the shipment becomes in_transit.'; +$_lang['setting_ms3_order_status_transitions'] = 'Allowed order status transitions'; +$_lang['setting_ms3_order_status_transitions_desc'] = 'Optional allow-list of status edges in addition to final/fixed rules. Empty = no matrix (default final/fixed only). Format: CSV pairs from:to (e.g. 2:3,3:4,2:5) or JSON [[2,3],[3,4]].'; $_lang['setting_ms3_customer_cancel_allowed_statuses'] = 'Statuses from which customer can cancel order'; $_lang['setting_ms3_customer_cancel_allowed_statuses_desc'] = 'Comma-separated status IDs. Default: New and Paid (2,3). Empty = use ms3_status_new and ms3_status_paid.'; $_lang['setting_ms3_status_for_stat'] = 'Status IDs for statistics'; diff --git a/core/components/minishop3/lexicon/ru/default.inc.php b/core/components/minishop3/lexicon/ru/default.inc.php index 67747b55..e0881105 100644 --- a/core/components/minishop3/lexicon/ru/default.inc.php +++ b/core/components/minishop3/lexicon/ru/default.inc.php @@ -214,6 +214,8 @@ $_lang['ms3_err_status_fixed'] = 'Установлен фиксирующий статус. Вы не можете сменить его на более ранний.'; $_lang['ms3_err_status_wrong'] = 'Неверный статус заказа.'; $_lang['ms3_err_status_same'] = 'Этот статус уже установлен.'; +$_lang['ms3_err_status_transition'] = 'Такой переход статуса не разрешён.'; +$_lang['ms3_err_status_transitions_invalid'] = 'Некорректный allow-list переходов статусов заказа.'; $_lang['ms3_err_register_globals'] = 'Ошибка: php параметр register_globals должен быть выключен.'; $_lang['ms3_err_link_equal'] = 'Вы пытаетесь добавить товару ссылку на самого себя'; $_lang['ms3_err_no_link'] = 'Тип связи не найден'; diff --git a/core/components/minishop3/lexicon/ru/setting.inc.php b/core/components/minishop3/lexicon/ru/setting.inc.php index 9b2523f1..dd8ef0c2 100644 --- a/core/components/minishop3/lexicon/ru/setting.inc.php +++ b/core/components/minishop3/lexicon/ru/setting.inc.php @@ -142,13 +142,15 @@ $_lang['setting_ms3_status_canceled'] = 'ID статуса отмены заказа'; $_lang['setting_ms3_status_canceled_desc'] = 'Какой статус нужно устанавливать при отмене заказа'; $_lang['setting_ms3_status_sent'] = 'ID статуса «отправлен»'; -$_lang['setting_ms3_status_sent_desc'] = 'Статус заказа при переходе отгрузки в shipped, если переход разрешён.'; +$_lang['setting_ms3_status_sent_desc'] = 'Статус заказа при переходе отгрузки в shipped, если переход разрешён. Также используется как shipped-статус для order lifecycle ports (по умолчанию seed id 4).'; $_lang['setting_ms3_shipment_enabled'] = 'Включить lifecycle отгрузки'; $_lang['setting_ms3_shipment_enabled_desc'] = 'Выкл. (по умолчанию): оформление и статусы заказа как сейчас. Вкл.: shipped ставит ms3_status_sent через OrderStatusService, cancelled/failed — ms3_status_canceled. create/setTracking работают и при выкл. Webhook при выкл. отвечает 404. Внешний WMS подменяется через ms3_shipment_lifecycle.'; $_lang['setting_ms3_shipment_on_delivered_status'] = 'ID статуса заказа при delivered'; $_lang['setting_ms3_shipment_on_delivered_status_desc'] = 'Необязательно. 0 (по умолчанию) не меняет статус заказа, когда отгрузка становится delivered. Сид sent финальный, поэтому оставьте 0, если не используете нефинальный sent.'; $_lang['setting_ms3_shipment_on_in_transit_status'] = 'ID статуса заказа при in_transit'; $_lang['setting_ms3_shipment_on_in_transit_status_desc'] = 'Необязательно. 0 (по умолчанию) не меняет статус заказа, когда отгрузка становится in_transit.'; +$_lang['setting_ms3_order_status_transitions'] = 'Разрешённые переходы статусов заказа'; +$_lang['setting_ms3_order_status_transitions_desc'] = 'Опциональный allow-list рёбер поверх правил final/fixed. Пусто — только final/fixed. Формат: CSV пары from:to (например 2:3,3:4,2:5) или JSON [[2,3],[3,4]].'; $_lang['setting_ms3_customer_cancel_allowed_statuses'] = 'Статусы, из которых покупатель может отменить заказ'; $_lang['setting_ms3_customer_cancel_allowed_statuses_desc'] = 'ID статусов через запятую. По умолчанию: «Новый» и «Оплачен» (2,3). Пусто — использовать ms3_status_new и ms3_status_paid.'; $_lang['setting_ms3_status_for_stat'] = 'ID статусов для статистики'; diff --git a/core/components/minishop3/src/ServiceRegistry.php b/core/components/minishop3/src/ServiceRegistry.php index ba076339..5e1c9ef7 100644 --- a/core/components/minishop3/src/ServiceRegistry.php +++ b/core/components/minishop3/src/ServiceRegistry.php @@ -108,7 +108,7 @@ class ServiceRegistry 'ms3_order_number_generator', ], 'ms3_order_finalize' => ['ms3_order_number_generator'], - 'ms3_order_status' => ['ms3_order_log'], + 'ms3_order_status' => ['ms3_order_log', 'ms3_order_lifecycle_ports'], 'ms3_payment_lifecycle' => ['ms3_order_status'], 'ms3_cart_mutation_handler' => [ 'ms3_order_draft_manager', @@ -286,6 +286,10 @@ class ServiceRegistry 'class' => \MiniShop3\Services\Order\OrderLogService::class, 'interface' => null, ], + 'ms3_order_lifecycle_ports' => [ + 'class' => \MiniShop3\Services\Order\NullOrderLifecyclePorts::class, + 'interface' => \MiniShop3\Services\Order\OrderLifecyclePortsInterface::class, + ], 'ms3_order_status' => [ 'class' => \MiniShop3\Services\Order\OrderStatusService::class, 'interface' => null, @@ -722,19 +726,16 @@ protected function validateClass( return $fallbackClass; } - if ($requiredInterface) { - $interfaces = class_implements($className); - if (!in_array($requiredInterface, $interfaces ?: [])) { - $this->modx->log( - modX::LOG_LEVEL_ERROR, - "[MiniShop3 ServiceRegistry] Class '{$className}' must implement {$requiredInterface}, " - . 'using fallback' - ); - return $fallbackClass; - } + if ($requiredInterface && !is_a($className, $requiredInterface, true)) { + $this->modx->log( + modX::LOG_LEVEL_ERROR, + "[MiniShop3 ServiceRegistry] Class '{$className}' must implement {$requiredInterface}, " + . 'using fallback' + ); + return $fallbackClass; } - if (!is_subclass_of($className, $fallbackClass)) { + if (!$requiredInterface && !is_subclass_of($className, $fallbackClass)) { $this->modx->log( modX::LOG_LEVEL_ERROR, "[MiniShop3 ServiceRegistry] Class '{$className}' must extend {$fallbackClass}, using fallback" diff --git a/core/components/minishop3/src/ServiceRegistryFactories.php b/core/components/minishop3/src/ServiceRegistryFactories.php index 64cef45d..0134f06c 100644 --- a/core/components/minishop3/src/ServiceRegistryFactories.php +++ b/core/components/minishop3/src/ServiceRegistryFactories.php @@ -184,11 +184,16 @@ public static function map(): array ); }, + 'ms3_order_lifecycle_ports' => static function (modX $modx, object $services, string $class): object { + return new $class(); + }, + 'ms3_order_status' => static function (modX $modx, object $services, string $class): object { return new $class( $modx, self::ms3($modx), - $services->get('ms3_order_log') + $services->get('ms3_order_log'), + $services->get('ms3_order_lifecycle_ports') ); }, diff --git a/core/components/minishop3/src/Services/Order/ManagerOrderMutationService.php b/core/components/minishop3/src/Services/Order/ManagerOrderMutationService.php index 769f417e..f681cfae 100644 --- a/core/components/minishop3/src/Services/Order/ManagerOrderMutationService.php +++ b/core/components/minishop3/src/Services/Order/ManagerOrderMutationService.php @@ -201,12 +201,19 @@ public function update(array $params = []): array // Store old values for logging $oldStatusId = (int)$order->get('status_id'); + $pendingStatusId = array_key_exists('status_id', $params) + ? (int) $params['status_id'] + : null; // Get editable order fields from msModelField configuration $orderFields = $this->presenter->getModelFieldNames('msOrder'); $changedOrderFields = []; foreach ($orderFields as $field) { + // Non-draft status changes go only through OrderStatusService (#592). + if ($field === 'status_id') { + continue; + } if (array_key_exists($field, $params)) { $oldValue = $order->get($field); $newValue = $params[$field]; @@ -257,8 +264,7 @@ public function update(array $params = []): array return $this->error('Failed to update order', HttpStatus::INTERNAL_SERVER_ERROR); } - // Log order field changes (excluding status_id which is logged separately) - unset($changedOrderFields['status_id']); + // Log order field changes (status_id is logged by OrderStatusService) if (!empty($changedOrderFields)) { $this->getOrderLog()->addEntry( $id, @@ -325,15 +331,10 @@ public function update(array $params = []): array } // Handle status change via OrderStatusService (sends notifications) - $newStatusId = (int)$order->get('status_id'); - if ($oldStatusId !== $newStatusId) { - // Revert status to old value - OrderStatusService will change it properly - $order->set('status_id', $oldStatusId); - $order->save(); - + if ($pendingStatusId !== null && $pendingStatusId !== $oldStatusId) { /** @var OrderStatusService $orderStatusService */ $orderStatusService = $this->modx->services->get('ms3_order_status'); - $result = $orderStatusService->change((int)$order->get('id'), $newStatusId); + $result = $orderStatusService->change((int)$order->get('id'), $pendingStatusId); if ($result !== true) { return $this->error( diff --git a/core/components/minishop3/src/Services/Order/NullOrderLifecyclePorts.php b/core/components/minishop3/src/Services/Order/NullOrderLifecyclePorts.php new file mode 100644 index 00000000..bc4df83d --- /dev/null +++ b/core/components/minishop3/src/Services/Order/NullOrderLifecyclePorts.php @@ -0,0 +1,28 @@ +modx = $modx; $this->ms3 = $ms3; $this->orderLog = $orderLog; + $this->lifecyclePorts = $lifecyclePorts ?? new NullOrderLifecyclePorts(); $this->modx->lexicon->load('minishop3:default'); } @@ -90,15 +108,22 @@ public function ensure(int $orderId, int $statusId, bool $skipNotifications = fa } /** - * Switch order status + * Switch order status (single gate for non-draft transitions). * * @param int $orderId The id of msOrder * @param int $statusId The id of msOrderStatus * @param bool $skipNotifications Skip sending notifications (for admin finalization) + * @param array{idempotent?: bool} $options idempotent=true → same status is success no-op * @return bool|string True on success, error message on failure */ - public function change(int $orderId, int $statusId, bool $skipNotifications = false): bool|string - { + public function change( + int $orderId, + int $statusId, + bool $skipNotifications = false, + array $options = [] + ): bool|string { + $idempotent = !empty($options['idempotent']); + /** @var msOrder|null $msOrder */ $msOrder = $this->modx->getObject(msOrder::class, ['id' => $orderId]); if (!$msOrder) { @@ -115,26 +140,26 @@ public function change(int $orderId, int $statusId, bool $skipNotifications = fa return $this->modx->lexicon('ms3_err_status_nf'); } + $storedStatusId = $msOrder->get('status_id'); + $previousStatusId = $storedStatusId !== null ? (int) $storedStatusId : null; + /** @var msOrderStatusModel|null $oldStatus */ - $oldStatus = $this->modx->getObject( - msOrderStatusModel::class, - ['id' => $msOrder->get('status_id'), 'active' => 1] - ); + $oldStatus = $previousStatusId !== null + ? $this->modx->getObject(msOrderStatusModel::class, ['id' => $previousStatusId]) + : null; - if ($oldStatus) { - $transitionError = $this->validateStatusTransition($oldStatus, $status); - if ($transitionError !== null) { - return $transitionError; - } + if ($previousStatusId === $statusId) { + return $idempotent ? true : $this->modx->lexicon('ms3_err_status_same'); } - if ($msOrder->get('status_id') == $statusId) { - return $this->modx->lexicon('ms3_err_status_same'); + $transitionError = $this->validateStatusTransition($oldStatus, $status); + if ($transitionError !== null) { + return $transitionError; } $eventParams = [ 'msOrder' => $msOrder, - 'old_status' => $oldStatus?->get('id'), + 'old_status' => $previousStatusId, 'status' => $statusId, ]; $response = $this->ms3->utils->invokeEvent('msOnBeforeChangeOrderStatus', $eventParams); @@ -154,8 +179,8 @@ public function change(int $orderId, int $statusId, bool $skipNotifications = fa if (!$status) { return $this->modx->lexicon('ms3_err_status_nf'); } - if ($msOrder->get('status_id') == $statusId) { - return $this->modx->lexicon('ms3_err_status_same'); + if ($previousStatusId === $statusId) { + return $idempotent ? true : $this->modx->lexicon('ms3_err_status_same'); } $transitionError = $this->validateStatusTransition($oldStatus, $status); @@ -164,20 +189,22 @@ public function change(int $orderId, int $statusId, bool $skipNotifications = fa } } - $msOrder->set('status_id', $statusId); - - if (!$msOrder->save()) { - return $this->modx->lexicon('ms3_err_unknown'); + $persistError = $this->persistStatusWithPorts($msOrder, $statusId, $previousStatusId); + if ($persistError !== null) { + return $persistError; } + // Log before the after-event: status is already committed, and a plugin + // error must not erase the history of that transition. $this->orderLog->add($msOrder->get('id'), $statusId, 'status'); $response = $this->ms3->utils->invokeEvent('msOnChangeOrderStatus', [ 'msOrder' => $msOrder, - 'old_status' => $oldStatus?->get('id'), + 'old_status' => $previousStatusId, 'status' => $statusId, ]); if (!$response['success']) { + // Status (and in-TX domain work) already committed — do not compensate. return $response['message']; } @@ -193,7 +220,110 @@ public function change(int $orderId, int $statusId, bool $skipNotifications = fa } /** - * Validate transition from old status to new (final/fixed rules). + * In-TX domain hooks then persist status_id. + * Joins an already open transaction instead of starting a nested one. + * On failure rolls back only the transaction this method opened. + */ + protected function persistStatusWithPorts( + msOrder $msOrder, + int $statusId, + ?int $previousStatusId + ): ?string { + $ownsTx = $this->beginOwnedTransaction(); + + try { + $portError = $this->runLifecyclePorts($msOrder, $statusId, $previousStatusId); + if ($portError !== null) { + $this->rollbackOwnedTransaction($ownsTx); + + return $portError; + } + + $msOrder->set('status_id', $statusId); + if (!$msOrder->save()) { + $this->rollbackOwnedTransaction($ownsTx); + $msOrder->set('status_id', $previousStatusId); + + return $this->modx->lexicon('ms3_err_unknown'); + } + + $this->commitOwnedTransaction($ownsTx); + } catch (\Throwable $e) { + $this->rollbackOwnedTransaction($ownsTx); + $msOrder->set('status_id', $previousStatusId); + $this->modx->log( + modX::LOG_LEVEL_ERROR, + '[OrderStatusService] persistStatusWithPorts: ' . $e->getMessage() + ); + + return $this->modx->lexicon('ms3_err_unknown'); + } + + return null; + } + + /** + * Start a transaction only when none is active. + * PDO::beginTransaction() throws if one is already open. + * + * xPDO exposes the connection as public $pdo and has no inTransaction(). + */ + private function beginOwnedTransaction(): bool + { + if ( + !is_callable([$this->modx, 'beginTransaction']) + || !is_callable([$this->modx, 'commit']) + || !is_callable([$this->modx, 'rollBack']) + ) { + return false; + } + if ($this->hasOpenTransaction()) { + return false; + } + + $this->modx->beginTransaction(); + + return true; + } + + /** + * Test doubles may define inTransaction() on the modX subclass. + * Production xPDO does not: the check is $modx->pdo->inTransaction(). + */ + private function hasOpenTransaction(): bool + { + if (method_exists($this->modx, 'inTransaction')) { + return (bool) $this->modx->inTransaction(); + } + + $pdo = $this->modx->pdo; + if (!$pdo instanceof \PDO) { + return false; + } + + return $pdo->inTransaction(); + } + + private function commitOwnedTransaction(bool $ownsTx): void + { + if ($ownsTx) { + $this->modx->commit(); + } + } + + private function rollbackOwnedTransaction(bool $ownsTx): void + { + if (!$ownsTx) { + return; + } + if (!$this->hasOpenTransaction()) { + return; + } + $this->modx->rollback(); + } + + /** + * Validate transition: final/fixed defaults + optional allow-list (ms3_order_status_transitions). */ protected function validateStatusTransition( ?msOrderStatusModel $oldStatus, @@ -211,6 +341,42 @@ protected function validateStatusTransition( return $this->modx->lexicon('ms3_err_status_fixed'); } + $edges = OrderStatusTransitionPolicy::resolve( + $this->modx->getOption('ms3_order_status_transitions', null, '') + ); + if ($edges['mode'] === OrderStatusTransitionPolicy::MODE_INVALID) { + return $this->modx->lexicon('ms3_err_status_transitions_invalid'); + } + if ( + $edges['mode'] === OrderStatusTransitionPolicy::MODE_ON + && !isset($edges['edges'][(int) $oldStatus->get('id')][(int) $newStatus->get('id')]) + ) { + return $this->modx->lexicon('ms3_err_status_transition'); + } + + return null; + } + + /** + * Invoke in-TX semantic lifecycle ports when the target matches configured status ids. + * May deny the transition before status_id is persisted (#589–#591 / #603). + */ + protected function runLifecyclePorts(msOrder $order, int $statusId, ?int $previousStatusId): ?string + { + $paidId = (int) $this->modx->getOption('ms3_status_paid', null, 3); + $canceledId = (int) $this->modx->getOption('ms3_status_canceled', null, 5); + $sentId = (int) $this->modx->getOption('ms3_status_sent', null, 4); + + if ($statusId === $paidId) { + return $this->lifecyclePorts->onOrderBecamePaid($order, $previousStatusId); + } + if ($statusId === $canceledId) { + return $this->lifecyclePorts->onOrderCancelled($order, $previousStatusId); + } + if ($statusId === $sentId) { + return $this->lifecyclePorts->onOrderShipped($order, $previousStatusId); + } + return null; } diff --git a/core/components/minishop3/src/Services/Order/OrderStatusTransitionPolicy.php b/core/components/minishop3/src/Services/Order/OrderStatusTransitionPolicy.php new file mode 100644 index 00000000..8d1eb13c --- /dev/null +++ b/core/components/minishop3/src/Services/Order/OrderStatusTransitionPolicy.php @@ -0,0 +1,89 @@ +>} + */ + public static function resolve(mixed $raw): array + { + if ($raw === null) { + return ['mode' => self::MODE_OFF, 'edges' => []]; + } + + if (is_array($raw)) { + return ['mode' => self::MODE_ON, 'edges' => self::fromPairList($raw)]; + } + + $value = trim((string) $raw); + if ($value === '') { + return ['mode' => self::MODE_OFF, 'edges' => []]; + } + + if (str_starts_with($value, '[')) { + $decoded = json_decode($value, true); + if (!is_array($decoded)) { + return ['mode' => self::MODE_INVALID, 'edges' => []]; + } + + return ['mode' => self::MODE_ON, 'edges' => self::fromPairList($decoded)]; + } + + $pairs = []; + foreach (array_filter(array_map('trim', explode(',', $value))) as $pair) { + $parts = array_map('trim', explode(':', $pair, 2)); + if (count($parts) !== 2) { + return ['mode' => self::MODE_INVALID, 'edges' => []]; + } + $pairs[] = $parts; + } + + $edges = self::fromPairList($pairs); + if ($edges === []) { + return ['mode' => self::MODE_INVALID, 'edges' => []]; + } + + return ['mode' => self::MODE_ON, 'edges' => $edges]; + } + + /** + * @param array $pairs + * @return array> + */ + private static function fromPairList(array $pairs): array + { + $edges = []; + foreach ($pairs as $pair) { + if (!is_array($pair) || count($pair) < 2) { + continue; + } + $from = (int) $pair[0]; + $to = (int) $pair[1]; + if ($from < 1 || $to < 1) { + continue; + } + $edges[$from][$to] = true; + } + + return $edges; + } +} diff --git a/core/components/minishop3/src/Services/Shipment/ShipmentLifecycleService.php b/core/components/minishop3/src/Services/Shipment/ShipmentLifecycleService.php index 6fae83bf..2ffe05bd 100644 --- a/core/components/minishop3/src/Services/Shipment/ShipmentLifecycleService.php +++ b/core/components/minishop3/src/Services/Shipment/ShipmentLifecycleService.php @@ -352,7 +352,7 @@ private function syncOrderStatus(int $orderId, string $shipmentStatus): void if ((int) $order->get('status_id') === $statusId) { return; } - $result = $this->orderStatus->change($orderId, $statusId); + $result = $this->orderStatus->ensure($orderId, $statusId); if ($result !== true) { $this->modx->log( modX::LOG_LEVEL_ERROR, diff --git a/core/components/minishop3/tests/Unit/Controllers/Api/Manager/OrderShipmentControllerTest.php b/core/components/minishop3/tests/Unit/Controllers/Api/Manager/OrderShipmentControllerTest.php index c5714b21..466da0da 100644 --- a/core/components/minishop3/tests/Unit/Controllers/Api/Manager/OrderShipmentControllerTest.php +++ b/core/components/minishop3/tests/Unit/Controllers/Api/Manager/OrderShipmentControllerTest.php @@ -141,7 +141,7 @@ private function modxWithLifecycle(): modX $store = new InMemoryShipmentStore(); $order = new StubMsOrder(['id' => 10, 'delivery_id' => 7, 'status_id' => 3]); $orderStatus = $this->createMock(OrderStatusService::class); - $orderStatus->method('change')->willReturn(true); + $orderStatus->method('ensure')->willReturn(true); $lifecycle = new ShipmentLifecycleService($store, $this->modx($order), $orderStatus); return $this->modx($order, ['ms3_shipment_lifecycle' => $lifecycle]); diff --git a/core/components/minishop3/tests/Unit/Controllers/Api/Web/DeliveryWebhookControllerTest.php b/core/components/minishop3/tests/Unit/Controllers/Api/Web/DeliveryWebhookControllerTest.php index fae0ce45..e2f0659d 100644 --- a/core/components/minishop3/tests/Unit/Controllers/Api/Web/DeliveryWebhookControllerTest.php +++ b/core/components/minishop3/tests/Unit/Controllers/Api/Web/DeliveryWebhookControllerTest.php @@ -212,7 +212,7 @@ protected function readRawRequestBody(): string private function lifecycle(InMemoryShipmentStore $store, msOrder $order): ShipmentLifecycleService { $orderStatus = $this->createMock(OrderStatusService::class); - $orderStatus->method('change')->willReturnCallback( + $orderStatus->method('ensure')->willReturnCallback( function (int $orderId, int $statusId): bool { $this->statusChanges[] = [$orderId, $statusId]; diff --git a/core/components/minishop3/tests/Unit/Services/Order/OrderStatusServiceLifecycleTest.php b/core/components/minishop3/tests/Unit/Services/Order/OrderStatusServiceLifecycleTest.php new file mode 100644 index 00000000..ee5b350b --- /dev/null +++ b/core/components/minishop3/tests/Unit/Services/Order/OrderStatusServiceLifecycleTest.php @@ -0,0 +1,488 @@ +makeHarness(statusId: 2); + $log = $this->recordingLog(); + $events = []; + + $service = $this->makeService($harness, $log, new NullOrderLifecyclePorts(), $events); + $result = $service->change(10, 2, true, ['idempotent' => true]); + + self::assertTrue($result); + self::assertSame([], $log->entries); + self::assertSame([], $events); + } + + public function testSameStatusWithoutIdempotentReturnsError(): void + { + $harness = $this->makeHarness(statusId: 2); + $log = $this->recordingLog(); + $events = []; + + $service = $this->makeService($harness, $log, new NullOrderLifecyclePorts(), $events); + $result = $service->change(10, 2); + + self::assertSame('ms3_err_status_same', $result); + self::assertSame([], $log->entries); + } + + public function testAllowListRejectsUnknownEdgeWithoutSave(): void + { + $harness = $this->makeHarness(statusId: 2, options: [ + 'ms3_order_status_transitions' => '2:3', + ]); + $log = $this->recordingLog(); + $events = []; + + $service = $this->makeService($harness, $log, new NullOrderLifecyclePorts(), $events); + $result = $service->change(10, 4); + + self::assertSame('ms3_err_status_transition', $result); + self::assertSame(2, $harness['order']->get('status_id')); + self::assertSame([], $log->entries); + self::assertSame([], $events); + } + + public function testInvalidAllowListConfigRejectedWithoutSave(): void + { + $harness = $this->makeHarness(statusId: 2, options: [ + 'ms3_order_status_transitions' => '[broken', + ]); + $log = $this->recordingLog(); + $events = []; + + $service = $this->makeService($harness, $log, new NullOrderLifecyclePorts(), $events); + $result = $service->change(10, 3); + + self::assertSame('ms3_err_status_transitions_invalid', $result); + self::assertSame(2, $harness['order']->get('status_id')); + self::assertSame([], $log->entries); + } + + public function testCancelAndShippedPortsAreInvoked(): void + { + $ports = new class implements OrderLifecyclePortsInterface { + public int $cancelCalls = 0; + public int $shipCalls = 0; + + public function onOrderBecamePaid(msOrder $order, ?int $previousStatusId): ?string + { + return null; + } + + public function onOrderCancelled(msOrder $order, ?int $previousStatusId): ?string + { + ++$this->cancelCalls; + + return null; + } + + public function onOrderShipped(msOrder $order, ?int $previousStatusId): ?string + { + ++$this->shipCalls; + + return null; + } + }; + + $cancelHarness = $this->makeHarness(statusId: 2); + $cancelLog = $this->recordingLog(); + $cancelEvents = []; + $cancelService = $this->makeService($cancelHarness, $cancelLog, $ports, $cancelEvents); + self::assertTrue($cancelService->change(10, 5, true)); + self::assertSame(1, $ports->cancelCalls); + + $shipHarness = $this->makeHarness(statusId: 2); + $shipLog = $this->recordingLog(); + $shipEvents = []; + $shipService = $this->makeService($shipHarness, $shipLog, $ports, $shipEvents); + self::assertTrue($shipService->change(10, 4, true)); + self::assertSame(1, $ports->shipCalls); + } + + public function testAfterEventFailureKeepsCommittedStatusAndStillLogs(): void + { + $harness = $this->makeHarness(statusId: 2); + $log = $this->recordingLog(); + $events = []; + + $service = $this->makeService( + $harness, + $log, + new NullOrderLifecyclePorts(), + $events, + afterFail: true + ); + $result = $service->change(10, 3, true); + + self::assertSame('after failed', $result); + self::assertSame(3, $harness['order']->get('status_id')); + self::assertSame([[10, 3, 'status']], $log->entries); + self::assertSame( + ['msOnBeforeChangeOrderStatus', 'log-present', 'msOnChangeOrderStatus'], + $events + ); + } + + public function testJoinsOuterTransactionWithoutBeginOrCommit(): void + { + $harness = $this->makeHarness(statusId: 2); + $harness['tx'] = (object) [ + 'outer' => true, + 'open' => false, + 'begins' => 0, + 'commits' => 0, + 'rollbacks' => 0, + ]; + $log = $this->recordingLog(); + $events = []; + $service = $this->makeService($harness, $log, new NullOrderLifecyclePorts(), $events); + + self::assertTrue($service->change(10, 3, true)); + self::assertSame(3, $harness['order']->get('status_id')); + self::assertSame(0, $harness['tx']->begins); + self::assertSame(0, $harness['tx']->commits); + self::assertSame(0, $harness['tx']->rollbacks); + } + + public function testOwnedTransactionRollsBackWhenPortDenies(): void + { + $harness = $this->makeHarness(statusId: 2); + $harness['tx'] = (object) [ + 'outer' => false, + 'open' => false, + 'begins' => 0, + 'commits' => 0, + 'rollbacks' => 0, + ]; + $log = $this->recordingLog(); + $events = []; + $ports = new class implements OrderLifecyclePortsInterface { + public function onOrderBecamePaid(msOrder $order, ?int $previousStatusId): ?string + { + return 'port failed'; + } + + public function onOrderCancelled(msOrder $order, ?int $previousStatusId): ?string + { + return null; + } + + public function onOrderShipped(msOrder $order, ?int $previousStatusId): ?string + { + return null; + } + }; + $service = $this->makeService($harness, $log, $ports, $events); + + self::assertSame('port failed', $service->change(10, 3, true)); + self::assertSame(2, $harness['order']->get('status_id')); + self::assertSame(1, $harness['tx']->begins); + self::assertSame(0, $harness['tx']->commits); + self::assertSame(1, $harness['tx']->rollbacks); + } + + public function testPaidPortFailureAbortsBeforeSave(): void + { + $harness = $this->makeHarness(statusId: 2); + $log = $this->recordingLog(); + $events = []; + $ports = new class implements OrderLifecyclePortsInterface { + public int $paidCalls = 0; + + public function onOrderBecamePaid(msOrder $order, ?int $previousStatusId): ?string + { + ++$this->paidCalls; + + return 'port failed'; + } + + public function onOrderCancelled(msOrder $order, ?int $previousStatusId): ?string + { + return null; + } + + public function onOrderShipped(msOrder $order, ?int $previousStatusId): ?string + { + return null; + } + }; + + $service = $this->makeService($harness, $log, $ports, $events); + $result = $service->change(10, 3, true); + + self::assertSame('port failed', $result); + self::assertSame(1, $ports->paidCalls); + self::assertSame(2, $harness['order']->get('status_id')); + self::assertSame([], $log->entries); + self::assertSame(['msOnBeforeChangeOrderStatus'], $events); + } + + public function testSuccessfulPaidTransitionLogsAndInvokesPort(): void + { + $harness = $this->makeHarness(statusId: 2); + $log = $this->recordingLog(); + $events = []; + $ports = new class implements OrderLifecyclePortsInterface { + public int $paidCalls = 0; + + public function onOrderBecamePaid(msOrder $order, ?int $previousStatusId): ?string + { + ++$this->paidCalls; + + return null; + } + + public function onOrderCancelled(msOrder $order, ?int $previousStatusId): ?string + { + return null; + } + + public function onOrderShipped(msOrder $order, ?int $previousStatusId): ?string + { + return null; + } + }; + + $service = $this->makeService($harness, $log, $ports, $events); + $result = $service->change(10, 3, true); + + self::assertTrue($result); + self::assertSame(1, $ports->paidCalls); + self::assertSame(3, $harness['order']->get('status_id')); + self::assertSame([[10, 3, 'status']], $log->entries); + self::assertSame(['msOnBeforeChangeOrderStatus', 'msOnChangeOrderStatus'], $events); + } + + /** + * @param array $options + * @return array{order: RecordingMsOrder, statuses: array, options: array} + */ + private function makeHarness(int $statusId, array $options = []): array + { + $order = new RecordingMsOrder([ + 'id' => 10, + 'status_id' => $statusId, + 'context' => 'web', + ]); + + $statuses = [ + 2 => $this->makeStatus(2, final: false, fixed: false, position: 2), + 3 => $this->makeStatus(3, final: false, fixed: true, position: 3), + 4 => $this->makeStatus(4, final: true, fixed: true, position: 4), + 5 => $this->makeStatus(5, final: true, fixed: false, position: 5), + ]; + + return [ + 'order' => $order, + 'statuses' => $statuses, + 'options' => array_merge([ + 'ms3_status_paid' => 3, + 'ms3_status_canceled' => 5, + 'ms3_status_sent' => 4, + 'ms3_order_status_transitions' => '', + ], $options), + ]; + } + + private function makeStatus(int $id, bool $final, bool $fixed, int $position): object + { + return new class($id, $final, $fixed, $position) extends msOrderStatus { + public function __construct( + private int $statusId, + private bool $isFinal, + private bool $isFixed, + private int $pos + ) { + } + + public function get($k, $format = null, $formatType = '') + { + return match ($k) { + 'id' => $this->statusId, + 'final' => $this->isFinal ? 1 : 0, + 'fixed' => $this->isFixed ? 1 : 0, + 'position' => $this->pos, + default => null, + }; + } + }; + } + + /** + * @return OrderLogService&object{entries: list} + */ + private function recordingLog(): OrderLogService + { + return new class extends OrderLogService { + /** @var list */ + public array $entries = []; + + public function __construct() + { + } + + public function add(int $order_id, mixed $entry, string $action, bool $visible = true): bool + { + $this->entries[] = [$order_id, $entry, $action]; + + return true; + } + }; + } + + /** + * @param array{order: RecordingMsOrder, statuses: array, options: array} $harness + * @param list $events + */ + private function makeService( + array $harness, + OrderLogService $log, + OrderLifecyclePortsInterface $ports, + array &$events, + bool $afterFail = false + ): OrderStatusService { + $modx = new class($harness, $afterFail) extends modX { + /** @var array{order: RecordingMsOrder, statuses: array, options: array, tx?: object} */ + private array $harness; + private bool $afterFail; + + public function __construct(array $harness, bool $afterFail) + { + parent::__construct(); + $this->harness = $harness; + $this->afterFail = $afterFail; + if (!isset($this->harness['tx'])) { + $this->harness['tx'] = (object) [ + 'outer' => false, + 'open' => false, + 'begins' => 0, + 'commits' => 0, + 'rollbacks' => 0, + ]; + } + } + + public function inTransaction(): bool + { + return $this->harness['tx']->outer || $this->harness['tx']->open; + } + + public function beginTransaction() + { + if ($this->inTransaction()) { + throw new \PDOException('There is already an active transaction'); + } + $this->harness['tx']->begins++; + $this->harness['tx']->open = true; + + return true; + } + + public function commit() + { + $this->harness['tx']->commits++; + $this->harness['tx']->open = false; + + return true; + } + + public function rollback() + { + $this->harness['tx']->rollbacks++; + $this->harness['tx']->open = false; + + return true; + } + + public function getOption(string $key, $options = null, $default = null) + { + return $this->harness['options'][$key] ?? $default; + } + + public function switchContext($contextKey, $force = false) + { + return true; + } + + public function getObject($className, $criteria = null, $cacheFlag = true) + { + if ($className === msOrder::class || $className === RecordingMsOrder::class) { + $id = is_array($criteria) ? (int) ($criteria['id'] ?? 0) : (int) $criteria; + return $id === (int) $this->harness['order']->get('id') + ? $this->harness['order'] + : null; + } + + if ($className === msOrderStatus::class || is_a($className, msOrderStatus::class, true)) { + $id = is_array($criteria) ? (int) ($criteria['id'] ?? 0) : (int) $criteria; + return $this->harness['statuses'][$id] ?? null; + } + + return null; + } + }; + + $ms3 = $this->createMock(MiniShop3::class); + $ms3->method('initialize')->willReturn(true); + + $utils = new class($events, $afterFail, $log) { + /** @var list */ + private array $events; + private bool $afterFail; + private OrderLogService $log; + + public function __construct(array &$events, bool $afterFail, OrderLogService $log) + { + $this->events = &$events; + $this->afterFail = $afterFail; + $this->log = $log; + } + + public function invokeEvent(string $eventName, array $params = [], $glue = '
'): array + { + if ($eventName === 'msOnChangeOrderStatus' && $this->afterFail) { + $entries = $this->log->entries ?? []; + $this->events[] = $entries === [] ? 'log-missing' : 'log-present'; + } + $this->events[] = $eventName; + if ($eventName === 'msOnChangeOrderStatus' && $this->afterFail) { + return ['success' => false, 'message' => 'after failed', 'data' => []]; + } + + return ['success' => true, 'message' => '', 'data' => $params]; + } + }; + $ms3->utils = $utils; + + return new OrderStatusService($modx, $ms3, $log, $ports); + } +} diff --git a/core/components/minishop3/tests/Unit/Services/Order/OrderStatusTransitionPolicyTest.php b/core/components/minishop3/tests/Unit/Services/Order/OrderStatusTransitionPolicyTest.php new file mode 100644 index 00000000..4f511ee3 --- /dev/null +++ b/core/components/minishop3/tests/Unit/Services/Order/OrderStatusTransitionPolicyTest.php @@ -0,0 +1,57 @@ + 10, 'delivery_id' => 7, 'status_id' => 3]); $modx = $this->modx($order, true); $orderStatus = $this->createMock(OrderStatusService::class); - $orderStatus->method('change')->willReturn('ms3_err_status_final'); + $orderStatus->method('ensure')->willReturn('ms3_err_status_final'); $service = new ShipmentLifecycleService($store, $modx, $orderStatus); $row = $service->create(10); $updated = $service->transition($row['id'], ShipmentStatus::SHIPPED, 'evt-1'); @@ -350,7 +350,7 @@ public function testWebhookDeliveryMismatchDoesNotCreateShipment(): void $store = new InMemoryShipmentStore(); $order = new StubMsOrder(['id' => 10, 'delivery_id' => 5, 'status_id' => 3]); $orderStatus = $this->createMock(OrderStatusService::class); - $orderStatus->method('change')->willReturnCallback( + $orderStatus->method('ensure')->willReturnCallback( function (int $orderId, int $statusId): bool { $this->statusChanges[] = [$orderId, $statusId]; @@ -377,7 +377,7 @@ private function service(InMemoryShipmentStore $store, bool $enabled = false): S { $order = new StubMsOrder(['id' => 10, 'delivery_id' => 7, 'status_id' => 3]); $orderStatus = $this->createMock(OrderStatusService::class); - $orderStatus->method('change')->willReturnCallback( + $orderStatus->method('ensure')->willReturnCallback( function (int $orderId, int $statusId): bool { $this->statusChanges[] = [$orderId, $statusId];