Skip to content

feat(order): lifecycle gate with in-TX ports and idempotency - #596

Open
Ibochkarev wants to merge 3 commits into
betafrom
feat/issue-592-order-lifecycle
Open

Ibochkarev wants to merge 3 commits into
betafrom
feat/issue-592-order-lifecycle

Conversation

@Ibochkarev

@Ibochkarev Ibochkarev commented Aug 17, 2026

Copy link
Copy Markdown
Member

Описание

OrderStatusService — единый gate для non-draft смены статуса: опциональный allow-list переходов, DI-порты lifecycle (пока no-op, точка входа для #603), режим idempotent / ensure() для интеграций.

Контракт change() (согласован с #603)

  1. Validate + msOnBeforeChangeOrderStatus (может отменить до любых записей).
  2. DB-транзакция: in-TX lifecycle ports (могут запретить) → status_id → commit. Откат только rollback транзакции. Компенсирующего rollbackStatus() нет.
  3. После commit: msOnChangeOrderStatus → журнал → уведомления. Ошибка плагина возвращается наружу, статус не откатывается (согласованность со складом).
  4. Порты — pre-persist domain hooks, не post-save() side-effects.

Manager update не пишет status_id в общий save() до вызова сервиса.

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

  • Новая функциональность (non-breaking change)
  • Исправление бага (non-breaking change)
  • Breaking change — поведение после ошибки msOnChangeOrderStatus: статус остаётся (как смысл beta), без compensating save из ранней редакции feat(order): lifecycle gate with in-TX ports and idempotency #596

Связанные Issues

Closes #592

Refs #603 (склад сядет на in-TX ports), #604 / #605

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

cd core/components/minishop3
php -l src/Services/Order/OrderStatusService.php
php -l src/Services/Order/OrderLifecyclePortsInterface.php
./vendor/bin/phpunit tests/Unit/Services/Order/OrderStatusServiceLifecycleTest.php \
  tests/Unit/Services/Order/OrderStatusTransitionPolicyTest.php
# exit 0 — 13 tests
  • Автоматические тесты
  • Ручное тестирование на живом MODX

Конфигурация: ветка feat/issue-592-order-lifecycle (rebase/merge beta + контракт PR comment)

Чеклист

  • Код соответствует стилю проекта
  • Лексиконы ru/en (удалён неиспользуемый ms3_err_status_rollback)
  • Не ломает default final/fixed; ports no-op
  • PHPStan — CI
  • CHANGELOG — на релизе

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

Порядок вливания линии: #605#604#596#603.
См. #596 (comment)

@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
@biz87

biz87 commented Sep 17, 2026

Copy link
Copy Markdown
Member

@Ibochkarev после ревью 14.09 ветки #596 и #603 только перебазированы на beta — сверил текущие головы (f4c35553, 32b2abb5), обе сливаются с beta без конфликтов, но между собой конфликтуют в OrderStatusService.php, ServiceRegistry*.php, настройках и лексиконах.

Открытыми остаются:

Модель смены статуса (#596 и #603 вместе)#596 (comment). В #596 по-прежнему порты после save() и компенсирующий rollbackStatus(), в #603 — своя транзакция и undoUncommittedNewStatus(). При объединении ошибка плагина в msOnChangeOrderStatus откатит статус, а остаток останется изменённым. Нужно решение, как будет устроен OrderStatusService::change(), — от него зависит, как переделывать оба PR.

#603, гонка внутри одного заказа. ProductStockInventory::reserve() и release() читают журнал через findReservation() без блокировки и только потом меняют stock. В release() транзакция охватывает increment() и запись журнала, но проверка состояния остаётся снаружи — два одновременных release() для одного заказа оба увидят reserved и оба вернут остаток. Нужен условный переход состояния журнала (UPDATE … WHERE state = 'reserved' и изменение stock только если запрос затронул строку) и MySQL-тест на два соединения для одного заказа.

#603, незаполненный остаток. setting_ms3_inventory_enabled_desc (ru/en) всё ещё не предупреждает, что NULL в stock считается нулём — после включения товары без заполненного остатка станут недоступны, — и что остаток общий на товар для всех вариантов опций.

Прежде чем смотреть дальше, нужен ответ по модели смены статуса.

@Ibochkarev

Copy link
Copy Markdown
Member Author

@biz87 Спасибо. По модели OrderStatusService::change() — решение такое (сводим #596 и #603 к одному контракту).

Контракт change()

  1. Одна DB-транзакция (до commit): доменные шаги, которые могут запретить переход (склад: reserve / commit / release) → запись status_idcommit. Откат только через rollback транзакции. Компенсирующего rollbackStatus() / undoUncommittedNewStatus() после commit не будет.
  2. После commit: msOnChangeOrderStatus → журнал → уведомления.
  3. msOnChangeOrderStatus не отменяет уже закоммиченный переход. Как в текущей beta по смыслу «статус уже применён»: ошибка плагина возвращается наружу, но status_id и склад остаются согласованными. Откат статуса повторным save() из feat(order): lifecycle gate with in-TX ports and idempotency #596 убираем — он как раз и даёт рассинхрон со складом. Если когда-нибудь понадобится отмена плагином — это отдельное решение: событие до commit, не компенсирующие save().
  4. Один idempotent-контракт: повторный вызов «уже в этом статусе» → успех без событий/уведомлений (свести $options['idempotent'] из feat(order): lifecycle gate with in-TX ports and idempotency #596 и ensure() из feat(core): add payment attempt lifecycle and webhook callback #604 к одному публичному API).
  5. Порты: не post-save() side-effects. Либо убрать, либо сделать in-TX domain hooks (через них же ходит склад feat(core): add opt-in inventory reserve, commit and release #603), с возможностью запретить переход до commit. Порта «стал Новым» для резерва либо нет как отдельной семантики — резерв вызывается явно как шаг политики статуса / координатора склада внутри п.1.

Порядок вливания

Согласен с предложенным: #605#604#596 (переделка под контракт выше) → #603 (склад садится на in-TX шаги, без своего компенсирующего undo после события).

#603 после модели

После переделки change() отдельно закроем:

  • условный переход журнала (UPDATE … WHERE state = 'reserved') + MySQL-тест на два соединения;
  • предупреждение в setting_ms3_inventory_enabled_desc (ru/en) про NULL→0 и общий остаток на товар.

Могу следующим шагом переписать #596 под этот контракт (без compensating rollback, порты/склад как pre-commit steps) и обновить описание PR.

… rollback

Ports run before status_id persist inside an optional DB transaction.
msOnChangeOrderStatus failure no longer compensates with a second save,
so status and future inventory stay consistent (#596 / #603 contract).
@Ibochkarev

Copy link
Copy Markdown
Member Author

@biz87 Переписал #596 под зафиксированный контракт:

  • порты до persist status_id, внутри optional DB TX;
  • убран compensating rollbackStatus();
  • ошибка msOnChangeOrderStatus оставляет закоммиченный статус, без журнала/notify;
  • удалён неиспользуемый ms3_err_status_rollback.

Ветка смержена с актуальной beta. Unit: OrderStatusServiceLifecycleTest + policy — 13 tests, exit 0.

Дальше по плану: #603 садится на in-TX hooks + conditional journal / lexicon.

@Ibochkarev Ibochkarev changed the title feat(order): lifecycle gate with ports, idempotency, and rollback feat(order): lifecycle gate with in-TX ports and idempotency Sep 17, 2026
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