Sylius / Sylius

Open Source eCommerce Framework on Symfony
https://sylius.com
Other
7.87k stars 2.09k forks source link

[Bug] Shipment's getShippingUnitTotal always return 0 #11515

Closed igormukhingmailcom closed 1 year ago

igormukhingmailcom commented 4 years ago

Sylius version affected: master / any

Description
During implementing shipping rate calculator (which have different rates based on order total), realized getShippingUnitTotal at Shipment always return 0.

https://github.com/Sylius/Sylius/blame/master/src/Sylius/Component/Shipping/Model/Shipment.php#L219

But I expect to have shipment items (order items) total there.

This bug's age is about 7 years, as getShippingUnitTotal never used by Sylius or covered by tests, when similar getShippingUnitCount is valid as used by PerUnitRateCalculator. I mean its another +1 for https://github.com/Sylius/Sylius/issues/10813 :)

Possible Solution
Calculate total from units.

I will provide PR if you guys OK with solution.

igormukhingmailcom commented 4 years ago

This implementation:

    public function getShippingUnitTotal(): int
    {
        return array_sum(array_map(function(ShipmentUnitInterface $shipmentUnit){
            Assert::isInstanceOf($shipmentUnit, OrderItemUnitInterface::class);
            return $shipmentUnit->getTotal();
        }, $this->units->toArray()));
    }

...is not valid I guess, as it adding dependency with order component to shipping component.

So ways I see:

  1. Move getShippingUnitTotal from Sylius\Component\Shipping\Model\ShipmentInterface to Sylius\Component\Core\Model\ShipmentInterface - implementation like in example above become valid then.
  2. Or add getShippingItemsTotal to ShippableInterface maybe
lchrusciel commented 4 years ago

ShippableInterface is no go for me. Let's just leave the current 0 on the Shipping\Model and reimplement new behavior in the Core. We could also add some error trigger for people, who would like to take the Shipping component alone.

After a while, I would be in favor of a free above some threshold shipping calculator.

jakubtobiasz commented 1 year ago

Hi @igormukhingmailcom 👋🏼

This bug has been fixed in #15025 and will be published along the nearest bugfix version.