Skip to content

feat(order): lifecycle gate with ports, idempotency, and rollback - #596

Open
Ibochkarev wants to merge 1 commit into
betafrom
feat/issue-592-order-lifecycle
Open

Ibochkarev wants to merge 1 commit into
betafrom
feat/issue-592-order-lifecycle

Conversation

@Ibochkarev

Copy link
Copy Markdown
Member

Описание

OrderStatusService становится единым gate для non-draft смены статуса: опциональный allow-list переходов, DI-порты lifecycle под #589#591 (пока no-op), режим idempotent для интеграций, rollback status_id если порт или msOnChangeOrderStatus вернули ошибку. Manager update больше не пишет status_id в общий save() до вызова сервиса. Пример в Payment docblock ведёт через gate.

Тип изменений

  • Новая функциональность (non-breaking change)
  • Исправление бага (non-breaking change)

Связанные Issues

Closes #592

Как это было протестировано?

Gate E (локально):

cd core/components/minishop3
php -l src/Services/Order/OrderStatusService.php   # exit 0
php -l src/Services/Order/OrderStatusTransitionPolicy.php
php -l src/Services/Order/ManagerOrderMutationService.php
./vendor/bin/phpunit tests/Unit/Services/Order/     # OK 18 tests
./vendor/bin/phpunit --testsuite Unit               # OK 195 tests (2 pre-existing deprecations)
php tests/OrderStatusFixedRevalidateTest.php        # OK
php tests/ServiceRegistryFactoryMapTest.php         # OK 70 factories
  • Автоматические тесты (composer test / phpunit Unit + smoke scripts)
  • Ручное тестирование на живом MODX

Конфигурация тестирования:

  • MiniShop3: ветка feat/issue-592-order-lifecycle
  • MODX: stubs / без полной установки
  • PHP: 8.4.17

Чеклист

  • Код соответствует стилю проекта
  • Лексиконы добавлены на двух языках (ru/en)
  • Изменения не ломают существующую функциональность (default = final/fixed, ports no-op)
  • PHPStan — CI job
  • CHANGELOG.md — не трогали (релизный процесс)

Дополнительные заметки

Gate A (AC)

AC Code Test
Single gate documented / manager path via service yes manager mutation + Payment docblock
Orchestration ports contract (#589#591) yes (ms3_order_lifecycle_ports) paid/cancel/shipped unit
Fail after save → rollback (no silent status+error) yes after-event + port fail tests
Idempotent same-status for integrations yes options['idempotent'] unit
Additive transition policy yes ms3_order_status_transitions policy + lifecycle unit
Keep msOrderStatus admin model yes n/a

Deferred (не blockers этого PR)

@Ibochkarev
Ibochkarev requested a review from biz87 August 17, 2026 05:21
@Ibochkarev
Ibochkarev force-pushed the feat/issue-592-order-lifecycle branch 2 times, most recently from 7292c16 to 596abac Compare August 17, 2026 10:20
@Ibochkarev Ibochkarev added the enhancement New feature or request label Aug 18, 2026
AgelxNash pushed a commit to AgelxNash/MiniShop3 that referenced this pull request Sep 6, 2026
@AgelxNash AgelxNash mentioned this pull request Sep 6, 2026
16 tasks
@AgelxNash

Copy link
Copy Markdown

Этот PR включён в тестовую интеграционную сборку всех открытых PR MiniShop3: AgelxNash/MiniShop3, ветка integration/open-prs-20260906 (28/28 открытых).

Сборка нужна, чтобы проверить совместимость взаимозависимых серий PR до их мержа — при последовательном слиянии они конфликтуют друг с другом. Это не ревью и не конкурирующий PR: авторство сохранено (1 PR = 1 коммит с исходным автором), ветка пересобирается по мере обновления PR.

Как вошёл в сборку: Слился чисто.

@AgelxNash

Copy link
Copy Markdown

Привет! Просто пожелание: удачи с этим PR 🚀 Работа нужная — пусть рассмотрят и смержат как можно скорее. Успехов!

@Ibochkarev
Ibochkarev force-pushed the feat/issue-592-order-lifecycle branch from 596abac to 56d165d Compare September 9, 2026 13:31
@biz87

biz87 commented Sep 14, 2026

Copy link
Copy Markdown
Member

Проверил. Сам по себе PR корректен: с текущей beta сливается чисто, smoke, PHPUnit и PHPStan зелёные. Битый формат ms3_order_status_transitions запрещает переходы, а не пропускает их. Но вливать его отдельно от #603 не предлагаю — ниже почему.

Порты не подходят тем, для кого сделаны

Докблок OrderLifecyclePortsInterface обещает реализации в #589#591, но ни #603, ни #604, ни #605 интерфейс не используют. И #603 не может использовать его в текущем виде:

Два разных отката

#596 при ошибке порта или msOnChangeOrderStatus возвращает прежний статус повторным save() (rollbackStatus()). #603 оборачивает резерв и save() в транзакцию, а при ошибке msOnChangeOrderStatus откатывает только переход в «Новый» (undoUncommittedNewStatus() с освобождением резерва).

Если объединить как есть: остаток зафиксирован или освобождён и закоммичен, затем плагин в msOnChangeOrderStatus возвращает ошибку, rollbackStatus() ставит прежний статус — а остаток остаётся изменённым.

Что предлагаю

Свести #596 и #603 к одной модели смены статуса в OrderStatusService::change():

  1. Транзакция: доменные шаги до сохранения (резерв, фиксация, освобождение остатка — могут запретить переход) → status_id → commit.
  2. После commit — msOnChangeOrderStatus, журнал, уведомления.
  3. Откат только через транзакцию, без компенсирующего save(). Отдельно решить, может ли msOnChangeOrderStatus отменить переход. В beta не может: статус остаётся, возвращается ошибка. Если должен мочь — событие придётся вызывать до commit.
  4. Один механизм «заказ уже в этом статусе — успех». Сейчас их два: $options['idempotent'] здесь и ensure() в feat(core): add payment attempt lifecycle and webhook callback #604.

Порты тогда либо убрать, либо превратить в шаг из п. 1, через который работает склад из #603.

После вливания любого PR линии остальные три конфликтуют, так что вливать придётся по одному с ребейзом. Предлагаю порядок #605#604#596#603: у #604 и #605 зависимость от OrderStatusService ограничивается публичным change(), и к моменту переделки #596 и #603 у модели будут реальные потребители.

Мелочи

  • Для всех сайтов меняется поведение: ошибка плагина в msOnChangeOrderStatus теперь откатывает статус, а запись в журнал заказа идёт после события. Это стоит вынести в описание PR и CHANGELOG.
  • testInvalidJsonIsNotDenyAll проверяет MODE_INVALID, который сервис трактует как запрет всех переходов, — название вводит в заблуждение.

@Ibochkarev

Copy link
Copy Markdown
Member Author

Rebased onto current beta (shipment lifecycle merged). Conflicts resolved: kept shipment settings + lexicon and added ms3_order_status_transitions. Lifecycle unit tests green locally.

@Ibochkarev
Ibochkarev force-pushed the feat/issue-592-order-lifecycle branch from 56d165d to 9adf16e Compare September 15, 2026 01:27
Make OrderStatusService the non-draft status gate: optional transition
allow-list, NullOrderLifecyclePorts for #589#591, idempotent retries,
and status rollback when after-event or a port fails.
@Ibochkarev
Ibochkarev force-pushed the feat/issue-592-order-lifecycle branch from 9adf16e to f4c3555 Compare September 16, 2026 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Core: make order lifecycle the orchestration layer for status-driven side effects

3 participants