Skip to content

fix(api): не перезаписывать properties заказа данными адреса#192

Closed
Ibochkarev wants to merge 1 commit intomodx-pro:betafrom
Ibochkarev:fix/191-order-api-properties-merge
Closed

fix(api): не перезаписывать properties заказа данными адреса#192
Ibochkarev wants to merge 1 commit intomodx-pro:betafrom
Ibochkarev:fix/191-order-api-properties-merge

Conversation

@Ibochkarev
Copy link
Copy Markdown
Member

Описание

В OrdersController данные заказа собирались через $order->toArray(), затем поверх мержились $address->toArray(). У msOrderAddress есть поле properties; при значении null оно перезаписывало msOrder.properties в ответе Manager API (GET /api/mgr/orders/{id}, draft/finalize).

Добавлен метод mergeAddressIntoOrderData(), который копирует поля адреса в payload заказа, исключая конфликтующие ключи: id, order_id, createdon, updatedon, properties.

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

  • Исправление бага (non-breaking change)
  • Новая функциональность (non-breaking change)
  • Breaking change (изменение, ломающее обратную совместимость)
  • Рефакторинг (без изменения функциональности)
  • Документация
  • Другое (опишите):

Связанные Issues

Closes #191

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

  • Ручное тестирование (сценарий из issue: заказ с заполненным ms3_orders.properties, у адреса properties = null — в ответе API object.properties совпадает с заказом)
  • Автоматические тесты (PHPStan, ESLint)
  • Тестирование на разных версиях PHP/MODX

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

  • MiniShop3: beta (1.8.0-beta1)
  • MODX: 3.x
  • PHP: php -l на OrdersController.php — без ошибок

Скриншоты (если применимо)

До После

Чеклист

  • Код соответствует стилю проекта
  • Добавлены/обновлены комментарии в сложных местах
  • Изменения не ломают существующую функциональность
  • Лексиконы добавлены на двух языках (ru/en) — не требуются
  • PHPStan проходит без новых ошибок
  • ESLint проходит без ошибок (для JS/Vue изменений)
  • Обновлён CHANGELOG.md (для значимых изменений)

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

Запись в CHANGELOG.md в секции 1.8.0-beta1 → Исправлено.

…o#191)

OrdersController мержил toArray() адреса в данные заказа, из-за чего
msOrderAddress.properties (null) затирал msOrder.properties в ответе
GET /api/mgr/orders/{id} и связанных эндпоинтах.
@Ibochkarev Ibochkarev requested a review from biz87 April 12, 2026 06:48
@Ibochkarev Ibochkarev changed the title fix(api): не перезаписывать properties заказа данными адреса (#191) fix(api): не перезаписывать properties заказа данными адреса Apr 12, 2026
@biz87
Copy link
Copy Markdown
Member

biz87 commented Apr 12, 2026

Закрываю в пользу #195, где реализован минимальный фикс той же проблемы.

Найденные проблемы в этом PR

1. ModelFieldDefaults — класс не существует в проекте

PR импортирует MiniShop3\Services\ModelFieldDefaults и вызывает ModelFieldDefaults::getRowsForModel(), но этого класса нет в кодовой базе. PR не соберётся.

2. Debug-блок на 30+ строк

Блок с ms3_api_debug — отладочный код, не должен попадать в production. Системной настройки ms3_api_debug не существует.

3. Scope creep

  • Создание msOrderAddress на лету в update() — новая функциональность, не относящаяся к issue [Bug] /api/mgr/orders/{id} overwrites msOrder.properties with msOrderAddress.properties #191
  • Fallback для отсутствующего адреса в get() с null-заполнением полей — тоже новая логика, завязанная на несуществующий ModelFieldDefaults
  • Загрузка лексикона в formatOrder() — побочный фикс, не связанный с проблемой

Что взято в #195

Ядро фикса корректное — выделен mergeAddressIntoOrderData() с исключением конфликтующих полей (properties, id, order_id, createdon, updatedon). В #195 реализован только этот минимальный фикс: замена array_merge на новый метод в get(), create(), finalize() + getOne('Address') вместо getObject. PHPStan без новых ошибок.

@biz87 biz87 closed this Apr 12, 2026
@Ibochkarev Ibochkarev deleted the fix/191-order-api-properties-merge branch April 12, 2026 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] /api/mgr/orders/{id} overwrites msOrder.properties with msOrderAddress.properties

2 participants