Conversation
…tus to chunk - After msOnGetStatusCart, data.status.total_cost may differ from the sum of line items; use it for total.cost and cost_formatted when numeric. - Pass status into Fenom chunk data (and return=data) including empty-cart early exits so templates match cart->get() API.
biz87
left a comment
There was a problem hiding this comment.
Ревью
PR решает реальную проблему — рассинхронизация total.cost в чанке с status.total_cost после плагинов. Изменение точечное, обратную совместимость не ломает, описание подробное. Но есть несколько вопросов.
1. Early returns не синхронизируют total.cost с status.total_cost
В early returns (строки 53-64) status теперь передаётся, но логика перезаписи total.cost из status.total_cost есть только в основном пути (после строки ~198).
Если плагин на msOnGetStatusCart выставит total_cost при пустой корзине (например, фиксированная стоимость доставки) — total.cost останется 0, а status.total_cost будет другим. Та же рассинхронизация, которую PR исправляет.
Если это осознанное решение — стоит добавить комментарий. Если нет — нужно применить ту же логику перед early returns.
2. Только total_cost, но не другие агрегаты
Синхронизируется только cost. Если плагины могут корректировать total_count, total_weight, total_discount в статусе — аналогичная проблема остаётся. Стоит либо пояснить, почему только cost, либо покрыть и остальные поля.
3. cost vs discount — противоречие
Если total_cost из статуса уже включает скидку/доставку, то total.discount (посчитанный по строкам) становится несогласованным: cost != sum(products) - discount. Разработчики чанков могут получить противоречивые данные.
4. Мутация $outputData после создания
Массив создаётся, а потом total.cost перезаписывается отдельно. Читаемее было бы сделать перезапись до формирования $outputData:
if (isset($status['total_cost']) && is_numeric($status['total_cost'])) {
$total['cost'] = (float) $status['total_cost'];
}
$outputData = [
'total' => $total,
'products' => $products,
'status' => $status,
];
$outputData['total']['cost_formatted'] = $ms3->format->price($outputData['total']['cost'], true);Линейный data flow без промежуточной мутации.
В целом направление правильное, но хотелось бы услышать мнение по пунктам 1-3 перед мержем.
…near outputData - Apply status totals (cost/count/weight/discount/positions) before early returns so plugins can set e.g. total_cost on empty cart (review #197) - Document partial-plugin override vs line-level discount - Format cost/weight for chunk on all paths; build outputData without post-mutation - CHANGELOG: note #197 follow-up
|
@biz87 Спасибо за ревью. Учёл все пункты во втором коммите: на ранних выходах перед CHANGELOG обновлён краткой строкой про #197. Если что-то ещё покажется спорным — напиши, поправлю. |
biz87
left a comment
There was a problem hiding this comment.
Все замечания из предыдущего ревью учтены:
- ✅ Early returns —
$applyStatusToTotalи$formatTotalForDisplayвызываются перед обоими ранними выходами - ✅ Все агрегаты — покрыты
total_cost,total_count,total_weight,total_discount,total_positionsчерез маппинг с правильными типами (int/float) - ✅
costvsdiscount— добавлен docblock, явно объясняющий, что status-поля каноничны для header totals, а row-leveldiscount_*остаётся per-position - ✅ Нет мутации
$outputData—$totalмодифицируется до сборки$outputData, data flow линейный
Код чистый: замыкания static, маппинг расширяем, is_numeric() проверка на месте, CHANGELOG обновлён.
LGTM 👍
Описание
Сниппет
ms3_cartпосле событияmsOnGetStatusCartможет получать в$data['status']агрегаты (total_cost,total_countи т.д.), уже скорректированные плагинами. При этом сумма по строкам корзины ($total['cost']из позиций) не всегда совпадает с этим итогом.Изменения:
return=data: если вstatusесть числовойtotal_cost, вtotal.costподставляется именно он, аcost_formattedпересчитывается от этого значения. Так подпись «к оплате» в Fenom-чанке совпадает с тем, что отдаётCart::get()после плагинов.$outputDataдобавлена передачаstatus, чтобы в чанкеms3_cart(и приreturn=data) были доступны те же поля статуса, что и при полной корзине — без дублирования запросов к API корзины.Зачем это нужно
msOnGetStatusCartчасто меняют итоговую сумму (скидки, округления, доставка в превью и т.п.). Раньше чанк мог показывать сумму по строкам, аCart::get()— уже «официальный»total_costиз статуса; поведение расходилось.{$status.total_cost}, флаги доставки или текст из статуса — без полного набораstatusв Fenom это было невозможно на пустой корзине.Как использовать
Fenom (чанк корзины,
&return=tpl``):{$total.cost}/{$total.cost_formatted}— после правки это согласовано сstatus.total_cost, если он числовой.{$status.total_cost},{$status.total_count}, и т.д. (структура та же, что в$data['status']у сниппета).JSON /
&return=data``:statusрядом сtotalиproducts.total.costкак основной итог для отображения, если полагаетесь на плагины статуса.Тип изменений
Связанные Issues
NA
Как это было протестировано?
Скриншоты (если применимо)
Чеклист
Дополнительные заметки
statusв плейсхолдеры; старые шаблоны, которые его не используют, не ломаются.