Skip to content

Commit 633cbc2

Browse files
committed
ACP2E-3176: [Cloud] quick order large amount of SKU performance
1 parent 71432ae commit 633cbc2

File tree

4 files changed

+136
-5
lines changed

4 files changed

+136
-5
lines changed

app/code/Magento/Msrp/Model/Quote/Address/CanApplyMsrp.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ public function __construct(\Magento\Msrp\Helper\Data $msrpHelper)
2121
}
2222

2323
/**
24+
* Checks whether MSRP can be applied to the address
25+
*
2426
* @param \Magento\Quote\Model\Quote\Address $address
2527
* @return bool
2628
*/
@@ -29,8 +31,8 @@ public function isCanApplyMsrp($address)
2931
$canApplyMsrp = false;
3032
foreach ($address->getAllItems() as $item) {
3133
if (!$item->getParentItemId()
32-
&& $this->msrpHelper->isShowBeforeOrderConfirm($item->getProductId())
33-
&& $this->msrpHelper->isMinimalPriceLessMsrp($item->getProductId())
34+
&& $this->msrpHelper->isShowBeforeOrderConfirm($item->getProduct())
35+
&& $this->msrpHelper->isMinimalPriceLessMsrp($item->getProduct())
3436
) {
3537
$canApplyMsrp = true;
3638
break;
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
<?php
2+
/**
3+
* Copyright 2024 Adobe
4+
* All Rights Reserved.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Msrp\Test\Unit\Model\Quote\Address;
9+
10+
use Magento\Catalog\Model\Product;
11+
use Magento\Msrp\Helper\Data;
12+
use Magento\Msrp\Model\Quote\Address\CanApplyMsrp;
13+
use Magento\Quote\Model\Quote\Address;
14+
use Magento\Quote\Model\Quote\Item;
15+
use PHPUnit\Framework\MockObject\MockObject;
16+
use PHPUnit\Framework\TestCase;
17+
18+
class CanApplyMsrpTest extends TestCase
19+
{
20+
/**
21+
* @var CanApplyMsrp|null
22+
*/
23+
private $canApplyMsrp;
24+
25+
/**
26+
* @var Data|MockObject|null
27+
*/
28+
private $msrpHelper;
29+
30+
/**
31+
* @var Address|MockObject|null
32+
*/
33+
private $address;
34+
35+
protected function setUp(): void
36+
{
37+
$this->msrpHelper = $this->createMock(Data::class);
38+
$this->address = $this->createMock(Address::class);
39+
$this->canApplyMsrp = new CanApplyMsrp($this->msrpHelper);
40+
}
41+
42+
public function testIsCanApplyMsrpWhenIsShowBeforeOrderConfirmAndIsMinimalPriceLessMsrpReturnTrue(): void
43+
{
44+
$item = $this->createPartialMock(Item::class, ['getProduct']);
45+
$product = $this->createMock(Product::class);
46+
$item->expects($this->exactly(2))->method('getProduct')->willReturn($product);
47+
$this->msrpHelper->expects($this->once())->method('isShowBeforeOrderConfirm')->with($product)->willReturn(true);
48+
$this->msrpHelper->expects($this->once())->method('isMinimalPriceLessMsrp')->with($product)->willReturn(true);
49+
$this->address->expects($this->once())->method('getAllItems')->willReturn([$item]);
50+
$this->assertTrue($this->canApplyMsrp->isCanApplyMsrp($this->address));
51+
}
52+
53+
public function testIsCanApplyMsrpWhenIsMinimalPriceLessMsrpReturnsFalse(): void
54+
{
55+
$item = $this->createPartialMock(Item::class, ['getProduct']);
56+
$product = $this->createMock(Product::class);
57+
$item->expects($this->exactly(2))->method('getProduct')->willReturn($product);
58+
$this->msrpHelper->expects($this->once())->method('isShowBeforeOrderConfirm')->with($product)->willReturn(true);
59+
$this->msrpHelper->expects($this->once())->method('isMinimalPriceLessMsrp')->with($product)->willReturn(false);
60+
$this->address->expects($this->once())->method('getAllItems')->willReturn([$item]);
61+
$this->assertFalse($this->canApplyMsrp->isCanApplyMsrp($this->address));
62+
}
63+
64+
public function testIsCanApplyMsrpWhenIsShowBeforeOrderConfirmReturnsFalse(): void
65+
{
66+
$item = $this->createPartialMock(Item::class, ['getProduct']);
67+
$product = $this->createMock(Product::class);
68+
$item->expects($this->exactly(1))->method('getProduct')->willReturn($product);
69+
$this->msrpHelper->expects($this->once())
70+
->method('isShowBeforeOrderConfirm')
71+
->with($product)
72+
->willReturn(false);
73+
$this->msrpHelper->expects($this->never())->method('isMinimalPriceLessMsrp')->with($product);
74+
$this->address->expects($this->once())->method('getAllItems')->willReturn([$item]);
75+
$this->assertFalse($this->canApplyMsrp->isCanApplyMsrp($this->address));
76+
}
77+
}

app/code/Magento/SalesRule/Model/Rule/Condition/Product.php

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@
77

88
/**
99
* Product rule condition data model
10-
*
11-
* @author Magento Core Team <core@magentocommerce.com>
1210
*/
1311
class Product extends \Magento\Rule\Model\Condition\Product\AbstractProduct
1412
{
@@ -194,7 +192,26 @@ public function validate(\Magento\Framework\Model\AbstractModel $model)
194192
}
195193
}
196194

197-
return parent::validate($product);
195+
/**
196+
* \Magento\Rule\Model\Condition\AbstractCondition::validate() will attempt to reload the product
197+
* if the attribute value is missing in the product. We need to ensure that logic is not executed
198+
* because not only it is unnecessary as all attributes used in rules conditions are loaded into
199+
* the product model, but it also causes performance issues.
200+
* @see \Magento\Rule\Model\Condition\AbstractCondition::validate()
201+
* @see \Magento\SalesRule\Model\Plugin\QuoteConfigProductAttributes::afterLoadAttributes()
202+
*/
203+
$hasAttribute = $product->hasData($attrCode);
204+
// value is most likely null if $hasAttribute === false, but it could be provided from custom attributes
205+
$value = $product->getData($attrCode);
206+
if (!$hasAttribute) {
207+
$product->setData($attrCode, $value);
208+
}
209+
$isValid = parent::validate($product);
210+
if (!$hasAttribute && $value === null) {
211+
$product->unsetData($attrCode);
212+
}
213+
214+
return $isValid;
198215
}
199216

200217
/**

app/code/Magento/SalesRule/Test/Unit/Model/Rule/Condition/ProductTest.php

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Magento\Eav\Model\Entity\AttributeLoaderInterface;
1919
use Magento\Eav\Model\ResourceModel\Entity\Attribute\Set\Collection;
2020
use Magento\Framework\App\ScopeResolverInterface;
21+
use Magento\Framework\DataObject;
2122
use Magento\Framework\DB\Adapter\AdapterInterface;
2223
use Magento\Framework\DB\Select;
2324
use Magento\Framework\Locale\Format;
@@ -336,4 +337,38 @@ public static function localisationProvider(): array
336337
'smallPrice' => [false, '1,500.03', '>=', 1000],
337338
];
338339
}
340+
341+
public function testValidateWhenAttributeValueIsMissingInTheProduct(): void
342+
{
343+
$attributeCode = 'test_attr';
344+
$attribute = new DataObject();
345+
$attribute->setBackendType('varchar');
346+
$attribute->setFrontendInput('text');
347+
348+
$newResource = $this->createPartialMock(Product::class, ['getAttribute']);
349+
$newResource->expects($this->any())
350+
->method('getAttribute')
351+
->with($attributeCode)
352+
->willReturn($attribute);
353+
354+
$product = $this->getMockBuilder(\Magento\Catalog\Model\Product::class)
355+
->disableOriginalConstructor()
356+
->onlyMethods(['getId', 'load', 'getResource'])
357+
->getMock();
358+
$product->method('getId')
359+
->willReturn(1);
360+
$product->expects($this->never())
361+
->method('load')
362+
->willReturnSelf();
363+
$product->expects($this->atLeastOnce())
364+
->method('getResource')
365+
->willReturn($newResource);
366+
367+
$item = $this->createMock(AbstractItem::class);
368+
$item->expects($this->any())
369+
->method('getProduct')
370+
->willReturn($product);
371+
$this->model->setAttribute($attributeCode);
372+
$this->model->validate($item);
373+
}
339374
}

0 commit comments

Comments
 (0)