View Issue Details

Jump to Notes
IDProjectCategoryView StatusDate SubmittedLast Update
0006252OXID eShop (all versions)1.03. Basket, checkout processpublic2015-10-31 11:472024-02-28 11:00
Reportermario_lorenz Assigned To 
PrioritynormalSeverityfeatureReproducibilityalways
Status acknowledgedResolutionopen 
Product Version4.9.6 / 5.2.6 
Summary0006252: bug in method isForBasket (class oxdelivery.php)
DescriptionI have three deliverys D.A, D.B, D.C

Each delivery has an own deliveryset
DS.A -> D.A
DS.B -> D.B
DS.C -> D.C

I have two Articles X and Y

Article X is connect with delivery D.A and D.B
Article Y is connect with delivery D.A and D.C

First i put article X in my basket. In the checkout the Delivery-Sets DS.A and DS.B are shown.
Now i delete article X from the basket and i put article Y in the basket.
In the checkout the Delivery-Sets DS.A and DS.C are shown.

Both situations are just as I expected it.

Now i put both articles in my Basket. Normally i would expect, that only Delivery-Set DS.A are shown, because only DS.A is valid for both, but all three Delivery-Sets are shown. Thats a mistake.
Steps To ReproduceCreate three deliverys
Create three deliverysets

Connect each of the deliverys with exact one deliveryset

Connect two different Articles like this:
Article X is connect with delivery A and B
Article Y is connect with delivery A and C

Put both articles in the basket. Go to the checkout.
Additional InformationI helped me in the moment with an overloading of the method.
So I get my expected behavior. But it could be that I've forgotten something.

(look at variable $bIsForAllArticlesInBasket)

public function isForBasket($oBasket)
{
  // amount for conditional check
  $blHasArticles = $this->hasArticles();
  $blHasCategories = $this->hasCategories();
  $blUse = true;
  $iAmount = 0;
  $blForBasket = false;
  $bIsForAllArticlesInBasket = true;

  // category & article check
  if ($blHasCategories || $blHasArticles)
  {
    $blUse = false;

    $aDeliveryArticles = $this->getArticles();
    $aDeliveryCategories = $this->getCategories();

    foreach ($oBasket->getContents() as $oContent)
    {
      //V FS#1954 - load delivery for variants from parent article
      $oArticle = $oContent->getArticle(false);
      $sProductId = $oArticle->getProductId();
      $sParentId = $oArticle->getParentId();

      if ($blHasArticles && (in_array($sProductId, $aDeliveryArticles) || ($sParentId && in_array($sParentId, $aDeliveryArticles)))) {
        $blUse = true;
        $iArtAmount = $this->getDeliveryAmount($oContent);
        if ($this->getCalculationRule() != self::CALCULATION_RULE_ONCE_PER_CART)
        {
          if ($this->_isForArticle($oContent, $iArtAmount))
          {
            $blForBasket = true;
          }
        }
        if (!$blForBasket)
        {
          $iAmount += $iArtAmount;
        }
      }
      elseif ($blHasCategories)
      {
        if (isset(self::$_aProductList[$sProductId]))
        {
          $oProduct = self::$_aProductList[$sProductId];
        }
        else
        {
          $oProduct = oxNew('oxArticle');
          $oProduct->setSkipAssign(true);

          if (!$oProduct->load($sProductId))
          {
            continue;
          }
          $oProduct->setId($sProductId);
          self::$_aProductList[$sProductId] = $oProduct;
        }

        foreach ($aDeliveryCategories as $sCatId)
        {
          if ($oProduct->inCategory($sCatId))
          {
            $blUse = true;
            $iArtAmount = $this->getDeliveryAmount($oContent);
            if ($this->getCalculationRule() != self::CALCULATION_RULE_ONCE_PER_CART)
            {
              if ($this->_isForArticle($oContent, $iArtAmount))
              {
                $blForBasket = true;
              }
            }
            if (!$blForBasket)
            {
              $iAmount += $iArtAmount;
            }
            //HR#5650 product might be in multiple rule categories, counting it once is enough
            break;
          }
        }
      }
      else
      {
        $bIsForAllArticlesInBasket = false;
      }
    }
  }
  else
  {
    // regular amounts check
    foreach ($oBasket->getContents() as $oContent)
    {
      $iArtAmount = $this->getDeliveryAmount($oContent);
      if ($this->getCalculationRule() != self::CALCULATION_RULE_ONCE_PER_CART)
      {
        if ($this->_isForArticle($oContent, $iArtAmount))
        {
          $blForBasket = true;
        }
      }
      if (!$blForBasket)
      {
        $iAmount += $iArtAmount;
      }
    }
  }

  //#M1130: Single article in Basket, checked as free shipping, is not buyable (step 3 no payments found)
  if (!$blForBasket && $blUse && $bIsForAllArticlesInBasket && ($this->_checkDeliveryAmount($iAmount) || $this->_blFreeShipping))
  {
    $blForBasket = true;
  }

  return $blForBasket;
}
TagsProduct domain and basket rewrite
ThemeNot defined
BrowserNot defined
PHP VersionNot defined
Database VersionNot defined

Activities

leofonic

2015-11-03 17:09

reporter   ~0011301

This is a duplicate of 0001659, which has eventually been marked resolved and moved to uservoice. Method isForBasket serves two purposes:

1. Shipping cost calculation (needs match if one article is valid)
2. Selection of valid deliverysets (needs match if all articles are valid)

This will not work at the same time. If you change isForBasket to satisfy number 2, number 1 will go wrong (0002149)

This method has to be refactored so both cases can be handled.

SvenBrunk

2024-02-28 10:59

administrator   ~0016314

Last edited: 2024-02-28 11:00

 This is indeed a feature request. I marked it as relevant for the upcoming Basket rewrite.

For this to properly work we actually need order splitting and an option for the customer to see what is happening and why.
In the current Apex Demoshop you already have this situation if you for example put the Test-Customer Download, a car and some other stuff into the basket.

You would not be able to check out such a basket without order splitting, as you have a download, a car delivery and a normal package in your basket.
In the real world the download would just be independent as it is a "virtual" item (with no costs involved) and the shop owner could just put the other item into the trunk of the car, but you can already see the issue with the representation in the shop.

What I would expect in a viable solution is:
- If there is a shipping method like in your example that can handle all products at the same time, preselect it and tell the customer that additional shipping costs will apply when he changes the shipping method
- Shipping costs for ALL three different shippings in the same basket if necessary (other shipping method selected or none applies to multiple products)
- Order splitting (optional) for the ERP with 3 orders with 3 different shipping methods. (or as many as applicable)

All of this needs a new implementation