Skip to content

Commit 25f6bcf

Browse files
slavvkaslavvka
authored
Merge pull request #691 from magento-south/BUGS
[SOUTH] Bugs: - MAGETWO-52925 Simple child product without a special price still shown as "was (original price)" #4442 #5097 #6645 - for mainline - MAGETWO-60098 Configurable product option price is displayed incorrectly per website - MAGETWO-56793 [GITHUB][PR] Fix Magento\Review\Model\ResourceModel\Rating\Option not instantiable in setup scripts #5465 - MAGETWO-58078 [FT] CreateProductAttributeEntityFromProductPageTest fails - MAGETWO-61725 [GITHUB] Improve address save flow to allow to use custom validators #7552
2 parents cd637e5 + e512fed commit 25f6bcf

File tree

13 files changed

+235
-263
lines changed

13 files changed

+235
-263
lines changed

‎app/code/Magento/Catalog/Model/ResourceModel/Product/StatusBaseSelectProcessor.php

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
use Magento\Eav\Model\Config;
1212
use Magento\Framework\DB\Select;
1313
use Magento\Framework\EntityManager\MetadataPool;
14+
use Magento\Store\Api\StoreResolverInterface;
15+
use Magento\Store\Model\Store;
1416

1517
/**
1618
* Class StatusBaseSelectProcessor
@@ -27,16 +29,24 @@ class StatusBaseSelectProcessor implements BaseSelectProcessorInterface
2729
*/
2830
private $metadataPool;
2931

32+
/**
33+
* @var StoreResolverInterface
34+
*/
35+
private $storeResolver;
36+
3037
/**
3138
* @param Config $eavConfig
3239
* @param MetadataPool $metadataPool
40+
* @param StoreResolverInterface $storeResolver
3341
*/
3442
public function __construct(
3543
Config $eavConfig,
36-
MetadataPool $metadataPool
44+
MetadataPool $metadataPool,
45+
StoreResolverInterface $storeResolver
3746
) {
3847
$this->eavConfig = $eavConfig;
3948
$this->metadataPool = $metadataPool;
49+
$this->storeResolver = $storeResolver;
4050
}
4151

4252
/**
@@ -48,13 +58,23 @@ public function process(Select $select)
4858
$linkField = $this->metadataPool->getMetadata(ProductInterface::class)->getLinkField();
4959
$statusAttribute = $this->eavConfig->getAttribute(Product::ENTITY, ProductInterface::STATUS);
5060

51-
$select->join(
61+
$select->joinLeft(
62+
['status_global_attr' => $statusAttribute->getBackendTable()],
63+
"status_global_attr.{$linkField} = " . self::PRODUCT_TABLE_ALIAS . ".{$linkField}"
64+
. ' AND status_global_attr.attribute_id = ' . (int)$statusAttribute->getAttributeId()
65+
. ' AND status_global_attr.store_id = ' . Store::DEFAULT_STORE_ID,
66+
[]
67+
);
68+
69+
$select->joinLeft(
5270
['status_attr' => $statusAttribute->getBackendTable()],
53-
sprintf('status_attr.%s = %s.%1$s', $linkField, self::PRODUCT_TABLE_ALIAS),
71+
"status_attr.{$linkField} = " . self::PRODUCT_TABLE_ALIAS . ".{$linkField}"
72+
. ' AND status_attr.attribute_id = ' . (int)$statusAttribute->getAttributeId()
73+
. ' AND status_attr.store_id = ' . $this->storeResolver->getCurrentStoreId(),
5474
[]
55-
)
56-
->where('status_attr.attribute_id = ?', $statusAttribute->getAttributeId())
57-
->where('status_attr.value = ?', Status::STATUS_ENABLED);
75+
);
76+
77+
$select->where('IFNULL(status_attr.value, status_global_attr.value) = ?', Status::STATUS_ENABLED);
5878

5979
return $select;
6080
}

‎app/code/Magento/Catalog/Test/Unit/Model/ResourceModel/Product/StatusBaseSelectProcessorTest.php

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,12 @@
1616
use Magento\Framework\EntityManager\EntityMetadataInterface;
1717
use Magento\Framework\EntityManager\MetadataPool;
1818
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
19+
use Magento\Store\Api\StoreResolverInterface;
20+
use Magento\Store\Model\Store;
1921

22+
/**
23+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
24+
*/
2025
class StatusBaseSelectProcessorTest extends \PHPUnit_Framework_TestCase
2126
{
2227
/**
@@ -29,6 +34,11 @@ class StatusBaseSelectProcessorTest extends \PHPUnit_Framework_TestCase
2934
*/
3035
private $metadataPool;
3136

37+
/**
38+
* @var StoreResolverInterface|\PHPUnit_Framework_MockObject_MockObject
39+
*/
40+
private $storeResolver;
41+
3242
/**
3343
* @var Select|\PHPUnit_Framework_MockObject_MockObject
3444
*/
@@ -43,19 +53,22 @@ protected function setUp()
4353
{
4454
$this->eavConfig = $this->getMockBuilder(Config::class)->disableOriginalConstructor()->getMock();
4555
$this->metadataPool = $this->getMockBuilder(MetadataPool::class)->disableOriginalConstructor()->getMock();
56+
$this->storeResolver = $this->getMockBuilder(StoreResolverInterface::class)->getMock();
4657
$this->select = $this->getMockBuilder(Select::class)->disableOriginalConstructor()->getMock();
4758

4859
$this->statusBaseSelectProcessor = (new ObjectManager($this))->getObject(StatusBaseSelectProcessor::class, [
4960
'eavConfig' => $this->eavConfig,
5061
'metadataPool' => $this->metadataPool,
62+
'storeResolver' => $this->storeResolver,
5163
]);
5264
}
5365

5466
public function testProcess()
5567
{
5668
$linkField = 'link_field';
5769
$backendTable = 'backend_table';
58-
$attributeId = 'attribute_id';
70+
$attributeId = 2;
71+
$currentStoreId = 1;
5972

6073
$metadata = $this->getMock(EntityMetadataInterface::class);
6174
$metadata->expects($this->once())
@@ -66,35 +79,49 @@ public function testProcess()
6679
->with(ProductInterface::class)
6780
->willReturn($metadata);
6881

82+
/** @var AttributeInterface|\PHPUnit_Framework_MockObject_MockObject $statusAttribute */
6983
$statusAttribute = $this->getMockBuilder(AttributeInterface::class)
7084
->setMethods(['getBackendTable', 'getAttributeId'])
7185
->getMock();
72-
$statusAttribute->expects($this->once())
86+
$statusAttribute->expects($this->atLeastOnce())
7387
->method('getBackendTable')
7488
->willReturn($backendTable);
75-
$statusAttribute->expects($this->once())
89+
$statusAttribute->expects($this->atLeastOnce())
7690
->method('getAttributeId')
7791
->willReturn($attributeId);
7892
$this->eavConfig->expects($this->once())
7993
->method('getAttribute')
8094
->with(Product::ENTITY, ProductInterface::STATUS)
8195
->willReturn($statusAttribute);
8296

83-
$this->select->expects($this->once())
84-
->method('join')
97+
$this->storeResolver->expects($this->once())
98+
->method('getCurrentStoreId')
99+
->willReturn($currentStoreId);
100+
101+
$this->select->expects($this->at(0))
102+
->method('joinLeft')
85103
->with(
86-
['status_attr' => $backendTable],
87-
sprintf('status_attr.%s = %s.%1$s', $linkField, BaseSelectProcessorInterface::PRODUCT_TABLE_ALIAS),
104+
['status_global_attr' => $backendTable],
105+
"status_global_attr.{$linkField} = "
106+
. BaseSelectProcessorInterface::PRODUCT_TABLE_ALIAS . ".{$linkField}"
107+
. " AND status_global_attr.attribute_id = {$attributeId}"
108+
. ' AND status_global_attr.store_id = ' . Store::DEFAULT_STORE_ID,
88109
[]
89110
)
90111
->willReturnSelf();
91112
$this->select->expects($this->at(1))
92-
->method('where')
93-
->with('status_attr.attribute_id = ?', $attributeId)
113+
->method('joinLeft')
114+
->with(
115+
['status_attr' => $backendTable],
116+
"status_attr.{$linkField} = " . BaseSelectProcessorInterface::PRODUCT_TABLE_ALIAS . ".{$linkField}"
117+
. " AND status_attr.attribute_id = {$attributeId}"
118+
. " AND status_attr.store_id = {$currentStoreId}",
119+
[]
120+
)
94121
->willReturnSelf();
95122
$this->select->expects($this->at(2))
96123
->method('where')
97-
->with('status_attr.value = ?', Status::STATUS_ENABLED)
124+
->with('IFNULL(status_attr.value, status_global_attr.value) = ?', Status::STATUS_ENABLED)
98125
->willReturnSelf();
99126

100127
$this->assertEquals($this->select, $this->statusBaseSelectProcessor->process($this->select));

‎app/code/Magento/ConfigurableProduct/Model/ResourceModel/Product/Indexer/Price/Configurable.php

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,45 @@
99

1010
use Magento\Catalog\Api\Data\ProductInterface;
1111
use Magento\Catalog\Model\Product\Attribute\Source\Status as ProductStatus;
12+
use Magento\Catalog\Model\Product\Attribute\Source\Status;
13+
use Magento\Store\Api\StoreResolverInterface;
14+
use Magento\Store\Model\Store;
1215

16+
/**
17+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
18+
*/
1319
class Configurable extends \Magento\Catalog\Model\ResourceModel\Product\Indexer\Price\DefaultPrice
1420
{
21+
/**
22+
* @var StoreResolverInterface
23+
*/
24+
private $storeResolver;
25+
26+
/**
27+
* Class constructor
28+
*
29+
* @param \Magento\Framework\Model\ResourceModel\Db\Context $context
30+
* @param \Magento\Framework\Indexer\Table\StrategyInterface $tableStrategy
31+
* @param \Magento\Eav\Model\Config $eavConfig
32+
* @param \Magento\Framework\Event\ManagerInterface $eventManager
33+
* @param \Magento\Framework\Module\Manager $moduleManager
34+
* @param string $connectionName
35+
* @param StoreResolverInterface $storeResolver
36+
*/
37+
public function __construct(
38+
\Magento\Framework\Model\ResourceModel\Db\Context $context,
39+
\Magento\Framework\Indexer\Table\StrategyInterface $tableStrategy,
40+
\Magento\Eav\Model\Config $eavConfig,
41+
\Magento\Framework\Event\ManagerInterface $eventManager,
42+
\Magento\Framework\Module\Manager $moduleManager,
43+
$connectionName = null,
44+
StoreResolverInterface $storeResolver = null
45+
) {
46+
parent::__construct($context, $tableStrategy, $eavConfig, $eventManager, $moduleManager, $connectionName);
47+
$this->storeResolver = $storeResolver ?:
48+
\Magento\Framework\App\ObjectManager::getInstance()->get(StoreResolverInterface::class);
49+
}
50+
1551
/**
1652
* Reindex temporary (price result data) for all products
1753
*
@@ -190,16 +226,21 @@ protected function _applyConfigurableOption()
190226
[]
191227
)->where(
192228
'le.required_options=0'
193-
)->join(
194-
['product_status' => $this->getTable($statusAttribute->getBackend()->getTable())],
195-
sprintf(
196-
'le.%1$s = product_status.%1$s AND product_status.attribute_id = %2$s',
197-
$linkField,
198-
$statusAttribute->getAttributeId()
199-
),
229+
)->joinLeft(
230+
['status_global_attr' => $statusAttribute->getBackendTable()],
231+
"status_global_attr.{$linkField} = le.{$linkField}"
232+
. ' AND status_global_attr.attribute_id = ' . (int)$statusAttribute->getAttributeId()
233+
. ' AND status_global_attr.store_id = ' . Store::DEFAULT_STORE_ID,
234+
[]
235+
)->joinLeft(
236+
['status_attr' => $statusAttribute->getBackendTable()],
237+
"status_attr.{$linkField} = le.{$linkField}"
238+
. ' AND status_attr.attribute_id = ' . (int)$statusAttribute->getAttributeId()
239+
. ' AND status_attr.store_id = ' . $this->storeResolver->getCurrentStoreId(),
200240
[]
201241
)->where(
202-
'product_status.value=' . ProductStatus::STATUS_ENABLED
242+
'IFNULL(status_attr.value, status_global_attr.value) = ?',
243+
Status::STATUS_ENABLED
203244
)->group(
204245
['e.entity_id', 'i.customer_group_id', 'i.website_id', 'l.product_id']
205246
);

‎app/code/Magento/Customer/Model/Address/AbstractAddress.php

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,8 @@ public function getDataModel($defaultBillingAddressId = null, $defaultShippingAd
554554
/**
555555
* Validate address attribute values
556556
*
557+
*
558+
*
557559
* @return bool|array
558560
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
559561
* @SuppressWarnings(PHPMD.NPathComplexity)
@@ -562,23 +564,24 @@ public function validate()
562564
{
563565
$errors = [];
564566
if (!\Zend_Validate::is($this->getFirstname(), 'NotEmpty')) {
565-
$errors[] = __('Please enter the first name.');
567+
$errors[] = __('%fieldName is a required field.', ['fieldName' => 'firstname']);
566568
}
567569

568570
if (!\Zend_Validate::is($this->getLastname(), 'NotEmpty')) {
569-
$errors[] = __('Please enter the last name.');
571+
$errors[] = __('%fieldName is a required field.', ['fieldName' => 'lastname']);
570572
}
571573

572574
if (!\Zend_Validate::is($this->getStreetLine(1), 'NotEmpty')) {
573-
$errors[] = __('Please enter the street.');
575+
$errors[] = __('%fieldName is a required field.', ['fieldName' => 'street']);
574576
}
575577

576578
if (!\Zend_Validate::is($this->getCity(), 'NotEmpty')) {
577-
$errors[] = __('Please enter the city.');
579+
$errors[] = __('%fieldName is a required field.', ['fieldName' => 'city']);
578580
}
579581

580582
if (!\Zend_Validate::is($this->getTelephone(), 'NotEmpty')) {
581-
$errors[] = __('Please enter the phone number.');
583+
$errors[] = __('%fieldName is a required field.', ['fieldName' => 'telephone']);
584+
582585
}
583586

584587
$_havingOptionalZip = $this->_directoryData->getCountriesWithOptionalZip();
@@ -590,11 +593,11 @@ public function validate()
590593
'NotEmpty'
591594
)
592595
) {
593-
$errors[] = __('Please enter the zip/postal code.');
596+
$errors[] = __('%fieldName is a required field.', ['fieldName' => 'postcode']);
594597
}
595598

596599
if (!\Zend_Validate::is($this->getCountryId(), 'NotEmpty')) {
597-
$errors[] = __('Please enter the country.');
600+
$errors[] = __('%fieldName is a required field.', ['fieldName' => 'countryId']);
598601
}
599602

600603
if ($this->getCountryModel()->getRegionCollection()->getSize() && !\Zend_Validate::is(
@@ -604,7 +607,7 @@ public function validate()
604607
$this->getCountryId()
605608
)
606609
) {
607-
$errors[] = __('Please enter the state/province.');
610+
$errors[] = __('%fieldName is a required field.', ['fieldName' => 'regionId']);
608611
}
609612

610613
if (empty($errors) || $this->getShouldIgnoreValidation()) {

‎app/code/Magento/Customer/Model/ResourceModel/AddressRepository.php

Lines changed: 6 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,12 @@ public function save(\Magento\Customer\Api\Data\AddressInterface $address)
124124
$addressModel->updateData($address);
125125
}
126126

127-
$inputException = $this->_validate($addressModel);
128-
if ($inputException->wasErrorAdded()) {
127+
$errors = $addressModel->validate();
128+
if ($errors !== true) {
129+
$inputException = new InputException();
130+
foreach ($errors as $error) {
131+
$inputException->addError($error);
132+
}
129133
throw $inputException;
130134
}
131135
$addressModel->save();
@@ -255,70 +259,6 @@ public function deleteById($addressId)
255259
return true;
256260
}
257261

258-
/**
259-
* Validate Customer Addresses attribute values.
260-
*
261-
* @param CustomerAddressModel $customerAddressModel the model to validate
262-
* @return InputException
263-
*
264-
* @SuppressWarnings(PHPMD.NPathComplexity)
265-
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
266-
*/
267-
private function _validate(CustomerAddressModel $customerAddressModel)
268-
{
269-
$exception = new InputException();
270-
if ($customerAddressModel->getShouldIgnoreValidation()) {
271-
return $exception;
272-
}
273-
274-
if (!\Zend_Validate::is($customerAddressModel->getFirstname(), 'NotEmpty')) {
275-
$exception->addError(__('%fieldName is a required field.', ['fieldName' => 'firstname']));
276-
}
277-
278-
if (!\Zend_Validate::is($customerAddressModel->getLastname(), 'NotEmpty')) {
279-
$exception->addError(__('%fieldName is a required field.', ['fieldName' => 'lastname']));
280-
}
281-
282-
if (!\Zend_Validate::is($customerAddressModel->getStreetLine(1), 'NotEmpty')) {
283-
$exception->addError(__('%fieldName is a required field.', ['fieldName' => 'street']));
284-
}
285-
286-
if (!\Zend_Validate::is($customerAddressModel->getCity(), 'NotEmpty')) {
287-
$exception->addError(__('%fieldName is a required field.', ['fieldName' => 'city']));
288-
}
289-
290-
if (!\Zend_Validate::is($customerAddressModel->getTelephone(), 'NotEmpty')) {
291-
$exception->addError(__('%fieldName is a required field.', ['fieldName' => 'telephone']));
292-
}
293-
294-
$havingOptionalZip = $this->directoryData->getCountriesWithOptionalZip();
295-
if (!in_array($customerAddressModel->getCountryId(), $havingOptionalZip)
296-
&& !\Zend_Validate::is($customerAddressModel->getPostcode(), 'NotEmpty')
297-
) {
298-
$exception->addError(__('%fieldName is a required field.', ['fieldName' => 'postcode']));
299-
}
300-
301-
if (!\Zend_Validate::is($customerAddressModel->getCountryId(), 'NotEmpty')) {
302-
$exception->addError(__('%fieldName is a required field.', ['fieldName' => 'countryId']));
303-
}
304-
305-
if ($this->directoryData->isRegionRequired($customerAddressModel->getCountryId())) {
306-
$regionCollection = $customerAddressModel->getCountryModel()->getRegionCollection();
307-
if (!$regionCollection->count() && empty($customerAddressModel->getRegion())) {
308-
$exception->addError(__('%fieldName is a required field.', ['fieldName' => 'region']));
309-
} elseif (
310-
$regionCollection->count()
311-
&& !in_array(
312-
$customerAddressModel->getRegionId(),
313-
array_column($regionCollection->getData(), 'region_id')
314-
)
315-
) {
316-
$exception->addError(__('%fieldName is a required field.', ['fieldName' => 'regionId']));
317-
}
318-
}
319-
return $exception;
320-
}
321-
322262
/**
323263
* Retrieve collection processor
324264
*

0 commit comments

Comments
 (0)