Skip to content

fix(web-api): normalize options JSON string in cart/change-option - #638

Merged
biz87 merged 1 commit into
betafrom
fix/issue-635-cart-change-option-options-string
Sep 7, 2026
Merged

fix(web-api): normalize options JSON string in cart/change-option#638
biz87 merged 1 commit into
betafrom
fix/issue-635-cart-change-option-options-string

Conversation

@Ibochkarev

Copy link
Copy Markdown
Member

Описание

POST /api/v1/cart/change-option принимает options как JSON-строку (например "{\"size\":\"L\"}"), а не только как объект. Нормализация через CartItemManager::normalizeOptions() на HTTP-границе, по тому же принципу, что и cart/add в #632/#633.

Невалидный PHP-тип (42, true) → 400 с ms3_err_cart_options. Пустая карта после decode → 400 с ms3_cart_change_options_error (как раньше).

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

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

Связанные Issues

Closes #635

Связано: #632, #633 (симметрия cart/add).

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

cd core/components/minishop3
composer ci:php   # exit 0 — php -l (627 files), smoke (89), PHPUnit 256 tests

Новые тесты в HeadlessStorefrontErrorsTest:

  • testChangeOptionJsonStringOptionsReturns200

  • testChangeOptionInvalidOptionsTypeReturns400

  • Ручное тестирование

  • Автоматические тесты (composer ci:php / composer test, npm run lint:ci, composer stan / GitHub Actions CI)

  • Тестирование на разных версиях PHP/MODX

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

  • MiniShop3: ветка fix/issue-635-cart-change-option-options-string
  • MODX: journey stub (PHPUnit integration)
  • PHP: 8.4.17

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

Не применимо (API).

Чеклист

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

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

CartItemManager::normalizeOptions() сделан static, чтобы контроллер мог вызывать его без инстанса MiniShop3 (в journey-тестах JourneyMs3 не наследует MiniShop3). CartMutationHandler::add по-прежнему вызывает $this->itemManager->normalizeOptions() — DI-подмена ms3_cart_item_manager сохраняется.

Невалидная JSON-строка ("{bad") по-прежнему даёт пустой map и ms3_cart_change_options_error — поведение как у пустого [], в scope issue не менялось.

@Ibochkarev
Ibochkarev requested a review from biz87 August 25, 2026 05:00
@Ibochkarev Ibochkarev added the bug Something isn't working label Aug 25, 2026
@AgelxNash AgelxNash mentioned this pull request Sep 6, 2026
16 tasks
AgelxNash pushed a commit to AgelxNash/MiniShop3 that referenced this pull request Sep 6, 2026
…change-option

Conflict resolution: keep PR 633 cart/add JSON-string tests alongside
PR 638 change-option tests (both sets retained).
@AgelxNash

Copy link
Copy Markdown

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

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

Как вошёл в сборку: Ветка PR не содержала тестов из #633 (cart/add JSON-string), поэтому в интеграционной ветке сохранены оба набора: cart/add от #633 и change-option от #638 (HeadlessStorefrontErrorsTest).

@AgelxNash

Copy link
Copy Markdown

Удачи с PR! Пусть дойдёт до релиза как можно скорее — спасибо за вклад в MiniShop3! 🚀

@biz87

biz87 commented Sep 7, 2026

Copy link
Copy Markdown
Member

PR проверен и к вливанию годен — возвращаю только на ребейз, содержательных претензий нет.

#633 влит в beta (74537739), после этого здесь появился конфликт.

Что проверено локально, на ветке с домердженной актуальной beta, и отдельно на объединении с #633:

  • продуктовый код конфликтов не даёт вовсе — CartController.php сливается автоматически, PR правят разные методы (add() против changeOption()), а CartItemManager.php обе стороны приводят к идентичному тексту;
  • на объединённом коде: smoke 90/90, PHPUnit 263 (9 skipped — mysql-группа), PHPStan по изменённым файлам 0 ошибок;
  • арифметика тестов сходится, при разрешении конфликта ни один тест не теряется и не дублируется;
  • normalizeOptions() как public static — DI-подмена ms3_cart_item_manager сохраняется: CartMutationHandler.php:58 по-прежнему зовёт метод через инстанс, поздняя статическая привязка отработает и для переопределённого сабкласса;
  • обратная совместимость цела: collectOptions() в assets/components/minishop3/js/web/ms3.js не тронут и всегда отдаёт объект, старый путь is_array($options) работает как раньше.

Что нужно сделать при ребейзе

1. Два конфликта в тестах — механический union. Оба PR вставили свои тест-методы в одну точку:

  • core/components/minishop3/tests/Integration/WebApi/HeadlessStorefrontErrorsTest.php
  • core/components/minishop3/tests/Unit/Services/Cart/CartItemManagerPureTest.php

Разрешается сохранением тел с обеих сторон, семантика не пересекается.

2. Дубль лексикона, который git не поймает. Оба PR добавляют ключ ms3_err_cart_options, но в разные позиции файлов — поэтому конфликта не возникает, строки просто складываются. После авто-мержа ключ определён по два раза в каждом из четырёх файлов (проверил, воспроизводится):

  • core/components/minishop3/lexicon/ru/cart.inc.php
  • core/components/minishop3/lexicon/en/cart.inc.php
  • core/components/minishop3/lexicon/ru/default.inc.php
  • core/components/minishop3/lexicon/en/default.inc.php

Значения одинаковые, поэтому функционально безвредно — последнее определение побеждает. Но это четыре мёртвых дубля, и убрать их надо руками: ребейз сам этого не сделает.

Мелочь на усмотрение

CartController::add() и changeOption() зовут CartItemManager::normalizeOptions() напрямую по имени класса, а не через DI-инстанс — хотя докблок класса обещает «Can be overridden via DI to customize cart behavior». Для этого конкретного вызова кастомный сабкласс через ms3_cart_item_manager не сработает. Не баг и не блокер, просто архитектурная непоследовательность — одинаковая, кстати, и в #633.

После ребейза с убранным дублем — вливаем.

Closes #635 — headless clients can send options as a JSON string on
change-option, matching the cart/add contract from #632/#633.
@Ibochkarev
Ibochkarev force-pushed the fix/issue-635-cart-change-option-options-string branch from 3c7fd34 to c5bd17b Compare September 7, 2026 10:35
@Ibochkarev

Copy link
Copy Markdown
Member Author

Rebased onto current beta (после #633).

  • Конфликты в HeadlessStorefrontErrorsTest и CartItemManagerPureTest — union обоих наборов тестов
  • Дубли ms3_err_cart_options в ru/en cart.inc.php и default.inc.php убраны (оставлено по одному ключу)
  • composer ci:php: php -l 631, smoke 91, PHPUnit 271 (9 skipped mysql)

Замечание про прямой вызов CartItemManager::normalizeOptions() vs DI — согласен, не блокер; при желании можно вынести отдельно.

@biz87
biz87 merged commit 0f6276f into beta Sep 7, 2026
3 checks passed
@biz87
biz87 deleted the fix/issue-635-cart-change-option-options-string branch September 7, 2026 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] POST /api/v1/cart/change-option не принимает options как JSON-строку

3 participants