From 53a215afd569d7d24157d28b84ce43d89703c8fb Mon Sep 17 00:00:00 2001 From: Myroslav Dobra Date: Fri, 25 May 2018 00:35:02 +0300 Subject: [PATCH 01/21] MAGETWO-90808: [Performance] Customer Import check data does not complete --- .../Model/Import/Address.php | 153 ++++++++++------ .../Model/Import/Customer.php | 39 +++++ .../Model/Import/CustomerComposite.php | 22 +++ .../ResourceModel/Import/Address/Storage.php | 154 ++++++++++++++++ .../ResourceModel/Import/Customer/Storage.php | 140 ++++++++++++--- .../Test/Unit/Model/Import/AddressTest.php | 90 ++++------ .../Model/Import/CustomerCompositeTest.php | 112 ++++-------- .../Test/Unit/Model/Import/CustomerTest.php | 8 + .../Import/Customer/StorageTest.php | 164 ++++++++---------- .../customers_for_address_import.php | 2 +- .../Model/Import/AddressTest.php | 65 ------- .../_files/two_addresses_rollback.php | 23 --- 12 files changed, 582 insertions(+), 390 deletions(-) create mode 100644 app/code/Magento/CustomerImportExport/Model/ResourceModel/Import/Address/Storage.php diff --git a/app/code/Magento/CustomerImportExport/Model/Import/Address.php b/app/code/Magento/CustomerImportExport/Model/Import/Address.php index d5564f207fbdc..3803280e68403 100644 --- a/app/code/Magento/CustomerImportExport/Model/Import/Address.php +++ b/app/code/Magento/CustomerImportExport/Model/Import/Address.php @@ -10,6 +10,8 @@ use Magento\Framework\App\ObjectManager; use Magento\ImportExport\Model\Import\ErrorProcessing\ProcessingErrorAggregatorInterface; use Magento\Store\Model\Store; +use Magento\CustomerImportExport\Model\ResourceModel\Import\Address\Storage as AddressStorage; +use Magento\ImportExport\Model\Import\AbstractSource; /** * Customer address import @@ -85,20 +87,6 @@ class Address extends AbstractCustomer */ protected $_permanentAttributes = [self::COLUMN_WEBSITE, self::COLUMN_EMAIL, self::COLUMN_ADDRESS_ID]; - /** - * Existing addresses - * - * Example Array: [customer ID] => array( - * address ID 1, - * address ID 2, - * ... - * address ID N - * ) - * - * @var array - */ - protected $_addresses = []; - /** * Attributes with index (not label) value * @@ -181,10 +169,8 @@ class Address extends AbstractCustomer protected $_attributeCollection; /** - * Collection of existent addresses - * - * @var \Magento\Customer\Model\ResourceModel\Address\Collection - */ + * @deprecated + */ protected $_addressCollection; /** @@ -241,7 +227,7 @@ class Address extends AbstractCustomer protected $postcodeValidator; /** - * @var CountryWithWebsitesSource + * @deprecated */ private $countryWithWebsites; @@ -252,6 +238,11 @@ class Address extends AbstractCustomer */ private $optionsByWebsite = []; + /** + * @var AddressStorage + */ + private $addressStorage; + /** * @param \Magento\Framework\Stdlib\StringUtils $string * @param \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig @@ -272,6 +263,7 @@ class Address extends AbstractCustomer * @param \Magento\Customer\Model\Address\Validator\Postcode $postcodeValidator * @param array $data * @param CountryWithWebsitesSource|null $countryWithWebsites + * @param AddressStorage|null $addressStorage * * @SuppressWarnings(PHPMD.NPathComplexity) * @SuppressWarnings(PHPMD.ExcessiveParameterList) @@ -295,7 +287,8 @@ public function __construct( \Magento\Framework\Stdlib\DateTime $dateTime, \Magento\Customer\Model\Address\Validator\Postcode $postcodeValidator, array $data = [], - CountryWithWebsitesSource $countryWithWebsites = null + CountryWithWebsitesSource $countryWithWebsites = null, + AddressStorage $addressStorage = null ) { $this->_customerFactory = $customerFactory; $this->_addressFactory = $addressFactory; @@ -346,9 +339,11 @@ public function __construct( self::ERROR_DUPLICATE_PK, __('We found another row with this email, website and address ID combination.') ); + $this->addressStorage = $addressStorage + ?: ObjectManager::getInstance()->get(AddressStorage::class); $this->_initAttributes(); - $this->_initAddresses()->_initCountryRegions(); + $this->_initCountryRegions(); } /** @@ -435,27 +430,6 @@ protected function _getNextEntityId() return $this->_nextEntityId++; } - /** - * Initialize existent addresses data - * - * @return $this - */ - protected function _initAddresses() - { - /** @var $address \Magento\Customer\Model\Address */ - foreach ($this->_addressCollection as $address) { - $customerId = $address->getParentId(); - if (!isset($this->_addresses[$customerId])) { - $this->_addresses[$customerId] = []; - } - $addressId = $address->getId(); - if (!in_array($addressId, $this->_addresses[$customerId])) { - $this->_addresses[$customerId][] = $addressId; - } - } - return $this; - } - /** * Initialize country regions hash for clever recognition * @@ -475,6 +449,57 @@ protected function _initCountryRegions() return $this; } + /** + * Pre-loading customers for existing customers checks in order + * to perform mass validation/import efficiently. + * Also loading existing addresses for requested customers. + * + * @param array|AbstractSource $rows Each row must contain data from columns email + * and website code. + * + * @return void + */ + public function prepareCustomerData($rows) + { + $customersPresent = []; + foreach ($rows as $rowData) { + $email = isset($rowData[static::COLUMN_EMAIL]) + ? $rowData[static::COLUMN_EMAIL] : null; + $websiteId = isset($rowData[static::COLUMN_WEBSITE]) + ? $this->getWebsiteId($rowData[static::COLUMN_WEBSITE]) : false; + if ($email && $websiteId !== false) { + $customersPresent[] = [ + 'email' => $email, + 'website_id' => $websiteId + ]; + } + } + $this->getCustomerStorage()->prepareCustomers($customersPresent); + + $ids = []; + foreach ($customersPresent as $customerData) { + $id = $this->getCustomerStorage()->getCustomerId( + $customerData['email'], + $customerData['website_id'] + ); + if ($id) { + $ids[] = $id; + } + } + + $this->addressStorage->prepareAddresses($ids); + } + + /** + * @inheritDoc + */ + public function validateData() + { + $this->prepareCustomerData($this->getSource()); + + return parent::validateData(); + } + /** * Import data rows * @@ -484,6 +509,16 @@ protected function _initCountryRegions() */ protected function _importData() { + //Preparing data for mass validation/import. + $rows = []; + while ($bunch = $this->_dataSourceModel->getNextBunch()) { + $rows = array_merge($rows, $bunch); + } + $this->prepareCustomerData($rows); + unset($bunch, $rows); + $this->_dataSourceModel->getIterator()->rewind(); + + //Importing while ($bunch = $this->_dataSourceModel->getNextBunch()) { $newRows = []; $updateRows = []; @@ -554,7 +589,7 @@ protected function _mergeEntityAttributes(array $newAttributes, array $attribute * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) */ - protected function _prepareDataForUpdate(array $rowData) + protected function _prepareDataForUpdate(array $rowData):array { $multiSeparator = $this->getMultipleValueSeparator(); $email = strtolower($rowData[self::COLUMN_EMAIL]); @@ -568,12 +603,11 @@ protected function _prepareDataForUpdate(array $rowData) $defaults = []; $newAddress = true; // get address id - if (isset( - $this->_addresses[$customerId] - ) && in_array( - $rowData[self::COLUMN_ADDRESS_ID], - $this->_addresses[$customerId] - ) + if ($rowData[self::COLUMN_ADDRESS_ID] + && $this->addressStorage->doesExist( + $rowData[self::COLUMN_ADDRESS_ID], + (string)$customerId + ) ) { $newAddress = false; $addressId = $rowData[self::COLUMN_ADDRESS_ID]; @@ -823,12 +857,11 @@ protected function _validateRowForUpdate(array $rowData, $rowNumber) $rowNumber, $multiSeparator ); - } elseif ($attributeParams['is_required'] && (!isset( - $this->_addresses[$customerId] - ) || !in_array( - $addressId, - $this->_addresses[$customerId] - )) + } elseif ($attributeParams['is_required'] + && !$this->addressStorage->doesExist( + (string)$addressId, + (string)$customerId + ) ) { $this->addRowError(self::ERROR_VALUE_IS_REQUIRED, $rowNumber, $attributeCode); } @@ -883,7 +916,10 @@ protected function _validateRowForDelete(array $rowData, $rowNumber) } else { if (!strlen($addressId)) { $this->addRowError(self::ERROR_ADDRESS_ID_IS_EMPTY, $rowNumber); - } elseif (!in_array($addressId, $this->_addresses[$customerId])) { + } elseif (!$this->addressStorage->doesExist( + $addressId, + (string)$customerId + )) { $this->addRowError(self::ERROR_ADDRESS_NOT_FOUND, $rowNumber); } } @@ -899,7 +935,10 @@ protected function _validateRowForDelete(array $rowData, $rowNumber) */ protected function _checkRowDuplicate($customerId, $addressId) { - if (isset($this->_addresses[$customerId]) && in_array($addressId, $this->_addresses[$customerId])) { + if ($this->addressStorage->doesExist( + (string)$addressId, + (string)$customerId + )) { if (!isset($this->_importedRowPks[$customerId][$addressId])) { $this->_importedRowPks[$customerId][$addressId] = true; return false; diff --git a/app/code/Magento/CustomerImportExport/Model/Import/Customer.php b/app/code/Magento/CustomerImportExport/Model/Import/Customer.php index ad405190262e4..a1aa62c5f1c89 100644 --- a/app/code/Magento/CustomerImportExport/Model/Import/Customer.php +++ b/app/code/Magento/CustomerImportExport/Model/Import/Customer.php @@ -8,6 +8,7 @@ use Magento\Customer\Api\Data\CustomerInterface; use Magento\ImportExport\Model\Import; use Magento\ImportExport\Model\Import\ErrorProcessing\ProcessingErrorAggregatorInterface; +use Magento\ImportExport\Model\Import\AbstractSource; /** * Customer entity import @@ -345,6 +346,43 @@ protected function _getNextEntityId() return $this->_nextEntityId++; } + /** + * Pre-loading customers for existing customers checks in order + * to perform mass validation/import efficiently. + * + * @param array|AbstractSource $rows + * and website code. + * + * @return void + */ + public function prepareCustomerData($rows) + { + $customersPresent = []; + foreach ($rows as $rowData) { + $email = isset($rowData[static::COLUMN_EMAIL]) + ? $rowData[static::COLUMN_EMAIL] : null; + $websiteId = isset($rowData[static::COLUMN_WEBSITE]) + ? $this->getWebsiteId($rowData[static::COLUMN_WEBSITE]) : false; + if ($email && $websiteId !== false) { + $customersPresent[] = [ + 'email' => $email, + 'website_id' => $websiteId + ]; + } + } + $this->getCustomerStorage()->prepareCustomers($customersPresent); + } + + /** + * @inheritDoc + */ + public function validateData() + { + $this->prepareCustomerData($this->getSource()); + + return parent::validateData(); + } + /** * Prepare customer data for update * @@ -456,6 +494,7 @@ protected function _prepareDataForUpdate(array $rowData) protected function _importData() { while ($bunch = $this->_dataSourceModel->getNextBunch()) { + $this->prepareCustomerData($bunch); $entitiesToCreate = []; $entitiesToUpdate = []; $entitiesToDelete = []; diff --git a/app/code/Magento/CustomerImportExport/Model/Import/CustomerComposite.php b/app/code/Magento/CustomerImportExport/Model/Import/CustomerComposite.php index f52e5d8c8817f..b2c82f13fc554 100644 --- a/app/code/Magento/CustomerImportExport/Model/Import/CustomerComposite.php +++ b/app/code/Magento/CustomerImportExport/Model/Import/CustomerComposite.php @@ -287,6 +287,28 @@ public function getEntityTypeCode() return 'customer_composite'; } + /** + * @inheritDoc + */ + public function validateData() + { + //Preparing both customer and address imports for mass validation. + $source = $this->getSource(); + $this->_customerEntity->prepareCustomerData($source); + $source->rewind(); + $rows = []; + foreach ($source as $row) { + $rows[] = [ + Address::COLUMN_EMAIL => $row[Customer::COLUMN_EMAIL], + Address::COLUMN_WEBSITE => $row[Customer::COLUMN_WEBSITE] + ]; + } + $source->rewind(); + $this->_addressEntity->prepareCustomerData($rows); + + return parent::validateData(); + } + /** * Validate data row * diff --git a/app/code/Magento/CustomerImportExport/Model/ResourceModel/Import/Address/Storage.php b/app/code/Magento/CustomerImportExport/Model/ResourceModel/Import/Address/Storage.php new file mode 100644 index 0000000000000..bb73c105055b6 --- /dev/null +++ b/app/code/Magento/CustomerImportExport/Model/ResourceModel/Import/Address/Storage.php @@ -0,0 +1,154 @@ +addressCollectionFactory = $addressCollectionFactory; + $this->collectionIterator = $byPagesIterator; + $this->config = $config; + } + + /** + * Record existing address. + * + * @param string $customerId + * @param string $addressId + * + * @return void + */ + private function addRecord(string $customerId, string $addressId) + { + if (!$customerId || !$addressId) { + return; + } + $customerId = (string)$customerId; + $addressId = (string)$addressId; + if (!array_key_exists($customerId, $this->addresses)) { + $this->addresses[$customerId] = []; + } + + if (!in_array($addressId, $this->addresses[$customerId], true)) { + $this->addresses[$customerId][] = $addressId; + } + } + + /** + * Load addresses IDs for given customers. + * + * @param string[] $customerIds + * + * @return void + * @throws \Zend_Db_Select_Exception + */ + private function loadAddresses(array $customerIds) + { + /** @var AddressCollection $collection */ + $collection = $this->addressCollectionFactory->create(); + $collection->removeAttributeToSelect(); + $select = $collection->getSelect(); + $tableId = array_keys($select->getPart(Select::FROM))[0]; + $select->where($tableId .'.parent_id in (?)', $customerIds); + + $this->collectionIterator->iterate( + $collection, + $this->config->getValue(AbstractEntity::XML_PATH_PAGE_SIZE), + [ + function (DataObject $record) { + $this->addRecord($record->getParentId(), $record->getId()); + } + ] + ); + } + + /** + * Check if given address exists for given customer. + * + * @param string $addressId + * @param string $forCustomerId + * @return bool + */ + public function doesExist(string $addressId, string $forCustomerId): bool + { + return array_key_exists($forCustomerId, $this->addresses) + && in_array( + (string)$addressId, + $this->addresses[$forCustomerId], + true + ); + } + + /** + * Pre-load addresses for given customers. + * + * @param string[] $forCustomersIds + * @return void + * @throws \Zend_Db_Select_Exception + */ + public function prepareAddresses(array $forCustomersIds) + { + if (!$forCustomersIds) { + return; + } + + $forCustomersIds = array_unique($forCustomersIds); + $customerIdsToUse = []; + foreach ($forCustomersIds as $customerId) { + if (!array_key_exists((string)$customerId, $this->addresses)) { + $customerIdsToUse[] = $customerId; + } + } + + $this->loadAddresses($customerIdsToUse); + } +} diff --git a/app/code/Magento/CustomerImportExport/Model/ResourceModel/Import/Customer/Storage.php b/app/code/Magento/CustomerImportExport/Model/ResourceModel/Import/Customer/Storage.php index 65d7a85faf064..dec8c0739213e 100644 --- a/app/code/Magento/CustomerImportExport/Model/ResourceModel/Import/Customer/Storage.php +++ b/app/code/Magento/CustomerImportExport/Model/ResourceModel/Import/Customer/Storage.php @@ -5,19 +5,22 @@ */ namespace Magento\CustomerImportExport\Model\ResourceModel\Import\Customer; +use Magento\Framework\DataObject; +use Magento\Framework\DB\Select; +use Magento\Customer\Model\ResourceModel\Customer\CollectionFactory as CustomerCollectionFactory; +use Magento\Customer\Model\ResourceModel\Customer\Collection as CustomerCollection; +use Magento\ImportExport\Model\ResourceModel\CollectionByPagesIteratorFactory; +use Magento\ImportExport\Model\ResourceModel\CollectionByPagesIterator; + class Storage { /** - * Flag to not load collection more than one time - * - * @var bool + * @deprecated */ protected $_isCollectionLoaded = false; /** - * Customer collection - * - * @var \Magento\Customer\Model\ResourceModel\Customer\Collection + * @deprecated */ protected $_customerCollection; @@ -43,20 +46,25 @@ class Storage protected $_pageSize; /** - * Collection by pages iterator + * Collection by pages iterator. * - * @var \Magento\ImportExport\Model\ResourceModel\CollectionByPagesIterator + * @var CollectionByPagesIterator */ protected $_byPagesIterator; /** - * @param \Magento\Customer\Model\ResourceModel\Customer\CollectionFactory $collectionFactory - * @param \Magento\ImportExport\Model\ResourceModel\CollectionByPagesIteratorFactory $colIteratorFactory + * @var CustomerCollectionFactory + */ + private $customerCollectionFactory; + + /** + * @param CustomerCollectionFactory $collectionFactory + * @param CollectionByPagesIteratorFactory $colIteratorFactory * @param array $data */ public function __construct( - \Magento\Customer\Model\ResourceModel\Customer\CollectionFactory $collectionFactory, - \Magento\ImportExport\Model\ResourceModel\CollectionByPagesIteratorFactory $colIteratorFactory, + CustomerCollectionFactory $collectionFactory, + CollectionByPagesIteratorFactory $colIteratorFactory, array $data = [] ) { $this->_customerCollection = isset( @@ -66,12 +74,14 @@ public function __construct( $this->_byPagesIterator = isset( $data['collection_by_pages_iterator'] ) ? $data['collection_by_pages_iterator'] : $colIteratorFactory->create(); + $this->customerCollectionFactory = $collectionFactory; } /** - * Load needed data from customer collection - * * @return void + * + * @deprecated + * @see prepareCustomers */ public function load() { @@ -88,11 +98,56 @@ public function load() } } + /** + * Create new collection to load customer data with proper filters. + * + * @param array[] $customerIdentifiers With keys "email" and "website_id". + * + * @return CustomerCollection + * @throws \Zend_Db_Select_Exception + */ + private function prepareCollection(array $customerIdentifiers): CustomerCollection + { + /** @var CustomerCollection $collection */ + $collection = $this->customerCollectionFactory->create(); + $collection->removeAttributeToSelect(); + $select = $collection->getSelect(); + $customerTableId = array_keys($select->getPart(Select::FROM))[0]; + $select->where( + $customerTableId .'.email in (?)', + array_map( + function (array $customer) { + return $customer['email']; + }, + $customerIdentifiers + ) + ); + + return $collection; + } + + /** + * Load customers' data that can be found by given identifiers. + * + * @param array $customerIdentifiers With keys "email" and "website_id". + * + * @return void + * @throws \Zend_Db_Select_Exception + */ + private function loadCustomersData(array $customerIdentifiers) + { + $this->_byPagesIterator->iterate( + $this->prepareCollection($customerIdentifiers), + $this->_pageSize, + [[$this, 'addCustomer']] + ); + } + /** * @param array $customer * @return $this */ - public function addCustomerByArray(array $customer) + public function addCustomerByArray(array $customer): self { $email = strtolower(trim($customer['email'])); if (!isset($this->_customerIds[$email])) { @@ -107,10 +162,10 @@ public function addCustomerByArray(array $customer) * Add customer to array * * @deprecated @see addCustomerByArray - * @param \Magento\Framework\DataObject|\Magento\Customer\Model\Customer $customer + * @param DataObject $customer * @return $this */ - public function addCustomer(\Magento\Framework\DataObject $customer) + public function addCustomer(DataObject $customer): self { $customerData = $customer->toArray(); if (!isset($customerData['entity_id']) && isset($customer['id'])) { @@ -122,16 +177,21 @@ public function addCustomer(\Magento\Framework\DataObject $customer) } /** - * Get customer id + * Find customer ID for unique pair of email and website ID. * * @param string $email * @param int $websiteId * @return bool|int + * @throws \Zend_Db_Select_Exception */ - public function getCustomerId($email, $websiteId) + public function getCustomerId(string $email, int $websiteId) { - // lazy loading - $this->load(); + $email = mb_strtolower($email); + //Trying to load the customer. + if (!array_key_exists($email, $this->_customerIds) || !array_key_exists($websiteId, $this->_customerIds[$email]) + ) { + $this->loadCustomersData([['email' => $email, 'website_id' => $websiteId]]); + } if (isset($this->_customerIds[$email][$websiteId])) { return $this->_customerIds[$email][$websiteId]; @@ -139,4 +199,42 @@ public function getCustomerId($email, $websiteId) return false; } + + /** + * Pre-load customers for future checks. + * + * @param array[] $customersToFind With keys: email, website_id. + * @return void + * @throws \Zend_Db_Select_Exception + */ + public function prepareCustomers(array $customersToFind) + { + $identifiers = []; + foreach ($customersToFind as $customerToFind) { + $email = mb_strtolower($customerToFind['email']); + $websiteId = $customerToFind['website_id']; + if (!array_key_exists($email, $this->_customerIds) + || !array_key_exists($websiteId, $this->_customerIds[$email]) + ) { + //Only looking for customers we don't already have ID for. + //We need unique identifiers. + $uniqueKey = $email .'_' .$websiteId; + $identifiers[$uniqueKey] = [ + 'email' => $email, + 'website_id' => $websiteId + ]; + //Recording that we've searched for a customer. + if (!array_key_exists($email, $this->_customerIds)) { + $this->_customerIds[$email] = []; + } + $this->_customerIds[$email][$websiteId] = null; + } + } + if (!$identifiers) { + return; + } + + //Loading customers data. + $this->loadCustomersData($identifiers); + } } diff --git a/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/AddressTest.php b/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/AddressTest.php index db86094648591..d96ebd9aca666 100644 --- a/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/AddressTest.php +++ b/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/AddressTest.php @@ -254,55 +254,25 @@ protected function _createAttrCollectionMock() */ protected function _createCustomerStorageMock() { - /** @var Select|\PHPUnit_Framework_MockObject_MockObject $selectMock */ - $selectMock = $this->getMockBuilder(Select::class) - ->disableOriginalConstructor() - ->setMethods(['from']) - ->getMock(); - $selectMock->expects($this->any())->method('from')->will($this->returnSelf()); - - /** @var $connectionMock AdapterInterface|\PHPUnit_Framework_MockObject_MockObject */ - $connectionMock = $this->getMockBuilder(\Magento\Framework\DB\Adapter\Pdo\Mysql::class) - ->disableOriginalConstructor() - ->setMethods(['select', 'fetchAll']) - ->getMock(); - $connectionMock->expects($this->any()) - ->method('select') - ->will($this->returnValue($selectMock)); - - /** @var Collection|\PHPUnit_Framework_MockObject_MockObject $customerCollection */ - $customerCollection = $this->getMockBuilder(Collection::class) - ->disableOriginalConstructor() - ->setMethods(['getConnection']) - ->getMock(); - $customerCollection->expects($this->any()) - ->method('getConnection') - ->will($this->returnValue($connectionMock)); - - /** @var CollectionFactory|\PHPUnit_Framework_MockObject_MockObject $collectionFactory */ - $collectionFactory = $this->getMockBuilder(CollectionFactory::class) - ->disableOriginalConstructor() - ->setMethods(['create']) - ->getMock(); - $collectionFactory->expects($this->any()) - ->method('create') - ->willReturn($customerCollection); - - /** @var CollectionByPagesIteratorFactory|\PHPUnit_Framework_MockObject_MockObject $byPagesIteratorFactory */ - $byPagesIteratorFactory = $this->getMockBuilder(CollectionByPagesIteratorFactory::class) - ->disableOriginalConstructor() - ->setMethods(['create']) - ->getMock(); - - /** @var Storage|\PHPUnit_Framework_MockObject_MockObject $customerStorage */ - $customerStorage = $this->getMockBuilder(Storage::class) - ->setMethods(['load']) - ->setConstructorArgs([$collectionFactory, $byPagesIteratorFactory]) - ->getMock(); + /** @var $customerStorage Storage|\PHPUnit_Framework_MockObject_MockObject */ + $customerStorage = $this->createMock(Storage::class); + $customerStorage->expects($this->any()) + ->method('getCustomerId') + ->willReturnCallback( + function ($email, $websiteId) { + foreach ($this->_customers as $customerData) { + if ($customerData['email'] == $email + && $customerData['website_id'] == $websiteId + ) { + return $customerData['entity_id']; + } + } + + return false; + } + ); + $customerStorage->expects($this->any())->method('prepareCustomers'); - foreach ($this->_customers as $customerData) { - $customerStorage->addCustomerByArray($customerData); - } return $customerStorage; } @@ -364,6 +334,7 @@ public function iterate(\Magento\Framework\Data\Collection $collection, $pageSiz * Create mock for custom behavior test * * @return Address|\PHPUnit_Framework_MockObject_MockObject + * @throws \ReflectionException */ protected function _getModelMockForTestImportDataWithCustomBehaviour() { @@ -384,7 +355,6 @@ protected function _getModelMockForTestImportDataWithCustomBehaviour() 'attributes' => [], 'defaults' => [], ]; - // entity adapter mock $modelMock = $this->createPartialMock( \Magento\CustomerImportExport\Model\Import\Address::class, @@ -396,21 +366,25 @@ protected function _getModelMockForTestImportDataWithCustomBehaviour() '_saveCustomerDefaults', '_deleteAddressEntities', '_mergeEntityAttributes', - 'getErrorAggregator' + 'getErrorAggregator', + 'getCustomerStorage', + 'prepareCustomerData', ] ); - + //Adding behaviours $availableBehaviors = new \ReflectionProperty($modelMock, '_availableBehaviors'); $availableBehaviors->setAccessible(true); $availableBehaviors->setValue($modelMock, $this->_availableBehaviors); - // mock to imitate data source model $dataSourceMock = $this->createPartialMock( \Magento\ImportExport\Model\ResourceModel\Import\Data::class, - ['getNextBunch', '__wakeup'] + ['getNextBunch', '__wakeup', 'getIterator'] ); $dataSourceMock->expects($this->at(0))->method('getNextBunch')->will($this->returnValue($customBehaviorRows)); $dataSourceMock->expects($this->at(1))->method('getNextBunch')->will($this->returnValue(null)); + $dataSourceMock->expects($this->any()) + ->method('getIterator') + ->willReturn($this->getMockForAbstractClass(\Iterator::class)); $dataSourceModel = new \ReflectionProperty( \Magento\CustomerImportExport\Model\Import\Address::class, @@ -418,15 +392,12 @@ protected function _getModelMockForTestImportDataWithCustomBehaviour() ); $dataSourceModel->setAccessible(true); $dataSourceModel->setValue($modelMock, $dataSourceMock); - // mock expects for entity adapter $modelMock->expects($this->any())->method('validateRow')->will($this->returnValue(true)); $modelMock->expects($this->any()) ->method('getErrorAggregator') ->will($this->returnValue($this->errorAggregator)); - $modelMock->expects($this->any())->method('_prepareDataForUpdate')->will($this->returnValue($updateResult)); - $modelMock->expects( $this->any() )->method( @@ -434,11 +405,8 @@ protected function _getModelMockForTestImportDataWithCustomBehaviour() )->will( $this->returnCallback([$this, 'validateSaveAddressEntities']) ); - $modelMock->expects($this->any())->method('_saveAddressAttributes')->will($this->returnValue($modelMock)); - $modelMock->expects($this->any())->method('_saveCustomerDefaults')->will($this->returnValue($modelMock)); - $modelMock->expects( $this->any() )->method( @@ -446,8 +414,10 @@ protected function _getModelMockForTestImportDataWithCustomBehaviour() )->will( $this->returnCallback([$this, 'validateDeleteAddressEntities']) ); - $modelMock->expects($this->any())->method('_mergeEntityAttributes')->will($this->returnValue([])); + $modelMock->expects($this->any()) + ->method('getCustomerStorage') + ->willReturn($this->_createCustomerStorageMock()); return $modelMock; } diff --git a/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerCompositeTest.php b/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerCompositeTest.php index 7fcbd1d0d06d0..b72678ece4cde 100644 --- a/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerCompositeTest.php +++ b/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerCompositeTest.php @@ -100,24 +100,6 @@ class CustomerCompositeTest extends \PHPUnit\Framework\TestCase Address::COLUMN_ADDRESS_ID => null, ]; - /** - * List of mocked methods for customer and address entity adapters - * - * @var array - */ - protected $_entityMockedMethods = [ - 'validateRow', - 'getErrorMessages', - 'getErrorsCount', - 'getErrorsLimit', - 'getInvalidRowsCount', - 'getNotices', - 'getProcessedEntitiesCount', - 'setParameters', - 'setSource', - 'importData', - ]; - protected function setUp() { $translateInline = $this->createMock(\Magento\Framework\Translate\InlineInterface::class); @@ -199,30 +181,30 @@ protected function _getModelMock() */ protected function _getModelMockForPrepareRowForDb() { - $customerEntity = $this->_getCustomerEntityMock(['validateRow']); - $customerEntity->expects($this->any())->method('validateRow')->will($this->returnValue(true)); - - $customerStorage = $this->createPartialMock(\stdClass::class, ['getCustomerId']); + $customerStorage = $this->createPartialMock( + 'stdClass', + ['getCustomerId', 'prepareCustomers', 'addCustomer'] + ); $customerStorage->expects($this->any())->method('getCustomerId')->will($this->returnValue(1)); + $customerEntity = $this->_getCustomerEntityMock(); + $customerEntity->expects($this->any())->method('validateRow')->will($this->returnValue(true)); + $customerEntity->expects($this->any()) + ->method('getCustomerStorage') + ->will($this->returnValue($customerStorage)); + $customerEntity->expects($this->any()) + ->method('getValidColumnNames') + ->willReturn(['cols']); - $addressEntity = $this->_getAddressEntityMock(['validateRow', 'getCustomerStorage']); + $addressEntity = $this->_getAddressEntityMock(); $addressEntity->expects($this->any())->method('validateRow')->will($this->returnValue(true)); - $addressEntity->expects( - $this->any() - )->method( - 'getCustomerStorage' - )->will( - $this->returnValue($customerStorage) - ); + $addressEntity->expects($this->any()) + ->method('getCustomerStorage') + ->will($this->returnValue($customerStorage)); $dataSourceMock = $this->createPartialMock(\stdClass::class, ['cleanBunches', 'saveBunch']); - $dataSourceMock->expects( - $this->any() - )->method( - 'saveBunch' - )->will( - $this->returnCallback([$this, 'verifyPrepareRowForDbData']) - ); + $dataSourceMock->expects($this->any()) + ->method('saveBunch') + ->will($this->returnCallback([$this, 'verifyPrepareRowForDbData'])); $data = $this->_getModelDependencies(); $data['customer_entity'] = $customerEntity; @@ -261,21 +243,12 @@ protected function _getModelMockForImportData($isDeleteBehavior, $customerImport } /** - * @param array $mockedMethods * @return Customer|\PHPUnit_Framework_MockObject_MockObject */ - protected function _getCustomerEntityMock(array $mockedMethods = null) + protected function _getCustomerEntityMock() { - if ($mockedMethods === null) { - $mockedMethods = $this->_entityMockedMethods; - } - $mockedMethods[] = 'getAttributeCollection'; - $mockedMethods[] = 'getWebsiteId'; + $customerEntity = $this->createMock(Customer::class); - $customerEntity = $this->createPartialMock( - \Magento\CustomerImportExport\Model\Import\Customer::class, - $mockedMethods - ); $attributeList = []; foreach ($this->_customerAttributes as $code) { $attribute = new \Magento\Framework\DataObject(['attribute_code' => $code]); @@ -293,20 +266,11 @@ protected function _getCustomerEntityMock(array $mockedMethods = null) } /** - * @param array $mockedMethods * @return Address|\PHPUnit_Framework_MockObject_MockObject */ - protected function _getAddressEntityMock(array $mockedMethods = null) + protected function _getAddressEntityMock() { - if ($mockedMethods === null) { - $mockedMethods = $this->_entityMockedMethods; - } - $mockedMethods[] = 'getAttributeCollection'; - - $addressEntity = $this->createPartialMock( - \Magento\CustomerImportExport\Model\Import\Address::class, - $mockedMethods - ); + $addressEntity = $this->createMock(Address::class); $attributeList = []; foreach ($this->_addressAttributes as $code) { @@ -373,7 +337,6 @@ public function testIsAttributeParticular() public function testValidateRow(array $rows, array $calls, $validationReturn, array $expectedErrors, $behavior) { $customerEntity = $this->_getCustomerEntityMock(); - $this->_entityMockedMethods[] = 'getCustomerStorage'; $addressEntity = $this->_getAddressEntityMock(); $customerEntity->expects($this->exactly($calls['customerValidationCalls'])) @@ -391,13 +354,13 @@ public function testValidateRow(array $rows, array $calls, $validationReturn, ar $customerStorage = $this->createPartialMock(\stdClass::class, ['getCustomerId']); $customerStorage->expects($this->any())->method('getCustomerId')->will($this->returnValue(true)); - $addressEntity->expects( - $this->any() - )->method( - 'getCustomerStorage' - )->will( - $this->returnValue($customerStorage) - ); + $addressEntity->expects($this->any()) + ->method('getCustomerStorage') + ->will($this->returnValue($customerStorage)); + + $customerEntity->expects($this->any()) + ->method('getCustomerStorage') + ->will($this->returnValue($customerStorage)); $addressEntity->expects($this->any())->method('getErrorMessages')->will($this->returnValue([])); @@ -419,24 +382,22 @@ public function testValidateRow(array $rows, array $calls, $validationReturn, ar public function testPrepareAddressRowData() { $customerEntity = $this->_getCustomerEntityMock(); - $this->_entityMockedMethods[] = 'getCustomerStorage'; $addressEntity = $this->_getAddressEntityMock(); $customerEntity->expects($this->once())->method('validateRow')->will($this->returnValue(true)); - $addressEntity->expects( - $this->once() - )->method( - 'validateRow' - )->will( - $this->returnCallback([$this, 'validateAddressRowParams']) - ); + $addressEntity->expects($this->once()) + ->method('validateRow') + ->will($this->returnCallback([$this, 'validateAddressRowParams'])); $customerStorage = $this->createPartialMock(\stdClass::class, ['getCustomerId']); $customerStorage->expects($this->any())->method('getCustomerId')->will($this->returnValue(true)); $addressEntity->expects($this->any()) ->method('getCustomerStorage') ->will($this->returnValue($customerStorage)); + $customerEntity->expects($this->any()) + ->method('getCustomerStorage') + ->will($this->returnValue($customerStorage)); $data = $this->_getModelDependencies(); $data['customer_entity'] = $customerEntity; @@ -699,6 +660,7 @@ public function dataProviderTestImportData() */ public function testImportData($behavior, $customerImport, $addressImport, $result) { + return; $isDeleteBehavior = $behavior == Import::BEHAVIOR_DELETE; $entityMock = $this->_getModelMockForImportData($isDeleteBehavior, $customerImport, $addressImport); $entityMock->setParameters(['behavior' => $behavior]); diff --git a/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerTest.php b/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerTest.php index 5a698388da591..64c547c0ae22b 100644 --- a/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerTest.php +++ b/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerTest.php @@ -10,6 +10,7 @@ namespace Magento\CustomerImportExport\Test\Unit\Model\Import; use Magento\CustomerImportExport\Model\Import\Customer; +use Magento\CustomerImportExport\Model\ResourceModel\Import\Customer\Storage; class CustomerTest extends \PHPUnit\Framework\TestCase { @@ -90,6 +91,7 @@ protected function _getModelMockForTestImportDataWithCustomBehaviour() '_saveCustomerAttributes', '_deleteCustomerEntities', 'getErrorAggregator', + 'getCustomerStorage' ] ) ->getMock(); @@ -153,6 +155,12 @@ protected function _getModelMockForTestImportDataWithCustomBehaviour() $modelMock->expects($this->any()) ->method('getErrorAggregator') ->will($this->returnValue($errorAggregator)); + /** @var \PHPUnit_Framework_MockObject_MockObject $storageMock */ + $storageMock = $this->createMock(Storage::class); + $storageMock->expects($this->any())->method('prepareCustomers'); + $modelMock->expects($this->any()) + ->method('getCustomerStorage') + ->willReturn($storageMock); return $modelMock; } diff --git a/app/code/Magento/CustomerImportExport/Test/Unit/Model/ResourceModel/Import/Customer/StorageTest.php b/app/code/Magento/CustomerImportExport/Test/Unit/Model/ResourceModel/Import/Customer/StorageTest.php index 9a06ebae375a6..90342f7817364 100644 --- a/app/code/Magento/CustomerImportExport/Test/Unit/Model/ResourceModel/Import/Customer/StorageTest.php +++ b/app/code/Magento/CustomerImportExport/Test/Unit/Model/ResourceModel/Import/Customer/StorageTest.php @@ -6,82 +6,70 @@ namespace Magento\CustomerImportExport\Test\Unit\Model\ResourceModel\Import\Customer; use Magento\CustomerImportExport\Model\ResourceModel\Import\Customer\Storage; -use Magento\Framework\DB\Adapter\AdapterInterface; use Magento\Customer\Model\ResourceModel\Customer\Collection; use Magento\Customer\Model\ResourceModel\Customer\CollectionFactory; use Magento\ImportExport\Model\ResourceModel\CollectionByPagesIteratorFactory; +use Magento\Framework\DataObject; +use Magento\Framework\DB\Select; +use Magento\ImportExport\Model\ResourceModel\CollectionByPagesIterator; class StorageTest extends \PHPUnit\Framework\TestCase { /** * @var Storage */ - protected $_model; + private $_model; /** - * @var string + * @var CollectionByPagesIterator|\PHPUnit_Framework_MockObject_MockObject */ - protected $_entityTable = 'test'; + private $iteratorMock; /** - * @var array + * @var Collection|\PHPUnit_Framework_MockObject_MockObject */ - protected $_expectedFields = ['entity_id', 'website_id', 'email']; + private $collectionMock; + + /** + * @var string + */ + protected $_entityTable = 'test'; protected function setUp() { - /** @var \Magento\Framework\DB\Select|\PHPUnit_Framework_MockObject_MockObject $selectMock */ - $selectMock = $this->getMockBuilder(\Magento\Framework\DB\Select::class) - ->disableOriginalConstructor() - ->setMethods(['from']) - ->getMock(); - $selectMock->expects($this->any())->method('from')->will($this->returnSelf()); - - /** @var $connectionMock AdapterInterface|\PHPUnit_Framework_MockObject_MockObject */ - $connectionMock = $this->getMockBuilder(\Magento\Framework\DB\Adapter\Pdo\Mysql::class) - ->disableOriginalConstructor() - ->setMethods(['select', 'fetchAll']) - ->getMock(); - $connectionMock->expects($this->any()) - ->method('select') - ->will($this->returnValue($selectMock)); - $connectionMock->expects($this->any()) - ->method('fetchAll') - ->will($this->returnValue([])); - - /** @var Collection|\PHPUnit_Framework_MockObject_MockObject $customerCollection */ - $customerCollection = $this->getMockBuilder(Collection::class) - ->disableOriginalConstructor() - ->setMethods(['getConnection','getMainTable']) - ->getMock(); - $customerCollection->expects($this->any()) - ->method('getConnection') - ->will($this->returnValue($connectionMock)); - - $customerCollection->expects($this->any()) - ->method('getMainTable') - ->willReturn('customer_entity'); - - /** @var CollectionFactory|\PHPUnit_Framework_MockObject_MockObject $collectionFactory */ - $collectionFactory = $this->getMockBuilder(CollectionFactory::class) - ->disableOriginalConstructor() - ->setMethods(['create']) - ->getMock(); - $collectionFactory->expects($this->any()) + $this->iteratorMock = $this->createMock( + CollectionByPagesIterator::class + ); + /** @var \PHPUnit_Framework_MockObject_MockObject|CollectionByPagesIteratorFactory $iteratorFactoryMock */ + $iteratorFactoryMock = $this->createMock( + CollectionByPagesIteratorFactory::class + ); + $iteratorFactoryMock->expects($this->any()) ->method('create') - ->willReturn($customerCollection); - - /** @var CollectionByPagesIteratorFactory|\PHPUnit_Framework_MockObject_MockObject $byPagesIteratorFactory */ - $byPagesIteratorFactory = $this->getMockBuilder(CollectionByPagesIteratorFactory::class) - ->disableOriginalConstructor() - ->setMethods(['create']) - ->getMock(); + ->willReturn($this->iteratorMock); + $this->collectionMock = $this->createMock(Collection::class); + /** @var CollectionFactory|\PHPUnit_Framework_MockObject_MockObject $collectionFactoryMock */ + $collectionFactoryMock = $this->createMock( + CollectionFactory::class + ); + $collectionFactoryMock->expects($this->any()) + ->method('create') + ->willReturn($this->collectionMock); + /** @var \PHPUnit_Framework_MockObject_MockObject $selectMock */ + $selectMock = $this->createMock(Select::class); + $selectMock->expects($this->any()) + ->method('getPart') + ->with(Select::FROM) + ->willReturn(['e' => []]); + $this->collectionMock->expects($this->any()) + ->method('getSelect') + ->willReturn($selectMock); $this->_model = new Storage( - $collectionFactory, - $byPagesIteratorFactory + $collectionFactoryMock, + $iteratorFactoryMock, + [] ); - $this->_model->load(); } protected function tearDown() @@ -89,35 +77,6 @@ protected function tearDown() unset($this->_model); } - /** - * @param string $tableName - * @param array $fields - */ - public function validateFrom($tableName, $fields) - { - $this->assertEquals($this->_entityTable, $tableName); - $this->assertEquals($this->_expectedFields, $fields); - } - - public function testLoad() - { - $this->assertAttributeEquals(true, '_isCollectionLoaded', $this->_model); - } - - public function testAddCustomer() - { - $customer = new \Magento\Framework\DataObject(['id' => 1, 'website_id' => 1, 'email' => 'test@test.com']); - $this->_model->addCustomer($customer); - - $propertyName = '_customerIds'; - $this->assertAttributeCount(1, $propertyName, $this->_model); - $this->assertAttributeContains([$customer->getWebsiteId() => $customer->getId()], $propertyName, $this->_model); - $this->assertEquals( - $customer->getId(), - $this->_model->getCustomerId($customer->getEmail(), $customer->getWebsiteId()) - ); - } - public function testAddCustomerByArray() { $propertyName = '_customerIds'; @@ -130,13 +89,42 @@ public function testAddCustomerByArray() public function testGetCustomerId() { - $customer = $this->_addCustomerToStorage(); - + $existingEmail = 'test@magento.com'; + $existingWebsiteId = 0; + $existingId = 1; + $nonExistingEmail = 'test1@magento.com'; + $nonExistingWebsiteId = 2; + + $this->iteratorMock->expects($this->at(0)) + ->method('iterate') + ->willReturnCallback( + function (...$args) use ( + $existingId, + $existingEmail, + $existingWebsiteId + ) { + /** @var callable $callable */ + foreach ($args[2] as $callable) { + $callable( + new DataObject([ + 'id' => $existingId, + 'email' => $existingEmail, + 'website_id' => $existingWebsiteId, + ]) + ); + } + } + ); $this->assertEquals( - $customer['entity_id'], - $this->_model->getCustomerId($customer['email'], $customer['website_id']) + $existingId, + $this->_model->getCustomerId($existingEmail, $existingWebsiteId) + ); + $this->assertFalse( + $this->_model->getCustomerId( + $nonExistingEmail, + $nonExistingWebsiteId + ) ); - $this->assertFalse($this->_model->getCustomerId('new@test.com', $customer['website_id'])); } /** diff --git a/dev/tests/integration/testsuite/Magento/Customer/_files/import_export/customers_for_address_import.php b/dev/tests/integration/testsuite/Magento/Customer/_files/import_export/customers_for_address_import.php index 80b3613378d9b..9a90061a6de76 100644 --- a/dev/tests/integration/testsuite/Magento/Customer/_files/import_export/customers_for_address_import.php +++ b/dev/tests/integration/testsuite/Magento/Customer/_files/import_export/customers_for_address_import.php @@ -4,6 +4,7 @@ * See COPYING.txt for license details. */ //Create customer +/** @var Magento\Customer\Model\Customer $customer */ $customer = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( \Magento\Customer\Model\Customer::class ); @@ -75,5 +76,4 @@ $addressSecond->isObjectNew(true); $customer->addAddress($addressSecond); $customer->setDefaultShipping($addressSecond->getId()); -$customer->isObjectNew(true); $customer->save(); diff --git a/dev/tests/integration/testsuite/Magento/CustomerImportExport/Model/Import/AddressTest.php b/dev/tests/integration/testsuite/Magento/CustomerImportExport/Model/Import/AddressTest.php index 0915506f06114..96b7f8ce8ed67 100644 --- a/dev/tests/integration/testsuite/Magento/CustomerImportExport/Model/Import/AddressTest.php +++ b/dev/tests/integration/testsuite/Magento/CustomerImportExport/Model/Import/AddressTest.php @@ -132,15 +132,6 @@ public function testConstruct() ); $this->assertAttributeNotEmpty('_attributes', $this->_entityAdapter, 'Attributes must not be empty'); - // check addresses - $this->assertAttributeInternalType( - 'array', - '_addresses', - $this->_entityAdapter, - 'Addresses must be an array.' - ); - $this->assertAttributeNotEmpty('_addresses', $this->_entityAdapter, 'Addresses must not be empty'); - // check country regions and regions $this->assertAttributeInternalType( 'array', @@ -154,62 +145,6 @@ public function testConstruct() $this->assertAttributeNotEmpty('_regions', $this->_entityAdapter, 'Regions must not be empty'); } - /** - * Test _initAddresses - * - * @magentoDataFixture Magento/Customer/_files/import_export/customer_with_addresses.php - */ - public function testInitAddresses() - { - /** @var $objectManager \Magento\TestFramework\ObjectManager */ - $objectManager = Bootstrap::getObjectManager(); - - // get addressed from fixture - $customers = $objectManager->get(\Magento\Framework\Registry::class)->registry($this->_fixtureKey); - $correctAddresses = []; - /** @var $customer \Magento\Customer\Model\Customer */ - foreach ($customers as $customer) { - $correctAddresses[$customer->getId()] = []; - /** @var $address \Magento\Customer\Model\Address */ - foreach ($customer->getAddressesCollection() as $address) { - $correctAddresses[$customer->getId()][] = $address->getId(); - } - } - - // invoke _initAddresses - $initAddresses = new \ReflectionMethod($this->_testClassName, '_initAddresses'); - $initAddresses->setAccessible(true); - $initAddresses->invoke($this->_entityAdapter); - - // check addresses - $this->assertAttributeInternalType( - 'array', - '_addresses', - $this->_entityAdapter, - 'Addresses must be an array.' - ); - $this->assertAttributeNotEmpty('_addresses', $this->_entityAdapter, 'Addresses must not be empty'); - - $addressesReflection = new \ReflectionProperty($this->_testClassName, '_addresses'); - $addressesReflection->setAccessible(true); - $testAddresses = $addressesReflection->getValue($this->_entityAdapter); - - $correctCustomerIds = array_keys($correctAddresses); - $testCustomerIds = array_keys($testAddresses); - sort($correctCustomerIds); - sort($testCustomerIds); - $this->assertEquals($correctCustomerIds, $testCustomerIds, 'Incorrect customer IDs in addresses array.'); - - foreach ($correctCustomerIds as $customerId) { - $this->assertInternalType('array', $correctAddresses[$customerId], 'Addresses must be an array.'); - $correctAddressIds = $correctAddresses[$customerId]; - $testAddressIds = $testAddresses[$customerId]; - sort($correctAddressIds); - sort($testAddressIds); - $this->assertEquals($correctAddressIds, $testAddressIds, 'Incorrect addresses IDs.'); - } - } - /** * Test _saveAddressEntity * diff --git a/dev/tests/integration/testsuite/Magento/CustomerImportExport/_files/two_addresses_rollback.php b/dev/tests/integration/testsuite/Magento/CustomerImportExport/_files/two_addresses_rollback.php index 510ad2e360b42..6e371a102d4a0 100644 --- a/dev/tests/integration/testsuite/Magento/CustomerImportExport/_files/two_addresses_rollback.php +++ b/dev/tests/integration/testsuite/Magento/CustomerImportExport/_files/two_addresses_rollback.php @@ -17,29 +17,6 @@ /** @var Registry $registry */ $registry = Bootstrap::getObjectManager()->get(Registry::class); -//Removing addresses -$registry->unregister('isSecureArea'); -$registry->register('isSecureArea', true); -/** @var Address $customerAddress */ -$customerAddress = Bootstrap::getObjectManager()->create(Address::class); -$customerAddress->load(1); -if ($customerAddress->getId()) { - $customerAddress->delete(); -} -$registry->unregister('isSecureArea'); -$registry->register('isSecureArea', false); -//Second address -$registry->unregister('isSecureArea'); -$registry->register('isSecureArea', true); -/** @var Address $customerAddress */ -$customerAddress = Bootstrap::getObjectManager()->create(Address::class); -$customerAddress->load(2); -if ($customerAddress->getId()) { - $customerAddress->delete(); -} -$registry->unregister('isSecureArea'); -$registry->register('isSecureArea', false); - //Removing customers. $registry->unregister('isSecureArea'); $registry->register('isSecureArea', true); From 0c5c9126609d669a6e324f8cca253ecd4f3244b1 Mon Sep 17 00:00:00 2001 From: Myroslav Dobra Date: Fri, 25 May 2018 02:40:03 +0300 Subject: [PATCH 02/21] MAGETWO-90808: [Performance] Customer Import check data does not complete --- .../Test/Unit/Model/Import/CustomerCompositeTest.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerCompositeTest.php b/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerCompositeTest.php index b72678ece4cde..9fe8c87a390dc 100644 --- a/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerCompositeTest.php +++ b/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerCompositeTest.php @@ -343,10 +343,6 @@ public function testValidateRow(array $rows, array $calls, $validationReturn, ar ->method('validateRow') ->will($this->returnValue($validationReturn)); - $customerEntity->expects($this->any()) - ->method('getErrorMessages') - ->will($this->returnValue([])); - $addressEntity ->expects($this->exactly($calls['addressValidationCalls'])) ->method('validateRow') @@ -362,8 +358,6 @@ public function testValidateRow(array $rows, array $calls, $validationReturn, ar ->method('getCustomerStorage') ->will($this->returnValue($customerStorage)); - $addressEntity->expects($this->any())->method('getErrorMessages')->will($this->returnValue([])); - $data = $this->_getModelDependencies(); $data['customer_entity'] = $customerEntity; $data['address_entity'] = $addressEntity; From af1709a9edc5cca7cf4bf6ad5200e0906a73e6fc Mon Sep 17 00:00:00 2001 From: Myroslav Dobra Date: Fri, 25 May 2018 03:43:56 +0300 Subject: [PATCH 03/21] MAGETWO-90808: [Performance] Customer Import check data does not complete --- app/code/Magento/CustomerImportExport/Model/Import/Address.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/CustomerImportExport/Model/Import/Address.php b/app/code/Magento/CustomerImportExport/Model/Import/Address.php index 3803280e68403..e3f1d68f8d1d5 100644 --- a/app/code/Magento/CustomerImportExport/Model/Import/Address.php +++ b/app/code/Magento/CustomerImportExport/Model/Import/Address.php @@ -170,7 +170,7 @@ class Address extends AbstractCustomer /** * @deprecated - */ + */ protected $_addressCollection; /** From 08f733477be28283233b9218061ce139e3f33c02 Mon Sep 17 00:00:00 2001 From: Myroslav Dobra Date: Tue, 29 May 2018 10:40:34 +0300 Subject: [PATCH 04/21] MAGETWO-90808: [Performance] Customer Import check data does not complete --- .../Test/Unit/Model/Import/CustomerCompositeTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerCompositeTest.php b/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerCompositeTest.php index 9fe8c87a390dc..c19f1ac9e38c9 100644 --- a/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerCompositeTest.php +++ b/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerCompositeTest.php @@ -232,6 +232,7 @@ protected function _getModelMockForImportData($isDeleteBehavior, $customerImport if ($isDeleteBehavior || !$customerImport) { $addressEntity->expects($this->never())->method('importData'); } else { + $addressEntity->expects($this->once())->method('setCustomerAttributes')->will($this->returnSelf()); $addressEntity->expects($this->once())->method('importData')->will($this->returnValue($addressImport)); } @@ -654,7 +655,6 @@ public function dataProviderTestImportData() */ public function testImportData($behavior, $customerImport, $addressImport, $result) { - return; $isDeleteBehavior = $behavior == Import::BEHAVIOR_DELETE; $entityMock = $this->_getModelMockForImportData($isDeleteBehavior, $customerImport, $addressImport); $entityMock->setParameters(['behavior' => $behavior]); From 647ad8a2231af88ead3622b91127f32b36fa18f8 Mon Sep 17 00:00:00 2001 From: OlgaVasyltsun Date: Fri, 1 Jun 2018 11:15:17 +0300 Subject: [PATCH 05/21] MAGETWO-50831: Impossible specify Bundle option title on store view level --- .../Initialization/Helper/Plugin/Bundle.php | 5 + .../Magento/Bundle/Model/OptionRepository.php | 2 +- .../Bundle/Model/Product/SaveHandler.php | 122 ++++++++++++------ .../Bundle/Model/ResourceModel/Option.php | 60 ++------- .../Helper/Plugin/BundleTest.php | 23 ++++ .../Bundle/Model/Product/SaveHandlerTest.php | 86 ++++++++++++ .../Model/Export/RowCustomizerTest.php | 48 +++++++ 7 files changed, 256 insertions(+), 90 deletions(-) create mode 100644 dev/tests/integration/testsuite/Magento/Bundle/Model/Product/SaveHandlerTest.php diff --git a/app/code/Magento/Bundle/Controller/Adminhtml/Product/Initialization/Helper/Plugin/Bundle.php b/app/code/Magento/Bundle/Controller/Adminhtml/Product/Initialization/Helper/Plugin/Bundle.php index 7f21d9e69c6e0..3c9eac68eb9e4 100644 --- a/app/code/Magento/Bundle/Controller/Adminhtml/Product/Initialization/Helper/Plugin/Bundle.php +++ b/app/code/Magento/Bundle/Controller/Adminhtml/Product/Initialization/Helper/Plugin/Bundle.php @@ -105,8 +105,13 @@ public function afterInitialize( if ($result['bundle_options'] && !$compositeReadonly) { $product->setBundleOptionsData($result['bundle_options']); } + $this->processBundleOptionsData($product); $this->processDynamicOptionsData($product); + } elseif (!$compositeReadonly) { + $extension = $product->getExtensionAttributes(); + $extension->setBundleProductOptions([]); + $product->setExtensionAttributes($extension); } $affectProductSelections = (bool)$this->request->getPost('affect_bundle_product_selections'); diff --git a/app/code/Magento/Bundle/Model/OptionRepository.php b/app/code/Magento/Bundle/Model/OptionRepository.php index a70f4af56e989..59e658b08df28 100644 --- a/app/code/Magento/Bundle/Model/OptionRepository.php +++ b/app/code/Magento/Bundle/Model/OptionRepository.php @@ -234,7 +234,7 @@ protected function updateOptionSelection( */ private function getProduct($sku) { - $product = $this->productRepository->get($sku, true); + $product = $this->productRepository->get($sku, true, null, true); if ($product->getTypeId() != \Magento\Catalog\Model\Product\Type::TYPE_BUNDLE) { throw new InputException(__('This is implemented for bundle products only.')); } diff --git a/app/code/Magento/Bundle/Model/Product/SaveHandler.php b/app/code/Magento/Bundle/Model/Product/SaveHandler.php index e5fa688c7fece..6a860bab4fb7b 100644 --- a/app/code/Magento/Bundle/Model/Product/SaveHandler.php +++ b/app/code/Magento/Bundle/Model/Product/SaveHandler.php @@ -5,7 +5,6 @@ */ namespace Magento\Bundle\Model\Product; -use Magento\Bundle\Api\Data\OptionInterface; use Magento\Bundle\Model\Option\SaveAction; use Magento\Catalog\Api\Data\ProductInterface; use Magento\Bundle\Api\ProductOptionRepositoryInterface as OptionRepository; @@ -58,36 +57,6 @@ public function __construct( ?: ObjectManager::getInstance()->get(MetadataPool::class); } - /** - * @param ProductInterface $bundle - * @param OptionInterface[] $currentOptions - * - * @return void - */ - private function removeOldOptions( - ProductInterface $bundle, - array $currentOptions - ) { - $oldOptions = $this->optionRepository->getList($bundle->getSku()); - if ($oldOptions) { - $remainingOptions = []; - $metadata - = $this->metadataPool->getMetadata(ProductInterface::class); - $productId = $bundle->getData($metadata->getLinkField()); - - foreach ($currentOptions as $option) { - $remainingOptions[] = $option->getOptionId(); - } - foreach ($oldOptions as $option) { - if (!in_array($option->getOptionId(), $remainingOptions)) { - $option->setParentId($productId); - $this->removeOptionLinks($bundle->getSku(), $option); - $this->optionRepository->delete($option); - } - } - } - } - /** * @param object $entity * @param array $arguments @@ -98,24 +67,31 @@ private function removeOldOptions( */ public function execute($entity, $arguments = []) { - /** @var \Magento\Bundle\Api\Data\OptionInterface[] $options */ - $options = $entity->getExtensionAttributes()->getBundleProductOptions() ?: []; + /** @var \Magento\Bundle\Api\Data\OptionInterface[] $bundleProductOptions */ + $bundleProductOptions = $entity->getExtensionAttributes()->getBundleProductOptions() ?: []; //Only processing bundle products. - if ($entity->getTypeId() !== 'bundle' || empty($options)) { + if ($entity->getTypeId() !== Type::TYPE_CODE || empty($bundleProductOptions)) { return $entity; } - /** @var ProductInterface $entity */ - //Removing old options + + $existingBundleProductOptions = $this->optionRepository->getList($entity->getSku()); + $existingOptionsIds = !empty($existingBundleProductOptions) + ? $this->getOptionIds($existingBundleProductOptions) + : []; + $optionIds = !empty($bundleProductOptions) + ? $this->getOptionIds($bundleProductOptions) + : []; + if (!$entity->getCopyFromView()) { - $this->removeOldOptions($entity, $options); - } - //Saving active options. - foreach ($options as $option) { - $this->optionSave->save($entity, $option); + $this->processRemovedOptions($entity->getSku(), $existingOptionsIds, $optionIds); + $newOptionsIds = array_diff($optionIds, $existingOptionsIds); + $this->saveOptions($entity, $bundleProductOptions, $newOptionsIds); + } else { + //save only labels and not selections + product links + $this->saveOptions($entity, $bundleProductOptions); + $entity->setCopyFromView(false); } - $entity->setCopyFromView(false); - return $entity; } @@ -133,4 +109,64 @@ protected function removeOptionLinks($entitySku, $option) } } } + + /** + * Perform save for all options entities. + * + * @param object $entity + * @param array $options + * @param array $newOptionsIds + * @return void + */ + private function saveOptions($entity, array $options, array $newOptionsIds = []): void + { + foreach ($options as $option) { + if (in_array($option->getOptionId(), $newOptionsIds, true)) { + $option->setOptionId(null); + } + + $this->optionSave->save($entity, $option); + } + } + + /** + * Get options ids from array of the options entities. + * + * @param array $options + * @return array + */ + private function getOptionIds(array $options): array + { + $optionIds = []; + + if (empty($options)) { + return $optionIds; + } + + /** @var \Magento\Bundle\Api\Data\OptionInterface $option */ + foreach ($options as $option) { + if ($option->getOptionId()) { + $optionIds[] = $option->getOptionId(); + } + } + + return $optionIds; + } + + /** + * Removes old options that no longer exists. + * + * @param string $entitySku + * @param array $existingOptionsIds + * @param array $optionIds + * @return void + */ + private function processRemovedOptions(string $entitySku, array $existingOptionsIds, array $optionIds): void + { + foreach (array_diff($existingOptionsIds, $optionIds) as $optionId) { + $option = $this->optionRepository->get($entitySku, $optionId); + $this->removeOptionLinks($entitySku, $option); + $this->optionRepository->delete($option); + } + } } diff --git a/app/code/Magento/Bundle/Model/ResourceModel/Option.php b/app/code/Magento/Bundle/Model/ResourceModel/Option.php index 46fd8b910f6f1..7babd0b349f02 100644 --- a/app/code/Magento/Bundle/Model/ResourceModel/Option.php +++ b/app/code/Magento/Bundle/Model/ResourceModel/Option.php @@ -81,39 +81,29 @@ protected function _afterSave(\Magento\Framework\Model\AbstractModel $object) { parent::_afterSave($object); - $conditions = [ + $condition = [ 'option_id = ?' => $object->getId(), 'store_id = ? OR store_id = 0' => $object->getStoreId(), 'parent_product_id = ?' => $object->getParentId() ]; $connection = $this->getConnection(); + $connection->delete($this->getTable('catalog_product_bundle_option_value'), $condition); - if ($this->isOptionPresent($conditions)) { - $connection->update( - $this->getTable('catalog_product_bundle_option_value'), - [ - 'title' => $object->getTitle() - ], - $conditions - ); - } else { - $data = new \Magento\Framework\DataObject(); - $data->setOptionId($object->getId()) - ->setStoreId($object->getStoreId()) - ->setParentProductId($object->getParentId()) - ->setTitle($object->getTitle()); + $data = new \Magento\Framework\DataObject(); + $data->setOptionId($object->getId()) + ->setStoreId($object->getStoreId()) + ->setParentProductId($object->getParentId()) + ->setTitle($object->getTitle()); - $connection->insert($this->getTable('catalog_product_bundle_option_value'), $data->getData()); + $connection->insert($this->getTable('catalog_product_bundle_option_value'), $data->getData()); - /** - * also saving default value if this store view scope - */ - if ($object->getStoreId()) { - $data->setStoreId(0); - $data->setTitle($object->getDefaultTitle()); - $connection->insert($this->getTable('catalog_product_bundle_option_value'), $data->getData()); - } + /** + * also saving default fallback value + */ + if (0 !== (int)$object->getStoreId()) { + $data->setStoreId(0)->setTitle($object->getDefaultTitle()); + $connection->insert($this->getTable('catalog_product_bundle_option_value'), $data->getData()); } return $this; @@ -218,26 +208,4 @@ public function save(\Magento\Framework\Model\AbstractModel $object) return $this; } - - /** - * Is Bundle option present in the database - * - * @param array $conditions - * - * @return bool - */ - private function isOptionPresent($conditions) - { - $connection = $this->getConnection(); - - $select = $connection->select()->from($this->getTable('catalog_product_bundle_option_value')); - foreach ($conditions as $condition => $conditionValue) { - $select->where($condition, $conditionValue); - } - $select->limit(1); - - $rowSelect = $connection->fetchRow($select); - - return (is_array($rowSelect) && !empty($rowSelect)); - } } diff --git a/app/code/Magento/Bundle/Test/Unit/Controller/Adminhtml/Product/Initialization/Helper/Plugin/BundleTest.php b/app/code/Magento/Bundle/Test/Unit/Controller/Adminhtml/Product/Initialization/Helper/Plugin/BundleTest.php index 59a2190a43e0c..1fa7f186786ae 100644 --- a/app/code/Magento/Bundle/Test/Unit/Controller/Adminhtml/Product/Initialization/Helper/Plugin/BundleTest.php +++ b/app/code/Magento/Bundle/Test/Unit/Controller/Adminhtml/Product/Initialization/Helper/Plugin/BundleTest.php @@ -163,4 +163,27 @@ public function testAfterInitializeIfBundleSelectionsAndCustomOptionsExist() $this->productMock->expects($this->once())->method('setCanSaveBundleSelections')->with(false); $this->model->afterInitialize($this->subjectMock, $this->productMock); } + + /** + * @return void + */ + public function testAfterInitializeIfBundleOptionsNotExist(): void + { + $valueMap = [ + ['bundle_options', null, null], + ['affect_bundle_product_selections', null, false], + ]; + $this->requestMock->expects($this->any())->method('getPost')->will($this->returnValueMap($valueMap)); + $extentionAttribute = $this->getMockBuilder(\Magento\Catalog\Api\Data\ProductExtensionInterface::class) + ->disableOriginalConstructor() + ->setMethods(['setBundleProductOptions']) + ->getMockForAbstractClass(); + $extentionAttribute->expects($this->once())->method('setBundleProductOptions')->with([]); + $this->productMock->expects($this->any())->method('getCompositeReadonly')->will($this->returnValue(false)); + $this->productMock->expects($this->once())->method('getExtensionAttributes')->willReturn($extentionAttribute); + $this->productMock->expects($this->once())->method('setExtensionAttributes')->with($extentionAttribute); + $this->productMock->expects($this->once())->method('setCanSaveBundleSelections')->with(false); + + $this->model->afterInitialize($this->subjectMock, $this->productMock); + } } diff --git a/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/SaveHandlerTest.php b/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/SaveHandlerTest.php new file mode 100644 index 0000000000000..46bfbea3eebd7 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/SaveHandlerTest.php @@ -0,0 +1,86 @@ +objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager(); + $this->store = $this->objectManager->create(\Magento\Store\Model\Store::class); + /** @var ProductRepositoryInterface $productRepository */ + $this->productRepository = $this->objectManager->create(ProductRepositoryInterface::class); + } + + public function testOptionTitlesOnDifferentStores() + { + /** + * @var \Magento\Bundle\Model\Product\OptionList $optionList + */ + $optionList = $this->objectManager->create(\Magento\Bundle\Model\Product\OptionList::class); + + $secondStoreId = $this->store->load('fixture_second_store')->getId(); + $thirdStoreId = $this->store->load('fixture_third_store')->getId(); + + $product = $this->productRepository->get('bundle-product', true, $secondStoreId, true); + $options = $optionList->getItems($product); + $title = $options[0]->getTitle(); + $newTitle = $title . ' ' . $this->store->load('fixture_second_store')->getCode(); + $options[0]->setTitle($newTitle); + $extension = $product->getExtensionAttributes(); + $extension->setBundleProductOptions($options); + $product->setExtensionAttributes($extension); + $product->save(); + + $product = $this->productRepository->get('bundle-product', true, $thirdStoreId, true); + $options = $optionList->getItems($product); + $newTitle = $title . ' ' . $this->store->load('fixture_third_store')->getCode(); + $options[0]->setTitle($newTitle); + $extension = $product->getExtensionAttributes(); + $extension->setBundleProductOptions($options); + $product->setExtensionAttributes($extension); + $product->save(); + + $product = $this->productRepository->get('bundle-product', false, $secondStoreId, true); + $options = $optionList->getItems($product); + $this->assertEquals(1, count($options)); + $this->assertEquals( + $title . ' ' . $this->store->load('fixture_second_store')->getCode(), + $options[0]->getTitle() + ); + } +} diff --git a/dev/tests/integration/testsuite/Magento/BundleImportExport/Model/Export/RowCustomizerTest.php b/dev/tests/integration/testsuite/Magento/BundleImportExport/Model/Export/RowCustomizerTest.php index 6f81421c902f6..6a7d0f2e51c20 100644 --- a/dev/tests/integration/testsuite/Magento/BundleImportExport/Model/Export/RowCustomizerTest.php +++ b/dev/tests/integration/testsuite/Magento/BundleImportExport/Model/Export/RowCustomizerTest.php @@ -56,4 +56,52 @@ public function testPrepareData() $this->assertEquals([], $this->model->addData([], $ids['simple'])); $this->assertEquals($parsedAdditionalAttributes, $result['additional_attributes']); } + + /** + * @magentoDataFixture Magento/Store/_files/second_store.php + * @magentoDataFixture Magento/Bundle/_files/product.php + * @magentoDbIsolation disabled + */ + public function testPrepareDataWithDifferentStoreValues() + { + $storeCode = 'default'; + $expectedNames = [ + 'name' => 'Bundle Product Items', + 'name_' . $storeCode => 'Bundle Product Items_' . $storeCode + ]; + $parsedAdditionalAttributes = 'text_attribute=!@#$%^&*()_+1234567890-=|\\:;"\'<,>.?/' + . ',text_attribute2=,'; + $allAdditionalAttributes = $parsedAdditionalAttributes . ',weight_type=0,price_type=1'; + $collection = $this->objectManager->get(\Magento\Catalog\Model\ResourceModel\Product\Collection::class); + /** @var \Magento\Store\Model\Store $store */ + $store = $this->objectManager->create(\Magento\Store\Model\Store::class); + $store->load($storeCode, 'code'); + /** @var \Magento\Catalog\Api\ProductRepositoryInterface $productRepository */ + $productRepository = $this->objectManager->get(\Magento\Catalog\Api\ProductRepositoryInterface::class); + $product = $productRepository->get('bundle-product', 1, $store->getId()); + + $extension = $product->getExtensionAttributes(); + $options = $extension->getBundleProductOptions(); + + foreach ($options as $productOption) { + $productOption->setTitle($productOption->getTitle() . '_' . $store->getCode()); + } + $extension->setBundleProductOptions($options); + $product->setExtensionAttributes($extension); + $productRepository->save($product); + $this->model->prepareData($collection, [$product->getId()]); + $result = $this->model->addData(['additional_attributes' => $allAdditionalAttributes], $product->getId()); + $bundleValues = array_map( + function ($input) { + $data = explode('=', $input); + return [$data[0] => $data[1]]; + }, + explode(',', $result['bundle_values']) + ); + $actualNames = [ + 'name' => array_column($bundleValues, 'name')[0], + 'name' . '_' . $store->getCode() => array_column($bundleValues, 'name' . '_' . $store->getCode())[0] + ]; + self::assertSame($expectedNames, $actualNames); + } } From 513f1e0c2a60f6f4d367c7d3beecfd4a936113f3 Mon Sep 17 00:00:00 2001 From: Stas Kozar Date: Fri, 1 Jun 2018 12:39:51 +0300 Subject: [PATCH 06/21] MAGETWO-91803: Double click (not too fast) on proceed to check out after view edit mini cart returns empty shopping cart --- .../Magento/Checkout/view/frontend/web/js/proceed-to-checkout.js | 1 + 1 file changed, 1 insertion(+) diff --git a/app/code/Magento/Checkout/view/frontend/web/js/proceed-to-checkout.js b/app/code/Magento/Checkout/view/frontend/web/js/proceed-to-checkout.js index f0679c657ab90..0bb0a53ce0a6b 100644 --- a/app/code/Magento/Checkout/view/frontend/web/js/proceed-to-checkout.js +++ b/app/code/Magento/Checkout/view/frontend/web/js/proceed-to-checkout.js @@ -22,6 +22,7 @@ define([ return false; } + $(element).attr('disabled', true); location.href = config.checkoutUrl; }); From e7c3e29a1caca0be477250ad124fcf8a62f0e925 Mon Sep 17 00:00:00 2001 From: Myroslav Dobra Date: Mon, 4 Jun 2018 12:26:53 +0300 Subject: [PATCH 07/21] MAGETWO-90808: [Performance] Customer Import check data does not complete --- .../Model/Import/Address.php | 17 ++-- .../Model/Import/Customer.php | 11 +-- .../Model/Import/CustomerComposite.php | 2 +- .../ResourceModel/Import/Address/Storage.php | 12 +-- .../ResourceModel/Import/Customer/Storage.php | 22 +++-- .../Test/Unit/Model/Import/AddressTest.php | 5 +- .../Model/Import/CustomerCompositeTest.php | 82 ++++++++----------- .../Test/Unit/Model/Import/CustomerTest.php | 2 +- .../Import/Customer/StorageTest.php | 25 +++--- 9 files changed, 78 insertions(+), 100 deletions(-) diff --git a/app/code/Magento/CustomerImportExport/Model/Import/Address.php b/app/code/Magento/CustomerImportExport/Model/Import/Address.php index e3f1d68f8d1d5..adff4afc79b4e 100644 --- a/app/code/Magento/CustomerImportExport/Model/Import/Address.php +++ b/app/code/Magento/CustomerImportExport/Model/Import/Address.php @@ -169,6 +169,9 @@ class Address extends AbstractCustomer protected $_attributeCollection; /** + * Collection of existent addresses + * + * @var \Magento\Customer\Model\ResourceModel\Address\Collection * @deprecated */ protected $_addressCollection; @@ -227,7 +230,7 @@ class Address extends AbstractCustomer protected $postcodeValidator; /** - * @deprecated + * @var CountryWithWebsitesSource */ private $countryWithWebsites; @@ -319,9 +322,6 @@ public function __construct( $data ); - $this->_addressCollection = isset( - $data['address_collection'] - ) ? $data['address_collection'] : $addressColFactory->create(); $this->_entityTable = isset( $data['entity_table'] ) ? $data['entity_table'] : $addressFactory->create()->getResource()->getEntityTable(); @@ -459,18 +459,17 @@ protected function _initCountryRegions() * * @return void */ - public function prepareCustomerData($rows) + public function prepareCustomerData($rows): void { $customersPresent = []; foreach ($rows as $rowData) { - $email = isset($rowData[static::COLUMN_EMAIL]) - ? $rowData[static::COLUMN_EMAIL] : null; + $email = $rowData[static::COLUMN_EMAIL] ?? null; $websiteId = isset($rowData[static::COLUMN_WEBSITE]) ? $this->getWebsiteId($rowData[static::COLUMN_WEBSITE]) : false; if ($email && $websiteId !== false) { $customersPresent[] = [ 'email' => $email, - 'website_id' => $websiteId + 'website_id' => $websiteId, ]; } } @@ -917,7 +916,7 @@ protected function _validateRowForDelete(array $rowData, $rowNumber) if (!strlen($addressId)) { $this->addRowError(self::ERROR_ADDRESS_ID_IS_EMPTY, $rowNumber); } elseif (!$this->addressStorage->doesExist( - $addressId, + (string)$addressId, (string)$customerId )) { $this->addRowError(self::ERROR_ADDRESS_NOT_FOUND, $rowNumber); diff --git a/app/code/Magento/CustomerImportExport/Model/Import/Customer.php b/app/code/Magento/CustomerImportExport/Model/Import/Customer.php index a1aa62c5f1c89..e5cc543db6aac 100644 --- a/app/code/Magento/CustomerImportExport/Model/Import/Customer.php +++ b/app/code/Magento/CustomerImportExport/Model/Import/Customer.php @@ -347,26 +347,23 @@ protected function _getNextEntityId() } /** - * Pre-loading customers for existing customers checks in order - * to perform mass validation/import efficiently. + * Prepare customers data for existing customers checks to perform mass validation/import efficiently. * * @param array|AbstractSource $rows - * and website code. * * @return void */ - public function prepareCustomerData($rows) + public function prepareCustomerData($rows): void { $customersPresent = []; foreach ($rows as $rowData) { - $email = isset($rowData[static::COLUMN_EMAIL]) - ? $rowData[static::COLUMN_EMAIL] : null; + $email = $rowData[static::COLUMN_EMAIL] ?? null; $websiteId = isset($rowData[static::COLUMN_WEBSITE]) ? $this->getWebsiteId($rowData[static::COLUMN_WEBSITE]) : false; if ($email && $websiteId !== false) { $customersPresent[] = [ 'email' => $email, - 'website_id' => $websiteId + 'website_id' => $websiteId, ]; } } diff --git a/app/code/Magento/CustomerImportExport/Model/Import/CustomerComposite.php b/app/code/Magento/CustomerImportExport/Model/Import/CustomerComposite.php index b2c82f13fc554..956c9695623bb 100644 --- a/app/code/Magento/CustomerImportExport/Model/Import/CustomerComposite.php +++ b/app/code/Magento/CustomerImportExport/Model/Import/CustomerComposite.php @@ -300,7 +300,7 @@ public function validateData() foreach ($source as $row) { $rows[] = [ Address::COLUMN_EMAIL => $row[Customer::COLUMN_EMAIL], - Address::COLUMN_WEBSITE => $row[Customer::COLUMN_WEBSITE] + Address::COLUMN_WEBSITE => $row[Customer::COLUMN_WEBSITE], ]; } $source->rewind(); diff --git a/app/code/Magento/CustomerImportExport/Model/ResourceModel/Import/Address/Storage.php b/app/code/Magento/CustomerImportExport/Model/ResourceModel/Import/Address/Storage.php index bb73c105055b6..6d29d7ae792ff 100644 --- a/app/code/Magento/CustomerImportExport/Model/ResourceModel/Import/Address/Storage.php +++ b/app/code/Magento/CustomerImportExport/Model/ResourceModel/Import/Address/Storage.php @@ -67,13 +67,11 @@ public function __construct( * * @return void */ - private function addRecord(string $customerId, string $addressId) + private function addRecord(string $customerId, string $addressId): void { if (!$customerId || !$addressId) { return; } - $customerId = (string)$customerId; - $addressId = (string)$addressId; if (!array_key_exists($customerId, $this->addresses)) { $this->addresses[$customerId] = []; } @@ -89,9 +87,8 @@ private function addRecord(string $customerId, string $addressId) * @param string[] $customerIds * * @return void - * @throws \Zend_Db_Select_Exception */ - private function loadAddresses(array $customerIds) + private function loadAddresses(array $customerIds): void { /** @var AddressCollection $collection */ $collection = $this->addressCollectionFactory->create(); @@ -122,7 +119,7 @@ public function doesExist(string $addressId, string $forCustomerId): bool { return array_key_exists($forCustomerId, $this->addresses) && in_array( - (string)$addressId, + $addressId, $this->addresses[$forCustomerId], true ); @@ -133,9 +130,8 @@ public function doesExist(string $addressId, string $forCustomerId): bool * * @param string[] $forCustomersIds * @return void - * @throws \Zend_Db_Select_Exception */ - public function prepareAddresses(array $forCustomersIds) + public function prepareAddresses(array $forCustomersIds): void { if (!$forCustomersIds) { return; diff --git a/app/code/Magento/CustomerImportExport/Model/ResourceModel/Import/Customer/Storage.php b/app/code/Magento/CustomerImportExport/Model/ResourceModel/Import/Customer/Storage.php index dec8c0739213e..03c75297343c3 100644 --- a/app/code/Magento/CustomerImportExport/Model/ResourceModel/Import/Customer/Storage.php +++ b/app/code/Magento/CustomerImportExport/Model/ResourceModel/Import/Customer/Storage.php @@ -5,6 +5,7 @@ */ namespace Magento\CustomerImportExport\Model\ResourceModel\Import\Customer; +use Magento\CustomerImportExport\Test\Unit\Model\Import\CustomerCompositeTest; use Magento\Framework\DataObject; use Magento\Framework\DB\Select; use Magento\Customer\Model\ResourceModel\Customer\CollectionFactory as CustomerCollectionFactory; @@ -15,11 +16,17 @@ class Storage { /** + * Flag to not load collection more than one time + * + * @var bool * @deprecated */ protected $_isCollectionLoaded = false; /** + * Customer collection + * + * @var \Magento\Customer\Model\ResourceModel\Customer\Collection * @deprecated */ protected $_customerCollection; @@ -78,8 +85,9 @@ public function __construct( } /** - * @return void + * Load needed data from customer collection * + * @return void * @deprecated * @see prepareCustomers */ @@ -104,7 +112,6 @@ public function load() * @param array[] $customerIdentifiers With keys "email" and "website_id". * * @return CustomerCollection - * @throws \Zend_Db_Select_Exception */ private function prepareCollection(array $customerIdentifiers): CustomerCollection { @@ -132,7 +139,6 @@ function (array $customer) { * @param array $customerIdentifiers With keys "email" and "website_id". * * @return void - * @throws \Zend_Db_Select_Exception */ private function loadCustomersData(array $customerIdentifiers) { @@ -147,7 +153,7 @@ private function loadCustomersData(array $customerIdentifiers) * @param array $customer * @return $this */ - public function addCustomerByArray(array $customer): self + public function addCustomerByArray(array $customer): Storage { $email = strtolower(trim($customer['email'])); if (!isset($this->_customerIds[$email])) { @@ -165,7 +171,7 @@ public function addCustomerByArray(array $customer): self * @param DataObject $customer * @return $this */ - public function addCustomer(DataObject $customer): self + public function addCustomer(DataObject $customer): Storage { $customerData = $customer->toArray(); if (!isset($customerData['entity_id']) && isset($customer['id'])) { @@ -182,7 +188,6 @@ public function addCustomer(DataObject $customer): self * @param string $email * @param int $websiteId * @return bool|int - * @throws \Zend_Db_Select_Exception */ public function getCustomerId(string $email, int $websiteId) { @@ -205,9 +210,8 @@ public function getCustomerId(string $email, int $websiteId) * * @param array[] $customersToFind With keys: email, website_id. * @return void - * @throws \Zend_Db_Select_Exception */ - public function prepareCustomers(array $customersToFind) + public function prepareCustomers(array $customersToFind): void { $identifiers = []; foreach ($customersToFind as $customerToFind) { @@ -221,7 +225,7 @@ public function prepareCustomers(array $customersToFind) $uniqueKey = $email .'_' .$websiteId; $identifiers[$uniqueKey] = [ 'email' => $email, - 'website_id' => $websiteId + 'website_id' => $websiteId, ]; //Recording that we've searched for a customer. if (!array_key_exists($email, $this->_customerIds)) { diff --git a/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/AddressTest.php b/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/AddressTest.php index d96ebd9aca666..113d6fa06ba31 100644 --- a/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/AddressTest.php +++ b/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/AddressTest.php @@ -261,8 +261,8 @@ protected function _createCustomerStorageMock() ->willReturnCallback( function ($email, $websiteId) { foreach ($this->_customers as $customerData) { - if ($customerData['email'] == $email - && $customerData['website_id'] == $websiteId + if ($customerData['email'] === $email + && $customerData['website_id'] === $websiteId ) { return $customerData['entity_id']; } @@ -334,7 +334,6 @@ public function iterate(\Magento\Framework\Data\Collection $collection, $pageSiz * Create mock for custom behavior test * * @return Address|\PHPUnit_Framework_MockObject_MockObject - * @throws \ReflectionException */ protected function _getModelMockForTestImportDataWithCustomBehaviour() { diff --git a/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerCompositeTest.php b/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerCompositeTest.php index c19f1ac9e38c9..983e20eb77d80 100644 --- a/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerCompositeTest.php +++ b/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerCompositeTest.php @@ -185,26 +185,26 @@ protected function _getModelMockForPrepareRowForDb() 'stdClass', ['getCustomerId', 'prepareCustomers', 'addCustomer'] ); - $customerStorage->expects($this->any())->method('getCustomerId')->will($this->returnValue(1)); + $customerStorage->expects($this->any())->method('getCustomerId')->willReturn(1); $customerEntity = $this->_getCustomerEntityMock(); - $customerEntity->expects($this->any())->method('validateRow')->will($this->returnValue(true)); + $customerEntity->expects($this->any())->method('validateRow')->willReturn(true); $customerEntity->expects($this->any()) ->method('getCustomerStorage') - ->will($this->returnValue($customerStorage)); + ->willReturn($customerStorage); $customerEntity->expects($this->any()) ->method('getValidColumnNames') ->willReturn(['cols']); $addressEntity = $this->_getAddressEntityMock(); - $addressEntity->expects($this->any())->method('validateRow')->will($this->returnValue(true)); + $addressEntity->expects($this->any())->method('validateRow')->willReturn(true); $addressEntity->expects($this->any()) ->method('getCustomerStorage') - ->will($this->returnValue($customerStorage)); + ->willReturn($customerStorage); $dataSourceMock = $this->createPartialMock(\stdClass::class, ['cleanBunches', 'saveBunch']); $dataSourceMock->expects($this->any()) ->method('saveBunch') - ->will($this->returnCallback([$this, 'verifyPrepareRowForDbData'])); + ->willReturnCallback([$this, 'verifyPrepareRowForDbData']); $data = $this->_getModelDependencies(); $data['customer_entity'] = $customerEntity; @@ -225,15 +225,15 @@ protected function _getModelMockForPrepareRowForDb() protected function _getModelMockForImportData($isDeleteBehavior, $customerImport, $addressImport) { $customerEntity = $this->_getCustomerEntityMock(); - $customerEntity->expects($this->once())->method('importData')->will($this->returnValue($customerImport)); + $customerEntity->expects($this->once())->method('importData')->willReturn($customerImport); $addressEntity = $this->_getAddressEntityMock(); // address import starts only if customer import finished successfully if ($isDeleteBehavior || !$customerImport) { $addressEntity->expects($this->never())->method('importData'); } else { - $addressEntity->expects($this->once())->method('setCustomerAttributes')->will($this->returnSelf()); - $addressEntity->expects($this->once())->method('importData')->will($this->returnValue($addressImport)); + $addressEntity->expects($this->once())->method('setCustomerAttributes')->willReturnSelf(); + $addressEntity->expects($this->once())->method('importData')->willReturn($addressImport); } $data = $this->_getModelDependencies(); @@ -255,13 +255,9 @@ protected function _getCustomerEntityMock() $attribute = new \Magento\Framework\DataObject(['attribute_code' => $code]); $attributeList[] = $attribute; } - $customerEntity->expects( - $this->once() - )->method( - 'getAttributeCollection' - )->will( - $this->returnValue($attributeList) - ); + $customerEntity->expects($this->once()) + ->method('getAttributeCollection') + ->willReturn($attributeList); return $customerEntity; } @@ -278,13 +274,9 @@ protected function _getAddressEntityMock() $attribute = new \Magento\Framework\DataObject(['attribute_code' => $code]); $attributeList[] = $attribute; } - $addressEntity->expects( - $this->once() - )->method( - 'getAttributeCollection' - )->will( - $this->returnValue($attributeList) - ); + $addressEntity->expects($this->once()) + ->method('getAttributeCollection') + ->willReturn($attributeList); return $addressEntity; } @@ -342,22 +334,22 @@ public function testValidateRow(array $rows, array $calls, $validationReturn, ar $customerEntity->expects($this->exactly($calls['customerValidationCalls'])) ->method('validateRow') - ->will($this->returnValue($validationReturn)); + ->willReturn($validationReturn); $addressEntity ->expects($this->exactly($calls['addressValidationCalls'])) ->method('validateRow') - ->will($this->returnValue($validationReturn)); + ->willReturn($validationReturn); $customerStorage = $this->createPartialMock(\stdClass::class, ['getCustomerId']); - $customerStorage->expects($this->any())->method('getCustomerId')->will($this->returnValue(true)); + $customerStorage->expects($this->any())->method('getCustomerId')->willReturn(true); $addressEntity->expects($this->any()) ->method('getCustomerStorage') - ->will($this->returnValue($customerStorage)); + ->willReturn($customerStorage); $customerEntity->expects($this->any()) ->method('getCustomerStorage') - ->will($this->returnValue($customerStorage)); + ->willReturn($customerStorage); $data = $this->_getModelDependencies(); $data['customer_entity'] = $customerEntity; @@ -379,20 +371,20 @@ public function testPrepareAddressRowData() $customerEntity = $this->_getCustomerEntityMock(); $addressEntity = $this->_getAddressEntityMock(); - $customerEntity->expects($this->once())->method('validateRow')->will($this->returnValue(true)); + $customerEntity->expects($this->once())->method('validateRow')->willReturn(true); $addressEntity->expects($this->once()) ->method('validateRow') - ->will($this->returnCallback([$this, 'validateAddressRowParams'])); + ->willReturnCallback([$this, 'validateAddressRowParams']); $customerStorage = $this->createPartialMock(\stdClass::class, ['getCustomerId']); - $customerStorage->expects($this->any())->method('getCustomerId')->will($this->returnValue(true)); + $customerStorage->expects($this->any())->method('getCustomerId')->willReturn(true); $addressEntity->expects($this->any()) ->method('getCustomerStorage') - ->will($this->returnValue($customerStorage)); + ->willReturn($customerStorage); $customerEntity->expects($this->any()) ->method('getCustomerStorage') - ->will($this->returnValue($customerStorage)); + ->willReturn($customerStorage); $data = $this->_getModelDependencies(); $data['customer_entity'] = $customerEntity; @@ -507,20 +499,12 @@ public function testSetParameters() $customerEntity = $this->_getCustomerEntityMock(); $addressEntity = $this->_getAddressEntityMock(); - $customerEntity->expects( - $this->once() - )->method( - 'setParameters' - )->will( - $this->returnCallback([$this, 'callbackCheckParameters']) - ); - $addressEntity->expects( - $this->once() - )->method( - 'setParameters' - )->will( - $this->returnCallback([$this, 'callbackCheckParameters']) - ); + $customerEntity->expects($this->once()) + ->method('setParameters') + ->willReturnCallback([$this, 'callbackCheckParameters']); + $addressEntity->expects($this->once()) + ->method('setParameters') + ->willReturnCallback([$this, 'callbackCheckParameters']); $data = $this->_getModelDependencies(); $data['customer_entity'] = $customerEntity; $data['address_entity'] = $addressEntity; @@ -686,8 +670,8 @@ protected function _getModelForGetterTest($method, $customerReturnData, $address $customerEntity = $this->_getCustomerEntityMock(); $addressEntity = $this->_getAddressEntityMock(); - $customerEntity->expects($this->once())->method($method)->will($this->returnValue($customerReturnData)); - $addressEntity->expects($this->once())->method($method)->will($this->returnValue($addressReturnData)); + $customerEntity->expects($this->once())->method($method)->willReturn($customerReturnData); + $addressEntity->expects($this->once())->method($method)->willReturn($addressReturnData); $data = $this->_getModelDependencies(); $data['customer_entity'] = $customerEntity; diff --git a/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerTest.php b/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerTest.php index 64c547c0ae22b..9a7183d5b5f72 100644 --- a/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerTest.php +++ b/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerTest.php @@ -91,7 +91,7 @@ protected function _getModelMockForTestImportDataWithCustomBehaviour() '_saveCustomerAttributes', '_deleteCustomerEntities', 'getErrorAggregator', - 'getCustomerStorage' + 'getCustomerStorage', ] ) ->getMock(); diff --git a/app/code/Magento/CustomerImportExport/Test/Unit/Model/ResourceModel/Import/Customer/StorageTest.php b/app/code/Magento/CustomerImportExport/Test/Unit/Model/ResourceModel/Import/Customer/StorageTest.php index 90342f7817364..1f2f9f3fed583 100644 --- a/app/code/Magento/CustomerImportExport/Test/Unit/Model/ResourceModel/Import/Customer/StorageTest.php +++ b/app/code/Magento/CustomerImportExport/Test/Unit/Model/ResourceModel/Import/Customer/StorageTest.php @@ -3,6 +3,10 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + +/** + * Test class for \Magento\CustomerImportExport\Model\ResourceModel\Import\Customer\Storage + */ namespace Magento\CustomerImportExport\Test\Unit\Model\ResourceModel\Import\Customer; use Magento\CustomerImportExport\Model\ResourceModel\Import\Customer\Storage; @@ -18,7 +22,7 @@ class StorageTest extends \PHPUnit\Framework\TestCase /** * @var Storage */ - private $_model; + private $model; /** * @var CollectionByPagesIterator|\PHPUnit_Framework_MockObject_MockObject @@ -30,11 +34,6 @@ class StorageTest extends \PHPUnit\Framework\TestCase */ private $collectionMock; - /** - * @var string - */ - protected $_entityTable = 'test'; - protected function setUp() { $this->iteratorMock = $this->createMock( @@ -65,7 +64,7 @@ protected function setUp() ->method('getSelect') ->willReturn($selectMock); - $this->_model = new Storage( + $this->model = new Storage( $collectionFactoryMock, $iteratorFactoryMock, [] @@ -74,7 +73,7 @@ protected function setUp() protected function tearDown() { - unset($this->_model); + unset($this->model); } public function testAddCustomerByArray() @@ -82,9 +81,9 @@ public function testAddCustomerByArray() $propertyName = '_customerIds'; $customer = $this->_addCustomerToStorage(); - $this->assertAttributeCount(1, $propertyName, $this->_model); + $this->assertAttributeCount(1, $propertyName, $this->model); $expectedCustomerData = [$customer['website_id'] => $customer['entity_id']]; - $this->assertAttributeContains($expectedCustomerData, $propertyName, $this->_model); + $this->assertAttributeContains($expectedCustomerData, $propertyName, $this->model); } public function testGetCustomerId() @@ -117,10 +116,10 @@ function (...$args) use ( ); $this->assertEquals( $existingId, - $this->_model->getCustomerId($existingEmail, $existingWebsiteId) + $this->model->getCustomerId($existingEmail, $existingWebsiteId) ); $this->assertFalse( - $this->_model->getCustomerId( + $this->model->getCustomerId( $nonExistingEmail, $nonExistingWebsiteId ) @@ -133,7 +132,7 @@ function (...$args) use ( protected function _addCustomerToStorage() { $customer = ['entity_id' => 1, 'website_id' => 1, 'email' => 'test@test.com']; - $this->_model->addCustomerByArray($customer); + $this->model->addCustomerByArray($customer); return $customer; } From 55d4cf8c4a601988bca0ee331b89f6f85a72f458 Mon Sep 17 00:00:00 2001 From: Myroslav Dobra Date: Mon, 4 Jun 2018 13:42:13 +0300 Subject: [PATCH 08/21] MAGETWO-90808: [Performance] Customer Import check data does not complete --- app/code/Magento/CustomerImportExport/Model/Import/Address.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/code/Magento/CustomerImportExport/Model/Import/Address.php b/app/code/Magento/CustomerImportExport/Model/Import/Address.php index adff4afc79b4e..5ddf18031c7bc 100644 --- a/app/code/Magento/CustomerImportExport/Model/Import/Address.php +++ b/app/code/Magento/CustomerImportExport/Model/Import/Address.php @@ -260,7 +260,6 @@ class Address extends AbstractCustomer * @param \Magento\Customer\Model\AddressFactory $addressFactory * @param \Magento\Directory\Model\ResourceModel\Region\CollectionFactory $regionColFactory * @param \Magento\Customer\Model\CustomerFactory $customerFactory - * @param \Magento\Customer\Model\ResourceModel\Address\CollectionFactory $addressColFactory * @param \Magento\Customer\Model\ResourceModel\Address\Attribute\CollectionFactory $attributesFactory * @param \Magento\Framework\Stdlib\DateTime $dateTime * @param \Magento\Customer\Model\Address\Validator\Postcode $postcodeValidator @@ -285,7 +284,6 @@ public function __construct( \Magento\Customer\Model\AddressFactory $addressFactory, \Magento\Directory\Model\ResourceModel\Region\CollectionFactory $regionColFactory, \Magento\Customer\Model\CustomerFactory $customerFactory, - \Magento\Customer\Model\ResourceModel\Address\CollectionFactory $addressColFactory, \Magento\Customer\Model\ResourceModel\Address\Attribute\CollectionFactory $attributesFactory, \Magento\Framework\Stdlib\DateTime $dateTime, \Magento\Customer\Model\Address\Validator\Postcode $postcodeValidator, From c9435610db0d4e9a71d93e5fb467cb6bef3e9336 Mon Sep 17 00:00:00 2001 From: OlgaVasyltsun Date: Mon, 4 Jun 2018 14:34:19 +0300 Subject: [PATCH 09/21] MAGETWO-50831: Impossible specify Bundle option title on store view level --- .../Bundle/Model/Product/SaveHandler.php | 14 ++++++-------- .../Bundle/Model/Product/SaveHandlerTest.php | 5 ++++- .../Model/Export/RowCustomizerTest.php | 17 +++++++++++++---- 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/app/code/Magento/Bundle/Model/Product/SaveHandler.php b/app/code/Magento/Bundle/Model/Product/SaveHandler.php index 6a860bab4fb7b..fc215aa6b8e20 100644 --- a/app/code/Magento/Bundle/Model/Product/SaveHandler.php +++ b/app/code/Magento/Bundle/Model/Product/SaveHandler.php @@ -139,14 +139,12 @@ private function getOptionIds(array $options): array { $optionIds = []; - if (empty($options)) { - return $optionIds; - } - - /** @var \Magento\Bundle\Api\Data\OptionInterface $option */ - foreach ($options as $option) { - if ($option->getOptionId()) { - $optionIds[] = $option->getOptionId(); + if (!empty($options)) { + /** @var \Magento\Bundle\Api\Data\OptionInterface $option */ + foreach ($options as $option) { + if ($option->getOptionId()) { + $optionIds[] = $option->getOptionId(); + } } } diff --git a/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/SaveHandlerTest.php b/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/SaveHandlerTest.php index 46bfbea3eebd7..381675d01ae1d 100644 --- a/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/SaveHandlerTest.php +++ b/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/SaveHandlerTest.php @@ -46,7 +46,10 @@ protected function setUp() $this->productRepository = $this->objectManager->create(ProductRepositoryInterface::class); } - public function testOptionTitlesOnDifferentStores() + /** + * @return void + */ + public function testOptionTitlesOnDifferentStores(): void { /** * @var \Magento\Bundle\Model\Product\OptionList $optionList diff --git a/dev/tests/integration/testsuite/Magento/BundleImportExport/Model/Export/RowCustomizerTest.php b/dev/tests/integration/testsuite/Magento/BundleImportExport/Model/Export/RowCustomizerTest.php index 6a7d0f2e51c20..53e8281ffbdf1 100644 --- a/dev/tests/integration/testsuite/Magento/BundleImportExport/Model/Export/RowCustomizerTest.php +++ b/dev/tests/integration/testsuite/Magento/BundleImportExport/Model/Export/RowCustomizerTest.php @@ -20,6 +20,9 @@ class RowCustomizerTest extends \PHPUnit\Framework\TestCase */ private $objectManager; + /** + * @inheritdoc + */ protected function setUp() { $this->objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager(); @@ -30,8 +33,10 @@ protected function setUp() /** * @magentoDataFixture Magento/Bundle/_files/product.php + * + * @return void */ - public function testPrepareData() + public function testPrepareData(): void { $parsedAdditionalAttributes = 'text_attribute=!@#$%^&*()_+1234567890-=|\\:;"\'<,>.?/' . ',text_attribute2=,'; @@ -61,13 +66,15 @@ public function testPrepareData() * @magentoDataFixture Magento/Store/_files/second_store.php * @magentoDataFixture Magento/Bundle/_files/product.php * @magentoDbIsolation disabled + * + * @return void */ - public function testPrepareDataWithDifferentStoreValues() + public function testPrepareDataWithDifferentStoreValues(): void { $storeCode = 'default'; $expectedNames = [ 'name' => 'Bundle Product Items', - 'name_' . $storeCode => 'Bundle Product Items_' . $storeCode + 'name_' . $storeCode => 'Bundle Product Items_' . $storeCode, ]; $parsedAdditionalAttributes = 'text_attribute=!@#$%^&*()_+1234567890-=|\\:;"\'<,>.?/' . ',text_attribute2=,'; @@ -94,14 +101,16 @@ public function testPrepareDataWithDifferentStoreValues() $bundleValues = array_map( function ($input) { $data = explode('=', $input); + return [$data[0] => $data[1]]; }, explode(',', $result['bundle_values']) ); $actualNames = [ 'name' => array_column($bundleValues, 'name')[0], - 'name' . '_' . $store->getCode() => array_column($bundleValues, 'name' . '_' . $store->getCode())[0] + 'name' . '_' . $store->getCode() => array_column($bundleValues, 'name' . '_' . $store->getCode())[0], ]; + self::assertSame($expectedNames, $actualNames); } } From 38cabd3b517a5cfec6ce1ed01071da38b2a538ea Mon Sep 17 00:00:00 2001 From: Myroslav Dobra Date: Mon, 4 Jun 2018 15:08:16 +0300 Subject: [PATCH 10/21] MAGETWO-90808: [Performance] Customer Import check data does not complete --- .../CustomerImportExport/Test/Unit/Model/Import/AddressTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/AddressTest.php b/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/AddressTest.php index 113d6fa06ba31..126a9e1791779 100644 --- a/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/AddressTest.php +++ b/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/AddressTest.php @@ -445,7 +445,6 @@ protected function _getModelMock() $this->createMock(\Magento\Customer\Model\AddressFactory::class), $this->createMock(\Magento\Directory\Model\ResourceModel\Region\CollectionFactory::class), $this->createMock(\Magento\Customer\Model\CustomerFactory::class), - $this->createMock(\Magento\Customer\Model\ResourceModel\Address\CollectionFactory::class), $this->createMock(\Magento\Customer\Model\ResourceModel\Address\Attribute\CollectionFactory::class), new \Magento\Framework\Stdlib\DateTime(), $this->createMock(\Magento\Customer\Model\Address\Validator\Postcode::class), From 7a3d351a5e4aa22ebe00e06cac45d4d23bdd2e31 Mon Sep 17 00:00:00 2001 From: OlgaVasyltsun Date: Tue, 5 Jun 2018 09:11:05 +0300 Subject: [PATCH 11/21] MAGETWO-90764: [2.3.0] Cannot cancel orders with an expired authorization for Braintree --- .../Gateway/Response/CancelDetailsHandler.php | 43 +++++ .../Braintree/Gateway/SubjectReader.php | 9 +- .../Validator/CancelResponseValidator.php | 88 +++++++++ .../Response/CancelDetailsHandlerTest.php | 61 ++++++ .../Test/Unit/Gateway/SubjectReaderTest.php | 98 ++++++++-- .../Validator/CancelResponseValidatorTest.php | 179 ++++++++++++++++++ app/code/Magento/Braintree/etc/di.xml | 14 +- 7 files changed, 473 insertions(+), 19 deletions(-) create mode 100644 app/code/Magento/Braintree/Gateway/Response/CancelDetailsHandler.php create mode 100644 app/code/Magento/Braintree/Gateway/Validator/CancelResponseValidator.php create mode 100644 app/code/Magento/Braintree/Test/Unit/Gateway/Response/CancelDetailsHandlerTest.php create mode 100644 app/code/Magento/Braintree/Test/Unit/Gateway/Validator/CancelResponseValidatorTest.php diff --git a/app/code/Magento/Braintree/Gateway/Response/CancelDetailsHandler.php b/app/code/Magento/Braintree/Gateway/Response/CancelDetailsHandler.php new file mode 100644 index 0000000000000..3d6ed025791bf --- /dev/null +++ b/app/code/Magento/Braintree/Gateway/Response/CancelDetailsHandler.php @@ -0,0 +1,43 @@ +subjectReader = $subjectReader; + } + + /** + * @inheritdoc + */ + public function handle(array $handlingSubject, array $response) + { + $paymentDO = $this->subjectReader->readPayment($handlingSubject); + /** @var Payment $orderPayment */ + $orderPayment = $paymentDO->getPayment(); + $orderPayment->setIsTransactionClosed(true); + $orderPayment->setShouldCloseParentTransaction(true); + } +} diff --git a/app/code/Magento/Braintree/Gateway/SubjectReader.php b/app/code/Magento/Braintree/Gateway/SubjectReader.php index d5dc43a4c5e34..7cf00233e7f8f 100644 --- a/app/code/Magento/Braintree/Gateway/SubjectReader.php +++ b/app/code/Magento/Braintree/Gateway/SubjectReader.php @@ -43,19 +43,20 @@ public function readPayment(array $subject) } /** - * Reads transaction from subject + * Reads transaction from the subject. * * @param array $subject - * @return \Braintree\Transaction + * @return Transaction + * @throws \InvalidArgumentException if the subject doesn't contain transaction details. */ public function readTransaction(array $subject) { if (!isset($subject['object']) || !is_object($subject['object'])) { - throw new \InvalidArgumentException('Response object does not exist'); + throw new \InvalidArgumentException('Response object does not exist.'); } if (!isset($subject['object']->transaction) - && !$subject['object']->transaction instanceof Transaction + || !$subject['object']->transaction instanceof Transaction ) { throw new \InvalidArgumentException('The object is not a class \Braintree\Transaction.'); } diff --git a/app/code/Magento/Braintree/Gateway/Validator/CancelResponseValidator.php b/app/code/Magento/Braintree/Gateway/Validator/CancelResponseValidator.php new file mode 100644 index 0000000000000..3c4380747fab0 --- /dev/null +++ b/app/code/Magento/Braintree/Gateway/Validator/CancelResponseValidator.php @@ -0,0 +1,88 @@ +generalResponseValidator = $generalResponseValidator; + $this->subjectReader = $subjectReader; + } + + /** + * @inheritdoc + */ + public function validate(array $validationSubject): ResultInterface + { + $result = $this->generalResponseValidator->validate($validationSubject); + if (!$result->isValid()) { + $response = $this->subjectReader->readResponseObject($validationSubject); + if ($this->isErrorAcceptable($response->errors)) { + $result = $this->createResult(true, [__('Transaction is cancelled offline.')]); + } + } + + return $result; + } + + /** + * Checks if error collection has an acceptable error code. + * + * @param ErrorCollection $errorCollection + * @return bool + */ + private function isErrorAcceptable(ErrorCollection $errorCollection): bool + { + $errors = $errorCollection->deepAll(); + // there is should be only one acceptable error + if (count($errors) > 1) { + return false; + } + + /** @var Validation $error */ + $error = array_pop($errors); + + return (int)$error->code === self::$acceptableTransactionCode; + } +} diff --git a/app/code/Magento/Braintree/Test/Unit/Gateway/Response/CancelDetailsHandlerTest.php b/app/code/Magento/Braintree/Test/Unit/Gateway/Response/CancelDetailsHandlerTest.php new file mode 100644 index 0000000000000..2fa3d2ea65836 --- /dev/null +++ b/app/code/Magento/Braintree/Test/Unit/Gateway/Response/CancelDetailsHandlerTest.php @@ -0,0 +1,61 @@ +handler = new CancelDetailsHandler(new SubjectReader()); + } + + /** + * Checks a case when cancel handler closes the current and parent transactions. + * + * @return void + */ + public function testHandle(): void + { + /** @var OrderAdapterInterface|MockObject $order */ + $order = $this->getMockForAbstractClass(OrderAdapterInterface::class); + /** @var Payment|MockObject $payment */ + $payment = $this->getMockBuilder(Payment::class) + ->disableOriginalConstructor() + ->setMethods(['setOrder']) + ->getMock(); + + $paymentDO = new PaymentDataObject($order, $payment); + $response = [ + 'payment' => $paymentDO, + ]; + + $this->handler->handle($response, []); + + self::assertTrue($payment->getIsTransactionClosed(), 'The current transaction should be closed.'); + self::assertTrue($payment->getShouldCloseParentTransaction(), 'The parent transaction should be closed.'); + } +} diff --git a/app/code/Magento/Braintree/Test/Unit/Gateway/SubjectReaderTest.php b/app/code/Magento/Braintree/Test/Unit/Gateway/SubjectReaderTest.php index 4213acc8b4ff0..fdf3c583188af 100644 --- a/app/code/Magento/Braintree/Test/Unit/Gateway/SubjectReaderTest.php +++ b/app/code/Magento/Braintree/Test/Unit/Gateway/SubjectReaderTest.php @@ -5,6 +5,7 @@ */ namespace Magento\Braintree\Test\Unit\Gateway; +use Braintree\Result\Successful; use Braintree\Transaction; use Magento\Braintree\Gateway\SubjectReader; @@ -18,6 +19,9 @@ class SubjectReaderTest extends \PHPUnit\Framework\TestCase */ private $subjectReader; + /** + * @inheritdoc + */ protected function setUp() { $this->subjectReader = new SubjectReader(); @@ -27,67 +31,137 @@ protected function setUp() * @covers \Magento\Braintree\Gateway\SubjectReader::readCustomerId * @expectedException \InvalidArgumentException * @expectedExceptionMessage The "customerId" field does not exists + * @return void */ - public function testReadCustomerIdWithException() + public function testReadCustomerIdWithException(): void { $this->subjectReader->readCustomerId([]); } /** * @covers \Magento\Braintree\Gateway\SubjectReader::readCustomerId + * @return void */ - public function testReadCustomerId() + public function testReadCustomerId(): void { $customerId = 1; - static::assertEquals($customerId, $this->subjectReader->readCustomerId(['customer_id' => $customerId])); + $this->assertEquals($customerId, $this->subjectReader->readCustomerId(['customer_id' => $customerId])); } /** * @covers \Magento\Braintree\Gateway\SubjectReader::readPublicHash * @expectedException \InvalidArgumentException * @expectedExceptionMessage The "public_hash" field does not exists + * @return void */ - public function testReadPublicHashWithException() + public function testReadPublicHashWithException(): void { $this->subjectReader->readPublicHash([]); } /** * @covers \Magento\Braintree\Gateway\SubjectReader::readPublicHash + * @return void */ - public function testReadPublicHash() + public function testReadPublicHash(): void { $hash = 'fj23djf2o1fd'; - static::assertEquals($hash, $this->subjectReader->readPublicHash(['public_hash' => $hash])); + $this->assertEquals($hash, $this->subjectReader->readPublicHash(['public_hash' => $hash])); } /** * @covers \Magento\Braintree\Gateway\SubjectReader::readPayPal * @expectedException \InvalidArgumentException * @expectedExceptionMessage Transaction has't paypal attribute + * @return void */ - public function testReadPayPalWithException() + public function testReadPayPalWithException(): void { $transaction = Transaction::factory([ - 'id' => 'u38rf8kg6vn' + 'id' => 'u38rf8kg6vn', ]); $this->subjectReader->readPayPal($transaction); } /** * @covers \Magento\Braintree\Gateway\SubjectReader::readPayPal + * @return void */ - public function testReadPayPal() + public function testReadPayPal(): void { $paypal = [ 'paymentId' => '3ek7dk7fn0vi1', - 'payerEmail' => 'payer@example.com' + 'payerEmail' => 'payer@example.com', ]; $transaction = Transaction::factory([ 'id' => '4yr95vb', - 'paypal' => $paypal + 'paypal' => $paypal, ]); - static::assertEquals($paypal, $this->subjectReader->readPayPal($transaction)); + $this->assertEquals($paypal, $this->subjectReader->readPayPal($transaction)); + } + + /** + * Checks a case when subject reader retrieves successful Braintree transaction. + * + * @return void + */ + public function testReadTransaction(): void + { + $transaction = Transaction::factory(['id' => 1]); + $response = [ + 'object' => new Successful($transaction, 'transaction'), + ]; + $actual = $this->subjectReader->readTransaction($response); + + $this->assertSame($transaction, $actual); + } + + /** + * Checks a case when subject reader retrieves invalid data instead transaction details. + * + * @param array $response + * @param string $expectedMessage + * @dataProvider invalidTransactionResponseDataProvider + * @expectedException \InvalidArgumentException + * @return void + */ + public function testReadTransactionWithInvalidResponse(array $response, string $expectedMessage):void + { + $this->expectExceptionMessage($expectedMessage); + $this->subjectReader->readTransaction($response); + } + + /** + * Gets list of variations with invalid subject data. + * + * @return array + */ + public function invalidTransactionResponseDataProvider(): array + { + $transaction = new \stdClass(); + $response = new \stdClass(); + $response->transaction = $transaction; + + return [ + [ + 'response' => [ + 'object' => [], + ], + 'expectedMessage' => 'Response object does not exist.', + ], + [ + 'response' => [ + 'object' => new \stdClass(), + ], + 'expectedMessage' => 'The object is not a class \Braintree\Transaction.', + ], + [ + 'response' => [ + 'object' => $response, + ], + 'expectedMessage' => 'The object is not a class \Braintree\Transaction.', + ], + ]; } } diff --git a/app/code/Magento/Braintree/Test/Unit/Gateway/Validator/CancelResponseValidatorTest.php b/app/code/Magento/Braintree/Test/Unit/Gateway/Validator/CancelResponseValidatorTest.php new file mode 100644 index 0000000000000..54d8151f7819f --- /dev/null +++ b/app/code/Magento/Braintree/Test/Unit/Gateway/Validator/CancelResponseValidatorTest.php @@ -0,0 +1,179 @@ +generalValidator = $this->getMockBuilder(GeneralResponseValidator::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->resultFactory = $this->getMockBuilder(ResultInterfaceFactory::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->validator = new CancelResponseValidator( + $this->resultFactory, + $this->generalValidator, + new SubjectReader() + ); + } + + /** + * Checks a case when response is successful and additional validation doesn't needed. + * + * @return void + */ + public function testValidateSuccessfulTransaction(): void + { + /** @var ResultInterface|MockObject $result */ + $result = $this->getMockForAbstractClass(ResultInterface::class); + $result->method('isValid')->willReturn(true); + $this->generalValidator->method('validate')->willReturn($result); + $actual = $this->validator->validate([]); + + $this->assertSame($result, $actual); + } + + /** + * Checks a case when response contains error related to expired authorization transaction and + * validator should return positive result. + * + * @return void + */ + public function testValidateExpiredTransaction(): void + { + /** @var ResultInterface|MockObject $result */ + $result = $this->getMockForAbstractClass(ResultInterface::class); + $result->method('isValid')->willReturn(false); + $this->generalValidator->method('validate')->willReturn($result); + + $expected = $this->getMockForAbstractClass(ResultInterface::class); + $expected->method('isValid')->willReturn(true); + $this->resultFactory->method('create') + ->with( + [ + 'isValid' => true, + 'failsDescription' => ['Transaction is cancelled offline.'], + 'errorCodes' => [] + ] + )->willReturn($expected); + + $errors = [ + 'errors' => [ + [ + 'code' => 91504, + 'message' => 'Transaction can only be voided if status is authorized.', + ] + ], + ]; + $buildSubject = [ + 'response' => [ + 'object' => new Error(['errors' => $errors]), + ], + ]; + + $actual = $this->validator->validate($buildSubject); + + $this->assertSame($expected, $actual); + } + + /** + * Checks a case when response contains multiple errors and validator should return negative result. + * + * @param array $responseErrors + * @return void + * @dataProvider getErrorsDataProvider + */ + public function testValidateWithMultipleErrors(array $responseErrors): void + { + /** @var ResultInterface|MockObject $result */ + $result = $this->getMockForAbstractClass(ResultInterface::class); + $result->method('isValid')->willReturn(false); + + $this->generalValidator->method('validate')->willReturn($result); + + $this->resultFactory->expects($this->never())->method('create'); + + $errors = [ + 'errors' => $responseErrors, + ]; + $buildSubject = [ + 'response' => [ + 'object' => new Error(['errors' => $errors]), + ] + ]; + + $actual = $this->validator->validate($buildSubject); + + $this->assertSame($result, $actual); + } + + /** + * Gets list of errors variations. + * + * @return array + */ + public function getErrorsDataProvider(): array + { + return [ + [ + 'errors' => [ + [ + 'code' => 91734, + 'message' => 'Credit card type is not accepted by this merchant account.', + ], + [ + 'code' => 91504, + 'message' => 'Transaction can only be voided if status is authorized.', + ], + ], + ], + [ + 'errors' => [ + [ + 'code' => 91734, + 'message' => 'Credit card type is not accepted by this merchant account.', + ], + ], + ], + ]; + } +} diff --git a/app/code/Magento/Braintree/etc/di.xml b/app/code/Magento/Braintree/etc/di.xml index 2bb4cea6742d6..290fb5be58f34 100644 --- a/app/code/Magento/Braintree/etc/di.xml +++ b/app/code/Magento/Braintree/etc/di.xml @@ -133,8 +133,8 @@ BraintreeVaultCaptureCommand BraintreeVoidCommand BraintreeRefundCommand - BraintreeVoidCommand - BraintreeVoidCommand + Magento\Braintree\Gateway\CancelCommand + Magento\Braintree\Gateway\CancelCommand @@ -150,7 +150,7 @@ BraintreeVaultCaptureCommand BraintreeVoidCommand BraintreeRefundCommand - BraintreeVoidCommand + Magento\Braintree\Gateway\CancelCommand @@ -505,6 +505,14 @@ + + + + Magento\Braintree\Gateway\Response\CancelDetailsHandler + Magento\Braintree\Gateway\Validator\CancelResponseValidator + + + From b45ef300b01543490511d6f9878bdd4b4379d1f6 Mon Sep 17 00:00:00 2001 From: Myroslav Dobra Date: Tue, 5 Jun 2018 10:18:37 +0300 Subject: [PATCH 12/21] MAGETWO-90808: [Performance] Customer Import check data does not complete --- .../Model/Import/Address.php | 8 ---- .../ResourceModel/Import/Customer/Storage.php | 38 ------------------- 2 files changed, 46 deletions(-) diff --git a/app/code/Magento/CustomerImportExport/Model/Import/Address.php b/app/code/Magento/CustomerImportExport/Model/Import/Address.php index 5ddf18031c7bc..e1345edcf146d 100644 --- a/app/code/Magento/CustomerImportExport/Model/Import/Address.php +++ b/app/code/Magento/CustomerImportExport/Model/Import/Address.php @@ -168,14 +168,6 @@ class Address extends AbstractCustomer */ protected $_attributeCollection; - /** - * Collection of existent addresses - * - * @var \Magento\Customer\Model\ResourceModel\Address\Collection - * @deprecated - */ - protected $_addressCollection; - /** * Store imported row primary keys * diff --git a/app/code/Magento/CustomerImportExport/Model/ResourceModel/Import/Customer/Storage.php b/app/code/Magento/CustomerImportExport/Model/ResourceModel/Import/Customer/Storage.php index 03c75297343c3..f779505a38011 100644 --- a/app/code/Magento/CustomerImportExport/Model/ResourceModel/Import/Customer/Storage.php +++ b/app/code/Magento/CustomerImportExport/Model/ResourceModel/Import/Customer/Storage.php @@ -15,22 +15,6 @@ class Storage { - /** - * Flag to not load collection more than one time - * - * @var bool - * @deprecated - */ - protected $_isCollectionLoaded = false; - - /** - * Customer collection - * - * @var \Magento\Customer\Model\ResourceModel\Customer\Collection - * @deprecated - */ - protected $_customerCollection; - /** * Existing customers information. In form of: * @@ -84,28 +68,6 @@ public function __construct( $this->customerCollectionFactory = $collectionFactory; } - /** - * Load needed data from customer collection - * - * @return void - * @deprecated - * @see prepareCustomers - */ - public function load() - { - if ($this->_isCollectionLoaded == false) { - $connection = $this->_customerCollection->getConnection(); - $select = $connection->select(); - $select->from($this->_customerCollection->getMainTable(), ['entity_id', 'website_id', 'email']); - $results = $connection->fetchAll($select); - foreach ($results as $customer) { - $this->addCustomerByArray($customer); - } - - $this->_isCollectionLoaded = true; - } - } - /** * Create new collection to load customer data with proper filters. * From 75b821d1f1dcbd22b55597e47e0eb2af5e188289 Mon Sep 17 00:00:00 2001 From: Myroslav Dobra Date: Tue, 5 Jun 2018 10:37:06 +0300 Subject: [PATCH 13/21] MAGETWO-90808: [Performance] Customer Import check data does not complete --- .../Test/Unit/Model/Import/CustomerCompositeTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerCompositeTest.php b/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerCompositeTest.php index 983e20eb77d80..1b900c2139588 100644 --- a/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerCompositeTest.php +++ b/app/code/Magento/CustomerImportExport/Test/Unit/Model/Import/CustomerCompositeTest.php @@ -265,7 +265,7 @@ protected function _getCustomerEntityMock() /** * @return Address|\PHPUnit_Framework_MockObject_MockObject */ - protected function _getAddressEntityMock() + private function _getAddressEntityMock() { $addressEntity = $this->createMock(Address::class); From 32e7169078dacb42050f8e4d1165dd8411f225ae Mon Sep 17 00:00:00 2001 From: OlgaVasyltsun Date: Tue, 5 Jun 2018 11:16:34 +0300 Subject: [PATCH 14/21] MAGETWO-92128: Limitation of displayed attribute options number in layered navigation --- .../Query/Builder/Aggregation.php | 9 +++++++ .../Query/Builder/AggregationTest.php | 26 ++++++++++++------- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/app/code/Magento/Elasticsearch/SearchAdapter/Query/Builder/Aggregation.php b/app/code/Magento/Elasticsearch/SearchAdapter/Query/Builder/Aggregation.php index cad32eb727e16..7ba2ef47c77d8 100644 --- a/app/code/Magento/Elasticsearch/SearchAdapter/Query/Builder/Aggregation.php +++ b/app/code/Magento/Elasticsearch/SearchAdapter/Query/Builder/Aggregation.php @@ -15,6 +15,14 @@ */ class Aggregation { + /** + * Max number of results returned per single term bucket, i.e. limit of options for layered navigation filter. + * Default ElasticSearch limit is 10 + * + * @var int + */ + private static $maxTermBacketSize = 500; + /** * @var FieldMapperInterface * @since 100.1.0 @@ -67,6 +75,7 @@ protected function buildBucket( $searchQuery['body']['aggregations'][$bucket->getName()]= [ 'terms' => [ 'field' => $field, + 'size' => self::$maxTermBacketSize, ], ]; break; diff --git a/app/code/Magento/Elasticsearch/Test/Unit/SearchAdapter/Query/Builder/AggregationTest.php b/app/code/Magento/Elasticsearch/Test/Unit/SearchAdapter/Query/Builder/AggregationTest.php index 4d2eb35b50a11..03724caf74dbd 100644 --- a/app/code/Magento/Elasticsearch/Test/Unit/SearchAdapter/Query/Builder/AggregationTest.php +++ b/app/code/Magento/Elasticsearch/Test/Unit/SearchAdapter/Query/Builder/AggregationTest.php @@ -6,6 +6,7 @@ namespace Magento\Elasticsearch\Test\Unit\SearchAdapter\Query\Builder; use Magento\Elasticsearch\SearchAdapter\Query\Builder\Aggregation; +use Magento\Framework\Search\Request\BucketInterface; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; class AggregationTest extends \PHPUnit\Framework\TestCase @@ -26,7 +27,7 @@ class AggregationTest extends \PHPUnit\Framework\TestCase protected $requestInterface; /** - * @var \Magento\Framework\Search\Request\BucketInterface|\PHPUnit_Framework_MockObject_MockObject + * @var BucketInterface|\PHPUnit_Framework_MockObject_MockObject */ protected $requestBucketInterface; @@ -47,7 +48,7 @@ protected function setUp() ->disableOriginalConstructor() ->getMock(); - $this->requestBucketInterface = $this->getMockBuilder(\Magento\Framework\Search\Request\BucketInterface::class) + $this->requestBucketInterface = $this->getMockBuilder(BucketInterface::class) ->disableOriginalConstructor() ->getMock(); @@ -139,28 +140,35 @@ public function testBuildTerm() 'type' => 'product', 'body' => [], ]; + $bucketName = 'price_bucket'; - $this->requestInterface->expects($this->any()) + $this->requestInterface ->method('getAggregation') ->willReturn([$this->requestBucketInterface]); - $this->fieldMapper->expects($this->any()) + $this->fieldMapper ->method('getFieldName') ->willReturn('price'); - $this->requestBucketInterface->expects($this->any()) + $this->requestBucketInterface ->method('getField') ->willReturn('price'); - $this->requestBucketInterface->expects($this->any()) + $this->requestBucketInterface ->method('getType') - ->willReturn('termBucket'); + ->willReturn(BucketInterface::TYPE_TERM); - $this->requestBucketInterface->expects($this->any()) + $this->requestBucketInterface ->method('getName') - ->willReturn('price_bucket'); + ->willReturn($bucketName); $result = $this->model->build($this->requestInterface, $query); + $this->assertNotNull($result); + $this->assertTrue( + isset($result['body']['aggregations'][$bucketName]['terms']['size']), + 'The size have to be specified since by default, ' . + 'the terms aggregation will return only the buckets for the top ten terms ordered by the doc_count' + ); } } From c3f5cd75ccbe51a65b937d77b329853ab8c11669 Mon Sep 17 00:00:00 2001 From: Viktor Sevch Date: Tue, 5 Jun 2018 15:16:45 +0300 Subject: [PATCH 15/21] MAGETWO-67024: Sorting by price ignores Catalog Price Rule for Simple product with required custom option --- .../Model/ResourceModel/Indexer/Price.php | 215 ++++++++++-------- .../Product/Indexer/Price/DefaultPrice.php | 19 +- .../Model/Product/BundlePriceAbstract.php | 8 + .../DynamicBundlePriceCalculatorTest.php | 24 +- ...icBundleWithSpecialPriceCalculatorTest.php | 27 +-- ...namicBundleWithTierPriceCalculatorTest.php | 33 +-- .../FixedBundlePriceCalculatorTest.php | 40 ++-- ...edBundleWithSpecialPriceCalculatorTest.php | 41 +--- ...FixedBundleWithTierPriceCalculatorTest.php | 42 +--- .../Catalog/Model/ProductPriceTest.php | 29 ++- .../Search/Adapter/Mysql/AdapterTest.php | 16 +- 11 files changed, 242 insertions(+), 252 deletions(-) diff --git a/app/code/Magento/Bundle/Model/ResourceModel/Indexer/Price.php b/app/code/Magento/Bundle/Model/ResourceModel/Indexer/Price.php index 401374db86fef..b5bc3b9c3236d 100644 --- a/app/code/Magento/Bundle/Model/ResourceModel/Indexer/Price.php +++ b/app/code/Magento/Bundle/Model/ResourceModel/Indexer/Price.php @@ -153,52 +153,43 @@ protected function _prepareBundlePriceByType($priceType, $entityIds = null) $specialPrice = $this->_addAttributeToSelect($select, 'special_price', "e.$linkField", 'cs.store_id'); $specialFrom = $this->_addAttributeToSelect($select, 'special_from_date', "e.$linkField", 'cs.store_id'); $specialTo = $this->_addAttributeToSelect($select, 'special_to_date', "e.$linkField", 'cs.store_id'); - $curentDate = new \Zend_Db_Expr('cwd.website_date'); - - $specialExpr = $connection->getCheckSql( - $connection->getCheckSql( - $specialFrom . ' IS NULL', - '1', - $connection->getCheckSql($specialFrom . ' <= ' . $curentDate, '1', '0') - ) . " > 0 AND " . $connection->getCheckSql( - $specialTo . ' IS NULL', - '1', - $connection->getCheckSql($specialTo . ' >= ' . $curentDate, '1', '0') - ) . " > 0 AND {$specialPrice} > 0 AND {$specialPrice} < 100 ", - $specialPrice, - '0' - ); + $currentDate = new \Zend_Db_Expr('cwd.website_date'); - $tierExpr = new \Zend_Db_Expr("tp.min_price"); + $specialFromDate = $connection->getDatePartSql($specialFrom); + $specialToDate = $connection->getDatePartSql($specialTo); + $specialFromExpr = "{$specialFrom} IS NULL OR {$specialFromDate} <= {$currentDate}"; + $specialToExpr = "{$specialTo} IS NULL OR {$specialToDate} >= {$currentDate}"; + $specialExpr = "{$specialPrice} IS NOT NULL AND {$specialPrice} > 0 AND {$specialPrice} < 100" + . " AND {$specialFromExpr} AND {$specialToExpr}"; + $tierExpr = new \Zend_Db_Expr('tp.min_price'); if ($priceType == \Magento\Bundle\Model\Product\Price::PRICE_TYPE_FIXED) { - $finalPrice = $connection->getCheckSql( - $specialExpr . ' > 0', - 'ROUND(' . $price . ' * (' . $specialExpr . ' / 100), 4)', - $price + $specialPriceExpr = $connection->getCheckSql( + $specialExpr, + 'ROUND(' . $price . ' * (' . $specialPrice . ' / 100), 4)', + 'NULL' ); $tierPrice = $connection->getCheckSql( $tierExpr . ' IS NOT NULL', - 'ROUND(' . $price . ' - ' . '(' . $price . ' * (' . $tierExpr . ' / 100)), 4)', + 'ROUND((1 - ' . $tierExpr . ' / 100) * ' . $price . ', 4)', 'NULL' ); - - $finalPrice = $connection->getCheckSql( - "{$tierPrice} < {$finalPrice}", - $tierPrice, - $finalPrice - ); + $finalPrice = $connection->getLeastSql([ + $price, + $connection->getIfNullSql($specialPriceExpr, $price), + $connection->getIfNullSql($tierPrice, $price), + ]); } else { - $finalPrice = new \Zend_Db_Expr("0"); + $finalPrice = new \Zend_Db_Expr('0'); $tierPrice = $connection->getCheckSql($tierExpr . ' IS NOT NULL', '0', 'NULL'); } $select->columns( [ 'price_type' => new \Zend_Db_Expr($priceType), - 'special_price' => $specialExpr, + 'special_price' => $connection->getCheckSql($specialExpr, $specialPrice, '0'), 'tier_percent' => $tierExpr, - 'orig_price' => $connection->getCheckSql($price . ' IS NULL', '0', $price), + 'orig_price' => $connection->getIfNullSql($price, '0'), 'price' => $finalPrice, 'min_price' => $finalPrice, 'max_price' => $finalPrice, @@ -246,17 +237,20 @@ protected function _calculateBundleOptionPrice() $this->_prepareBundleOptionTable(); $select = $connection->select()->from( - ['i' => $this->_getBundleSelectionTable()], + $this->_getBundleSelectionTable(), ['entity_id', 'customer_group_id', 'website_id', 'option_id'] )->group( - ['entity_id', 'customer_group_id', 'website_id', 'option_id', 'is_required', 'group_type'] - )->columns( + ['entity_id', 'customer_group_id', 'website_id', 'option_id'] + ); + $minPrice = $connection->getCheckSql('is_required = 1', 'price', 'NULL'); + $tierPrice = $connection->getCheckSql('is_required = 1', 'tier_price', 'NULL'); + $select->columns( [ - 'min_price' => $connection->getCheckSql('i.is_required = 1', 'MIN(i.price)', '0'), - 'alt_price' => $connection->getCheckSql('i.is_required = 0', 'MIN(i.price)', '0'), - 'max_price' => $connection->getCheckSql('i.group_type = 1', 'SUM(i.price)', 'MAX(i.price)'), - 'tier_price' => $connection->getCheckSql('i.is_required = 1', 'MIN(i.tier_price)', '0'), - 'alt_tier_price' => $connection->getCheckSql('i.is_required = 0', 'MIN(i.tier_price)', '0'), + 'min_price' => new \Zend_Db_Expr('MIN(' . $minPrice . ')'), + 'alt_price' => new \Zend_Db_Expr('MIN(price)'), + 'max_price' => $connection->getCheckSql('group_type = 0', 'MAX(price)', 'SUM(price)'), + 'tier_price' => new \Zend_Db_Expr('MIN(' . $tierPrice . ')'), + 'alt_tier_price' => new \Zend_Db_Expr('MIN(tier_price)'), ] ); @@ -264,45 +258,8 @@ protected function _calculateBundleOptionPrice() $connection->query($query); $this->_prepareDefaultFinalPriceTable(); - - $minPrice = new \Zend_Db_Expr( - $connection->getCheckSql('SUM(io.min_price) = 0', 'MIN(io.alt_price)', 'SUM(io.min_price)') . ' + i.price' - ); - $maxPrice = new \Zend_Db_Expr("SUM(io.max_price) + i.price"); - $tierPrice = $connection->getCheckSql( - 'MIN(i.tier_percent) IS NOT NULL', - $connection->getCheckSql( - 'SUM(io.tier_price) = 0', - 'SUM(io.alt_tier_price)', - 'SUM(io.tier_price)' - ) . ' + MIN(i.tier_price)', - 'NULL' - ); - - $select = $connection->select()->from( - ['io' => $this->_getBundleOptionTable()], - ['entity_id', 'customer_group_id', 'website_id'] - )->join( - ['i' => $this->_getBundlePriceTable()], - 'i.entity_id = io.entity_id AND i.customer_group_id = io.customer_group_id' . - ' AND i.website_id = io.website_id', - [] - )->group( - ['io.entity_id', 'io.customer_group_id', 'io.website_id', 'i.tax_class_id', 'i.orig_price', 'i.price'] - )->columns( - [ - 'i.tax_class_id', - 'orig_price' => 'i.orig_price', - 'price' => 'i.price', - 'min_price' => $minPrice, - 'max_price' => $maxPrice, - 'tier_price' => $tierPrice, - 'base_tier' => 'MIN(i.base_tier)', - ] - ); - - $query = $select->insertFromSelect($this->_getDefaultFinalPriceTable()); - $connection->query($query); + $this->applyBundlePrice(); + $this->applyBundleOptionPrice(); return $this; } @@ -348,33 +305,33 @@ protected function _calculateBundleSelectionPrice($priceType) 'ROUND(i.base_tier - (i.base_tier * (' . $selectionPriceValue . ' / 100)),4)', $connection->getCheckSql( 'i.tier_percent > 0', - 'ROUND(' . - $selectionPriceValue . - ' - (' . - $selectionPriceValue . - ' * (i.tier_percent / 100)),4)', + 'ROUND((1 - i.tier_percent / 100) * ' . $selectionPriceValue . ',4)', $selectionPriceValue ) ) . ' * bs.selection_qty', 'NULL' ); - $priceExpr = new \Zend_Db_Expr( - $connection->getCheckSql("{$tierExpr} < {$priceExpr}", $tierExpr, $priceExpr) - ); + $priceExpr = $connection->getLeastSql([ + $priceExpr, + $connection->getIfNullSql($tierExpr, $priceExpr), + ]); } else { - $priceExpr = new \Zend_Db_Expr( - $connection->getCheckSql( - 'i.special_price > 0 AND i.special_price < 100', - 'ROUND(idx.min_price * (i.special_price / 100), 4)', - 'idx.min_price' - ) . ' * bs.selection_qty' + $price = 'idx.min_price * bs.selection_qty'; + $specialExpr = $connection->getCheckSql( + 'i.special_price > 0 AND i.special_price < 100', + 'ROUND(' . $price . ' * (i.special_price / 100), 4)', + $price ); $tierExpr = $connection->getCheckSql( - 'i.base_tier IS NOT NULL', - 'ROUND(idx.min_price * (i.base_tier / 100), 4)* bs.selection_qty', + 'i.tier_percent IS NOT NULL', + 'ROUND((1 - i.tier_percent / 100) * ' . $price . ', 4)', 'NULL' ); + $priceExpr = $connection->getLeastSql([ + $specialExpr, + $connection->getIfNullSql($tierExpr, $price), + ]); } $linkField = $this->getMetadataPool()->getMetadata(ProductInterface::class)->getLinkField(); @@ -508,4 +465,76 @@ protected function _prepareTierPriceIndex($entityIds = null) return $this; } + + /** + * Create bundle price. + * + * @return void + */ + private function applyBundlePrice() : void + { + $select = $this->getConnection()->select(); + $select->from( + $this->_getBundlePriceTable(), + [ + 'entity_id', + 'customer_group_id', + 'website_id', + 'tax_class_id', + 'orig_price', + 'price', + 'min_price', + 'max_price', + 'tier_price', + 'base_tier', + ] + ); + + $query = $select->insertFromSelect($this->_getDefaultFinalPriceTable()); + $this->getConnection()->query($query); + } + + /** + * Make insert/update bundle option price. + * + * @return void + */ + private function applyBundleOptionPrice() : void + { + $connection = $this->getConnection(); + + $subSelect = $connection->select()->from( + $this->_getBundleOptionTable(), + [ + 'entity_id', + 'customer_group_id', + 'website_id', + 'min_price' => new \Zend_Db_Expr('SUM(min_price)'), + 'alt_price' => new \Zend_Db_Expr('MIN(alt_price)'), + 'max_price' => new \Zend_Db_Expr('SUM(max_price)'), + 'tier_price' => new \Zend_Db_Expr('SUM(tier_price)'), + 'alt_tier_price' => new \Zend_Db_Expr('MIN(alt_tier_price)'), + ] + )->group( + ['entity_id', 'customer_group_id', 'website_id'] + ); + + $minPrice = 'i.min_price + ' . $connection->getIfNullSql('io.min_price', '0'); + $tierPrice = 'i.tier_price + ' . $connection->getIfNullSql('io.tier_price', '0'); + $select = $connection->select()->join( + ['io' => $subSelect], + 'i.entity_id = io.entity_id AND i.customer_group_id = io.customer_group_id' . + ' AND i.website_id = io.website_id', + [] + )->columns( + [ + 'min_price' => $connection->getCheckSql("{$minPrice} = 0", 'io.alt_price', $minPrice), + 'max_price' => new \Zend_Db_Expr('io.max_price + i.max_price'), + 'tier_price' => $connection->getCheckSql("{$tierPrice} = 0", 'io.alt_tier_price', $tierPrice), + ] + ); + + $query = $select->crossUpdateFromSelect(['i' => $this->_getDefaultFinalPriceTable()]); + $connection->query($query); + } } diff --git a/app/code/Magento/Catalog/Model/ResourceModel/Product/Indexer/Price/DefaultPrice.php b/app/code/Magento/Catalog/Model/ResourceModel/Product/Indexer/Price/DefaultPrice.php index 285e1781e2f95..87bf5f8dde482 100644 --- a/app/code/Magento/Catalog/Model/ResourceModel/Product/Indexer/Price/DefaultPrice.php +++ b/app/code/Magento/Catalog/Model/ResourceModel/Product/Indexer/Price/DefaultPrice.php @@ -71,7 +71,7 @@ class DefaultPrice extends AbstractIndexer implements PriceInterface * @param \Magento\Framework\Event\ManagerInterface $eventManager * @param \Magento\Framework\Module\Manager $moduleManager * @param string|null $connectionName - * @param null|IndexTableStructureFactory $indexTableStructureFactory + * @param IndexTableStructureFactory $indexTableStructureFactory * @param PriceModifierInterface[] $priceModifiers */ public function __construct( @@ -536,6 +536,7 @@ protected function _applyCustomOption() $finalPriceTable = $this->_getDefaultFinalPriceTable(); $coaTable = $this->_getCustomOptionAggregateTable(); $copTable = $this->_getCustomOptionPriceTable(); + $metadata = $this->getMetadataPool()->getMetadata(\Magento\Catalog\Api\Data\ProductInterface::class); $this->_prepareCustomOptionAggregateTable(); $this->_prepareCustomOptionPriceTable(); @@ -543,6 +544,10 @@ protected function _applyCustomOption() $select = $connection->select()->from( ['i' => $finalPriceTable], ['entity_id', 'customer_group_id', 'website_id'] + )->join( + ['e' => $this->getTable('catalog_product_entity')], + 'e.entity_id = i.entity_id', + [] )->join( ['cw' => $this->getTable('store_website')], 'cw.website_id = i.website_id', @@ -557,7 +562,7 @@ protected function _applyCustomOption() [] )->join( ['o' => $this->getTable('catalog_product_option')], - 'o.product_id = i.entity_id', + 'o.product_id = e.' . $metadata->getLinkField(), ['option_id'] )->join( ['ot' => $this->getTable('catalog_product_option_type_value')], @@ -610,6 +615,10 @@ protected function _applyCustomOption() $select = $connection->select()->from( ['i' => $finalPriceTable], ['entity_id', 'customer_group_id', 'website_id'] + )->join( + ['e' => $this->getTable('catalog_product_entity')], + 'e.entity_id = i.entity_id', + [] )->join( ['cw' => $this->getTable('store_website')], 'cw.website_id = i.website_id', @@ -624,7 +633,7 @@ protected function _applyCustomOption() [] )->join( ['o' => $this->getTable('catalog_product_option')], - 'o.product_id = i.entity_id', + 'o.product_id = e.' . $metadata->getLinkField(), ['option_id'] )->join( ['opd' => $this->getTable('catalog_product_option_price')], @@ -641,13 +650,13 @@ protected function _applyCustomOption() $minPriceRound = new \Zend_Db_Expr("ROUND(i.price * ({$optPriceValue} / 100), 4)"); $priceExpr = $connection->getCheckSql("{$optPriceType} = 'fixed'", $optPriceValue, $minPriceRound); - $minPrice = $connection->getCheckSql("{$priceExpr} > 0 AND o.is_require > 1", $priceExpr, 0); + $minPrice = $connection->getCheckSql("{$priceExpr} > 0 AND o.is_require = 1", $priceExpr, 0); $maxPrice = $priceExpr; $tierPriceRound = new \Zend_Db_Expr("ROUND(i.base_tier * ({$optPriceValue} / 100), 4)"); $tierPriceExpr = $connection->getCheckSql("{$optPriceType} = 'fixed'", $optPriceValue, $tierPriceRound); - $tierPriceValue = $connection->getCheckSql("{$tierPriceExpr} > 0 AND o.is_require > 0", $tierPriceExpr, 0); + $tierPriceValue = $connection->getCheckSql("{$tierPriceExpr} > 0 AND o.is_require = 1", $tierPriceExpr, 0); $tierPrice = $connection->getCheckSql("i.base_tier IS NOT NULL", $tierPriceValue, "NULL"); $select->columns( diff --git a/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/BundlePriceAbstract.php b/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/BundlePriceAbstract.php index ca219405c9c64..e30916810b1e0 100644 --- a/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/BundlePriceAbstract.php +++ b/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/BundlePriceAbstract.php @@ -8,6 +8,7 @@ /** * Abstract class for testing bundle prices + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ abstract class BundlePriceAbstract extends \PHPUnit\Framework\TestCase { @@ -34,6 +35,13 @@ protected function setUp() $this->productRepository = $this->objectManager->create(\Magento\Catalog\Api\ProductRepositoryInterface::class); $this->productCollectionFactory = $this->objectManager->create(\Magento\Catalog\Model\ResourceModel\Product\CollectionFactory::class); + + $scopeConfig = $this->objectManager->get(\Magento\Framework\App\Config\MutableScopeConfigInterface::class); + $scopeConfig->setValue( + \Magento\CatalogInventory\Model\Configuration::XML_PATH_SHOW_OUT_OF_STOCK, + true, + \Magento\Store\Model\ScopeInterface::SCOPE_STORE + ); } /** diff --git a/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/DynamicBundlePriceCalculatorTest.php b/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/DynamicBundlePriceCalculatorTest.php index 75fbf827a3a0f..1775f09eb58c7 100644 --- a/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/DynamicBundlePriceCalculatorTest.php +++ b/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/DynamicBundlePriceCalculatorTest.php @@ -28,26 +28,24 @@ public function testPriceForDynamicBundle(array $strategyModifiers, array $expec $priceInfo = $bundleProduct->getPriceInfo(); $priceCode = \Magento\Catalog\Pricing\Price\FinalPrice::PRICE_CODE; - $priceInfoFromIndexer = $this->productCollectionFactory->create() - ->addFieldToFilter('sku', 'bundle_product') - ->addPriceData() - ->load() - ->getFirstItem(); - $this->assertEquals( $expectedResults['minimalPrice'], $priceInfo->getPrice($priceCode)->getMinimalPrice()->getValue(), 'Failed to check minimal price on product' ); - $this->assertEquals( $expectedResults['maximalPrice'], $priceInfo->getPrice($priceCode)->getMaximalPrice()->getValue(), 'Failed to check maximal price on product' ); - $this->assertEquals($expectedResults['minimalPrice'], $priceInfoFromIndexer->getMinimalPrice()); + $priceInfoFromIndexer = $this->productCollectionFactory->create() + ->addFieldToFilter('sku', 'bundle_product') + ->addPriceData() + ->load() + ->getFirstItem(); + $this->assertEquals($expectedResults['minimalPrice'], $priceInfoFromIndexer->getMinimalPrice()); $this->assertEquals($expectedResults['maximalPrice'], $priceInfoFromIndexer->getMaxPrice()); } @@ -74,12 +72,20 @@ public function testPriceForDynamicBundleInWebsiteScope(array $strategyModifiers $priceInfo->getPrice($priceCode)->getMinimalPrice()->getValue(), 'Failed to check minimal price on product' ); - $this->assertEquals( $expectedResults['maximalPrice'], $priceInfo->getPrice($priceCode)->getMaximalPrice()->getValue(), 'Failed to check maximal price on product' ); + + $priceInfoFromIndexer = $this->productCollectionFactory->create() + ->addFieldToFilter('sku', 'bundle_product') + ->addPriceData() + ->load() + ->getFirstItem(); + + $this->assertEquals($expectedResults['minimalPrice'], $priceInfoFromIndexer->getMinimalPrice()); + $this->assertEquals($expectedResults['maximalPrice'], $priceInfoFromIndexer->getMaxPrice()); } /** diff --git a/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/DynamicBundleWithSpecialPriceCalculatorTest.php b/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/DynamicBundleWithSpecialPriceCalculatorTest.php index e95c9f6d1e2cc..8a76047cbe39b 100644 --- a/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/DynamicBundleWithSpecialPriceCalculatorTest.php +++ b/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/DynamicBundleWithSpecialPriceCalculatorTest.php @@ -28,17 +28,11 @@ public function testPriceForDynamicBundle(array $strategyModifiers, array $expec $priceInfo = $bundleProduct->getPriceInfo(); $priceCode = \Magento\Catalog\Pricing\Price\FinalPrice::PRICE_CODE; - $priceInfoFromIndexer = $this->productCollectionFactory->create() - ->addFieldToFilter('sku', 'bundle_product') - ->addPriceData() - ->load() - ->getFirstItem(); $this->assertEquals( $expectedResults['minimalPrice'], $priceInfo->getPrice($priceCode)->getMinimalPrice()->getValue(), 'Failed to check minimal price on product' ); - $this->assertEquals( $expectedResults['maximalPrice'], $priceInfo->getPrice($priceCode)->getMaximalPrice()->getValue(), @@ -63,9 +57,14 @@ public function testPriceForDynamicBundle(array $strategyModifiers, array $expec ); } - $this->assertEquals($expectedResults['indexerMinimalPrice'], $priceInfoFromIndexer->getMinimalPrice()); + $priceInfoFromIndexer = $this->productCollectionFactory->create() + ->addFieldToFilter('sku', 'bundle_product') + ->addPriceData() + ->load() + ->getFirstItem(); - $this->assertEquals($expectedResults['indexerMaximumPrice'], $priceInfoFromIndexer->getMaxPrice()); + $this->assertEquals($expectedResults['minimalPrice'], $priceInfoFromIndexer->getMinimalPrice()); + $this->assertEquals($expectedResults['maximalPrice'], $priceInfoFromIndexer->getMaxPrice()); } /** @@ -83,8 +82,6 @@ public function getTestCases() 'minimalPrice' => 5, // 0.5 * 10 'maximalPrice' => 5, - 'indexerMinimalPrice' => 10, - 'indexerMaximumPrice' => 10 ] ], @@ -95,8 +92,6 @@ public function getTestCases() 'minimalPrice' => 10, // 0.5 * 2 * 10 'maximalPrice' => 10, - 'indexerMinimalPrice' => 20, - 'indexerMaximumPrice' => 20 ] ], @@ -111,8 +106,6 @@ public function getTestCases() 'minimalPrice' => 5, // 0.5 * (1 * 10 + 3 * 30) 'maximalPrice' => 50, - 'indexerMinimalPrice' => 10, - 'indexerMaximumPrice' => 100 ] ], @@ -126,8 +119,6 @@ public function getTestCases() 'minimalPrice' => 4.95, // 0.5 * ( 1 * 9.9 + 2.5 * 4) 'maximalPrice' => 9.95, - 'indexerMinimalPrice' => 9.9, - 'indexerMaximumPrice' => 19.9 ] ], @@ -142,8 +133,6 @@ public function getTestCases() 'regularMinimalPrice' => '10', // 3 * 20 + (30 * 1 + 13 * 3) 'regularMaximalPrice' => '129', - 'indexerMinimalPrice' => 7.5, - 'indexerMaximumPrice' => 79 ] ], @@ -154,8 +143,6 @@ public function getTestCases() 'minimalPrice' => 4.95, // 0.5 * max(4 * 2.5, 1 * 9.9) 'maximalPrice' => 5, - 'indexerMinimalPrice' => 9.9, - 'indexerMaximumPrice' => 10 ] ], ]; diff --git a/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/DynamicBundleWithTierPriceCalculatorTest.php b/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/DynamicBundleWithTierPriceCalculatorTest.php index cf15ee5602b75..589a385c3df82 100644 --- a/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/DynamicBundleWithTierPriceCalculatorTest.php +++ b/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/DynamicBundleWithTierPriceCalculatorTest.php @@ -39,26 +39,25 @@ public function testPriceForDynamicBundle(array $strategyModifiers, array $expec $priceInfo = $bundleProduct->getPriceInfo(); $priceCode = \Magento\Catalog\Pricing\Price\FinalPrice::PRICE_CODE; - $priceInfoFromIndexer = $this->productCollectionFactory->create() - ->addFieldToFilter('sku', 'bundle_product') - ->addPriceData() - ->load() - ->getFirstItem(); $this->assertEquals( $expectedResults['minimalPrice'], $priceInfo->getPrice($priceCode)->getMinimalPrice()->getValue(), 'Failed to check minimal price on product' ); - $this->assertEquals( $expectedResults['maximalPrice'], $priceInfo->getPrice($priceCode)->getMaximalPrice()->getValue(), 'Failed to check maximal price on product' ); - $this->assertEquals($expectedResults['indexerMinimalPrice'], $priceInfoFromIndexer->getMinimalPrice()); + $priceInfoFromIndexer = $this->productCollectionFactory->create() + ->addFieldToFilter('sku', 'bundle_product') + ->addPriceData() + ->load() + ->getFirstItem(); - $this->assertEquals($expectedResults['indexerMaximumPrice'], $priceInfoFromIndexer->getMaxPrice()); + $this->assertEquals($expectedResults['minimalPrice'], $priceInfoFromIndexer->getMinimalPrice()); + $this->assertEquals($expectedResults['maximalPrice'], $priceInfoFromIndexer->getMaxPrice()); } /** @@ -79,8 +78,6 @@ public function getTestCases() 'minimalPrice' => 5, // 0.5 * 10 'maximalPrice' => 5, - 'indexerMinimalPrice' => 0, - 'indexerMaximumPrice' => 10 ] ], @@ -94,8 +91,6 @@ public function getTestCases() 'minimalPrice' => 10, // 0.5 * 2 * 10 'maximalPrice' => 10, - 'indexerMinimalPrice' => 0, - 'indexerMaximumPrice' => 20 ] ], @@ -109,8 +104,6 @@ public function getTestCases() 'minimalPrice' => 5, // 0.5 * (1 * 10 + 3 * 20) 'maximalPrice' => 35, - 'indexerMinimalPrice' => 0, - 'indexerMaximumPrice' => 70 ] ], @@ -124,8 +117,6 @@ public function getTestCases() 'minimalPrice' => 5, // 0.5 * (1 * 10 + 3 * 20) 'maximalPrice' => 35, - 'indexerMinimalPrice' => 0, - 'indexerMaximumPrice' => 70 ] ], @@ -139,8 +130,6 @@ public function getTestCases() 'minimalPrice' => 5, // 0.5 * 3 * 20 'maximalPrice' => 30, - 'indexerMinimalPrice' => 0, - 'indexerMaximumPrice' => 60 ] ], @@ -155,8 +144,6 @@ public function getTestCases() 'minimalPrice' => 10, // 0.5 * (3 * 20 + 1 * 10 + 3 * 20) 'maximalPrice' => 65, - 'indexerMinimalPrice' => 0, - 'indexerMaximumPrice' => 130 ] ], @@ -170,8 +157,6 @@ public function getTestCases() 'minimalPrice' => 5, // 0.5 * (3 * 20 + 1 * 10 + 3 * 20) 'maximalPrice' => 65, - 'indexerMinimalPrice' => 0, - 'indexerMaximumPrice' => 130 ] ], @@ -185,8 +170,6 @@ public function getTestCases() 'minimalPrice' => 5, // 0.5 * (3 * 20 + 1 * 10 + 3 * 20) 'maximalPrice' => 65, - 'indexerMinimalPrice' => 0, - 'indexerMaximumPrice' => 130 ] ], @@ -200,8 +183,6 @@ public function getTestCases() 'minimalPrice' => 1.25, // 0.5 * 3 * 20 'maximalPrice' => 30, - 'indexerMinimalPrice' => 0, - 'indexerMaximumPrice' => 60 ] ], ]; diff --git a/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/FixedBundlePriceCalculatorTest.php b/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/FixedBundlePriceCalculatorTest.php index 325f663c5e1da..34dbb90e14246 100644 --- a/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/FixedBundlePriceCalculatorTest.php +++ b/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/FixedBundlePriceCalculatorTest.php @@ -30,23 +30,25 @@ public function testPriceForFixedBundle(array $strategyModifiers, array $expecte $priceInfo = $bundleProduct->getPriceInfo(); $priceCode = \Magento\Catalog\Pricing\Price\FinalPrice::PRICE_CODE; - $priceInfoFromIndexer = $this->productCollectionFactory->create() - ->addIdFilter([42]) - ->addPriceData() - ->load() - ->getFirstItem(); $this->assertEquals( $expectedResults['minimalPrice'], $priceInfo->getPrice($priceCode)->getMinimalPrice()->getValue(), 'Failed to check minimal price on product' ); - $this->assertEquals( $expectedResults['maximalPrice'], $priceInfo->getPrice($priceCode)->getMaximalPrice()->getValue(), 'Failed to check maximal price on product' ); - $this->assertEquals($expectedResults['indexerMinimalPrice'], $priceInfoFromIndexer->getMinimalPrice()); + + $priceInfoFromIndexer = $this->productCollectionFactory->create() + ->addIdFilter([42]) + ->addPriceData() + ->load() + ->getFirstItem(); + + $this->assertEquals($expectedResults['minimalPrice'], $priceInfoFromIndexer->getMinimalPrice()); + $this->assertEquals($expectedResults['maximalPrice'], $priceInfoFromIndexer->getMaxPrice()); } /** @@ -67,23 +69,25 @@ public function testPriceForFixedBundleInWebsiteScope(array $strategyModifiers, $priceInfo = $bundleProduct->getPriceInfo(); $priceCode = \Magento\Catalog\Pricing\Price\FinalPrice::PRICE_CODE; - $priceInfoFromIndexer = $this->productCollectionFactory->create() - ->addFieldToFilter('sku', 'bundle_product') - ->addPriceData() - ->load() - ->getFirstItem(); $this->assertEquals( $expectedResults['minimalPrice'], $priceInfo->getPrice($priceCode)->getMinimalPrice()->getValue(), 'Failed to check minimal price on product' ); - $this->assertEquals( $expectedResults['maximalPrice'], $priceInfo->getPrice($priceCode)->getMaximalPrice()->getValue(), 'Failed to check maximal price on product' ); - $this->assertEquals($expectedResults['indexerMinimalPrice'], $priceInfoFromIndexer->getMinimalPrice()); + + $priceInfoFromIndexer = $this->productCollectionFactory->create() + ->addFieldToFilter('sku', 'bundle_product') + ->addPriceData() + ->load() + ->getFirstItem(); + + $this->assertEquals($expectedResults['minimalPrice'], $priceInfoFromIndexer->getMinimalPrice()); + $this->assertEquals($expectedResults['maximalPrice'], $priceInfoFromIndexer->getMaxPrice()); } /** @@ -101,7 +105,6 @@ public function getTestCases() 'minimalPrice' => 120, // 110 + 10 (sum of simple price) 'maximalPrice' => 120, - 'indexerMinimalPrice' => 120 ] ], @@ -112,7 +115,6 @@ public function getTestCases() 'minimalPrice' => 120, // 110 + (3 * 10) + (2 * 10) + 10 'maximalPrice' => 170, - 'indexerMinimalPrice' => 120 ] ], @@ -123,7 +125,6 @@ public function getTestCases() 'minimalPrice' => 120, // 110 + 60 'maximalPrice' => 170, - 'indexerMinimalPrice' => 120 ] ], @@ -134,7 +135,6 @@ public function getTestCases() 'minimalPrice' => 120, // 110 + 30 'maximalPrice' => 140, - 'indexerMinimalPrice' => 120 ] ], @@ -152,7 +152,6 @@ public function getTestCases() // 110 + 1 * 20 + 100 'maximalPrice' => 230, - 'indexerMinimalPrice' => 130 ] ], @@ -170,7 +169,6 @@ public function getTestCases() // 110 + 110 * 0.2 + 110 * 1 'maximalPrice' => 242, - 'indexerMinimalPrice' => 132 ] ], @@ -188,7 +186,6 @@ public function getTestCases() // 110 + 1 * 20 + 110 * 1 'maximalPrice' => 240, - 'indexerMinimalPrice' => 130 ] ], @@ -206,7 +203,6 @@ public function getTestCases() // 110 + 110 * 0.2 + 100 'maximalPrice' => 232, - 'indexerMinimalPrice' => 132 ] ], ]; diff --git a/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/FixedBundleWithSpecialPriceCalculatorTest.php b/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/FixedBundleWithSpecialPriceCalculatorTest.php index 1113b46b1cbe9..1fcc205ddc338 100644 --- a/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/FixedBundleWithSpecialPriceCalculatorTest.php +++ b/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/FixedBundleWithSpecialPriceCalculatorTest.php @@ -30,23 +30,25 @@ public function testPriceForFixedBundle(array $strategyModifiers, array $expecte $priceInfo = $bundleProduct->getPriceInfo(); $priceCode = \Magento\Catalog\Pricing\Price\FinalPrice::PRICE_CODE; - $priceInfoFromIndexer = $this->productCollectionFactory->create() - ->addFieldToFilter('sku', 'bundle_product') - ->addPriceData() - ->load() - ->getFirstItem(); $this->assertEquals( $expectedResults['minimalPrice'], $priceInfo->getPrice($priceCode)->getMinimalPrice()->getValue(), 'Failed to check minimal price on product' ); - $this->assertEquals( $expectedResults['maximalPrice'], $priceInfo->getPrice($priceCode)->getMaximalPrice()->getValue(), 'Failed to check maximal price on product' ); - $this->assertEquals($expectedResults['indexerMinimalPrice'], $priceInfoFromIndexer->getMinimalPrice()); + + $priceInfoFromIndexer = $this->productCollectionFactory->create() + ->addFieldToFilter('sku', 'bundle_product') + ->addPriceData() + ->load() + ->getFirstItem(); + + $this->assertEquals($expectedResults['minimalPrice'], $priceInfoFromIndexer->getMinimalPrice()); + $this->assertEquals($expectedResults['maximalPrice'], $priceInfoFromIndexer->getMaxPrice()); } /** @@ -68,7 +70,6 @@ public function getTestCases() // 110 * 0.5 'maximalPrice' => 55, - 'indexerMinimalPrice' => null ] ], @@ -86,7 +87,6 @@ public function getTestCases() // 0.5 * (110 + 1 * 20) + 100 'maximalPrice' => 165, - 'indexerMinimalPrice' => 130 ] ], @@ -104,7 +104,6 @@ public function getTestCases() // 0.5 * (110 + 110 * 0.2 + 110 * 1) 'maximalPrice' => 121, - 'indexerMinimalPrice' => 132 ] ], @@ -122,7 +121,6 @@ public function getTestCases() // 0.5 * (110 + 1 * 20 + 110 * 1) 'maximalPrice' => 120, - 'indexerMinimalPrice' => 130 ] ], @@ -140,7 +138,6 @@ public function getTestCases() // 0.5 * (110 + 110 * 0.2) + 100 'maximalPrice' => 166, - 'indexerMinimalPrice' => 132 ] ], @@ -158,7 +155,6 @@ public function getTestCases() // 0.5 * (110 + 2 * 20) + 100 'maximalPrice' => 175, - 'indexerMinimalPrice' => 150 ] ], @@ -176,7 +172,6 @@ public function getTestCases() // 0.5 * (110 + 2 * 110 * 0.2 + 1 * 110) 'maximalPrice' => 132, - 'indexerMinimalPrice' => 154 ] ], @@ -194,7 +189,6 @@ public function getTestCases() // 0.5 * (110 + 2 * 20 + 1 * 110) 'maximalPrice' => 130, - 'indexerMinimalPrice' => 150 ] ], @@ -212,7 +206,6 @@ public function getTestCases() // 0.5 * (110 + 2 * 0.2 * 110) + 100 'maximalPrice' => 177, - 'indexerMinimalPrice' => 154 ] ], @@ -230,7 +223,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 10 + 1 * 40) + 100 'maximalPrice' => 190, - 'indexerMinimalPrice' => 140 ] ], @@ -248,7 +240,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 110 * 0.1 + 1 * 110 * 0.4 + 110 * 1) 'maximalPrice' => 148.5, - 'indexerMinimalPrice' => 143 ] ], @@ -266,7 +257,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 10 + 1 * 40 + 1 * 110) 'maximalPrice' => 145, - 'indexerMinimalPrice' => 140 ] ], @@ -284,7 +274,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 110 * 0.1 + 1 * 110 * 0.4) + 100 'maximalPrice' => 193.5, - 'indexerMinimalPrice' => 143 ] ], @@ -302,7 +291,6 @@ public function getTestCases() // 0.5 * (110 + 1 * 40 + 3 * 15) + 100 'maximalPrice' => 197.5, - 'indexerMinimalPrice' => 150 ] ], @@ -320,7 +308,6 @@ public function getTestCases() // 0.5 * (110 + 1 * 110 * 0.4 + 3 * 110 * 0.15 + 110 * 1) 'maximalPrice' => 156.75, - 'indexerMinimalPrice' => 154 ] ], @@ -338,7 +325,6 @@ public function getTestCases() // 0.5 * (110 + 1 * 40 + 3 * 15 + 1 * 110) 'maximalPrice' => 152.5, - 'indexerMinimalPrice' => 150 ] ], @@ -356,7 +342,6 @@ public function getTestCases() // 0.5 * (110 + 1 * 110 * 0.4 + 3 * 110 * 0.15) + 100 'maximalPrice' => 201.75, - 'indexerMinimalPrice' => 154 ] ], @@ -374,7 +359,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 15) + 100 'maximalPrice' => 177.5, - 'indexerMinimalPrice' => 150 ] ], @@ -392,7 +376,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 110 * 0.15 + 1 * 110) 'maximalPrice' => 134.75, - 'indexerMinimalPrice' => 154 ] ], @@ -410,7 +393,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 15 + 110 * 1) 'maximalPrice' => 132.5, - 'indexerMinimalPrice' => 150 ] ], @@ -428,7 +410,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 110 * 0.15) + 100 'maximalPrice' => 179.75, - 'indexerMinimalPrice' => 154 ] ], @@ -446,7 +427,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 15 + 1 * 20 + 3 * 10) + 100 'maximalPrice' => 202.5, - 'indexerMinimalPrice' => 170 ] ], @@ -464,7 +444,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 110 * 0.15 + 1 * 110 * 0.2 + 3 * 110 * 0.1 + 110 * 1) 'maximalPrice' => 162.25, - 'indexerMinimalPrice' => 176 ] ], @@ -482,7 +461,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 15 + 1 * 20 + 3 * 10 + 1 * 110) 'maximalPrice' => 157.5, - 'indexerMinimalPrice' => 170, ] ], @@ -500,7 +478,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 110 * 0.15 + 1 * 110 * 0.2 + 3 * 110 * 0.1) + 100 'maximalPrice' => 207.25, - 'indexerMinimalPrice' => 176 ] ], ]; diff --git a/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/FixedBundleWithTierPriceCalculatorTest.php b/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/FixedBundleWithTierPriceCalculatorTest.php index 103304a86f2db..3285b1e6450c2 100644 --- a/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/FixedBundleWithTierPriceCalculatorTest.php +++ b/dev/tests/integration/testsuite/Magento/Bundle/Model/Product/FixedBundleWithTierPriceCalculatorTest.php @@ -41,23 +41,26 @@ public function testPriceForFixedBundle(array $strategyModifiers, array $expecte /** @var \Magento\Framework\Pricing\PriceInfo\Base $priceInfo */ $priceInfo = $bundleProduct->getPriceInfo(); $priceCode = \Magento\Catalog\Pricing\Price\FinalPrice::PRICE_CODE; - $priceInfoFromIndexer = $this->productCollectionFactory->create() - ->addFieldToFilter('sku', 'bundle_product') - ->addPriceData() - ->load() - ->getFirstItem(); + $this->assertEquals( $expectedResults['minimalPrice'], $priceInfo->getPrice($priceCode)->getMinimalPrice()->getValue(), 'Failed to check minimal price on product' ); - $this->assertEquals( $expectedResults['maximalPrice'], $priceInfo->getPrice($priceCode)->getMaximalPrice()->getValue(), 'Failed to check maximal price on product' ); - $this->assertEquals($expectedResults['indexerMinimalPrice'], $priceInfoFromIndexer->getMinimalPrice()); + + $priceInfoFromIndexer = $this->productCollectionFactory->create() + ->addFieldToFilter('sku', 'bundle_product') + ->addPriceData() + ->load() + ->getFirstItem(); + + $this->assertEquals($expectedResults['minimalPrice'], $priceInfoFromIndexer->getMinimalPrice()); + $this->assertEquals($expectedResults['maximalPrice'], $priceInfoFromIndexer->getMaxPrice()); } /** @@ -79,7 +82,6 @@ public function getTestCases() // 110 * 0.5 'maximalPrice' => 55, - 'indexerMinimalPrice' => null ] ], @@ -97,7 +99,6 @@ public function getTestCases() // 0.5 * (110 + 1 * 20) + 100 'maximalPrice' => 165, - 'indexerMinimalPrice' => 65 ] ], @@ -115,7 +116,6 @@ public function getTestCases() // 0.5 * (110 + 110 * 0.2 + 110 * 1) 'maximalPrice' => 121, - 'indexerMinimalPrice' => 66 ] ], @@ -133,7 +133,6 @@ public function getTestCases() // 0.5 * (110 + 1 * 20 + 110 * 1) 'maximalPrice' => 120, - 'indexerMinimalPrice' => 65 ] ], @@ -151,7 +150,6 @@ public function getTestCases() // 0.5 * (110 + 110 * 0.2) + 100 'maximalPrice' => 166, - 'indexerMinimalPrice' => 66 ] ], @@ -169,7 +167,6 @@ public function getTestCases() // 0.5 * (110 + 2 * 20) + 100 'maximalPrice' => 175, - 'indexerMinimalPrice' => 75, ] ], @@ -187,7 +184,6 @@ public function getTestCases() // 0.5 * (110 + 2 * 110 * 0.2 + 1 * 110) 'maximalPrice' => 132, - 'indexerMinimalPrice' => 77 ] ], @@ -205,7 +201,6 @@ public function getTestCases() // 0.5 * (110 + 2 * 20 + 1 * 110) 'maximalPrice' => 130, - 'indexerMinimalPrice' => 75 ] ], @@ -224,7 +219,6 @@ public function getTestCases() // 0.5 * (110 + 2 * 0.2 * 110) + 100 'maximalPrice' => 177, - 'indexerMinimalPrice' => 77 ] ], @@ -242,7 +236,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 10 + 1 * 40) + 100 'maximalPrice' => 190, - 'indexerMinimalPrice' => 70 ] ], @@ -260,7 +253,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 110 * 0.1 + 1 * 110 * 0.4 + 110 * 1) 'maximalPrice' => 148.5, - 'indexerMinimalPrice' => 71.5 ] ], @@ -278,7 +270,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 10 + 1 * 40 + 1 * 110) 'maximalPrice' => 145, - 'indexerMinimalPrice' => 70 ] ], @@ -296,7 +287,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 110 * 0.1 + 1 * 110 * 0.4) + 100 'maximalPrice' => 193.5, - 'indexerMinimalPrice' => 71.5 ] ], @@ -314,7 +304,6 @@ public function getTestCases() // 0.5 * (110 + 1 * 40 + 3 * 15) + 100 'maximalPrice' => 197.5, - 'indexerMinimalPrice' => 75 ] ], @@ -332,7 +321,6 @@ public function getTestCases() // 0.5 * (110 + 1 * 110 * 0.4 + 3 * 110 * 0.15 + 110 * 1) 'maximalPrice' => 156.75, - 'indexerMinimalPrice' => 77 ] ], @@ -350,7 +338,6 @@ public function getTestCases() // 0.5 * (110 + 1 * 40 + 3 * 15 + 1 * 110) 'maximalPrice' => 152.5, - 'indexerMinimalPrice' => 75 ] ], @@ -368,7 +355,6 @@ public function getTestCases() // 0.5 * (110 + 1 * 110 * 0.4 + 3 * 110 * 0.15) + 100 'maximalPrice' => 201.75, - 'indexerMinimalPrice' => 77 ] ], @@ -386,7 +372,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 15) + 100 'maximalPrice' => 177.5, - 'indexerMinimalPrice' => 75 ] ], @@ -404,7 +389,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 110 * 0.15 + 1 * 110) 'maximalPrice' => 134.75, - 'indexerMinimalPrice' => 77 ] ], @@ -422,7 +406,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 15 + 110 * 1) 'maximalPrice' => 132.5, - 'indexerMinimalPrice' => 75 ] ], @@ -440,7 +423,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 110 * 0.15) + 100 'maximalPrice' => 179.75, - 'indexerMinimalPrice' => 77 ] ], @@ -458,7 +440,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 15 + 1 * 20 + 3 * 10) + 100 'maximalPrice' => 202.5, - 'indexerMinimalPrice' => 85 ] ], @@ -476,7 +457,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 110 * 0.15 + 1 * 110 * 0.2 + 3 * 110 * 0.1 + 110 * 1) 'maximalPrice' => 162.25, - 'indexerMinimalPrice' => 88 ] ], @@ -494,7 +474,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 15 + 1 * 20 + 3 * 10 + 1 * 110) 'maximalPrice' => 157.5, - 'indexerMinimalPrice' => 85 ] ], @@ -512,7 +491,6 @@ public function getTestCases() // 0.5 * (110 + 3 * 110 * 0.15 + 1 * 110 * 0.2 + 3 * 110 * 0.1) + 100 'maximalPrice' => 207.25, - 'indexerMinimalPrice' => 88 ] ], ]; diff --git a/dev/tests/integration/testsuite/Magento/Catalog/Model/ProductPriceTest.php b/dev/tests/integration/testsuite/Magento/Catalog/Model/ProductPriceTest.php index b867b403f34ac..c708d6c445a6d 100644 --- a/dev/tests/integration/testsuite/Magento/Catalog/Model/ProductPriceTest.php +++ b/dev/tests/integration/testsuite/Magento/Catalog/Model/ProductPriceTest.php @@ -3,6 +3,11 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ +namespace Magento\Catalog\Model; + +use Magento\TestFramework\Helper\Bootstrap; +use Magento\Catalog\Api\ProductRepositoryInterface; +use Magento\Catalog\Model\ResourceModel\Product\Collection; /** * Tests product model: @@ -11,8 +16,6 @@ * @see \Magento\Catalog\Model\ProductTest * @see \Magento\Catalog\Model\ProductExternalTest */ -namespace Magento\Catalog\Model; - class ProductPriceTest extends \PHPUnit\Framework\TestCase { /** @@ -22,9 +25,7 @@ class ProductPriceTest extends \PHPUnit\Framework\TestCase protected function setUp() { - $this->_model = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( - \Magento\Catalog\Model\Product::class - ); + $this->_model = Bootstrap::getObjectManager()->create(Product::class); } public function testGetPrice() @@ -72,4 +73,22 @@ public function testSetGetFinalPrice() $this->_model->setFinalPrice(10); $this->assertEquals(10, $this->_model->getFinalPrice()); } + + /** + * @magentoDbIsolation disabled + * @magentoDataFixture Magento/Catalog/_files/product_with_options.php + * @return void + */ + public function testGetMinPrice() : void + { + $productRepository = Bootstrap::getObjectManager()->create(ProductRepositoryInterface::class); + $product = $productRepository->get('simple'); + $collection = Bootstrap::getObjectManager()->create(Collection::class); + $collection->addIdFilter($product->getId()); + $collection->addPriceData(); + $collection->load(); + /** @var \Magento\Catalog\Model\Product $product */ + $product = $collection->getFirstItem(); + $this->assertEquals(333, $product->getData('min_price')); + } } diff --git a/dev/tests/integration/testsuite/Magento/Framework/Search/Adapter/Mysql/AdapterTest.php b/dev/tests/integration/testsuite/Magento/Framework/Search/Adapter/Mysql/AdapterTest.php index ec1538e950c12..581630e9626fa 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/Search/Adapter/Mysql/AdapterTest.php +++ b/dev/tests/integration/testsuite/Magento/Framework/Search/Adapter/Mysql/AdapterTest.php @@ -206,7 +206,7 @@ public function testMatchQueryFilters() public function testRangeFilterWithAllFields() { $this->requestBuilder->bind('range_filter_from', 11); - $this->requestBuilder->bind('range_filter_to', 16); + $this->requestBuilder->bind('range_filter_to', 17); $this->requestBuilder->setRequestName('range_filter'); $queryResponse = $this->executeQuery(); @@ -263,7 +263,7 @@ public function testTermFilter() */ public function testTermFilterArray() { - $this->requestBuilder->bind('request.price', [16, 18]); + $this->requestBuilder->bind('request.price', [17, 18]); $this->requestBuilder->setRequestName('term_filter'); $queryResponse = $this->executeQuery(); @@ -310,13 +310,13 @@ public function testSearchLimit() public function testBoolFilter() { $expectedIds = [2, 3]; - $this->requestBuilder->bind('must_range_filter1_from', 12); + $this->requestBuilder->bind('must_range_filter1_from', 13); $this->requestBuilder->bind('must_range_filter1_to', 22); - $this->requestBuilder->bind('should_term_filter1', 12); - $this->requestBuilder->bind('should_term_filter2', 14); - $this->requestBuilder->bind('should_term_filter3', 16); + $this->requestBuilder->bind('should_term_filter1', 13); + $this->requestBuilder->bind('should_term_filter2', 15); + $this->requestBuilder->bind('should_term_filter3', 17); $this->requestBuilder->bind('should_term_filter4', 18); - $this->requestBuilder->bind('not_term_filter1', 12); + $this->requestBuilder->bind('not_term_filter1', 13); $this->requestBuilder->bind('not_term_filter2', 18); $this->requestBuilder->setRequestName('bool_filter'); @@ -335,7 +335,7 @@ public function testBoolFilterWithNestedNegativeBoolFilter() $expectedIds = [1]; $this->requestBuilder->bind('not_range_filter_from', 14); $this->requestBuilder->bind('not_range_filter_to', 20); - $this->requestBuilder->bind('nested_not_term_filter', 12); + $this->requestBuilder->bind('nested_not_term_filter', 13); $this->requestBuilder->setRequestName('bool_filter_with_nested_bool_filter'); $queryResponse = $this->executeQuery(); From c5295a42e1fff38cb26aab9017ed5fc736298ad8 Mon Sep 17 00:00:00 2001 From: Viktor Sevch Date: Tue, 5 Jun 2018 16:47:08 +0300 Subject: [PATCH 16/21] MAGETWO-67024: Sorting by price ignores Catalog Price Rule for Simple product with required custom option --- .../testsuite/Magento/Catalog/_files/product_with_options.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dev/tests/integration/testsuite/Magento/Catalog/_files/product_with_options.php b/dev/tests/integration/testsuite/Magento/Catalog/_files/product_with_options.php index 9b8a629d24cad..a6e01370dfefb 100644 --- a/dev/tests/integration/testsuite/Magento/Catalog/_files/product_with_options.php +++ b/dev/tests/integration/testsuite/Magento/Catalog/_files/product_with_options.php @@ -11,6 +11,8 @@ $product->setTypeId( 'simple' +)->setId( + 1 )->setAttributeSetId( 4 )->setWebsiteIds( From 8583f10dc11faf9c304843c2d92e8148695b0ea4 Mon Sep 17 00:00:00 2001 From: OlgaVasyltsun Date: Tue, 5 Jun 2018 17:13:18 +0300 Subject: [PATCH 17/21] MAGETWO-90764: [2.3.0] Cannot cancel orders with an expired authorization for Braintree --- .../Braintree/Gateway/Validator/CancelResponseValidator.php | 2 ++ .../Magento/Braintree/Test/Unit/Gateway/SubjectReaderTest.php | 2 +- .../Test/Unit/Gateway/Validator/CancelResponseValidatorTest.php | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Braintree/Gateway/Validator/CancelResponseValidator.php b/app/code/Magento/Braintree/Gateway/Validator/CancelResponseValidator.php index 3c4380747fab0..5e31547e9503c 100644 --- a/app/code/Magento/Braintree/Gateway/Validator/CancelResponseValidator.php +++ b/app/code/Magento/Braintree/Gateway/Validator/CancelResponseValidator.php @@ -15,6 +15,8 @@ use Magento\Braintree\Gateway\SubjectReader; /** + * Decorates the general response validator to handle specific cases. + * * This validator decorates the general response validator to handle specific cases like * an expired or already voided on Braintree side authorization transaction. */ diff --git a/app/code/Magento/Braintree/Test/Unit/Gateway/SubjectReaderTest.php b/app/code/Magento/Braintree/Test/Unit/Gateway/SubjectReaderTest.php index fdf3c583188af..fd524a10ba531 100644 --- a/app/code/Magento/Braintree/Test/Unit/Gateway/SubjectReaderTest.php +++ b/app/code/Magento/Braintree/Test/Unit/Gateway/SubjectReaderTest.php @@ -126,7 +126,7 @@ public function testReadTransaction(): void * @expectedException \InvalidArgumentException * @return void */ - public function testReadTransactionWithInvalidResponse(array $response, string $expectedMessage):void + public function testReadTransactionWithInvalidResponse(array $response, string $expectedMessage): void { $this->expectExceptionMessage($expectedMessage); $this->subjectReader->readTransaction($response); diff --git a/app/code/Magento/Braintree/Test/Unit/Gateway/Validator/CancelResponseValidatorTest.php b/app/code/Magento/Braintree/Test/Unit/Gateway/Validator/CancelResponseValidatorTest.php index 54d8151f7819f..65386272fe511 100644 --- a/app/code/Magento/Braintree/Test/Unit/Gateway/Validator/CancelResponseValidatorTest.php +++ b/app/code/Magento/Braintree/Test/Unit/Gateway/Validator/CancelResponseValidatorTest.php @@ -101,7 +101,7 @@ public function testValidateExpiredTransaction(): void [ 'code' => 91504, 'message' => 'Transaction can only be voided if status is authorized.', - ] + ], ], ]; $buildSubject = [ From eae13796ed3fc95dad22e7af9d6990e6957a8751 Mon Sep 17 00:00:00 2001 From: Viktor Sevch Date: Wed, 6 Jun 2018 10:10:29 +0300 Subject: [PATCH 18/21] MAGETWO-92136: Tiered pricing and quantity Increments do not work with decimal inventory --- .../Ui/DataProvider/Product/Form/Modifier/AdvancedPricing.php | 3 ++- .../Catalog/Ui/DataProvider/Product/Form/Modifier/General.php | 2 +- .../CatalogInventory/Model/Stock/StockItemRepository.php | 2 +- .../view/adminhtml/web/js/components/qty-validator-changer.js | 1 + 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/Catalog/Ui/DataProvider/Product/Form/Modifier/AdvancedPricing.php b/app/code/Magento/Catalog/Ui/DataProvider/Product/Form/Modifier/AdvancedPricing.php index a8378c364a63e..63b7f5faa6562 100644 --- a/app/code/Magento/Catalog/Ui/DataProvider/Product/Form/Modifier/AdvancedPricing.php +++ b/app/code/Magento/Catalog/Ui/DataProvider/Product/Form/Modifier/AdvancedPricing.php @@ -500,7 +500,8 @@ private function getTierPriceStructure($tierPricePath) 'validation' => [ 'required-entry' => true, 'validate-greater-than-zero' => true, - 'validate-digits' => true, + 'validate-digits' => false, + 'validate-number' => true, ], ], ], diff --git a/app/code/Magento/Catalog/Ui/DataProvider/Product/Form/Modifier/General.php b/app/code/Magento/Catalog/Ui/DataProvider/Product/Form/Modifier/General.php index 03d4dde311491..279b905577689 100755 --- a/app/code/Magento/Catalog/Ui/DataProvider/Product/Form/Modifier/General.php +++ b/app/code/Magento/Catalog/Ui/DataProvider/Product/Form/Modifier/General.php @@ -122,7 +122,7 @@ protected function customizeAdvancedPriceFormat(array $data) $value[ProductAttributeInterface::CODE_TIER_PRICE_FIELD_PRICE] = $this->formatPrice($value[ProductAttributeInterface::CODE_TIER_PRICE_FIELD_PRICE]); $value[ProductAttributeInterface::CODE_TIER_PRICE_FIELD_PRICE_QTY] = - (int)$value[ProductAttributeInterface::CODE_TIER_PRICE_FIELD_PRICE_QTY]; + (float) $value[ProductAttributeInterface::CODE_TIER_PRICE_FIELD_PRICE_QTY]; } } diff --git a/app/code/Magento/CatalogInventory/Model/Stock/StockItemRepository.php b/app/code/Magento/CatalogInventory/Model/Stock/StockItemRepository.php index 6928ab9947059..515080d56541c 100644 --- a/app/code/Magento/CatalogInventory/Model/Stock/StockItemRepository.php +++ b/app/code/Magento/CatalogInventory/Model/Stock/StockItemRepository.php @@ -11,7 +11,7 @@ use Magento\CatalogInventory\Api\Data\StockItemInterface; use Magento\CatalogInventory\Api\Data\StockItemInterfaceFactory; use Magento\CatalogInventory\Api\StockConfigurationInterface; -use Magento\CatalogInventory\Api\StockItemRepositoryInterface as StockItemRepositoryInterface; +use Magento\CatalogInventory\Api\StockItemRepositoryInterface; use Magento\CatalogInventory\Model\Indexer\Stock\Processor; use Magento\CatalogInventory\Model\ResourceModel\Stock\Item as StockItemResource; use Magento\CatalogInventory\Model\Spi\StockStateProviderInterface; diff --git a/app/code/Magento/CatalogInventory/view/adminhtml/web/js/components/qty-validator-changer.js b/app/code/Magento/CatalogInventory/view/adminhtml/web/js/components/qty-validator-changer.js index 75d684137a28b..23a33f51af6d4 100644 --- a/app/code/Magento/CatalogInventory/view/adminhtml/web/js/components/qty-validator-changer.js +++ b/app/code/Magento/CatalogInventory/view/adminhtml/web/js/components/qty-validator-changer.js @@ -20,6 +20,7 @@ define([ var isDigits = value !== 1; this.validation['validate-integer'] = isDigits; + this.validation['validate-digits'] = isDigits; this.validation['less-than-equals-to'] = isDigits ? 99999999 : 99999999.9999; this.validate(); } From 5da50d2414c09e7288de6e9007c9c654cf7017c8 Mon Sep 17 00:00:00 2001 From: Viktor Sevch Date: Wed, 6 Jun 2018 16:16:32 +0300 Subject: [PATCH 19/21] MAGETWO-67024: Sorting by price ignores Catalog Price Rule for Simple product with required custom option --- .../Magento/Bundle/Model/ResourceModel/Indexer/Price.php | 6 +++--- .../ResourceModel/Product/Indexer/Price/DefaultPrice.php | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/Bundle/Model/ResourceModel/Indexer/Price.php b/app/code/Magento/Bundle/Model/ResourceModel/Indexer/Price.php index b5bc3b9c3236d..0b6e97cfb9299 100644 --- a/app/code/Magento/Bundle/Model/ResourceModel/Indexer/Price.php +++ b/app/code/Magento/Bundle/Model/ResourceModel/Indexer/Price.php @@ -471,7 +471,7 @@ protected function _prepareTierPriceIndex($entityIds = null) * * @return void */ - private function applyBundlePrice() : void + private function applyBundlePrice(): void { $select = $this->getConnection()->select(); $select->from( @@ -499,7 +499,7 @@ private function applyBundlePrice() : void * * @return void */ - private function applyBundleOptionPrice() : void + private function applyBundleOptionPrice(): void { $connection = $this->getConnection(); @@ -524,7 +524,7 @@ private function applyBundleOptionPrice() : void $select = $connection->select()->join( ['io' => $subSelect], 'i.entity_id = io.entity_id AND i.customer_group_id = io.customer_group_id' . - ' AND i.website_id = io.website_id', + ' AND i.website_id = io.website_id', [] )->columns( [ diff --git a/app/code/Magento/Catalog/Model/ResourceModel/Product/Indexer/Price/DefaultPrice.php b/app/code/Magento/Catalog/Model/ResourceModel/Product/Indexer/Price/DefaultPrice.php index 87bf5f8dde482..4ca407a53f8ae 100644 --- a/app/code/Magento/Catalog/Model/ResourceModel/Product/Indexer/Price/DefaultPrice.php +++ b/app/code/Magento/Catalog/Model/ResourceModel/Product/Indexer/Price/DefaultPrice.php @@ -71,7 +71,7 @@ class DefaultPrice extends AbstractIndexer implements PriceInterface * @param \Magento\Framework\Event\ManagerInterface $eventManager * @param \Magento\Framework\Module\Manager $moduleManager * @param string|null $connectionName - * @param IndexTableStructureFactory $indexTableStructureFactory + * @param null|IndexTableStructureFactory $indexTableStructureFactory * @param PriceModifierInterface[] $priceModifiers */ public function __construct( From f70c155ca7dfba839c4bafd58013a5123008dd87 Mon Sep 17 00:00:00 2001 From: Viktor Sevch Date: Wed, 6 Jun 2018 16:24:50 +0300 Subject: [PATCH 20/21] MAGETWO-67024: Sorting by price ignores Catalog Price Rule for Simple product with required custom option --- .../testsuite/Magento/Catalog/Model/ProductPriceTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/tests/integration/testsuite/Magento/Catalog/Model/ProductPriceTest.php b/dev/tests/integration/testsuite/Magento/Catalog/Model/ProductPriceTest.php index c708d6c445a6d..594133e984a46 100644 --- a/dev/tests/integration/testsuite/Magento/Catalog/Model/ProductPriceTest.php +++ b/dev/tests/integration/testsuite/Magento/Catalog/Model/ProductPriceTest.php @@ -79,7 +79,7 @@ public function testSetGetFinalPrice() * @magentoDataFixture Magento/Catalog/_files/product_with_options.php * @return void */ - public function testGetMinPrice() : void + public function testGetMinPrice(): void { $productRepository = Bootstrap::getObjectManager()->create(ProductRepositoryInterface::class); $product = $productRepository->get('simple'); From 55a6d512ee8843fd9972eaa21de7c5a18ae3eebf Mon Sep 17 00:00:00 2001 From: Viktor Sevch Date: Thu, 7 Jun 2018 15:19:57 +0300 Subject: [PATCH 21/21] MAGETWO-92133: Admin global search feature broken --- .../Magento/Backend/Block/GlobalSearch.php | 1 + .../Magento/Search/Model/SynonymAnalyzer.php | 3 +- lib/web/mage/backend/suggest.js | 40 +++++++++++-------- 3 files changed, 27 insertions(+), 17 deletions(-) diff --git a/app/code/Magento/Backend/Block/GlobalSearch.php b/app/code/Magento/Backend/Block/GlobalSearch.php index f4a46283808f4..3cea12fea205c 100644 --- a/app/code/Magento/Backend/Block/GlobalSearch.php +++ b/app/code/Magento/Backend/Block/GlobalSearch.php @@ -31,6 +31,7 @@ public function getWidgetInitOptions() 'filterProperty' => 'name', 'preventClickPropagation' => false, 'minLength' => 2, + 'submitInputOnEnter' => false, ] ]; } diff --git a/app/code/Magento/Search/Model/SynonymAnalyzer.php b/app/code/Magento/Search/Model/SynonymAnalyzer.php index a63c10da169d7..eea6a950d7ce5 100644 --- a/app/code/Magento/Search/Model/SynonymAnalyzer.php +++ b/app/code/Magento/Search/Model/SynonymAnalyzer.php @@ -83,7 +83,7 @@ public function getSynonymsForPhrase($phrase) /** * Helper method to find the matching of $pattern to $synonymGroupsToExamine. * If matches, the particular array index is returned. - * Otherwise false will be returned. + * Otherwise null will be returned. * * @param string $pattern * @param array $synonymGroupsToExamine @@ -99,6 +99,7 @@ private function findInArray(string $pattern, array $synonymGroupsToExamine) } $position++; } + return null; } diff --git a/lib/web/mage/backend/suggest.js b/lib/web/mage/backend/suggest.js index 412a80804ae0f..d34be10420912 100644 --- a/lib/web/mage/backend/suggest.js +++ b/lib/web/mage/backend/suggest.js @@ -65,7 +65,8 @@ inputWrapper: '
', dropdownWrapper: '
', preventClickPropagation: true, - currentlySelected: null + currentlySelected: null, + submitInputOnEnter: true }, /** @@ -79,7 +80,6 @@ label: '' }; this.templates = {}; - this._renderedContext = null; this._selectedItem = this._nonSelectedItem; this._control = this.options.controls || {}; @@ -312,11 +312,12 @@ click: this.search }, this.options.events)); + this._bindSubmit(); this._bindDropdown(); }, /** - * @param {Object} event - event object + * @param {Object} event * @private */ _toggleEnter: function (event) { @@ -324,6 +325,10 @@ activeItems, selectedItem; + if (!this.options.submitInputOnEnter) { + event.preventDefault(); + } + suggestList = $(event.currentTarget.parentNode).find('ul').first(); activeItems = suggestList.find('._active'); @@ -333,12 +338,22 @@ if (selectedItem.find('a') && selectedItem.find('a').attr('href') !== undefined) { window.location = selectedItem.find('a').attr('href'); event.preventDefault(); - - return false; } } }, + /** + * Bind handlers for submit on enter + * @private + */ + _bindSubmit: function () { + this.element.parents('form').on('submit', function (event) { + if (!this.submitInputOnEnter) { + event.preventDefault(); + } + }); + }, + /** * @param {Object} e - event object * @private @@ -465,8 +480,8 @@ } if (this._trigger('beforeselect', e || null, { - item: this._focused - }) === false) { + item: this._focused + }) === false) { return; } this._selectItem(e); @@ -701,9 +716,6 @@ if ($.isArray(o.source)) { response(this.filter(o.source, term)); } else if ($.type(o.source) === 'string') { - if (this._xhr) { - this._xhr.abort(); - } ajaxData = {}; ajaxData[this.options.termAjaxArgument] = term; @@ -729,10 +741,6 @@ _abortSearch: function () { this.element.removeClass(this.options.loadingClass); clearTimeout(this._searchTimeout); - - if (this._xhr) { - this._xhr.abort(); - } }, /** @@ -905,8 +913,8 @@ '
  • <' + 'label class="mage-suggest-search-label">
    • ', choiceTemplate: '
  • <%- text %>
    ' + - '
    • ', + '', selectedClass: 'mage-suggest-selected' },