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
magento · Dec 26, 2016 · 25f6bcf · 25f6bcf
2 parents cd637e5 + e512fed
commit 25f6bcf
Show file tree

Hide file tree

Showing 13 changed files with 235 additions and 263 deletions.
diff --git a/app/code/Magento/Catalog/Model/ResourceModel/Product/StatusBaseSelectProcessor.php b/app/code/Magento/Catalog/Model/ResourceModel/Product/StatusBaseSelectProcessor.php
@@ -11,6 +11,8 @@
 use Magento\Eav\Model\Config;
 use Magento\Framework\DB\Select;
 use Magento\Framework\EntityManager\MetadataPool;
+use Magento\Store\Api\StoreResolverInterface;
+use Magento\Store\Model\Store;
 
 /**
  * Class StatusBaseSelectProcessor
@@ -27,16 +29,24 @@ class StatusBaseSelectProcessor implements BaseSelectProcessorInterface
      */
     private $metadataPool;
 
+    /**
+     * @var StoreResolverInterface
+     */
+    private $storeResolver;
+
     /**
      * @param Config $eavConfig
      * @param MetadataPool $metadataPool
+     * @param StoreResolverInterface $storeResolver
      */
     public function __construct(
         Config $eavConfig,
-        MetadataPool $metadataPool
+        MetadataPool $metadataPool,
+        StoreResolverInterface $storeResolver
     ) {
         $this->eavConfig = $eavConfig;
         $this->metadataPool = $metadataPool;
+        $this->storeResolver = $storeResolver;
     }
 
     /**
@@ -48,13 +58,23 @@ public function process(Select $select)
         $linkField = $this->metadataPool->getMetadata(ProductInterface::class)->getLinkField();
         $statusAttribute = $this->eavConfig->getAttribute(Product::ENTITY, ProductInterface::STATUS);
 
-        $select->join(
+        $select->joinLeft(
+            ['status_global_attr' => $statusAttribute->getBackendTable()],
+            "status_global_attr.{$linkField} = " . self::PRODUCT_TABLE_ALIAS . ".{$linkField}"
+            . ' AND status_global_attr.attribute_id = ' . (int)$statusAttribute->getAttributeId()
+            . ' AND status_global_attr.store_id = ' . Store::DEFAULT_STORE_ID,
+            []
+        );
+
+        $select->joinLeft(
             ['status_attr' => $statusAttribute->getBackendTable()],
-            sprintf('status_attr.%s = %s.%1$s', $linkField, self::PRODUCT_TABLE_ALIAS),
+            "status_attr.{$linkField} = " . self::PRODUCT_TABLE_ALIAS . ".{$linkField}"
+            . ' AND status_attr.attribute_id = ' . (int)$statusAttribute->getAttributeId()
+            . ' AND status_attr.store_id = ' . $this->storeResolver->getCurrentStoreId(),
             []
-        )
-            ->where('status_attr.attribute_id = ?', $statusAttribute->getAttributeId())
-            ->where('status_attr.value = ?', Status::STATUS_ENABLED);
+        );
+
+        $select->where('IFNULL(status_attr.value, status_global_attr.value) = ?', Status::STATUS_ENABLED);
 
         return $select;
     }

diff --git a/...e/Magento/Catalog/Test/Unit/Model/ResourceModel/Product/StatusBaseSelectProcessorTest.php b/...e/Magento/Catalog/Test/Unit/Model/ResourceModel/Product/StatusBaseSelectProcessorTest.php
@@ -16,7 +16,12 @@
 use Magento\Framework\EntityManager\EntityMetadataInterface;
 use Magento\Framework\EntityManager\MetadataPool;
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
+use Magento\Store\Api\StoreResolverInterface;
+use Magento\Store\Model\Store;
 
+/**
+ * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
+ */
 class StatusBaseSelectProcessorTest extends \PHPUnit_Framework_TestCase
 {
     /**
@@ -29,6 +34,11 @@ class StatusBaseSelectProcessorTest extends \PHPUnit_Framework_TestCase
      */
     private $metadataPool;
 
+    /**
+     * @var StoreResolverInterface|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $storeResolver;
+
     /**
      * @var Select|\PHPUnit_Framework_MockObject_MockObject
      */
@@ -43,19 +53,22 @@ protected function setUp()
     {
         $this->eavConfig = $this->getMockBuilder(Config::class)->disableOriginalConstructor()->getMock();
         $this->metadataPool = $this->getMockBuilder(MetadataPool::class)->disableOriginalConstructor()->getMock();
+        $this->storeResolver = $this->getMockBuilder(StoreResolverInterface::class)->getMock();
         $this->select = $this->getMockBuilder(Select::class)->disableOriginalConstructor()->getMock();
 
         $this->statusBaseSelectProcessor =  (new ObjectManager($this))->getObject(StatusBaseSelectProcessor::class, [
             'eavConfig' => $this->eavConfig,
             'metadataPool' => $this->metadataPool,
+            'storeResolver' => $this->storeResolver,
         ]);
     }
 
     public function testProcess()
     {
         $linkField = 'link_field';
         $backendTable = 'backend_table';
-        $attributeId = 'attribute_id';
+        $attributeId = 2;
+        $currentStoreId = 1;
 
         $metadata = $this->getMock(EntityMetadataInterface::class);
         $metadata->expects($this->once())
@@ -66,35 +79,49 @@ public function testProcess()
             ->with(ProductInterface::class)
             ->willReturn($metadata);
 
+        /** @var AttributeInterface|\PHPUnit_Framework_MockObject_MockObject $statusAttribute */
         $statusAttribute = $this->getMockBuilder(AttributeInterface::class)
             ->setMethods(['getBackendTable', 'getAttributeId'])
             ->getMock();
-        $statusAttribute->expects($this->once())
+        $statusAttribute->expects($this->atLeastOnce())
             ->method('getBackendTable')
             ->willReturn($backendTable);
-        $statusAttribute->expects($this->once())
+        $statusAttribute->expects($this->atLeastOnce())
             ->method('getAttributeId')
             ->willReturn($attributeId);
         $this->eavConfig->expects($this->once())
             ->method('getAttribute')
             ->with(Product::ENTITY, ProductInterface::STATUS)
             ->willReturn($statusAttribute);
 
-        $this->select->expects($this->once())
-            ->method('join')
+        $this->storeResolver->expects($this->once())
+            ->method('getCurrentStoreId')
+            ->willReturn($currentStoreId);
+
+        $this->select->expects($this->at(0))
+            ->method('joinLeft')
             ->with(
-                ['status_attr' => $backendTable],
-                sprintf('status_attr.%s = %s.%1$s', $linkField, BaseSelectProcessorInterface::PRODUCT_TABLE_ALIAS),
+                ['status_global_attr' => $backendTable],
+                "status_global_attr.{$linkField} = "
+                . BaseSelectProcessorInterface::PRODUCT_TABLE_ALIAS . ".{$linkField}"
+                . " AND status_global_attr.attribute_id = {$attributeId}"
+                . ' AND status_global_attr.store_id = ' . Store::DEFAULT_STORE_ID,
                 []
             )
             ->willReturnSelf();
         $this->select->expects($this->at(1))
-            ->method('where')
-            ->with('status_attr.attribute_id = ?', $attributeId)
+            ->method('joinLeft')
+            ->with(
+                ['status_attr' => $backendTable],
+                "status_attr.{$linkField} = " . BaseSelectProcessorInterface::PRODUCT_TABLE_ALIAS . ".{$linkField}"
+                . " AND status_attr.attribute_id = {$attributeId}"
+                . " AND status_attr.store_id = {$currentStoreId}",
+                []
+            )
             ->willReturnSelf();
         $this->select->expects($this->at(2))
             ->method('where')
-            ->with('status_attr.value = ?', Status::STATUS_ENABLED)
+            ->with('IFNULL(status_attr.value, status_global_attr.value) = ?', Status::STATUS_ENABLED)
             ->willReturnSelf();
 
         $this->assertEquals($this->select, $this->statusBaseSelectProcessor->process($this->select));

diff --git a/...de/Magento/ConfigurableProduct/Model/ResourceModel/Product/Indexer/Price/Configurable.php b/...de/Magento/ConfigurableProduct/Model/ResourceModel/Product/Indexer/Price/Configurable.php
@@ -9,9 +9,45 @@
 
 use Magento\Catalog\Api\Data\ProductInterface;
 use Magento\Catalog\Model\Product\Attribute\Source\Status as ProductStatus;
+use Magento\Catalog\Model\Product\Attribute\Source\Status;
+use Magento\Store\Api\StoreResolverInterface;
+use Magento\Store\Model\Store;
 
+/**
+ * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
+ */
 class Configurable extends \Magento\Catalog\Model\ResourceModel\Product\Indexer\Price\DefaultPrice
 {
+    /**
+     * @var StoreResolverInterface
+     */
+    private $storeResolver;
+
+    /**
+     * Class constructor
+     *
+     * @param \Magento\Framework\Model\ResourceModel\Db\Context $context
+     * @param \Magento\Framework\Indexer\Table\StrategyInterface $tableStrategy
+     * @param \Magento\Eav\Model\Config $eavConfig
+     * @param \Magento\Framework\Event\ManagerInterface $eventManager
+     * @param \Magento\Framework\Module\Manager $moduleManager
+     * @param string $connectionName
+     * @param StoreResolverInterface $storeResolver
+     */
+    public function __construct(
+        \Magento\Framework\Model\ResourceModel\Db\Context $context,
+        \Magento\Framework\Indexer\Table\StrategyInterface $tableStrategy,
+        \Magento\Eav\Model\Config $eavConfig,
+        \Magento\Framework\Event\ManagerInterface $eventManager,
+        \Magento\Framework\Module\Manager $moduleManager,
+        $connectionName = null,
+        StoreResolverInterface $storeResolver = null
+    ) {
+        parent::__construct($context, $tableStrategy, $eavConfig, $eventManager, $moduleManager, $connectionName);
+        $this->storeResolver = $storeResolver ?:
+            \Magento\Framework\App\ObjectManager::getInstance()->get(StoreResolverInterface::class);
+    }
+
     /**
      * Reindex temporary (price result data) for all products
      *
@@ -190,16 +226,21 @@ protected function _applyConfigurableOption()
             []
         )->where(
             'le.required_options=0'
-        )->join(
-            ['product_status' => $this->getTable($statusAttribute->getBackend()->getTable())],
-            sprintf(
-                'le.%1$s = product_status.%1$s AND product_status.attribute_id = %2$s',
-                $linkField,
-                $statusAttribute->getAttributeId()
-            ),
+        )->joinLeft(
+            ['status_global_attr' => $statusAttribute->getBackendTable()],
+            "status_global_attr.{$linkField} = le.{$linkField}"
+            . ' AND status_global_attr.attribute_id  = ' . (int)$statusAttribute->getAttributeId()
+            . ' AND status_global_attr.store_id  = ' . Store::DEFAULT_STORE_ID,
+            []
+        )->joinLeft(
+            ['status_attr' => $statusAttribute->getBackendTable()],
+            "status_attr.{$linkField} = le.{$linkField}"
+            . ' AND status_attr.attribute_id  = ' . (int)$statusAttribute->getAttributeId()
+            . ' AND status_attr.store_id  = ' . $this->storeResolver->getCurrentStoreId(),
             []
         )->where(
-            'product_status.value=' . ProductStatus::STATUS_ENABLED
+            'IFNULL(status_attr.value, status_global_attr.value) = ?',
+            Status::STATUS_ENABLED
         )->group(
             ['e.entity_id', 'i.customer_group_id', 'i.website_id', 'l.product_id']
         );

diff --git a/app/code/Magento/Customer/Model/Address/AbstractAddress.php b/app/code/Magento/Customer/Model/Address/AbstractAddress.php
@@ -554,6 +554,8 @@ public function getDataModel($defaultBillingAddressId = null, $defaultShippingAd
     /**
      * Validate address attribute values
      *
+     *
+     *
      * @return bool|array
      * @SuppressWarnings(PHPMD.CyclomaticComplexity)
      * @SuppressWarnings(PHPMD.NPathComplexity)
@@ -562,23 +564,24 @@ public function validate()
     {
         $errors = [];
         if (!\Zend_Validate::is($this->getFirstname(), 'NotEmpty')) {
-            $errors[] = __('Please enter the first name.');
+            $errors[] = __('%fieldName is a required field.', ['fieldName' => 'firstname']);
         }
 
         if (!\Zend_Validate::is($this->getLastname(), 'NotEmpty')) {
-            $errors[] = __('Please enter the last name.');
+            $errors[] = __('%fieldName is a required field.', ['fieldName' => 'lastname']);
         }
 
         if (!\Zend_Validate::is($this->getStreetLine(1), 'NotEmpty')) {
-            $errors[] = __('Please enter the street.');
+            $errors[] = __('%fieldName is a required field.', ['fieldName' => 'street']);
         }
 
         if (!\Zend_Validate::is($this->getCity(), 'NotEmpty')) {
-            $errors[] = __('Please enter the city.');
+            $errors[] = __('%fieldName is a required field.', ['fieldName' => 'city']);
         }
 
         if (!\Zend_Validate::is($this->getTelephone(), 'NotEmpty')) {
-            $errors[] = __('Please enter the phone number.');
+            $errors[] = __('%fieldName is a required field.', ['fieldName' => 'telephone']);
+
         }
 
         $_havingOptionalZip = $this->_directoryData->getCountriesWithOptionalZip();
@@ -590,11 +593,11 @@ public function validate()
             'NotEmpty'
         )
         ) {
-            $errors[] = __('Please enter the zip/postal code.');
+            $errors[] = __('%fieldName is a required field.', ['fieldName' => 'postcode']);
         }
 
         if (!\Zend_Validate::is($this->getCountryId(), 'NotEmpty')) {
-            $errors[] = __('Please enter the country.');
+            $errors[] = __('%fieldName is a required field.', ['fieldName' => 'countryId']);
         }
 
         if ($this->getCountryModel()->getRegionCollection()->getSize() && !\Zend_Validate::is(
@@ -604,7 +607,7 @@ public function validate()
             $this->getCountryId()
         )
         ) {
-            $errors[] = __('Please enter the state/province.');
+            $errors[] = __('%fieldName is a required field.', ['fieldName' => 'regionId']);
         }
 
         if (empty($errors) || $this->getShouldIgnoreValidation()) {

diff --git a/app/code/Magento/Customer/Model/ResourceModel/AddressRepository.php b/app/code/Magento/Customer/Model/ResourceModel/AddressRepository.php
@@ -124,8 +124,12 @@ public function save(\Magento\Customer\Api\Data\AddressInterface $address)
             $addressModel->updateData($address);
         }
 
-        $inputException = $this->_validate($addressModel);
-        if ($inputException->wasErrorAdded()) {
+        $errors = $addressModel->validate();
+        if ($errors !== true) {
+            $inputException = new InputException();
+            foreach ($errors as $error) {
+                $inputException->addError($error);
+            }
             throw $inputException;
         }
         $addressModel->save();
@@ -255,70 +259,6 @@ public function deleteById($addressId)
         return true;
     }
 
-    /**
-     * Validate Customer Addresses attribute values.
-     *
-     * @param CustomerAddressModel $customerAddressModel the model to validate
-     * @return InputException
-     *
-     * @SuppressWarnings(PHPMD.NPathComplexity)
-     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
-     */
-    private function _validate(CustomerAddressModel $customerAddressModel)
-    {
-        $exception = new InputException();
-        if ($customerAddressModel->getShouldIgnoreValidation()) {
-            return $exception;
-        }
-
-        if (!\Zend_Validate::is($customerAddressModel->getFirstname(), 'NotEmpty')) {
-            $exception->addError(__('%fieldName is a required field.', ['fieldName' => 'firstname']));
-        }
-
-        if (!\Zend_Validate::is($customerAddressModel->getLastname(), 'NotEmpty')) {
-            $exception->addError(__('%fieldName is a required field.', ['fieldName' => 'lastname']));
-        }
-
-        if (!\Zend_Validate::is($customerAddressModel->getStreetLine(1), 'NotEmpty')) {
-            $exception->addError(__('%fieldName is a required field.', ['fieldName' => 'street']));
-        }
-
-        if (!\Zend_Validate::is($customerAddressModel->getCity(), 'NotEmpty')) {
-            $exception->addError(__('%fieldName is a required field.', ['fieldName' => 'city']));
-        }
-
-        if (!\Zend_Validate::is($customerAddressModel->getTelephone(), 'NotEmpty')) {
-            $exception->addError(__('%fieldName is a required field.', ['fieldName' => 'telephone']));
-        }
-
-        $havingOptionalZip = $this->directoryData->getCountriesWithOptionalZip();
-        if (!in_array($customerAddressModel->getCountryId(), $havingOptionalZip)
-            && !\Zend_Validate::is($customerAddressModel->getPostcode(), 'NotEmpty')
-        ) {
-            $exception->addError(__('%fieldName is a required field.', ['fieldName' => 'postcode']));
-        }
-
-        if (!\Zend_Validate::is($customerAddressModel->getCountryId(), 'NotEmpty')) {
-            $exception->addError(__('%fieldName is a required field.', ['fieldName' => 'countryId']));
-        }
-
-        if ($this->directoryData->isRegionRequired($customerAddressModel->getCountryId())) {
-            $regionCollection = $customerAddressModel->getCountryModel()->getRegionCollection();
-            if (!$regionCollection->count() && empty($customerAddressModel->getRegion())) {
-                $exception->addError(__('%fieldName is a required field.', ['fieldName' => 'region']));
-            } elseif (
-                $regionCollection->count()
-                && !in_array(
-                    $customerAddressModel->getRegionId(),
-                    array_column($regionCollection->getData(), 'region_id')
-                )
-            ) {
-                $exception->addError(__('%fieldName is a required field.', ['fieldName' => 'regionId']));
-            }
-        }
-        return $exception;
-    }
-
     /**
      * Retrieve collection processor
      *