Цена доставки за вес не учитывалась при оформлении заказа - #810
GulomovCreative wants to merge 1 commit into
Conversation
Delivery::getCost() read the weight from the cart controller guarded by empty($this->ms3->cart). MiniShop3 exposes cart through __get() without __isset(), so the check was always true, the weight stayed 0 and weight_price never affected the delivery cost on checkout, order/cost and submit. The manager recalculation used the real order weight, so the two paths disagreed. The weight is now summed from the order products (weight x count) via OrderService::aggregateProductsTotals(), the same source as ManagerOrderCostRecalculator.
Ibochkarev
left a comment
There was a problem hiding this comment.
Вердикт: APPROVE
Claim из описания сходится с кодом: MiniShop3 отдаёт cart через __get() без __isset(), поэтому empty($this->ms3->cart) всегда был true, вес оставался нулём и weight_price на витрине молча не применялся. Менеджерский пересчёт шёл другим путём и получал реальный вес, суммы расходились.
Что сделано хорошо
- Правка удаляет слой, а не перекладывает его. Старый
getCost()поднимал cart-контроллер и сессию ради одного числа. Теперь одна строка через каноническийOrderService::aggregateProductsTotals(), который уже зовутOrderDraftManagerиManagerOrderCostRecalculator. Метод сжался с ~28 строк с ветвлениями до 6 строк без условий. - Слой правильный.
Controllers/Deliveryздесь доменный провайдер, не HTTP. Зависимость отmsOrderи статических хелперов вServices/Orderповторяет рисунокPayment::getCost()→OrderCostEngine. Старый вариант лез в$this->ms3->cartчерезservices->load(), это и была утечка границы. - BC не сломан. Сигнатура
getCost()не менялась, подклассы с собственным расчётом не затронуты. Кто звалparent::getCost(), начнёт получать надбавку за вес. Это и есть фикс. - Тест гоняет настоящий путь
DefaultDelivery::getCost→aggregateProductsTotals→OrderCostEngine. Наbetaпадает, на ветке проходит. Пять кейсов: вес × количество, пустой заказ, порог бесплатной доставки, процент + вес. - Побочный эффект старого кода ушёл: первый
empty()всегда вызывалservices->load(), второй никогда не доходил доcart->status().
Замечания (не блокеры)
getManyпротивgetIterator(Delivery.php:86). Формула общая с менеджерским пересчётом, источник выборки разный: тамgetIterator, здесьgetMany. Репозиторий уже знает про кэш связей xPDO (CartItemManager::loadItemsобходитgetMany). На штатных путях checkout и submit это не ломается: фасадыCartиOrderдержат разные экземплярыmsOrder, первыйgetManyна свежем объекте идёт в БД. Для программного создания заказаgetManyдаже лучше, потому что видитaddManyна несохранённом объекте. Follow-up: общий accessor «товары заказа» рядом сcalculateProductTotals, чтобыaggregateProductsTotalsникогда не получал кэш связей. Не этот PR.?? []мёртвый (Delivery.php:86).getMany()в xPDO возвращает массив, неnull. Та же идиома уже стоит вOrderDraftManager::recalculate:172. Сносить стоит вместе с accessor'ом из пункта 1, не точечно в багфиксе.- Тест закрывает формулу, не выборку (
DeliveryWeightCostTest.php:64-74).getManyподменён анонимным классом, реальный путь через xPDO тест не видит. Регрессию «источник снова стал пустым» он не поймает. Для двухфайлового фикса хватает. Интеграцию черезSqliteDraftCartHarnessTraitможно добавить отдельно.
Поведенческое предупреждение для релиз-нот
У магазинов с настроенным weight_price доставка на витрине вырастет (пример из PR: 390 → 414 при 0,4 кг и 60 ₽/кг). Это исправление молчаливого бага, но цифра в заказе изменится. Стоит упомянуть при подготовке релиза.
Проверки
php -l ок, smoke 122 ок, PHPUnit 738/2625 (3 падения HeadlessStorefrontCors* одинаковы на beta и ветке, к доставке не относятся). High-находок нет. Мержить можно.
Описание
Цена доставки за единицу веса (
weight_price) не учитывалась при оформлении заказа, в/api/v1/order/cost,/api/v1/order/cost/deliveryиOrder::submit(): в заказ записывалась стоимость без веса.Delivery::getCost()брал вес из контроллера корзины под проверкойempty($this->ms3->cart). УMiniShop3свойствоcartотдаётся через__get(), а__isset()нет, поэтомуempty()всегда возвращалtrue, блок сcart->status()не выполнялся и вес оставался0. При этом пересчёт в менеджере (ManagerOrderCostRecalculator) передаёт реальный вес заказа, и суммы на витрине и после пересчёта в админке расходились.Теперь вес считается по товарам самого заказа — вес × количество через
OrderService::aggregateProductsTotals(), так же, как вManagerOrderCostRecalculator. Расчёт больше не зависит от того, инициализирован ли контроллер корзины токеном покупателя (Web API, программное создание заказа).msOrderProduct.weight— вес одной штуки: так его записываетCartItemManagerи так же суммируютCartItemManager::getStatus()иOrderService::getOrderStatistics().Пример: доставка
price = 390,weight_price = 60, в корзине товар весом 0,4 кг — былоdelivery_cost = 390, стало414.Тип изменений
Связанные Issues
—
Как это было протестировано?
Вручную на MODX 3.2.4 + MiniShop3 1.14.1-beta2: доставка
390 + 60 ₽/кг, в корзине 0,4 кг —order/costвернулdelivery_cost = 414(без исправления —390). Порогfree_delivery_amountпо-прежнему обнуляет доставку, процентная цена складывается с весовой.composer ci:php/composer test,npm run lint:ci,composer stan/ GitHub Actions CI)