From 354d68802292eab502cdfbe115ede72d4604fba8 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Thu, 16 Jul 2026 10:29:05 +0600 Subject: [PATCH 1/3] fix(order): multiply draft weight by product count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OrderDraftManager::recalculate summed unit weight only, so draft msOrder.weight disagreed with cart status and undercharged weight-based delivery. Aggregate via OrderService with weight × count. --- .../src/Services/Order/OrderDraftManager.php | 13 +--- .../src/Services/Order/OrderService.php | 23 ++++++ .../minishop3/tests/OrderDraftWeightTest.php | 75 +++++++++++++++++++ 3 files changed, 101 insertions(+), 10 deletions(-) create mode 100644 core/components/minishop3/tests/OrderDraftWeightTest.php diff --git a/core/components/minishop3/src/Services/Order/OrderDraftManager.php b/core/components/minishop3/src/Services/Order/OrderDraftManager.php index f7450608..cfe08c40 100644 --- a/core/components/minishop3/src/Services/Order/OrderDraftManager.php +++ b/core/components/minishop3/src/Services/Order/OrderDraftManager.php @@ -150,16 +150,9 @@ public function getOrCreateDraft(string $token, string $ctx = 'web', ?int $custo public function recalculate(msOrder $draft): void { // TODO: event before recalculating order - $products = $draft->getMany('Products'); - $cart_cost = 0; - $weight = 0; - - if (!empty($products)) { - foreach ($products as $product) { - $weight += $product->get('weight'); - $cart_cost += $product->get('cost'); - } - } + $totals = OrderService::aggregateProductsTotals($draft->getMany('Products') ?? []); + $cart_cost = $totals['cart_cost']; + $weight = $totals['weight']; /** @var OrderService $orderService */ $orderService = $this->modx->services->get('ms3_order_service'); diff --git a/core/components/minishop3/src/Services/Order/OrderService.php b/core/components/minishop3/src/Services/Order/OrderService.php index a4417079..958df29b 100644 --- a/core/components/minishop3/src/Services/Order/OrderService.php +++ b/core/components/minishop3/src/Services/Order/OrderService.php @@ -63,6 +63,29 @@ public function clampComputedTotal( return 0.0; } + /** + * Sum cart_cost and total weight from order product lines. + * Weight is unit weight × count (cart status / draft / finalize / manager). + * + * @param iterable $products Objects with get('weight'|'cost'|'count') + * @return array{cart_cost: float, weight: float} + */ + public static function aggregateProductsTotals(iterable $products): array + { + $cartCost = 0.0; + $weight = 0.0; + + foreach ($products as $product) { + $cartCost += (float) $product->get('cost'); + $weight += (float) $product->get('weight') * (int) $product->get('count'); + } + + return [ + 'cart_cost' => $cartCost, + 'weight' => $weight, + ]; + } + /** * Recalculate products in order * diff --git a/core/components/minishop3/tests/OrderDraftWeightTest.php b/core/components/minishop3/tests/OrderDraftWeightTest.php new file mode 100644 index 00000000..ac702c28 --- /dev/null +++ b/core/components/minishop3/tests/OrderDraftWeightTest.php @@ -0,0 +1,75 @@ + $this->weight, + 'count' => $this->count, + 'cost' => $this->cost, + default => 0, + }; + } + }; +}; + +$empty = OrderService::aggregateProductsTotals([]); +$assertSame(0.0, $empty['weight'], 'empty weight'); +$assertSame(0.0, $empty['cart_cost'], 'empty cart_cost'); + +$single = OrderService::aggregateProductsTotals([ + $product(1.0, 3, 30.0), +]); +$assertSame(3.0, $single['weight'], 'weight=1 count=3 → total weight 3'); +$assertSame(30.0, $single['cart_cost'], 'line cost is not multiplied again'); + +$multi = OrderService::aggregateProductsTotals([ + $product(1.5, 2, 10.0), + $product(0.5, 4, 20.0), +]); +$assertSame(5.0, $multi['weight'], '1.5*2 + 0.5*4'); +$assertSame(30.0, $multi['cart_cost'], '10 + 20'); + +// Guard: unit weight alone must not be treated as total (the #375 bug) +if ($single['weight'] === 1.0) { + $fail('weight must include count; got unit weight only'); +} + +fwrite(STDOUT, "OK OrderDraftWeightTest\n"); +exit(0); From 1f143085d004b4ba50d45fedbe0916a0ecf5de6f Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Thu, 16 Jul 2026 10:30:51 +0600 Subject: [PATCH 2/3] refactor(order): reuse OrderService product totals aggregation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire draft, finalize, manager recalculate, and OrdersController through OrderService::aggregateProductsTotals so weight×count lives in one place. Also round totals to 6 decimals like the former manager helper. --- .../Controllers/Api/Manager/OrdersController.php | 12 +++--------- .../Services/Order/ManagerOrderCostRecalculator.php | 13 +------------ .../src/Services/Order/OrderFinalizeService.php | 12 +++--------- .../minishop3/src/Services/Order/OrderService.php | 4 ++-- 4 files changed, 9 insertions(+), 32 deletions(-) diff --git a/core/components/minishop3/src/Controllers/Api/Manager/OrdersController.php b/core/components/minishop3/src/Controllers/Api/Manager/OrdersController.php index ce13d719..d5057493 100644 --- a/core/components/minishop3/src/Controllers/Api/Manager/OrdersController.php +++ b/core/components/minishop3/src/Controllers/Api/Manager/OrdersController.php @@ -1396,21 +1396,15 @@ public function deleteProduct(array $params = []): array */ protected function recalculateOrderTotals(msOrder $order): void { - $cartCost = 0; - $weight = 0; - $products = $this->modx->getIterator(\MiniShop3\Model\msOrderProduct::class, [ 'order_id' => $order->get('id'), ]); - - foreach ($products as $product) { - $cartCost += (float)$product->get('cost'); - $weight += (float)$product->get('weight') * (int)$product->get('count'); - } + $totals = OrderService::aggregateProductsTotals($products); + $cartCost = $totals['cart_cost']; + $weight = $totals['weight']; $order->set('cart_cost', $cartCost); $order->set('weight', $weight); - // Recalculate total cost (cart + delivery; payment deltas are reflected in cost when persisted elsewhere) /** @var OrderService $orderService */ $orderService = $this->modx->services->get('ms3_order_service'); diff --git a/core/components/minishop3/src/Services/Order/ManagerOrderCostRecalculator.php b/core/components/minishop3/src/Services/Order/ManagerOrderCostRecalculator.php index 73f729da..4a5f2b9c 100644 --- a/core/components/minishop3/src/Services/Order/ManagerOrderCostRecalculator.php +++ b/core/components/minishop3/src/Services/Order/ManagerOrderCostRecalculator.php @@ -48,22 +48,11 @@ public function __construct(modX $modx, MiniShop3 $ms3) */ public function calculateProductTotals(msOrder $order): array { - $cartCost = 0.0; - $weight = 0.0; - $products = $this->modx->getIterator(msOrderProduct::class, [ 'order_id' => $order->get('id'), ]); - foreach ($products as $product) { - $cartCost += (float)$product->get('cost'); - $weight += (float)$product->get('weight') * (int)$product->get('count'); - } - - return [ - 'cart_cost' => round($cartCost, 6), - 'weight' => round($weight, 6), - ]; + return OrderService::aggregateProductsTotals($products); } /** diff --git a/core/components/minishop3/src/Services/Order/OrderFinalizeService.php b/core/components/minishop3/src/Services/Order/OrderFinalizeService.php index 893d1433..8de2cca3 100644 --- a/core/components/minishop3/src/Services/Order/OrderFinalizeService.php +++ b/core/components/minishop3/src/Services/Order/OrderFinalizeService.php @@ -380,18 +380,12 @@ protected function validateDeliveryRequiredFields(msOrder $order): array */ protected function calculateCosts(msOrder $order): array { - // Calculate cart cost from order products - $cartCost = 0; - $weight = 0; - $products = $this->modx->getIterator(msOrderProduct::class, [ 'order_id' => $order->get('id'), ]); - - foreach ($products as $product) { - $cartCost += (float) $product->get('cost'); - $weight += (float) $product->get('weight') * (int) $product->get('count'); - } + $totals = OrderService::aggregateProductsTotals($products); + $cartCost = $totals['cart_cost']; + $weight = $totals['weight']; // Calculate delivery cost $deliveryCost = 0; diff --git a/core/components/minishop3/src/Services/Order/OrderService.php b/core/components/minishop3/src/Services/Order/OrderService.php index 958df29b..cc722446 100644 --- a/core/components/minishop3/src/Services/Order/OrderService.php +++ b/core/components/minishop3/src/Services/Order/OrderService.php @@ -81,8 +81,8 @@ public static function aggregateProductsTotals(iterable $products): array } return [ - 'cart_cost' => $cartCost, - 'weight' => $weight, + 'cart_cost' => round($cartCost, 6), + 'weight' => round($weight, 6), ]; } From 784d191e6fa692201f61827f304125e56ec121bc Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Thu, 16 Jul 2026 10:32:03 +0600 Subject: [PATCH 3/3] fix(order): drop fake recalculate event TODOs; wire GetOrderCost MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Draft recalculate persists via msOnBefore/SaveOrder on save — do not invent unregistered recalculate events. Invoke the already-registered msOnBeforeGetOrderCost / msOnGetOrderCost from OrderCostCalculator::getTotalCost. --- .../Services/Order/OrderCostCalculator.php | 32 +++++++++++++++++++ .../src/Services/Order/OrderDraftManager.php | 8 +++-- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/core/components/minishop3/src/Services/Order/OrderCostCalculator.php b/core/components/minishop3/src/Services/Order/OrderCostCalculator.php index b497e978..ce777a6b 100644 --- a/core/components/minishop3/src/Services/Order/OrderCostCalculator.php +++ b/core/components/minishop3/src/Services/Order/OrderCostCalculator.php @@ -227,6 +227,17 @@ public function getTotalCost( string $ctx = 'web', bool $onlyCost = false ): array { + $before = $this->ms3->utils->invokeEvent('msOnBeforeGetOrderCost', [ + 'calculator' => $this, + 'cart' => $this->ms3->cart, + 'draft' => $draft, + 'with_cart' => true, + 'only_cost' => $onlyCost, + ]); + if (!$before['success']) { + return $this->error($before['message']); + } + $cartCostResponse = $this->getCartCost($draft, $token, $ctx); $cartCost = $cartCostResponse['success'] ? $cartCostResponse['data']['cost'] : 0; @@ -245,6 +256,27 @@ public function getTotalCost( (float) $paymentCost ); + $after = $this->ms3->utils->invokeEvent('msOnGetOrderCost', [ + 'calculator' => $this, + 'cart' => $this->ms3->cart, + 'draft' => $draft, + 'with_cart' => true, + 'only_cost' => $onlyCost, + 'cost' => $cost, + 'cart_cost' => $cartCost, + 'delivery_cost' => $deliveryCost, + 'payment_cost' => $paymentCost, + ]); + if (!$after['success']) { + return $this->error($after['message']); + } + + $cost = (float) ($after['data']['cost'] ?? $cost); + $cartCost = (float) ($after['data']['cart_cost'] ?? $cartCost); + $deliveryCost = (float) ($after['data']['delivery_cost'] ?? $deliveryCost); + $paymentCost = (float) ($after['data']['payment_cost'] ?? $paymentCost); + $cost = $orderService->clampComputedTotal($draft, $cartCost, $deliveryCost, $paymentCost); + if ($onlyCost) { return $this->success('ms3_order_getcost_success', ['cost' => $cost]); } diff --git a/core/components/minishop3/src/Services/Order/OrderDraftManager.php b/core/components/minishop3/src/Services/Order/OrderDraftManager.php index cfe08c40..c358b38e 100644 --- a/core/components/minishop3/src/Services/Order/OrderDraftManager.php +++ b/core/components/minishop3/src/Services/Order/OrderDraftManager.php @@ -145,11 +145,14 @@ public function getOrCreateDraft(string $token, string $ctx = 'web', ?int $custo } /** - * Recalculate order costs (cart, delivery, total) + * Recalculate and persist draft cart_cost / weight / cost from order products. + * + * Plugin hooks: $draft->save() fires msOnBeforeSaveOrder / msOnSaveOrder (see msOrder::save). + * Display-time cost adjustment uses msOnBeforeGetOrderCost / msOnGetOrderCost via OrderCostCalculator, + * not this persist path — do not invent a separate "recalculate" event here. */ public function recalculate(msOrder $draft): void { - // TODO: event before recalculating order $totals = OrderService::aggregateProductsTotals($draft->getMany('Products') ?? []); $cart_cost = $totals['cart_cost']; $weight = $totals['weight']; @@ -159,7 +162,6 @@ public function recalculate(msOrder $draft): void $delivery_cost = (float) $draft->get('delivery_cost'); $cost = $orderService->clampComputedTotal($draft, (float) $cart_cost, $delivery_cost, 0.0); - // TODO: event on recalculating order $draft->set('updatedon', time()); $draft->set('cart_cost', $cart_cost); $draft->set('cost', $cost);