Skip to content

Issue #31311 - Shipping Method Delimiter Conflict Fix #35134

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.

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 16 commits into
base: 2.4-develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
use Magento\Quote\Model\QuoteAddressValidator;
use Magento\Quote\Model\ShippingAssignmentFactory;
use Magento\Quote\Model\ShippingFactory;
use Magento\Sales\Model\Order;
use Psr\Log\LoggerInterface as Logger;

/**
Expand Down Expand Up @@ -184,7 +185,7 @@ public function saveAddressInformation(
$carrierCode = $addressInformation->getShippingCarrierCode();
$address->setLimitCarrier($carrierCode);
$methodCode = $addressInformation->getShippingMethodCode();
$quote = $this->prepareShippingAssignment($quote, $address, $carrierCode . '_' . $methodCode);
$quote = $this->prepareShippingAssignment($quote, $address, $carrierCode . Order::DELIMITER_SHIPPING_METHOD . $methodCode);

$quote->setIsMultiShipping(false);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
namespace Magento\Checkout\Model;

use Magento\Checkout\Api\Data\TotalsInformationInterface;
use Magento\Sales\Model\Order;

/**
* Class for management of totals information.
Expand Down Expand Up @@ -54,7 +55,7 @@ public function calculate(
$quote->setShippingAddress($addressInformation->getAddress());
if ($addressInformation->getShippingCarrierCode() && $addressInformation->getShippingMethodCode()) {
$shippingMethod = implode(
'_',
Order::DELIMITER_SHIPPING_METHOD,
[$addressInformation->getShippingCarrierCode(), $addressInformation->getShippingMethodCode()]
);
$quoteShippingAddress = $quote->getShippingAddress();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
use Magento\Quote\Model\ShippingAssignment;
use Magento\Quote\Model\ShippingAssignmentFactory;
use Magento\Quote\Model\ShippingFactory;
use Magento\Sales\Model\Order;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\MockObject\RuntimeException;
use PHPUnit\Framework\TestCase;
Expand Down Expand Up @@ -359,7 +360,7 @@ public function testSaveAddressInformationWithLocalizedException(): void
->method('getCountryId')
->willReturn('USA');

$this->setShippingAssignmentsMocks($carrierCode . '_' . $shippingMethod);
$this->setShippingAssignmentsMocks($carrierCode . Order::DELIMITER_SHIPPING_METHOD . $shippingMethod);

$this->quoteMock->expects($this->once())
->method('getItemsCount')
Expand Down Expand Up @@ -425,7 +426,7 @@ public function testSaveAddressInformationIfCanNotSaveQuote(): void
->method('getCountryId')
->willReturn('USA');

$this->setShippingAssignmentsMocks($carrierCode . '_' . $shippingMethod);
$this->setShippingAssignmentsMocks($carrierCode . Order::DELIMITER_SHIPPING_METHOD . $shippingMethod);

$this->quoteMock->expects($this->once())
->method('getItemsCount')
Expand Down Expand Up @@ -488,7 +489,7 @@ public function testSaveAddressInformationIfCarrierCodeIsInvalid(): void
->method('getCountryId')
->willReturn('USA');

$this->setShippingAssignmentsMocks($carrierCode . '_' . $shippingMethod);
$this->setShippingAssignmentsMocks($carrierCode . Order::DELIMITER_SHIPPING_METHOD . $shippingMethod);

$this->quoteMock->expects($this->once())
->method('getItemsCount')
Expand Down Expand Up @@ -563,7 +564,7 @@ public function testSaveAddressInformation(): void
->method('getCountryId')
->willReturn('USA');

$this->setShippingAssignmentsMocks($carrierCode . '_' . $shippingMethod);
$this->setShippingAssignmentsMocks($carrierCode . Order::DELIMITER_SHIPPING_METHOD . $shippingMethod);

$this->quoteMock->expects($this->once())
->method('getItemsCount')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ define([

if (!availableRate && selectedShippingRate) {
availableRate = _.find(ratesData, function (rate) {
return rate['carrier_code'] + '_' + rate['method_code'] === selectedShippingRate;
return rate['carrier_code'] + '::' + rate['method_code'] === selectedShippingRate;
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ define([
shippingRateGroups: ko.observableArray([]),
selectedShippingMethod: ko.computed(function () {
return quote.shippingMethod() ?
quote.shippingMethod()['carrier_code'] + '_' + quote.shippingMethod()['method_code'] :
quote.shippingMethod()['carrier_code'] + '::' + quote.shippingMethod()['method_code'] :
null;
}),

Expand Down Expand Up @@ -76,7 +76,7 @@ define([
*/
selectShippingMethod: function (methodData) {
selectShippingMethodAction(methodData);
checkoutData.setSelectedShippingRate(methodData['carrier_code'] + '_' + methodData['method_code']);
checkoutData.setSelectedShippingRate(methodData['carrier_code'] + '::' + methodData['method_code']);

return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ define([
isLoading: shippingService.isLoading,
isSelected: ko.computed(function () {
return quote.shippingMethod() ?
quote.shippingMethod()['carrier_code'] + '_' + quote.shippingMethod()['method_code'] :
quote.shippingMethod()['carrier_code'] + '::' + quote.shippingMethod()['method_code'] :
null;
}),

Expand All @@ -270,7 +270,7 @@ define([
*/
selectShippingMethod: function (shippingMethod) {
selectShippingMethodAction(shippingMethod);
checkoutData.setSelectedShippingRate(shippingMethod['carrier_code'] + '_' + shippingMethod['method_code']);
checkoutData.setSelectedShippingRate(shippingMethod['carrier_code'] + '::' + shippingMethod['method_code']);

return true;
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@
click: $parents[1].selectShippingMethod,
checked: $parents[1].selectedShippingMethod,
attr: {
value: carrier_code + '_' + method_code,
id: 's_method_' + carrier_code + '_' + method_code,
value: carrier_code + '::' + method_code,
id: 's_method_' + carrier_code + '::' + method_code,
disabled: false
}
"/>
<label class="label" data-bind="attr: {for: 's_method_' + carrier_code + '_' + method_code}">
<label class="label" data-bind="attr: {for: 's_method_' + carrier_code + '::' + method_code}">
<!-- ko text: $data.method_title --><!-- /ko -->
<each args="element.getRegion('price')" render=""></each>
</label>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
class="radio"
ifnot="method.error_message"
ko-checked="element.isSelected"
ko-value="method.carrier_code + '_' + method.method_code"
attr="'aria-labelledby': 'label_method_' + method.method_code + '_' + method.carrier_code + ' ' + 'label_carrier_' + method.method_code + '_' + method.carrier_code,
ko-value="method.carrier_code + '::' + method.method_code"
attr="'aria-labelledby': 'label_method_' + method.method_code + '::' + method.carrier_code + ' ' + 'label_carrier_' + method.method_code + '::' + method.carrier_code,
'checked': element.rates().length == 1 || element.isSelected" />
</td>
<!-- ko ifnot: (method.error_message) -->
Expand All @@ -21,10 +21,10 @@
</td>
<!-- /ko -->
<td class="col col-method"
attr="'id': 'label_method_' + method.method_code + '_' + method.carrier_code"
attr="'id': 'label_method_' + method.method_code + '::' + method.carrier_code"
text="method.method_title"></td>
<td class="col col-carrier"
attr="'id': 'label_carrier_' + method.method_code + '_' + method.carrier_code"
attr="'id': 'label_carrier_' + method.method_code + '::' + method.carrier_code"
text="method.carrier_title"></td>
</tr>
<tr class="row row-error"
Expand Down
3 changes: 2 additions & 1 deletion app/code/Magento/Quote/Model/Quote/Address/Rate.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
namespace Magento\Quote\Model\Quote\Address;

use Magento\Framework\Model\AbstractModel;
use Magento\Sales\Model\Order;

/**
* @api
Expand Down Expand Up @@ -98,7 +99,7 @@ public function importShippingRate(\Magento\Quote\Model\Quote\Address\RateResult
);
} elseif ($rate instanceof \Magento\Quote\Model\Quote\Address\RateResult\Method) {
$this->setCode(
$rate->getCarrier() . '_' . $rate->getMethod()
$rate->getCarrier() . Order::DELIMITER_SHIPPING_METHOD . $rate->getMethod()
)->setCarrier(
$rate->getCarrier()
)->setCarrierTitle(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use Magento\Quote\Model\ShippingFactory;
use Magento\Quote\Model\ShippingAddressManagement;
use Magento\Quote\Model\ShippingMethodManagement;
use Magento\Sales\Model\Order;

class ShippingProcessor
{
Expand Down Expand Up @@ -65,10 +66,9 @@ public function save(ShippingInterface $shipping, CartInterface $quote)
{
$this->shippingAddressManagement->assign($quote->getId(), $shipping->getAddress());
if (!empty($shipping->getMethod()) && $quote->getItemsCount() > 0) {
$nameComponents = explode('_', $shipping->getMethod());
$carrierCode = array_shift($nameComponents);
// carrier method code can contains more one name component
$methodCode = implode('_', $nameComponents);
$nameComponents = explode(Order::DELIMITER_SHIPPING_METHOD, $shipping->getMethod());
$carrierCode = $nameComponents[0];
$methodCode = $nameComponents[1];
$this->shippingMethodManagement->apply($quote->getId(), $carrierCode, $methodCode);
}
}
Expand Down
3 changes: 2 additions & 1 deletion app/code/Magento/Quote/Model/ShippingMethodManagement.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
use Magento\Quote\Model\Quote\Address\Rate;
use Magento\Quote\Model\Quote\TotalsCollector;
use Magento\Quote\Model\ResourceModel\Quote\Address as QuoteAddressResource;
use Magento\Sales\Model\Order;

/**
* Shipping method read service
Expand Down Expand Up @@ -224,7 +225,7 @@ public function apply($cartId, $carrierCode, $methodCode)
$this->quoteAddressResource->delete($shippingAddress);
throw new StateException(__('The shipping address is missing. Set the address and try again.'));
}
$shippingMethod = $carrierCode . '_' . $methodCode;
$shippingMethod = $carrierCode . Order::DELIMITER_SHIPPING_METHOD . $methodCode;
$shippingAddress->setShippingMethod($shippingMethod);
$shippingAssignments = $quote->getExtensionAttributes()->getShippingAssignments();
if (!empty($shippingAssignments)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ public function testSetMethodWithCouldNotSaveException()
->willReturn($countryId);
$this->shippingAddress->expects($this->once())
->method('setShippingMethod')
->with($carrierCode . '_' . $methodCode);
->with($carrierCode . Order::DELIMITER_SHIPPING_METHOD . $methodCode);
$this->quote->expects($this->once())
->method('getExtensionAttributes')
->willReturn($this->extensionAttributesMock);
Expand All @@ -469,7 +469,7 @@ public function testSetMethodWithCouldNotSaveException()

$this->shippingMock->expects($this->once())
->method('setMethod')
->with($carrierCode . '_' . $methodCode);
->with($carrierCode . Order::DELIMITER_SHIPPING_METHOD . $methodCode);

$this->shippingAssignmentBuilder->expects($this->once())
->method('setShipping')
Expand Down Expand Up @@ -527,7 +527,7 @@ public function testSetMethod()
$this->shippingAddress->expects($this->once())
->method('getCountryId')->willReturn($countryId);
$this->shippingAddress->expects($this->once())
->method('setShippingMethod')->with($carrierCode . '_' . $methodCode);
->method('setShippingMethod')->with($carrierCode . Order::DELIMITER_SHIPPING_METHOD . $methodCode);
$this->quote->expects($this->once())
->method('getExtensionAttributes')
->willReturn($this->extensionAttributesMock);
Expand All @@ -540,7 +540,7 @@ public function testSetMethod()

$this->shippingMock->expects($this->once())
->method('setMethod')
->with($carrierCode . '_' . $methodCode);
->with($carrierCode . Order::DELIMITER_SHIPPING_METHOD . $methodCode);

$this->shippingAssignmentBuilder->expects($this->once())
->method('setShipping')
Expand Down
7 changes: 6 additions & 1 deletion app/code/Magento/Sales/Model/Order.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@ class Order extends AbstractModel implements EntityInterface, OrderInterface
public const REPORT_DATE_TYPE_CREATED = 'created';

public const REPORT_DATE_TYPE_UPDATED = 'updated';

/**
* Delimiters
*/
public const DELIMITER_SHIPPING_METHOD = '::';

/**
* @var string
Expand Down Expand Up @@ -1372,7 +1377,7 @@ public function getShippingMethod($asObject = false)
if (!$asObject || !$shippingMethod) {
return $shippingMethod;
} else {
list($carrierCode, $method) = explode('_', $shippingMethod, 2);
list($carrierCode, $method) = preg_split("/".self::DELIMITER_SHIPPING_METHOD."/", $shippingMethod, 2);
return new \Magento\Framework\DataObject(['carrier_code' => $carrierCode, 'method' => $method]);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Magento\Quote\Api\CartRepositoryInterface;
use Magento\Quote\Api\Data\CartInterface as Quote;
use Magento\Sales\Api\PaymentFailuresInterface;
use Magento\Sales\Model\Order;
use Magento\Store\Model\ScopeInterface;
use Magento\Store\Model\Store;
use Psr\Log\LoggerInterface;
Expand Down Expand Up @@ -215,7 +216,7 @@ private function getShippingMethod(Quote $quote): string
$shippingInfo = $quote->getShippingAddress()->getShippingMethod();

if ($shippingInfo) {
$data = explode('_', $shippingInfo);
$data = preg_split("/".Order::DELIMITER_SHIPPING_METHOD."/", $shippingInfo);
$shippingMethod = $data[0];
}

Expand Down
4 changes: 3 additions & 1 deletion app/code/Magento/Shipping/Model/Config/Source/Allmethods.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
*/
namespace Magento\Shipping\Model\Config\Source;

use Magento\Sales\Model\Order;

/**
* @inheritdoc
*/
Expand Down Expand Up @@ -66,7 +68,7 @@ public function toOptionArray($isActiveOnlyFlag = false)
continue;
}
$methods[$carrierCode]['value'][] = [
'value' => $carrierCode . '_' . $methodCode,
'value' => $carrierCode . Order::DELIMITER_SHIPPING_METHOD . $methodCode,
'label' => '[' . $carrierCode . '] ' . $methodTitle,
];
}
Expand Down