Skip to content

Commit

Permalink
🔃 [Magento Community Engineering] Community Contributions
Browse files Browse the repository at this point in the history
Accepted Community Pull Requests:
 - #26623: #26622 - Check quote item for parentItem instead of parentItemId (by @aligent-lturner)
 - #24691: Allows additional payment checks in payment method list (by @jensscherbl)
 - #26423: Update getCustomer method in order class (by @sertlab)
 - #26621: Set of fixes introduced during #CoreReview 31.01.2020 (by @lbajsarowicz)
 - #26339: Module xml extra end tag removed (by @tejash-wagento)
 - #26546: [fixed My Wish List Product not showing properly between >768px and <… (by @hitesh-wagento)


Fixed GitHub Issues:
 - #26622: Fixed cart discount calculated incorrectly when product first added to cart. (reported by @aligent-lturner) has been fixed in #26623 by @aligent-lturner in 2.4-develop branch
   Related commits:
     1. e943bda
     2. 199afbf
     3. 90c3540
     4. 5f4cc5f
     5. 0e9c4ef
     6. 2c961af
     7. 75b03e0
     8. a037edf

 - #25268: $order->getCustomer() returns NULL for registered customer (reported by @qsolutions-pl) has been fixed in #26423 by @sertlab in 2.4-develop branch
   Related commits:
     1. 92816ab
     2. 427a4ad
     3. f8bcd47
     4. 80c737a
     5. 5a1f48e
     6. 57e2532
     7. 3ca45d8

 - #26338: Code cleanup for module xml extra end tag removed (reported by @tejash-wagento) has been fixed in #26339 by @tejash-wagento in 2.4-develop branch
   Related commits:
     1. 0653539
     2. 399fe8e

 - #26543: My Wish List Product not showing properly between >768px and <1023px (reported by @hitesh-wagento) has been fixed in #26546 by @hitesh-wagento in 2.4-develop branch
   Related commits:
     1. 55e2e64
     2. 6fbdbbe
     3. 13b3ba7
  • Loading branch information
slavvka committed Feb 9, 2020
2 parents 15b5c60 + f156cb4 commit 1027e02
Show file tree
Hide file tree
Showing 13 changed files with 160 additions and 62 deletions.
30 changes: 16 additions & 14 deletions app/code/Magento/CatalogWidget/Block/Product/ProductsList.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

use Magento\Catalog\Api\CategoryRepositoryInterface;
use Magento\Catalog\Block\Product\AbstractProduct;
use Magento\Catalog\Block\Product\Context;
use Magento\Catalog\Block\Product\Widget\Html\Pager;
use Magento\Catalog\Model\Product;
use Magento\Catalog\Model\Product\Visibility;
Expand All @@ -16,7 +17,7 @@
use Magento\Catalog\Pricing\Price\FinalPrice;
use Magento\CatalogWidget\Model\Rule;
use Magento\Framework\App\ActionInterface;
use Magento\Framework\App\Http\Context;
use Magento\Framework\App\Http\Context as HttpContext;
use Magento\Framework\App\ObjectManager;
use Magento\Framework\DataObject\IdentityInterface;
use Magento\Framework\Exception\LocalizedException;
Expand All @@ -27,7 +28,7 @@
use Magento\Framework\View\LayoutFactory;
use Magento\Framework\View\LayoutInterface;
use Magento\Rule\Model\Condition\Combine;
use Magento\Rule\Model\Condition\Sql\Builder;
use Magento\Rule\Model\Condition\Sql\Builder as SqlBuilder;
use Magento\Widget\Block\BlockInterface;
use Magento\Widget\Helper\Conditions;

Expand Down Expand Up @@ -69,7 +70,7 @@ class ProductsList extends AbstractProduct implements BlockInterface, IdentityIn
protected $pager;

/**
* @var Context
* @var HttpContext
*/
protected $httpContext;

Expand All @@ -88,7 +89,7 @@ class ProductsList extends AbstractProduct implements BlockInterface, IdentityIn
protected $productCollectionFactory;

/**
* @var Builder
* @var SqlBuilder
*/
protected $sqlBuilder;

Expand Down Expand Up @@ -135,34 +136,34 @@ class ProductsList extends AbstractProduct implements BlockInterface, IdentityIn
private $categoryRepository;

/**
* @param \Magento\Catalog\Block\Product\Context $context
* @param Context $context
* @param CollectionFactory $productCollectionFactory
* @param Visibility $catalogProductVisibility
* @param Context $httpContext
* @param Builder $sqlBuilder
* @param HttpContext $httpContext
* @param SqlBuilder $sqlBuilder
* @param Rule $rule
* @param Conditions $conditionsHelper
* @param CategoryRepositoryInterface $categoryRepository
* @param array $data
* @param Json|null $json
* @param LayoutFactory|null $layoutFactory
* @param EncoderInterface|null $urlEncoder
* @param CategoryRepositoryInterface|null $categoryRepository
*
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
*/
public function __construct(
\Magento\Catalog\Block\Product\Context $context,
Context $context,
CollectionFactory $productCollectionFactory,
Visibility $catalogProductVisibility,
Context $httpContext,
Builder $sqlBuilder,
HttpContext $httpContext,
SqlBuilder $sqlBuilder,
Rule $rule,
Conditions $conditionsHelper,
CategoryRepositoryInterface $categoryRepository,
array $data = [],
Json $json = null,
LayoutFactory $layoutFactory = null,
EncoderInterface $urlEncoder = null
EncoderInterface $urlEncoder = null,
CategoryRepositoryInterface $categoryRepository = null
) {
$this->productCollectionFactory = $productCollectionFactory;
$this->catalogProductVisibility = $catalogProductVisibility;
Expand All @@ -173,7 +174,8 @@ public function __construct(
$this->json = $json ?: ObjectManager::getInstance()->get(Json::class);
$this->layoutFactory = $layoutFactory ?: ObjectManager::getInstance()->get(LayoutFactory::class);
$this->urlEncoder = $urlEncoder ?: ObjectManager::getInstance()->get(EncoderInterface::class);
$this->categoryRepository = $categoryRepository;
$this->categoryRepository = $categoryRepository ?? ObjectManager::getInstance()
->get(CategoryRepositoryInterface::class);
parent::__construct(
$context,
$data
Expand Down
3 changes: 1 addition & 2 deletions app/code/Magento/Dhl/etc/module.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,5 @@
*/
-->
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd">
<module name="Magento_Dhl" >
</module>
<module name="Magento_Dhl"/>
</config>
29 changes: 21 additions & 8 deletions app/code/Magento/Payment/Model/MethodList.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,29 @@ class MethodList
*/
private $paymentMethodInstanceFactory;

/**
* @var array
*/
private $additionalChecks;

/**
* @param \Magento\Payment\Helper\Data $paymentHelper
* @param Checks\SpecificationFactory $specificationFactory
* @param Checks\SpecificationFactory $specificationFactory
* @param array $additionalChecks
*/
public function __construct(
\Magento\Payment\Helper\Data $paymentHelper,
\Magento\Payment\Model\Checks\SpecificationFactory $specificationFactory
\Magento\Payment\Model\Checks\SpecificationFactory $specificationFactory,
array $additionalChecks = []
) {
$this->paymentHelper = $paymentHelper;
$this->methodSpecificationFactory = $specificationFactory;
$this->additionalChecks = $additionalChecks;
}

/**
* Returns all available payment methods for the given quote.
*
* @param \Magento\Quote\Api\Data\CartInterface $quote
* @return \Magento\Payment\Model\MethodInterface[]
*/
Expand Down Expand Up @@ -80,12 +90,15 @@ public function getAvailableMethods(\Magento\Quote\Api\Data\CartInterface $quote
protected function _canUseMethod($method, \Magento\Quote\Api\Data\CartInterface $quote)
{
return $this->methodSpecificationFactory->create(
[
AbstractMethod::CHECK_USE_CHECKOUT,
AbstractMethod::CHECK_USE_FOR_COUNTRY,
AbstractMethod::CHECK_USE_FOR_CURRENCY,
AbstractMethod::CHECK_ORDER_TOTAL_MIN_MAX,
]
array_merge(
[
AbstractMethod::CHECK_USE_CHECKOUT,
AbstractMethod::CHECK_USE_FOR_COUNTRY,
AbstractMethod::CHECK_USE_FOR_CURRENCY,
AbstractMethod::CHECK_ORDER_TOTAL_MIN_MAX,
],
$this->additionalChecks
)
)->isApplicable(
$method,
$quote
Expand Down
34 changes: 25 additions & 9 deletions app/code/Magento/Payment/Test/Unit/Model/MethodListTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@

namespace Magento\Payment\Test\Unit\Model;

use Magento\Payment\Model\MethodList;
use Magento\Payment\Model\Method\AbstractMethod;
use Magento\Payment\Model\MethodList;

class MethodListTest extends \PHPUnit\Framework\TestCase
{
Expand Down Expand Up @@ -36,6 +36,11 @@ class MethodListTest extends \PHPUnit\Framework\TestCase
*/
protected $specificationFactoryMock;

/**
* @var array $additionalChecks
*/
private $additionalChecks;

protected function setUp()
{
$this->objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
Expand All @@ -49,10 +54,14 @@ protected function setUp()
)->disableOriginalConstructor()->getMock();

$this->specificationFactoryMock = $this->createMock(\Magento\Payment\Model\Checks\SpecificationFactory::class);

$this->additionalChecks = ['acme_custom_payment_method_check' => 'acme_custom_payment_method_check'];

$this->methodList = $this->objectManager->getObject(
\Magento\Payment\Model\MethodList::class,
[
'specificationFactory' => $this->specificationFactoryMock
'specificationFactory' => $this->specificationFactoryMock,
'additionalChecks' => $this->additionalChecks
]
);

Expand All @@ -68,6 +77,9 @@ protected function setUp()
);
}

/**
* Verify available payment methods
*/
public function testGetAvailableMethods()
{
$storeId = 1;
Expand All @@ -90,13 +102,17 @@ public function testGetAvailableMethods()

$this->specificationFactoryMock->expects($this->atLeastOnce())
->method('create')
->with([
AbstractMethod::CHECK_USE_CHECKOUT,
AbstractMethod::CHECK_USE_FOR_COUNTRY,
AbstractMethod::CHECK_USE_FOR_CURRENCY,
AbstractMethod::CHECK_ORDER_TOTAL_MIN_MAX
])
->will($this->returnValue($compositeMock));
->with(
array_merge(
[
AbstractMethod::CHECK_USE_CHECKOUT,
AbstractMethod::CHECK_USE_FOR_COUNTRY,
AbstractMethod::CHECK_USE_FOR_CURRENCY,
AbstractMethod::CHECK_ORDER_TOTAL_MIN_MAX
],
$this->additionalChecks
)
)->will($this->returnValue($compositeMock));

$methodMock = $this->getMockForAbstractClass(\Magento\Payment\Api\Data\PaymentMethodInterface::class);
$this->paymentMethodList->expects($this->once())
Expand Down
3 changes: 1 addition & 2 deletions app/code/Magento/Rss/etc/module.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,5 @@
*/
-->
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd">
<module name="Magento_Rss" >
</module>
<module name="Magento_Rss"/>
</config>
4 changes: 2 additions & 2 deletions app/code/Magento/Sales/Model/Order.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
* @method bool hasCustomerNoteNotify()
* @method bool hasForcedCanCreditmemo()
* @method bool getIsInProcess()
* @method \Magento\Customer\Model\Customer getCustomer()
* @method \Magento\Customer\Model\Customer|null getCustomer()
* @method \Magento\Sales\Model\Order setSendEmail(bool $value)
* @SuppressWarnings(PHPMD.ExcessivePublicCount)
* @SuppressWarnings(PHPMD.TooManyFields)
Expand Down Expand Up @@ -1326,7 +1326,7 @@ public function getTrackingNumbers()
*/
public function getShippingMethod($asObject = false)
{
$shippingMethod = parent::getShippingMethod();
$shippingMethod = $this->getData('shipping_method');
if (!$asObject || !$shippingMethod) {
return $shippingMethod;
} else {
Expand Down
32 changes: 25 additions & 7 deletions app/code/Magento/SalesRule/Model/Validator.php
Original file line number Diff line number Diff line change
Expand Up @@ -383,13 +383,7 @@ public function initTotals($items, Address $address)

foreach ($items as $item) {
//Skipping child items to avoid double calculations
if ($item->getParentItemId()) {
continue;
}
if (!$rule->getActions()->validate($item)) {
continue;
}
if (!$this->canApplyDiscount($item)) {
if (!$this->isValidItemForRule($item, $rule)) {
continue;
}
$qty = $this->validatorUtility->getItemQty($item, $rule);
Expand All @@ -409,6 +403,30 @@ public function initTotals($items, Address $address)
return $this;
}

/**
* Determine if quote item is valid for a given sales rule
*
* @param AbstractItem $item
* @param Rule $rule
* @return bool
*/
private function isValidItemForRule(AbstractItem $item, Rule $rule)
{
if ($item->getParentItemId()) {
return false;
}
if ($item->getParentItem()) {
return false;
}
if (!$rule->getActions()->validate($item)) {
return false;
}
if (!$this->canApplyDiscount($item)) {
return false;
}
return true;
}

/**
* Return item price
*
Expand Down
23 changes: 18 additions & 5 deletions app/code/Magento/SalesRule/Test/Unit/Model/ValidatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
use PHPUnit\Framework\MockObject\MockObject;

/**
* Class ValidatorTest
* Tests for Magento\SalesRule\Model\Validator
* @@SuppressWarnings(PHPMD.CouplingBetweenObjects)
*/
class ValidatorTest extends \PHPUnit\Framework\TestCase
Expand Down Expand Up @@ -346,11 +346,14 @@ public function testInitTotalsCanApplyDiscount()
'getDiscountCalculationPrice',
'getBaseDiscountCalculationPrice',
'getCalculationPrice',
'getParentItemId'
'getParentItemId',
'getParentItem'
]
);
$item2 = clone $item1;
$items = [$item1, $item2];
$item3 = clone $item1;
$item4 = clone $item1;
$items = [$item1, $item2, $item3, $item4];

$rule->expects($this->any())
->method('getSimpleAction')
Expand All @@ -367,12 +370,22 @@ public function testInitTotalsCanApplyDiscount()
$validator->expects($this->at(0))->method('isValid')->with($item1)->willReturn(false);
$validator->expects($this->at(1))->method('isValid')->with($item2)->willReturn(true);

$item1->expects($this->any())->method('getParentItemId')->willReturn(false);
$item1->expects($this->any())->method('getParentItemId')->willReturn(null);
$item1->expects($this->any())->method('getParentItem')->willReturn(null);
$item1->expects($this->never())->method('getDiscountCalculationPrice');
$item1->expects($this->never())->method('getBaseDiscountCalculationPrice');
$item2->expects($this->any())->method('getParentItemId')->willReturn(false);
$item2->expects($this->any())->method('getParentItemId')->willReturn(null);
$item2->expects($this->any())->method('getParentItem')->willReturn(null);
$item2->expects($this->any())->method('getDiscountCalculationPrice')->willReturn(50);
$item2->expects($this->once())->method('getBaseDiscountCalculationPrice')->willReturn(50);
$item3->expects($this->any())->method('getParentItemId')->willReturn(null);
$item3->expects($this->any())->method('getParentItem')->willReturn($item1);
$item3->expects($this->never())->method('getDiscountCalculationPrice');
$item3->expects($this->never())->method('getBaseDiscountCalculationPrice');
$item4->expects($this->any())->method('getParentItemId')->willReturn(12345);
$item4->expects($this->any())->method('getParentItem')->willReturn(null);
$item4->expects($this->never())->method('getDiscountCalculationPrice');
$item4->expects($this->never())->method('getBaseDiscountCalculationPrice');
$this->utility->expects($this->once())->method('getItemQty')->willReturn(1);
$this->utility->expects($this->any())->method('canProcessRule')->willReturn(true);

Expand Down
3 changes: 1 addition & 2 deletions app/code/Magento/Translation/etc/module.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,5 @@
*/
-->
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd">
<module name="Magento_Translation" >
</module>
<module name="Magento_Translation"/>
</config>
3 changes: 1 addition & 2 deletions app/code/Magento/Ups/etc/module.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,5 @@
*/
-->
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd">
<module name="Magento_Ups" >
</module>
<module name="Magento_Ups"/>
</config>
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,13 @@
.products-grid.wishlist {
.product {
&-item {
margin-left: 2%;
width: calc(~'(100% - 4%) / 3');

&:nth-child(3n + 1) {
margin-left: 0;
}

&-photo {
display: block;
margin-bottom: @indent__s;
Expand Down Expand Up @@ -185,6 +192,9 @@
.products-grid.wishlist {
.product-item {
border-bottom: 1px solid @secondary__color;
margin: 0;
width: 100%;

&:first-child {
border-top: 1px solid @secondary__color;
}
Expand Down
Loading

0 comments on commit 1027e02

Please sign in to comment.