21

Preface: This is meant to serve as both a written observation of Magento's architecture for the community (and myself), as well as an actual question. We're working with a heavily modified cart and checkout experience, but the root of this problem is within Magento's core logic.

Background

We've created a free shipping coupon using the standard shopping cart price rules functionality. There are no conditions on the coupon, and the only action is that Free Shipping is set to For matching items only. Since there are no conditions, this will set free_shipping to 1 for all of the sales quote items.

As is customary, we've also enabled the Free Shipping shipping method. The Freeshipping carrier model will provide rates whenever the request has free shipping or the subtotal matches or exceeds the threshold (but we're not using the threshold option). See Mage_Shipping_Model_Carrier_Freeshipping::collectRates:

$this->_updateFreeMethodQuote($request);

if (($request->getFreeShipping()) // <-- This is the condition we're relying on
    || ($request->getBaseSubtotalInclTax() >=
        $this->getConfigData('free_shipping_subtotal'))
) {
    /* Snip: Add $0.00 method to the result */
}

And Mage_Shipping_Model_Carrier_Freeshipping::_updateFreeMethodQuote looks like this:

protected function _updateFreeMethodQuote($request)
{
    $freeShipping = false;
    $items = $request->getAllItems();
    $c = count($items);
    for ($i = 0; $i < $c; $i++) {
        if ($items[$i]->getProduct() instanceof Mage_Catalog_Model_Product) {
            if ($items[$i]->getFreeShipping()) {
                $freeShipping = true;
            } else {
                return;
            }
        }
    }
    if ($freeShipping) {
        $request->setFreeShipping(true);
    }
}

So, as long as all of the items have free_shipping set to a truthy value (which they will, due to the coupon), we should get free shipping. And we do!

The problem

However, there's a major side effect: any shipping methods that rely on an item's row_weight (as is the case with our customized version of the FedEx carrier) will fail to calculate proper shipping rates because each item's row_weight is set to 0 when free shipping is active.

Interestingly enough, none of Magento's default shipping carriers actually rely on row_weight, but we'll get to that after we figure out why/when row_weight is set to 0.

Figuring out why row_weight is set to 0

This part was actually pretty easy to dig up. A big chunk of the shipping calculations happen in Mage_Sales_Model_Quote_Address_Total_Shipping::collect, including setting row_weight to 0:

public function collect(Mage_Sales_Model_Quote_Address $address)
{
    parent::collect($address);

    foreach ($items as $item) {
        /* Snip: Handling virtual items and parent items */

        if ($item->getHasChildren() && $item->isShipSeparately()) {
            /* Snip: Handling items with children */
        }
        else {
            if (!$item->getProduct()->isVirtual()) {
                $addressQty += $item->getQty();
            }
            $itemWeight = $item->getWeight();
            $rowWeight  = $itemWeight*$item->getQty();
            $addressWeight+= $rowWeight;
            if ($freeAddress || $item->getFreeShipping()===true) {
                $rowWeight = 0;
            } elseif (is_numeric($item->getFreeShipping())) {
                $freeQty = $item->getFreeShipping();
                if ($item->getQty()>$freeQty) {
                    $rowWeight = $itemWeight*($item->getQty()-$freeQty);
                }
                else {
                    $rowWeight = 0;
                }
            }
            $freeMethodWeight+= $rowWeight;
            $item->setRowWeight($rowWeight);
        }
    }

Why this doesn't affect Magento's default carriers

If you do a regex search for /row_?weight/i (e.g. getRowWeight, setRowWeight, setData('row_weight'), etc.) in Mage_Shipping (simple carriers) and Mage_Usa (FedEx, UPS, and some other carriers), nothing pops up. Why? Because the default carriers use the total address weight, not the individual item weights.

For example, let's look at Mage_Usa_Model_Shipping_Carrier_Fedex::setRequest:

public function setRequest(Mage_Shipping_Model_Rate_Request $request)
{
    $this->_request = $request;

    $r = new Varien_Object();

    /* Snip */

    $weight = $this->getTotalNumOfBoxes($request->getPackageWeight());
    $r->setWeight($weight);
    if ($request->getFreeMethodWeight()!= $request->getPackageWeight()) {
        $r->setFreeMethodWeight($request->getFreeMethodWeight());
    }

And where does the request get the package weight from? The answer is in Mage_Sales_Model_Quote_Address::requestShippingRates:

public function requestShippingRates(Mage_Sales_Model_Quote_Item_Abstract $item = null)
{
    /** @var $request Mage_Shipping_Model_Rate_Request */
    $request = Mage::getModel('shipping/rate_request');
    /* Snip */
    $request->setPackageWeight($item ? $item->getRowWeight() : $this->getWeight());

We can ignore the use of $item->getRowWeight() here because requestShippingRates is called without providing a specific item as a parameter in Mage_Sales_Model_Quote_Address_Total_Shipping::collect:

public function collect(Mage_Sales_Model_Quote_Address $address)
{
    parent::collect($address);

    foreach ($items as $item) {
        /* Snip: Handling virtual items and parent items */

        if ($item->getHasChildren() && $item->isShipSeparately()) {
            /* Snip: Handling items with children */
        }
        else {
            if (!$item->getProduct()->isVirtual()) {
                $addressQty += $item->getQty();
            }
            $itemWeight = $item->getWeight();
            $rowWeight  = $itemWeight*$item->getQty();
            $addressWeight+= $rowWeight;
            if ($freeAddress || $item->getFreeShipping()===true) {
                $rowWeight = 0;
            } elseif (is_numeric($item->getFreeShipping())) {
                $freeQty = $item->getFreeShipping();
                if ($item->getQty()>$freeQty) {
                    $rowWeight = $itemWeight*($item->getQty()-$freeQty);
                }
                else {
                    $rowWeight = 0;
                }
            }
            $freeMethodWeight+= $rowWeight;
            $item->setRowWeight($rowWeight);
        }
    }

    $address->setWeight($addressWeight);
    $address->setFreeMethodWeight($freeMethodWeight);

    $address->collectShippingRates();

This should look familiar, as this is the same place that each item's row_weight is set to 0 if free shipping is in effect. Notice how $addressWeight sums up each item's $rowWeight, but that's done before row_weight is set to 0.

Basically, the address weight will always be the total weight of all the items, regardless of the free_shipping value of each item. Since Magento's default carriers only rely on the address weight, the issue with row_weight doesn't show up.

So why do we need row_weight

We need row_weight because we've customized Magento's FedEx carrier to calculate separate rates for items that are coming from different origins, even if they're going to the same destination (and are thus part of the same address). For example, if you live in NJ, it's cheaper (and quicker) to ship an item from NJ than from CA - and if you have items from both NJ and CA in your order, you're able to see the cost (and estimated delivery date) of each shipment.

All in all, it looks like we can easily work around this issue by ignoring row_weight and using weight * qty directly. But, it does lead us to:

The question

Why does the Shipping total set the row_weight of the sales quote items to 0 if free shipping is in effect? This doesn't appear to be utilized anywhere.

Further observations

I neglected to mention that row_weight could actually be non-zero, but still less than weight * qty, if free_shipping is a number instead of true. I assume the purpose of this is to provide a solution to a scenario like this:

I have 3 items of the same product in my cart, each item weighing 2 lbs. I apply a free shipping coupon, but it's limited to a quantity of 2, so it only applies to 2 of the items. Now, when I look at the shipping rates, I'll be looking at shipping rates for 2 lbs (2 + 0 + 0) rather than 6 lbs (2 + 2 + 2).

This seems to make sense, but there are two major problems with it:

  • None of the default Magento carriers work like this (they use the address total weight, see above).

  • Even if some of the carriers worked like this, that would mean that I could pick any shipping method (e.g. overnight shipping) and only pay for the weight of 1 item - meaning, the merchant would have to cover the cost of the other 2 items. It would be up to the merchant to somehow figure out that I only paid for the weight of 1 item, and then ship the other 2 items using a more cost effective method, effectively creating a mismatch between what Magento displays and how the items were actually shipped out.

  • Heh, no takers? :) – Agop Oct 14 '15 at 15:50
  • So many up-votes, so little discussion! Such is the life of Magento development. – Agop Jan 7 '16 at 15:22
2

Figured I'd take a stab at this... ;)

Quite an interesting question you've posed here so here's why I think they did it, however I'm still working to track down when this particular case comes into play.

Going over the USPS carrier method, it looks like International Requests to their API provide itemized weights for each product. This is the only carrier that I can find that does this. Find the full method below, and the highlighted section below that.

protected function _formIntlShipmentRequest(Varien_Object $request)
    {
        $packageParams = $request->getPackageParams();
        $height = $packageParams->getHeight();
        $width = $packageParams->getWidth();
        $length = $packageParams->getLength();
        $girth = $packageParams->getGirth();
        $packageWeight = $request->getPackageWeight();
        if ($packageParams->getWeightUnits() != Zend_Measure_Weight::POUND) {
            $packageWeight = Mage::helper('usa')->convertMeasureWeight(
                $request->getPackageWeight(),
                $packageParams->getWeightUnits(),
                Zend_Measure_Weight::POUND
            );
        }
        if ($packageParams->getDimensionUnits() != Zend_Measure_Length::INCH) {
            $length = round(Mage::helper('usa')->convertMeasureDimension(
                $packageParams->getLength(),
                $packageParams->getDimensionUnits(),
                Zend_Measure_Length::INCH
            ));
            $width = round(Mage::helper('usa')->convertMeasureDimension(
                $packageParams->getWidth(),
                $packageParams->getDimensionUnits(),
                Zend_Measure_Length::INCH
            ));
            $height = round(Mage::helper('usa')->convertMeasureDimension(
                $packageParams->getHeight(),
                $packageParams->getDimensionUnits(),
                Zend_Measure_Length::INCH
            ));
        }
        if ($packageParams->getGirthDimensionUnits() != Zend_Measure_Length::INCH) {
            $girth = round(Mage::helper('usa')->convertMeasureDimension(
                $packageParams->getGirth(),
                $packageParams->getGirthDimensionUnits(),
                Zend_Measure_Length::INCH
            ));
        }

        $container = $request->getPackagingType();
        switch ($container) {
            case 'VARIABLE':
                $container = 'VARIABLE';
                break;
            case 'FLAT RATE ENVELOPE':
                $container = 'FLATRATEENV';
                break;
            case 'FLAT RATE BOX':
                $container = 'FLATRATEBOX';
                break;
            case 'RECTANGULAR':
                $container = 'RECTANGULAR';
                break;
            case 'NONRECTANGULAR':
                $container = 'NONRECTANGULAR';
                break;
            default:
                $container = 'VARIABLE';
        }
        $shippingMethod = $request->getShippingMethod();
        list($fromZip5, $fromZip4) = $this->_parseZip($request->getShipperAddressPostalCode());

        // the wrap node needs for remove xml declaration above
        $xmlWrap = new SimpleXMLElement('<?xml version = "1.0" encoding = "UTF-8"?><wrap/>');
        $method = '';
        $service = $this->getCode('service_to_code', $shippingMethod);
        if ($service == 'Priority') {
            $method = 'Priority';
            $rootNode = 'PriorityMailIntlRequest';
            $xml = $xmlWrap->addChild($rootNode);
        } else if ($service == 'First Class') {
            $method = 'FirstClass';
            $rootNode = 'FirstClassMailIntlRequest';
            $xml = $xmlWrap->addChild($rootNode);
        } else {
            $method = 'Express';
            $rootNode = 'ExpressMailIntlRequest';
            $xml = $xmlWrap->addChild($rootNode);
        }

        $xml->addAttribute('USERID', $this->getConfigData('userid'));
        $xml->addAttribute('PASSWORD', $this->getConfigData('password'));
        $xml->addChild('Option');
        $xml->addChild('Revision', self::DEFAULT_REVISION);
        $xml->addChild('ImageParameters');
        $xml->addChild('FromFirstName', $request->getShipperContactPersonFirstName());
        $xml->addChild('FromLastName', $request->getShipperContactPersonLastName());
        $xml->addChild('FromFirm', $request->getShipperContactCompanyName());
        $xml->addChild('FromAddress1', $request->getShipperAddressStreet2());
        $xml->addChild('FromAddress2', $request->getShipperAddressStreet1());
        $xml->addChild('FromCity', $request->getShipperAddressCity());
        $xml->addChild('FromState', $request->getShipperAddressStateOrProvinceCode());
        $xml->addChild('FromZip5', $fromZip5);
        $xml->addChild('FromZip4', $fromZip4);
        $xml->addChild('FromPhone', $request->getShipperContactPhoneNumber());
        if ($method != 'FirstClass') {
            if ($request->getReferenceData()) {
                $referenceData = $request->getReferenceData() . ' P' . $request->getPackageId();
            } else {
                $referenceData = $request->getOrderShipment()->getOrder()->getIncrementId()
                                 . ' P'
                                 . $request->getPackageId();
            }
            $xml->addChild('FromCustomsReference', 'Order #' . $referenceData);
        }
        $xml->addChild('ToFirstName', $request->getRecipientContactPersonFirstName());
        $xml->addChild('ToLastName', $request->getRecipientContactPersonLastName());
        $xml->addChild('ToFirm', $request->getRecipientContactCompanyName());
        $xml->addChild('ToAddress1', $request->getRecipientAddressStreet1());
        $xml->addChild('ToAddress2', $request->getRecipientAddressStreet2());
        $xml->addChild('ToCity', $request->getRecipientAddressCity());
        $xml->addChild('ToProvince', $request->getRecipientAddressStateOrProvinceCode());
        $xml->addChild('ToCountry', $this->_getCountryName($request->getRecipientAddressCountryCode()));
        $xml->addChild('ToPostalCode', $request->getRecipientAddressPostalCode());
        $xml->addChild('ToPOBoxFlag', 'N');
        $xml->addChild('ToPhone', $request->getRecipientContactPhoneNumber());
        $xml->addChild('ToFax');
        $xml->addChild('ToEmail');
        if ($method != 'FirstClass') {
            $xml->addChild('NonDeliveryOption', 'Return');
        }
        if ($method == 'FirstClass') {
            if (stripos($shippingMethod, 'Letter') !== false) {
                $xml->addChild('FirstClassMailType', 'LETTER');
            } else if (stripos($shippingMethod, 'Flat') !== false) {
                $xml->addChild('FirstClassMailType', 'FLAT');
            } else{
                $xml->addChild('FirstClassMailType', 'PARCEL');
            }
        }
        if ($method != 'FirstClass') {
            $xml->addChild('Container', $container);
        }
        $shippingContents = $xml->addChild('ShippingContents');
        $packageItems = $request->getPackageItems();
        // get countries of manufacture
        $countriesOfManufacture = array();
        $productIds = array();
        foreach ($packageItems as $itemShipment) {
                $item = new Varien_Object();
                $item->setData($itemShipment);

                $productIds[]= $item->getProductId();
        }
        $productCollection = Mage::getResourceModel('catalog/product_collection')
            ->addStoreFilter($request->getStoreId())
            ->addFieldToFilter('entity_id', array('in' => $productIds))
            ->addAttributeToSelect('country_of_manufacture');
        foreach ($productCollection as $product) {
            $countriesOfManufacture[$product->getId()] = $product->getCountryOfManufacture();
        }

        $packagePoundsWeight = $packageOuncesWeight = 0;
        // for ItemDetail
        foreach ($packageItems as $itemShipment) {
            $item = new Varien_Object();
            $item->setData($itemShipment);

            $itemWeight = $item->getWeight() * $item->getQty();
            if ($packageParams->getWeightUnits() != Zend_Measure_Weight::POUND) {
                $itemWeight = Mage::helper('usa')->convertMeasureWeight(
                    $itemWeight,
                    $packageParams->getWeightUnits(),
                    Zend_Measure_Weight::POUND
                );
            }
            if (!empty($countriesOfManufacture[$item->getProductId()])) {
                $countryOfManufacture = $this->_getCountryName(
                    $countriesOfManufacture[$item->getProductId()]
                );
            } else {
                $countryOfManufacture = '';
            }
            $itemDetail = $shippingContents->addChild('ItemDetail');
            $itemDetail->addChild('Description', $item->getName());
            $ceiledQty = ceil($item->getQty());
            if ($ceiledQty < 1) {
                $ceiledQty = 1;
            }
            $individualItemWeight = $itemWeight / $ceiledQty;
            $itemDetail->addChild('Quantity', $ceiledQty);
            $itemDetail->addChild('Value', $item->getCustomsValue() * $item->getQty());
            list($individualPoundsWeight, $individualOuncesWeight) = $this->_convertPoundOunces($individualItemWeight);
            $itemDetail->addChild('NetPounds', $individualPoundsWeight);
            $itemDetail->addChild('NetOunces', $individualOuncesWeight);
            $itemDetail->addChild('HSTariffNumber', 0);
            $itemDetail->addChild('CountryOfOrigin', $countryOfManufacture);

            list($itemPoundsWeight, $itemOuncesWeight) = $this->_convertPoundOunces($itemWeight);
            $packagePoundsWeight += $itemPoundsWeight;
            $packageOuncesWeight += $itemOuncesWeight;
        }
        $additionalPackagePoundsWeight = floor($packageOuncesWeight / self::OUNCES_POUND);
        $packagePoundsWeight += $additionalPackagePoundsWeight;
        $packageOuncesWeight -= $additionalPackagePoundsWeight * self::OUNCES_POUND;
        if ($packagePoundsWeight + $packageOuncesWeight / self::OUNCES_POUND < $packageWeight) {
            list($packagePoundsWeight, $packageOuncesWeight) = $this->_convertPoundOunces($packageWeight);
        }

        $xml->addChild('GrossPounds', $packagePoundsWeight);
        $xml->addChild('GrossOunces', $packageOuncesWeight);
        if ($packageParams->getContentType() == 'OTHER' && $packageParams->getContentTypeOther() != null) {
            $xml->addChild('ContentType', $packageParams->getContentType());
            $xml->addChild('ContentTypeOther ', $packageParams->getContentTypeOther());
        } else {
            $xml->addChild('ContentType', $packageParams->getContentType());
        }

        $xml->addChild('Agreement', 'y');
        $xml->addChild('ImageType', 'PDF');
        $xml->addChild('ImageLayout', 'ALLINONEFILE');
        if ($method == 'FirstClass') {
            $xml->addChild('Container', $container);
        }
        // set size
        if ($packageParams->getSize()) {
            $xml->addChild('Size', $packageParams->getSize());
        }
        // set dimensions
        $xml->addChild('Length', $length);
        $xml->addChild('Width', $width);
        $xml->addChild('Height', $height);
        if ($girth) {
            $xml->addChild('Girth', $girth);
        }

        $xml = $xmlWrap->{$rootNode}->asXML();
        return $xml;
    }

Particular section:

$packagePoundsWeight = $packageOuncesWeight = 0;
        // for ItemDetail
        foreach ($packageItems as $itemShipment) {
            $item = new Varien_Object();
            $item->setData($itemShipment);

            $itemWeight = $item->getWeight() * $item->getQty();
            if ($packageParams->getWeightUnits() != Zend_Measure_Weight::POUND) {
                $itemWeight = Mage::helper('usa')->convertMeasureWeight(
                    $itemWeight,
                    $packageParams->getWeightUnits(),
                    Zend_Measure_Weight::POUND
                );
            }
            if (!empty($countriesOfManufacture[$item->getProductId()])) {
                $countryOfManufacture = $this->_getCountryName(
                    $countriesOfManufacture[$item->getProductId()]
                );
            } else {
                $countryOfManufacture = '';
            }
            $itemDetail = $shippingContents->addChild('ItemDetail');
            $itemDetail->addChild('Description', $item->getName());
            $ceiledQty = ceil($item->getQty());
            if ($ceiledQty < 1) {
                $ceiledQty = 1;
            }
            $individualItemWeight = $itemWeight / $ceiledQty;
            $itemDetail->addChild('Quantity', $ceiledQty);
            $itemDetail->addChild('Value', $item->getCustomsValue() * $item->getQty());
            list($individualPoundsWeight, $individualOuncesWeight) = $this->_convertPoundOunces($individualItemWeight);
            $itemDetail->addChild('NetPounds', $individualPoundsWeight);
            $itemDetail->addChild('NetOunces', $individualOuncesWeight);
            $itemDetail->addChild('HSTariffNumber', 0);
            $itemDetail->addChild('CountryOfOrigin', $countryOfManufacture);

            list($itemPoundsWeight, $itemOuncesWeight) = $this->_convertPoundOunces($itemWeight);
            $packagePoundsWeight += $itemPoundsWeight;
            $packageOuncesWeight += $itemOuncesWeight;
        }

To me, it appears Magento is recalculating the package weight based on the actual item weights, and not leveraging the address weight. I wonder if the package items being passed do not have their weight zeroed, because that would lead to false item weights considering the rates response would be based on bad weight data.

Shot in the dark though.

1

I had a stab also on this and i found something interesting this line of code

$item->setRowWeight($rowWeight);

was introduced on version 1.1.5 before that the function was like this.

Magento Mirror Import Magento Release 1.1.5 - Shipping.php

Magento Mirror Import Magento Release 1.1.1 - Shipping.php

            else {
            if (!$item->getProduct()->getTypeInstance()->isVirtual()) {
                $addressQty += $item->getQty();
            }
            $itemWeight = $item->getWeight();
            $rowWeight  = $itemWeight*$item->getQty();
            $addressWeight+= $rowWeight;
            if ($freeAddress || $item->getFreeShipping()===true) {
                $rowWeight = 0;
            } elseif (is_numeric($item->getFreeShipping())) {
                $freeQty = $item->getFreeShipping();
                if ($item->getQty()>$freeQty) {
                    $rowWeight = $itemWeight*($item->getQty()-$freeQty);
                }
                else {
                    $rowWeight = 0;
                }
            }
            $freeMethodWeight+= $rowWeight;
        }
    }

My understanding is that Mage_Sales_Model_Quote_Address_Total_Shipping::collect need to calculate/update $addressWeight and $freeMethodWeight

    $addressWeight      = $address->getWeight();
    $freeMethodWeight   = $address->getFreeMethodWeight();

and $item->setRowWeight shod not be used in here because is related with the item not with the quote and address.

My bet is that this is a mistake. Probably row_total was never intended to be used in shipping methods that's why default modules are not using it.

I was not able to track down any change log that can explain why this was introduced in v 1.1.5.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.