Skip to content

Fix Shipping iterate over null #34187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: 2.4-develop
Choose a base branch
from
Open

Fix Shipping iterate over null #34187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
42813cc
Extract carriers status and config to separate class
thomas-kl1 Sep 29, 2021
389c86f
Merge branch '2.4-develop' into patch-9
thomas-kl1 Mar 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 75 additions & 74 deletions app/code/Magento/Shipping/Model/Carrier/AbstractCarrier.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,20 @@

namespace Magento\Shipping\Model\Carrier;

use Exception;
use Magento\Framework\App\Config\ScopeConfigInterface;
use Magento\Framework\App\ObjectManager;
use Magento\Framework\DataObject;
use Magento\Framework\Model\AbstractModel;
use Magento\Quote\Model\Quote\Address\RateRequest;
use Magento\Quote\Model\Quote\Address\RateResult\Error;
use Magento\Quote\Model\Quote\Address\RateResult\ErrorFactory;
use Magento\Quote\Model\Quote\Address\RateResult\Method;
use Magento\Shipping\Model\Config\CarrierStatus;
use Magento\Shipping\Model\Shipment\Request;
use Magento\Store\Model\ScopeInterface;
use Psr\Log\LoggerInterface;
use SimpleXMLElement;

/**
* Class AbstractCarrier
Expand All @@ -16,7 +28,7 @@
* @api
* @since 100.0.2
*/
abstract class AbstractCarrier extends \Magento\Framework\DataObject implements AbstractCarrierInterface
abstract class AbstractCarrier extends DataObject implements AbstractCarrierInterface
{
public const DEBUG_KEYS_MASK = '****';

Expand Down Expand Up @@ -72,36 +84,40 @@ abstract class AbstractCarrier extends \Magento\Framework\DataObject implements
/**
* Core store config
*
* @var \Magento\Framework\App\Config\ScopeConfigInterface
* @var ScopeConfigInterface
*/
protected $_scopeConfig;

/**
* @var \Magento\Quote\Model\Quote\Address\RateResult\ErrorFactory
* @var ErrorFactory
*/
protected $_rateErrorFactory;

/**
* @var \Psr\Log\LoggerInterface
* @var LoggerInterface
*/
protected $_logger;

protected CarrierStatus $carrierStatus;

/**
* @param \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig
* @param \Magento\Quote\Model\Quote\Address\RateResult\ErrorFactory $rateErrorFactory
* @param \Psr\Log\LoggerInterface $logger
* @param ScopeConfigInterface $scopeConfig
* @param ErrorFactory $rateErrorFactory
* @param LoggerInterface $logger
* @param array $data
*/
public function __construct(
\Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig,
\Magento\Quote\Model\Quote\Address\RateResult\ErrorFactory $rateErrorFactory,
\Psr\Log\LoggerInterface $logger,
array $data = []
ScopeConfigInterface $scopeConfig,
ErrorFactory $rateErrorFactory,
LoggerInterface $logger,
array $data = [],
CarrierStatus $carrierStatus = null
) {
parent::__construct($data);
$this->_scopeConfig = $scopeConfig;
$this->_rateErrorFactory = $rateErrorFactory;
$this->_logger = $logger;
$this->carrierStatus = $carrierStatus ?? ObjectManager::getInstance()->get(CarrierStatus::class);
}

/**
Expand All @@ -117,11 +133,7 @@ public function getConfigData($field)
}
$path = 'carriers/' . $this->_code . '/' . $field;

return $this->_scopeConfig->getValue(
$path,
\Magento\Store\Model\ScopeInterface::SCOPE_STORE,
$this->getStore()
);
return $this->_scopeConfig->getValue($path, ScopeInterface::SCOPE_STORE, $this->getStore());
}

/**
Expand All @@ -138,11 +150,7 @@ public function getConfigFlag($field)
}
$path = 'carriers/' . $this->_code . '/' . $field;

return $this->_scopeConfig->isSetFlag(
$path,
\Magento\Store\Model\ScopeInterface::SCOPE_STORE,
$this->getStore()
);
return $this->_scopeConfig->isSetFlag($path, ScopeInterface::SCOPE_STORE, $this->getStore());
}

/**
Expand All @@ -151,12 +159,12 @@ public function getConfigFlag($field)
* Implementation must be in overridden method
*
* @param Request $request
* @return \Magento\Framework\DataObject
* @return DataObject
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
public function requestToShipment($request)
{
return new \Magento\Framework\DataObject();
return new DataObject();
}

/**
Expand All @@ -165,35 +173,35 @@ public function requestToShipment($request)
* Implementation must be in overridden method
*
* @param Request $request
* @return \Magento\Framework\DataObject
* @return DataObject
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
public function returnOfShipment($request)
{
return new \Magento\Framework\DataObject();
return new DataObject();
}

/**
* Return container types of carrier
*
* @param \Magento\Framework\DataObject|null $params
* @param DataObject|null $params
* @return array
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
public function getContainerTypes(\Magento\Framework\DataObject $params = null)
public function getContainerTypes(DataObject $params = null)
{
return [];
}

/**
* Get allowed containers of carrier
*
* @param \Magento\Framework\DataObject|null $params
* @param DataObject|null $params
* @return array|bool
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
* @SuppressWarnings(PHPMD.NPathComplexity)
*/
protected function _getAllowedContainers(\Magento\Framework\DataObject $params = null)
protected function _getAllowedContainers(DataObject $params = null)
{
$containersAll = $this->getContainerTypesAll();
if (empty($containersAll)) {
Expand All @@ -215,14 +223,10 @@ protected function _getAllowedContainers(\Magento\Framework\DataObject $params =
return $containersAll;
}

if ($countryShipper == self::USA_COUNTRY_ID && $countryRecipient == self::USA_COUNTRY_ID) {
$direction = 'within_us';
if ($countryShipper === self::USA_COUNTRY_ID) {
$direction = $countryRecipient === self::USA_COUNTRY_ID ? 'within_us' : 'from_us';
} else {
if ($countryShipper == self::USA_COUNTRY_ID && $countryRecipient != self::USA_COUNTRY_ID) {
$direction = 'from_us';
} else {
return $containersAll;
}
return $containersAll;
}

foreach ($containersFilter as $dataItem) {
Expand Down Expand Up @@ -253,23 +257,23 @@ public function getCustomizableContainerTypes()
/**
* Return delivery confirmation types of carrier
*
* @param \Magento\Framework\DataObject|null $params
* @param DataObject|null $params
* @return array
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
public function getDeliveryConfirmationTypes(\Magento\Framework\DataObject $params = null)
public function getDeliveryConfirmationTypes(DataObject $params = null)
{
return [];
}

/**
* Validate request for available ship countries.
*
* @param \Magento\Framework\DataObject $request
* @return $this|bool|false|\Magento\Framework\Model\AbstractModel
* @param DataObject $request
* @return $this|bool|false|AbstractModel
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
*/
public function checkAvailableShipCountries(\Magento\Framework\DataObject $request)
public function checkAvailableShipCountries(DataObject $request)
{
$speCountriesAllow = $this->getConfigData('sallowspecific');
/*
Expand All @@ -283,30 +287,29 @@ public function checkAvailableShipCountries(\Magento\Framework\DataObject $reque
}
if ($availableCountries && in_array($request->getDestCountryId(), $availableCountries)) {
return $this;
} elseif ($showMethod && (!$availableCountries || $availableCountries && !in_array(
}
if ($showMethod && (!$availableCountries || $availableCountries && !in_array(
$request->getDestCountryId(),
$availableCountries
))
) {
))) {
/** @var Error $error */
$error = $this->_rateErrorFactory->create();
$error->setCarrier($this->_code);
$error->setCarrierTitle($this->getConfigData('title'));
$errorMsg = $this->getConfigData('specificerrmsg');
$error->setErrorMessage(
$errorMsg ? $errorMsg : __(
'Sorry, but we can\'t deliver to the destination country with this shipping module.'
)
$errorMsg
?: __('Sorry, but we can\'t deliver to the destination country with this shipping module.')
);

return $error;
} else {
/*
* The admin set not to show the shipping module if the delivery country
* is not within specific countries
*/
return false;
}

/*
* The admin set not to show the shipping module if the delivery country
* is not within specific countries
*/
return false;
}

return $this;
Expand All @@ -315,25 +318,25 @@ public function checkAvailableShipCountries(\Magento\Framework\DataObject $reque
/**
* Processing additional validation to check is carrier applicable.
*
* @param \Magento\Framework\DataObject $request
* @return $this|bool|\Magento\Framework\DataObject
* @param DataObject $request
* @return $this|bool|DataObject
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
* @since 100.2.6
*/
public function processAdditionalValidation(\Magento\Framework\DataObject $request)
public function processAdditionalValidation(DataObject $request)
{
return $this;
}

/**
* Processing additional validation to check is carrier applicable.
*
* @param \Magento\Framework\DataObject $request
* @return $this|bool|\Magento\Framework\DataObject
* @param DataObject $request
* @return $this|bool|DataObject
* @deprecated 100.2.6
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
public function proccessAdditionalValidation(\Magento\Framework\DataObject $request)
public function proccessAdditionalValidation(DataObject $request)
{
return $this->processAdditionalValidation($request);
}
Expand All @@ -345,9 +348,7 @@ public function proccessAdditionalValidation(\Magento\Framework\DataObject $requ
*/
public function isActive()
{
$active = $this->getConfigData('active');

return $active == 1 || $active == 'true';
return $this->carrierStatus->isEnabled($this->_code);
}

/**
Expand Down Expand Up @@ -393,7 +394,7 @@ public function getSortOrder()
/**
* Check if the request has free shipping weight
*
* @param \Magento\Quote\Model\Quote\Address\RateRequest $request
* @param RateRequest $request
* @return bool
*/
private function hasFreeMethodWeight($request): bool
Expand All @@ -410,7 +411,7 @@ private function hasFreeMethodWeight($request): bool
/**
* Allows free shipping when all product items have free shipping.
*
* @param \Magento\Quote\Model\Quote\Address\RateRequest $request
* @param RateRequest $request
* @return void
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
* @SuppressWarnings(PHPMD.NPathComplexity)
Expand Down Expand Up @@ -447,12 +448,12 @@ protected function _updateFreeMethodQuote($request)
// phpstan:ignore
$result = $this->_getQuotes();
if ($result && ($rates = $result->getAllRates()) && count($rates) > 0) {
if (count($rates) == 1 && $rates[0] instanceof \Magento\Quote\Model\Quote\Address\RateResult\Method) {
if (count($rates) == 1 && $rates[0] instanceof Method) {
$price = $rates[0]->getPrice();
}
if (count($rates) > 1) {
foreach ($rates as $rate) {
if ($rate instanceof \Magento\Quote\Model\Quote\Address\RateResult\Method &&
if ($rate instanceof Method &&
$rate->getMethod() == $freeMethod
) {
$price = $rate->getPrice();
Expand Down Expand Up @@ -640,11 +641,11 @@ public function getCarrierCode()
/**
* Return content types of package
*
* @param \Magento\Framework\DataObject $params
* @param DataObject $params
* @return array
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
public function getContentTypes(\Magento\Framework\DataObject $params)
public function getContentTypes(DataObject $params)
{
return [];
}
Expand All @@ -670,10 +671,10 @@ public function getContentTypes(\Magento\Framework\DataObject $params)
protected function filterDebugData($data)
{
try {
$xml = new \SimpleXMLElement($data);
$xml = new SimpleXMLElement($data);
$this->filterXmlData($xml);
$data = $xml->asXML();
} catch (\Exception $e) {
} catch (Exception $e) {
$this->_logger->critical($e);
}
return $data;
Expand All @@ -682,16 +683,16 @@ protected function filterDebugData($data)
/**
* Recursive replace sensitive xml nodes values by specified mask.
*
* @param \SimpleXMLElement $xml
* @param SimpleXMLElement $xml
* @return void
*/
private function filterXmlData(\SimpleXMLElement $xml)
private function filterXmlData(SimpleXMLElement $xml): void
{
/** @var \SimpleXMLElement $child */
/** @var SimpleXMLElement $child */
foreach ($xml->children() as $child) {
if ($child->count()) {
$this->filterXmlData($child);
} elseif (in_array((string) $child->getName(), $this->_debugReplacePrivateDataKeys)) {
} elseif (in_array($child->getName(), $this->_debugReplacePrivateDataKeys, true)) {
$child[0] = self::DEBUG_KEYS_MASK;
}
}
Expand Down
Loading