From a4e59fc137edd5c922126efd8c59914e5b630109 Mon Sep 17 00:00:00 2001 From: Nikita Shcherbatykh Date: Fri, 5 Oct 2018 10:30:28 +0300 Subject: [PATCH 01/10] MAGETWO-93393: Table rate shipment is taken from wrong website ID when creating an order through the admin --- .../Magento/Sales/Model/AdminOrder/Create.php | 17 ++- .../_files/tablerates_second_website.php | 40 +++++ .../tablerates_second_website_rollback.php | 13 ++ .../Controller/Adminhtml/Order/CreateTest.php | 140 +++++++++++++++++- .../_files/websites_different_countries.php | 14 +- 5 files changed, 218 insertions(+), 6 deletions(-) create mode 100644 dev/tests/integration/testsuite/Magento/OfflineShipping/_files/tablerates_second_website.php create mode 100644 dev/tests/integration/testsuite/Magento/OfflineShipping/_files/tablerates_second_website_rollback.php diff --git a/app/code/Magento/Sales/Model/AdminOrder/Create.php b/app/code/Magento/Sales/Model/AdminOrder/Create.php index c30bb5e328e8b..12d2a5396ddc4 100644 --- a/app/code/Magento/Sales/Model/AdminOrder/Create.php +++ b/app/code/Magento/Sales/Model/AdminOrder/Create.php @@ -16,6 +16,7 @@ use Magento\Quote\Model\Quote\Item; use Magento\Sales\Api\Data\OrderAddressInterface; use Magento\Sales\Model\Order; +use Magento\Store\Model\StoreManagerInterface; /** * Order create model @@ -243,6 +244,11 @@ class Create extends \Magento\Framework\DataObject implements \Magento\Checkout\ */ private $dataObjectConverter; + /** + * @var StoreManagerInterface + */ + private $storeManager; + /** * @param \Magento\Framework\ObjectManagerInterface $objectManager * @param \Magento\Framework\Event\ManagerInterface $eventManager @@ -274,6 +280,7 @@ class Create extends \Magento\Framework\DataObject implements \Magento\Checkout\ * @param array $data * @param \Magento\Framework\Serialize\Serializer\Json|null $serializer * @param ExtensibleDataObjectConverter|null $dataObjectConverter + * @param StoreManagerInterface $storeManager * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ public function __construct( @@ -306,7 +313,8 @@ public function __construct( \Magento\Quote\Model\QuoteFactory $quoteFactory, array $data = [], \Magento\Framework\Serialize\Serializer\Json $serializer = null, - ExtensibleDataObjectConverter $dataObjectConverter = null + ExtensibleDataObjectConverter $dataObjectConverter = null, + StoreManagerInterface $storeManager = null ) { $this->_objectManager = $objectManager; $this->_eventManager = $eventManager; @@ -340,6 +348,7 @@ public function __construct( parent::__construct($data); $this->dataObjectConverter = $dataObjectConverter ?: ObjectManager::getInstance() ->get(ExtensibleDataObjectConverter::class); + $this->storeManager = $storeManager ?: ObjectManager::getInstance()->get(StoreManagerInterface::class); } /** @@ -417,7 +426,8 @@ public function setRecollect($flag) /** * Recollect totals for customer cart. - * Set recollect totals flag for quote + * + * Set recollect totals flag for quote. * * @return $this */ @@ -1327,6 +1337,7 @@ protected function _createCustomerForm(\Magento\Customer\Api\Data\CustomerInterf /** * Set and validate Quote address + * * All errors added to _errors * * @param \Magento\Quote\Model\Quote\Address $address @@ -1530,6 +1541,8 @@ public function resetShippingMethod() */ public function collectShippingRates() { + $store = $this->getQuote()->getStore(); + $this->storeManager->setCurrentStore($store); $this->getQuote()->getShippingAddress()->setCollectShippingRates(true); $this->collectRates(); diff --git a/dev/tests/integration/testsuite/Magento/OfflineShipping/_files/tablerates_second_website.php b/dev/tests/integration/testsuite/Magento/OfflineShipping/_files/tablerates_second_website.php new file mode 100644 index 0000000000000..52551e6dc96d8 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/OfflineShipping/_files/tablerates_second_website.php @@ -0,0 +1,40 @@ +get(WebsiteRepositoryInterface::class); +$website = $websiteRepository->get('test'); + +/** @var ResourceConnection $resource */ +$resource = $objectManager->get(ResourceConnection::class); +$connection = $resource->getConnection(); +$resourceModel = $objectManager->create(Tablerate::class); +$entityTable = $resourceModel->getTable('shipping_tablerate'); +$data = + [ + 'website_id' => $website->getId(), + 'dest_country_id' => 'US', + 'dest_region_id' => 0, + 'dest_zip' => '*', + 'condition_name' => 'package_qty', + 'condition_value' => 1, + 'price' => 20, + 'cost' => 20 + ]; +$connection->query( + "INSERT INTO {$entityTable} (`website_id`, `dest_country_id`, `dest_region_id`, `dest_zip`, `condition_name`," + . "`condition_value`, `price`, `cost`) VALUES (:website_id, :dest_country_id, :dest_region_id, :dest_zip," + . " :condition_name, :condition_value, :price, :cost);", + $data +); diff --git a/dev/tests/integration/testsuite/Magento/OfflineShipping/_files/tablerates_second_website_rollback.php b/dev/tests/integration/testsuite/Magento/OfflineShipping/_files/tablerates_second_website_rollback.php new file mode 100644 index 0000000000000..9606b0eb605fd --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/OfflineShipping/_files/tablerates_second_website_rollback.php @@ -0,0 +1,13 @@ +get(\Magento\Framework\App\ResourceConnection::class); +$connection = $resource->getConnection(); +$resourceModel = $objectManager->create(\Magento\OfflineShipping\Model\ResourceModel\Carrier\Tablerate::class); +$entityTable = $resourceModel->getTable('shipping_tablerate'); +$connection->query("DELETE FROM {$entityTable};"); diff --git a/dev/tests/integration/testsuite/Magento/Sales/Controller/Adminhtml/Order/CreateTest.php b/dev/tests/integration/testsuite/Magento/Sales/Controller/Adminhtml/Order/CreateTest.php index e071dde26a263..01eb917135730 100644 --- a/dev/tests/integration/testsuite/Magento/Sales/Controller/Adminhtml/Order/CreateTest.php +++ b/dev/tests/integration/testsuite/Magento/Sales/Controller/Adminhtml/Order/CreateTest.php @@ -6,12 +6,26 @@ namespace Magento\Sales\Controller\Adminhtml\Order; use Magento\Customer\Api\CustomerRepositoryInterface; -use Magento\Backend\Model\Session\Quote; +use Magento\Backend\Model\Session\Quote as SessionQuote; +use Magento\Customer\Api\Data\CustomerInterface; +use Magento\Framework\Api\SearchCriteriaBuilder; +use Magento\Framework\App\Config\MutableScopeConfigInterface; +use Magento\Framework\Exception\NoSuchEntityException; use Magento\Quote\Api\CartRepositoryInterface; +use Magento\Framework\App\Request\Http as HttpRequest; +use Magento\Quote\Model\Quote; +use Magento\Store\Api\Data\StoreInterface; +use Magento\Store\Api\Data\WebsiteInterface; +use Magento\Store\Api\StoreRepositoryInterface; +use Magento\Store\Api\WebsiteRepositoryInterface; +use Magento\Store\Model\ScopeInterface; /** * @magentoAppArea adminhtml * @magentoDbIsolation enabled + * + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + * @SuppressWarnings(PHPMD.TooManyPublicMethods) */ class CreateTest extends \Magento\TestFramework\TestCase\AbstractBackendController { @@ -57,6 +71,57 @@ public function testLoadBlockActionData() } /** + * Tests that shipping method 'Table rates' shows rates according to selected website. + * + * @magentoAppArea adminhtml + * @magentoDataFixture Magento/Quote/Fixtures/quote_sec_website.php + * @magentoDataFixture Magento/OfflineShipping/_files/tablerates_second_website.php + * @magentoDbIsolation disabled + */ + public function testLoadBlockShippingMethod() + { + $store = $this->getStore('fixture_second_store'); + + /** @var MutableScopeConfigInterface $mutableScopeConfig */ + $mutableScopeConfig = $this->_objectManager->get(MutableScopeConfigInterface::class); + $mutableScopeConfig->setValue( + 'carriers/tablerate/active', + 1, + ScopeInterface::SCOPE_STORE, + $store->getCode() + ); + $mutableScopeConfig->setValue( + 'carriers/tablerate/condition_name', + 'package_qty', + ScopeInterface::SCOPE_STORE, + $store->getCode() + ); + + $website = $this->getWebsite('test'); + $customer = $this->getCustomer('customer.web@example.com', (int)$website->getId()); + $quote = $this->getQuoteById('0000032134'); + $session = $this->_objectManager->get(SessionQuote::class); + $session->setQuoteId($quote->getId()); + + $this->getRequest()->setMethod(HttpRequest::METHOD_POST); + $this->getRequest()->setPostValue( + [ + 'customer_id' => $customer->getId(), + 'collect_shipping_rates' => 1, + 'store_id' => $store->getId(), + 'json' => true + ] + ); + $this->dispatch('backend/sales/order_create/loadBlock/block/shipping_method'); + $body = $this->getResponse()->getBody(); + $expectedTableRatePrice = '$20.00<\/span>'; + + $this->assertContains($expectedTableRatePrice, $body, ''); + } + + /** + * Tests LoadBlock actions. + * * @dataProvider loadBlockActionsDataProvider */ public function testLoadBlockActions($block, $expected) @@ -80,6 +145,8 @@ public function loadBlockActionsDataProvider() } /** + * Tests action items. + * * @magentoDataFixture Magento/Catalog/_files/product_simple.php */ public function testLoadBlockActionItems() @@ -153,6 +220,8 @@ public function testIndexAction() } /** + * Tests ACL. + * * @param string $actionName * @param boolean $reordered * @param string $expectedResult @@ -162,7 +231,7 @@ public function testIndexAction() */ public function testGetAclResource($actionName, $reordered, $expectedResult) { - $this->_objectManager->get(Quote::class)->setReordered($reordered); + $this->_objectManager->get(SessionQuote::class)->setReordered($reordered); $orderController = $this->_objectManager->get( \Magento\Sales\Controller\Adminhtml\Order\Stub\OrderCreateStub::class ); @@ -251,7 +320,7 @@ public function testSyncBetweenQuoteAddresses() $quoteRepository = $this->_objectManager->get(CartRepositoryInterface::class); $quote = $quoteRepository->getActiveForCustomer($customer->getId()); - $session = $this->_objectManager->get(Quote::class); + $session = $this->_objectManager->get(SessionQuote::class); $session->setQuoteId($quote->getId()); $data = [ @@ -286,4 +355,69 @@ public function testSyncBetweenQuoteAddresses() self::assertEquals($data['city'], $shippingAddress->getCity()); self::assertEquals($data['street'], $shippingAddress->getStreet()); } + + /** + * Gets quote entity by reserved order id. + * + * @param string $reservedOrderId + * @return Quote + */ + private function getQuoteById(string $reservedOrderId): Quote + { + /** @var SearchCriteriaBuilder $searchCriteriaBuilder */ + $searchCriteriaBuilder = $this->_objectManager->get(SearchCriteriaBuilder::class); + $searchCriteria = $searchCriteriaBuilder->addFilter('reserved_order_id', $reservedOrderId) + ->create(); + + /** @var CartRepositoryInterface $repository */ + $repository = $this->_objectManager->get(CartRepositoryInterface::class); + $items = $repository->getList($searchCriteria) + ->getItems(); + + return array_pop($items); + } + + /** + * Gets website entity. + * + * @param string $code + * @return WebsiteInterface + * @throws NoSuchEntityException + */ + private function getWebsite(string $code): WebsiteInterface + { + /** @var WebsiteRepositoryInterface $repository */ + $repository = $this->_objectManager->get(WebsiteRepositoryInterface::class); + return $repository->get($code); + } + + /** + * Gets customer entity. + * + * @param string $email + * @param int $websiteId + * @return CustomerInterface + * @throws NoSuchEntityException + * @throws \Magento\Framework\Exception\LocalizedException + */ + private function getCustomer(string $email, int $websiteId): CustomerInterface + { + /** @var CustomerRepositoryInterface $repository */ + $repository = $this->_objectManager->get(CustomerRepositoryInterface::class); + return $repository->get($email, $websiteId); + } + + /** + * Gets store by code. + * + * @param string $code + * @return StoreInterface + * @throws \Magento\Framework\Exception\NoSuchEntityException + */ + private function getStore(string $code): StoreInterface + { + /** @var StoreRepositoryInterface $repository */ + $repository = $this->_objectManager->get(StoreRepositoryInterface::class); + return $repository->get($code); + } } diff --git a/dev/tests/integration/testsuite/Magento/Store/_files/websites_different_countries.php b/dev/tests/integration/testsuite/Magento/Store/_files/websites_different_countries.php index 6a00031434b3d..dd58f4eb7a984 100644 --- a/dev/tests/integration/testsuite/Magento/Store/_files/websites_different_countries.php +++ b/dev/tests/integration/testsuite/Magento/Store/_files/websites_different_countries.php @@ -10,6 +10,7 @@ use Magento\Store\Model\Store; use Magento\CatalogSearch\Model\Indexer\Fulltext as FulltextIndex; use Magento\Framework\App\Config\ReinitableConfigInterface; +use Magento\Store\Model\Group; $objectManager = Bootstrap::getObjectManager(); //Creating second website with a store. @@ -21,12 +22,23 @@ $website->setData([ 'code' => 'test', 'name' => 'Test Website', - 'default_group_id' => '1', 'is_default' => '0', ]); $website->save(); } +/** + * @var Group $storeGroup + */ +$storeGroup = $objectManager->create(Group::class); +$storeGroup->setCode('some_group') + ->setName('custom store group') + ->setWebsite($website); +$storeGroup->save($storeGroup); + +$website->setDefaultGroupId($storeGroup->getId()); +$website->save($website); + $websiteId = $website->getId(); $store = $objectManager->create(Store::class); $store->load('fixture_second_store', 'code'); From d265ee312fd5bcc8f9b8e33e8d9dfe4605dcb60a Mon Sep 17 00:00:00 2001 From: Mastiuhin Olexandr Date: Tue, 9 Oct 2018 20:19:21 +0300 Subject: [PATCH 02/10] MAGETWO-93222: Missing URL Keys upon product import --- .../Model/Import/Product.php | 52 ++++++++++++++----- .../Model/Import/ProductTest.php | 50 +++++++++++++++--- .../products_to_import_without_url_keys.csv | 1 - ...ts_to_import_without_url_keys_and_name.csv | 3 ++ 4 files changed, 83 insertions(+), 23 deletions(-) create mode 100644 dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/_files/products_to_import_without_url_keys_and_name.csv diff --git a/app/code/Magento/CatalogImportExport/Model/Import/Product.php b/app/code/Magento/CatalogImportExport/Model/Import/Product.php index 19e33ee267da7..ce7239abb7fd5 100644 --- a/app/code/Magento/CatalogImportExport/Model/Import/Product.php +++ b/app/code/Magento/CatalogImportExport/Model/Import/Product.php @@ -1574,8 +1574,14 @@ protected function _saveProducts() $rowScope = $this->getRowScope($rowData); $urlKey = $this->getUrlKey($rowData); - if (!empty($urlKey)) { + if (!empty($rowData[self::URL_KEY])) { + // If url_key column and its value were in the CSV file $rowData[self::URL_KEY] = $urlKey; + } else if($this->isNeedToChangeUrlKey($rowData)) { + // If url_key column was empty or even not declared in the CSV file but by the rules it is need to + // be setteed. In case when url_key is generating from name column we have to ensure that the bunch + // of products will pass for the event with url_key column. + $bunch[$rowNum][self::URL_KEY] = $rowData[self::URL_KEY] = $urlKey; } $rowSku = $rowData[self::COL_SKU]; @@ -2475,17 +2481,17 @@ public function validateRow(array $rowData, $rowNum) } /** + * Check if need to validate url key. + * * @param array $rowData * @return bool */ private function isNeedToValidateUrlKey($rowData) { - $urlKey = $this->getUrlKey($rowData); - - return (!empty($urlKey)) + return (!empty($rowData[self::URL_KEY]) || !empty($rowData[self::COL_NAME])) && (empty($rowData[self::COL_VISIBILITY]) - || $rowData[self::COL_VISIBILITY] - !== (string)Visibility::getOptionArray()[Visibility::VISIBILITY_NOT_VISIBLE]); + || $rowData[self::COL_VISIBILITY] + !== (string)Visibility::getOptionArray()[Visibility::VISIBILITY_NOT_VISIBLE]); } /** @@ -2785,8 +2791,11 @@ protected function getProductUrlSuffix($storeId = null) } /** + * Retrieve url key from provided row data. + * * @param array $rowData * @return string + * * @since 100.0.3 */ protected function getUrlKey($rowData) @@ -2794,14 +2803,8 @@ protected function getUrlKey($rowData) if (!empty($rowData[self::URL_KEY])) { return $this->productUrl->formatUrlKey($rowData[self::URL_KEY]); } - - /** - * If the product exists, assume it already has a URL Key and even - * if a name is provided in the import data, it should not be used - * to overwrite that existing URL Key the product already has. - */ - $isSkuExist = $this->isSkuExist($rowData[self::COL_SKU]); - if (!$isSkuExist && !empty($rowData[self::COL_NAME])) { + + if (!empty($rowData[self::COL_NAME])) { return $this->productUrl->formatUrlKey($rowData[self::COL_NAME]); } @@ -2820,6 +2823,27 @@ protected function getResource() return $this->_resource; } + /** + * Whether a url key is needed to be change. + * Returns false if + * + * @param array $rowData + * @return bool + */ + private function isNeedToChangeUrlKey(array $rowData): bool + { + $urlKey = $this->getUrlKey($rowData); + $productExists = $this->isSkuExist($rowData[self::COL_SKU]); + $markedToEraseUrlKey = isset($rowData[self::URL_KEY]); + // The product isn't new and the url key index wasn't marked for change. + if (!$urlKey && $productExists && !$markedToEraseUrlKey) { + // Seems there is no need to change the url key + return false; + } + + return true; + } + /** * Get product entity link field * diff --git a/dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/ProductTest.php b/dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/ProductTest.php index 019fffc8c0898..a9c24829f9ad8 100644 --- a/dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/ProductTest.php +++ b/dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/ProductTest.php @@ -1573,15 +1573,10 @@ public function testExistingProductWithUrlKeys() */ public function testImportWithoutUrlKeys() { - /** - * Products `simple1` and `simple2` are created by fixture so already - * have a URL Key, whereas `new-simple` is a new product so the import - * will generate it's URL Key from the name provided in the CSV. - */ $products = [ - 'simple1' => 'url-key', - 'simple2' => 'url-key2', - 'new-simple' => 'new-simple', + 'simple1' => 'simple-1', + 'simple2' => 'simple-2', + 'simple3' => 'simple-3' ]; $filesystem = $this->objectManager->create(\Magento\Framework\Filesystem::class); $directory = $filesystem->getDirectoryWrite(DirectoryList::ROOT); @@ -1645,6 +1640,45 @@ public function testImportWithUrlKeysWithSpaces() } } + /** + * Make sure the absence of a url_key column in the csv file won't erase the url key of the existing products. + * To reach the goal we need to not send the name column, as the url key is generated from it. + * + * @magentoDataFixture Magento/Catalog/_files/product_simple_with_url_key.php + * @magentoDbIsolation enabled + * @magentoAppIsolation enabled + */ + public function testImportWithoutUrlKeysAndName() + { + $products = [ + 'simple1' => 'url-key', + 'simple2' => 'url-key2', + ]; + $filesystem = $this->objectManager->create(\Magento\Framework\Filesystem::class); + $directory = $filesystem->getDirectoryWrite(DirectoryList::ROOT); + $source = $this->objectManager->create( + \Magento\ImportExport\Model\Import\Source\Csv::class, + [ + 'file' => __DIR__ . '/_files/products_to_import_without_url_keys_and_name.csv', + 'directory' => $directory + ] + ); + + $errors = $this->_model->setParameters( + ['behavior' => \Magento\ImportExport\Model\Import::BEHAVIOR_APPEND, 'entity' => 'catalog_product'] + ) + ->setSource($source) + ->validateData(); + + $this->assertTrue($errors->getErrorsCount() == 0); + $this->_model->importData(); + + $productRepository = $this->objectManager->create(\Magento\Catalog\Api\ProductRepositoryInterface::class); + foreach ($products as $productSku => $productUrlKey) { + $this->assertEquals($productUrlKey, $productRepository->get($productSku)->getUrlKey()); + } + } + /** * @magentoAppIsolation enabled */ diff --git a/dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/_files/products_to_import_without_url_keys.csv b/dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/_files/products_to_import_without_url_keys.csv index 80898c5440df7..ff1b9f4b02afb 100644 --- a/dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/_files/products_to_import_without_url_keys.csv +++ b/dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/_files/products_to_import_without_url_keys.csv @@ -2,4 +2,3 @@ sku,product_type,store_view_code,name,price,attribute_set_code,url_key simple1,simple,,"simple 1",25,Default,"" simple2,simple,,"simple 2",34,Default,"" simple3,simple,,"simple 3",58,Default,"" -new-simple,simple,,"new simple",25,Default,"" diff --git a/dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/_files/products_to_import_without_url_keys_and_name.csv b/dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/_files/products_to_import_without_url_keys_and_name.csv new file mode 100644 index 0000000000000..8ea6ab92a0295 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/_files/products_to_import_without_url_keys_and_name.csv @@ -0,0 +1,3 @@ +sku,price +simple1,25 +simple2,34 From f339363ef7e23a215445913de6ac357b3dbe1a91 Mon Sep 17 00:00:00 2001 From: Nikita Shcherbatykh Date: Wed, 10 Oct 2018 12:26:13 +0300 Subject: [PATCH 03/10] MAGETWO-93393: Table rate shipment is taken from wrong website ID when creating an order through the admin --- .../Controller/Adminhtml/Order/CreateTest.php | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/dev/tests/integration/testsuite/Magento/Sales/Controller/Adminhtml/Order/CreateTest.php b/dev/tests/integration/testsuite/Magento/Sales/Controller/Adminhtml/Order/CreateTest.php index 01eb917135730..7110f39ee532c 100644 --- a/dev/tests/integration/testsuite/Magento/Sales/Controller/Adminhtml/Order/CreateTest.php +++ b/dev/tests/integration/testsuite/Magento/Sales/Controller/Adminhtml/Order/CreateTest.php @@ -96,18 +96,27 @@ public function testLoadBlockShippingMethod() ScopeInterface::SCOPE_STORE, $store->getCode() ); - $website = $this->getWebsite('test'); $customer = $this->getCustomer('customer.web@example.com', (int)$website->getId()); $quote = $this->getQuoteById('0000032134'); $session = $this->_objectManager->get(SessionQuote::class); $session->setQuoteId($quote->getId()); - $this->getRequest()->setMethod(HttpRequest::METHOD_POST); + $data = [ + 'firstname' => 'John', + 'lastname' => 'Doe', + 'street' => ['Soborna 23'], + 'city' => 'Testcity', + 'country_id' => 'US', + 'region' => 'Alabama', + 'region_id' => 1 + ]; $this->getRequest()->setPostValue( [ - 'customer_id' => $customer->getId(), + 'order' => ['billing_address' => $data], + 'reset_shipping' => 1, 'collect_shipping_rates' => 1, + 'customer_id' => $customer->getId(), 'store_id' => $store->getId(), 'json' => true ] From 906faa659c720976e2355851fcbe0465ca77bb40 Mon Sep 17 00:00:00 2001 From: Nikita Shcherbatykh Date: Wed, 10 Oct 2018 14:22:10 +0300 Subject: [PATCH 04/10] MAGETWO-93155: Joins to grid collections causes MySQL exception due to ambiguous where clause --- .../ResourceModel/Order/Grid/Collection.php | 15 +++++++ .../Order/Grid/CollectionTest.php | 45 +++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 dev/tests/integration/testsuite/Magento/Sales/Model/ResourceModel/Order/Grid/CollectionTest.php diff --git a/app/code/Magento/Sales/Model/ResourceModel/Order/Grid/Collection.php b/app/code/Magento/Sales/Model/ResourceModel/Order/Grid/Collection.php index f6dd8f8527a53..82c612c1a781d 100644 --- a/app/code/Magento/Sales/Model/ResourceModel/Order/Grid/Collection.php +++ b/app/code/Magento/Sales/Model/ResourceModel/Order/Grid/Collection.php @@ -35,4 +35,19 @@ public function __construct( ) { parent::__construct($entityFactory, $logger, $fetchStrategy, $eventManager, $mainTable, $resourceModel); } + + /** + * @inheritdoc + */ + protected function _initSelect() + { + parent::_initSelect(); + + $tableDescription = $this->getConnection()->describeTable($this->getMainTable()); + foreach ($tableDescription as $columnInfo) { + $this->addFilterToMap($columnInfo['COLUMN_NAME'], 'main_table.' . $columnInfo['COLUMN_NAME']); + } + + return $this; + } } diff --git a/dev/tests/integration/testsuite/Magento/Sales/Model/ResourceModel/Order/Grid/CollectionTest.php b/dev/tests/integration/testsuite/Magento/Sales/Model/ResourceModel/Order/Grid/CollectionTest.php new file mode 100644 index 0000000000000..1649706a51f6b --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Sales/Model/ResourceModel/Order/Grid/CollectionTest.php @@ -0,0 +1,45 @@ +get(Collection::class); + $tableDescription = $gridCollection->getConnection() + ->describeTable($gridCollection->getMainTable()); + + $mapper = new \ReflectionMethod( + Collection::class, + '_getMapper' + ); + $mapper->setAccessible(true); + $map = $mapper->invoke($gridCollection); + + self::assertInternalType('array', $map); + self::assertArrayHasKey('fields', $map); + self::assertInternalType('array', $map['fields']); + self::assertCount(count($tableDescription), $map['fields']); + + foreach ($map['fields'] as $mappedName) { + self::assertContains('main_table.', $mappedName); + } + } +} From ee8f3b1ca91eb6dc76baf2d5e8fad331e25814e5 Mon Sep 17 00:00:00 2001 From: Nikita Shcherbatykh Date: Wed, 10 Oct 2018 14:57:35 +0300 Subject: [PATCH 05/10] MAGETWO-93155: Joins to grid collections causes MySQL exception due to ambiguous where clause --- .../Sales/Model/ResourceModel/Order/Grid/CollectionTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/tests/integration/testsuite/Magento/Sales/Model/ResourceModel/Order/Grid/CollectionTest.php b/dev/tests/integration/testsuite/Magento/Sales/Model/ResourceModel/Order/Grid/CollectionTest.php index 1649706a51f6b..8eda87ae43c29 100644 --- a/dev/tests/integration/testsuite/Magento/Sales/Model/ResourceModel/Order/Grid/CollectionTest.php +++ b/dev/tests/integration/testsuite/Magento/Sales/Model/ResourceModel/Order/Grid/CollectionTest.php @@ -17,7 +17,7 @@ class CollectionTest extends \PHPUnit\Framework\TestCase * @throws \ReflectionException * @return void */ - public function testCollectionCreate(): void + public function testCollectionCreate() { $objectManager = Bootstrap::getObjectManager(); From f08793ebf1def59a94593537570f401104a0bc38 Mon Sep 17 00:00:00 2001 From: Mastiuhin Olexandr Date: Wed, 10 Oct 2018 15:25:16 +0300 Subject: [PATCH 06/10] MAGETWO-93222: Missing URL Keys upon product import --- .../Magento/CatalogImportExport/Model/Import/Product.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/CatalogImportExport/Model/Import/Product.php b/app/code/Magento/CatalogImportExport/Model/Import/Product.php index ce7239abb7fd5..3cf167e9e37e7 100644 --- a/app/code/Magento/CatalogImportExport/Model/Import/Product.php +++ b/app/code/Magento/CatalogImportExport/Model/Import/Product.php @@ -1574,10 +1574,10 @@ protected function _saveProducts() $rowScope = $this->getRowScope($rowData); $urlKey = $this->getUrlKey($rowData); - if (!empty($rowData[self::URL_KEY])) { + if (!empty($rowData[self::URL_KEY])) { // If url_key column and its value were in the CSV file $rowData[self::URL_KEY] = $urlKey; - } else if($this->isNeedToChangeUrlKey($rowData)) { + } else if ($this->isNeedToChangeUrlKey($rowData)) { // If url_key column was empty or even not declared in the CSV file but by the rules it is need to // be setteed. In case when url_key is generating from name column we have to ensure that the bunch // of products will pass for the event with url_key column. @@ -2825,7 +2825,6 @@ protected function getResource() /** * Whether a url key is needed to be change. - * Returns false if * * @param array $rowData * @return bool From 0bb8a167b3ae7a5afa72cca5678ac236a9597a8e Mon Sep 17 00:00:00 2001 From: Mastiuhin Olexandr Date: Wed, 10 Oct 2018 18:56:44 +0300 Subject: [PATCH 07/10] MAGETWO-95585: Can't generate Plugin if object method returns self --- .../SourceClassWithNamespace.php | 12 +++++++++++ ...ceClassWithNamespaceInterceptor.php.sample | 13 ++++++++++++ .../SourceClassWithNamespaceProxy.php.sample | 8 +++++++ .../Code/Generator/Interceptor.php | 21 ++++++++++++++++++- .../ObjectManager/Code/Generator/Proxy.php | 21 ++++++++++++++++++- 5 files changed, 73 insertions(+), 2 deletions(-) diff --git a/dev/tests/integration/testsuite/Magento/Framework/Code/GeneratorTest/SourceClassWithNamespace.php b/dev/tests/integration/testsuite/Magento/Framework/Code/GeneratorTest/SourceClassWithNamespace.php index ba7df011eec9b..3d88ba391ee0a 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/Code/GeneratorTest/SourceClassWithNamespace.php +++ b/dev/tests/integration/testsuite/Magento/Framework/Code/GeneratorTest/SourceClassWithNamespace.php @@ -110,4 +110,16 @@ public static function publicChildStatic() final public function publicChildFinal() { } + + /** + * Test method + * + * @param bool $arg + * @return SourceClassWithNamespace + * + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ + public function publicWithSelf($arg = false): self + { + } } diff --git a/dev/tests/integration/testsuite/Magento/Framework/Code/_expected/SourceClassWithNamespaceInterceptor.php.sample b/dev/tests/integration/testsuite/Magento/Framework/Code/_expected/SourceClassWithNamespaceInterceptor.php.sample index cf8611866a885..3e0389052ac07 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/Code/_expected/SourceClassWithNamespaceInterceptor.php.sample +++ b/dev/tests/integration/testsuite/Magento/Framework/Code/_expected/SourceClassWithNamespaceInterceptor.php.sample @@ -54,6 +54,19 @@ class Interceptor extends \Magento\Framework\Code\GeneratorTest\SourceClassWithN } } + /** + * {@inheritdoc} + */ + public function publicWithSelf($arg = false) : \Magento\Framework\Code\GeneratorTest\SourceClassWithNamespace + { + $pluginInfo = $this->pluginList->getNext($this->subjectType, 'publicWithSelf'); + if (!$pluginInfo) { + return parent::publicWithSelf($arg); + } else { + return $this->___callPlugins('publicWithSelf', func_get_args(), $pluginInfo); + } + } + /** * {@inheritdoc} */ diff --git a/dev/tests/integration/testsuite/Magento/Framework/Code/_expected/SourceClassWithNamespaceProxy.php.sample b/dev/tests/integration/testsuite/Magento/Framework/Code/_expected/SourceClassWithNamespaceProxy.php.sample index 345ac49c6a4f4..30112dcb51485 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/Code/_expected/SourceClassWithNamespaceProxy.php.sample +++ b/dev/tests/integration/testsuite/Magento/Framework/Code/_expected/SourceClassWithNamespaceProxy.php.sample @@ -114,6 +114,14 @@ class Proxy extends \Magento\Framework\Code\GeneratorTest\SourceClassWithNamespa return $this->_getSubject()->publicChildWithoutParameters(); } + /** + * {@inheritdoc} + */ + public function publicWithSelf($arg = false) : \Magento\Framework\Code\GeneratorTest\SourceClassWithNamespace + { + return $this->_getSubject()->publicWithSelf($arg); + } + /** * {@inheritdoc} */ diff --git a/lib/internal/Magento/Framework/Interception/Code/Generator/Interceptor.php b/lib/internal/Magento/Framework/Interception/Code/Generator/Interceptor.php index 608907abcc0e1..59bba2c49631c 100644 --- a/lib/internal/Magento/Framework/Interception/Code/Generator/Interceptor.php +++ b/lib/internal/Magento/Framework/Interception/Code/Generator/Interceptor.php @@ -113,7 +113,7 @@ protected function _getMethodInfo(\ReflectionMethod $method) "} else {\n" . " return \$this->___callPlugins('{$method->getName()}', func_get_args(), \$pluginInfo);\n" . "}", - 'returnType' => $method->getReturnType(), + 'returnType' => $this->getReturnTypeValue($method->getReturnType()), 'docblock' => ['shortDescription' => '{@inheritdoc}'], ]; @@ -183,4 +183,23 @@ protected function _validateData() } return $result; } + + /** + * Returns return type + * + * @param mixed $returnType + * @return null|string + */ + private function getReturnTypeValue($returnType) + { + $returnTypeValue = null; + + if ($returnType) { + $returnTypeValue = ($returnType->getName() === 'self') + ? $this->getSourceClassName() + : $returnType->getName(); + } + + return $returnTypeValue; + } } diff --git a/lib/internal/Magento/Framework/ObjectManager/Code/Generator/Proxy.php b/lib/internal/Magento/Framework/ObjectManager/Code/Generator/Proxy.php index 0aaf67b5c7cfc..a08ff4677b3a9 100644 --- a/lib/internal/Magento/Framework/ObjectManager/Code/Generator/Proxy.php +++ b/lib/internal/Magento/Framework/ObjectManager/Code/Generator/Proxy.php @@ -165,7 +165,7 @@ protected function _getMethodInfo(\ReflectionMethod $method) 'parameters' => $parameters, 'body' => $this->_getMethodBody($method->getName(), $parameterNames), 'docblock' => ['shortDescription' => '{@inheritdoc}'], - 'returnType' => $method->getReturnType(), + 'returnType' => $this->getReturnTypeValue($method->getReturnType()), ]; return $methodInfo; @@ -245,4 +245,23 @@ protected function _validateData() } return $result; } + + /** + * Returns return type + * + * @param mixed $returnType + * @return null|string + */ + private function getReturnTypeValue($returnType) + { + $returnTypeValue = null; + + if ($returnType) { + $returnTypeValue = ($returnType->getName() === 'self') + ? $this->getSourceClassName() + : $returnType->getName(); + } + + return $returnTypeValue; + } } From de8d6991c29b8e7d783613b6c6c9b10d6ea698ac Mon Sep 17 00:00:00 2001 From: Serhiy Yelahin Date: Thu, 11 Oct 2018 09:21:29 +0300 Subject: [PATCH 08/10] MAGETWO-94857: Unable to create invoice for an order via Braintree PayPal --- .../Request/PayPal/VaultDataBuilder.php | 2 + .../Request/VaultCaptureDataBuilder.php | 5 ++ .../Request/VaultCaptureDataBuilderTest.php | 72 +++++++++++++++++-- app/code/Magento/Braintree/i18n/en_US.csv | 1 + 4 files changed, 74 insertions(+), 6 deletions(-) diff --git a/app/code/Magento/Braintree/Gateway/Request/PayPal/VaultDataBuilder.php b/app/code/Magento/Braintree/Gateway/Request/PayPal/VaultDataBuilder.php index a035c84b4cafd..4d63ee4125b74 100644 --- a/app/code/Magento/Braintree/Gateway/Request/PayPal/VaultDataBuilder.php +++ b/app/code/Magento/Braintree/Gateway/Request/PayPal/VaultDataBuilder.php @@ -49,6 +49,8 @@ public function build(array $buildSubject) $payment = $paymentDO->getPayment(); $data = $payment->getAdditionalInformation(); + // the payment token could be stored only if a customer checks the Vault flow on storefront + // see https://developers.braintreepayments.com/guides/paypal/vault/javascript/v2#invoking-the-vault-flow if (!empty($data[VaultConfigProvider::IS_ACTIVE_CODE])) { $result[self::$optionsKey] = [ self::$storeInVaultOnSuccess => true diff --git a/app/code/Magento/Braintree/Gateway/Request/VaultCaptureDataBuilder.php b/app/code/Magento/Braintree/Gateway/Request/VaultCaptureDataBuilder.php index 4280663178efb..78753af0ab11b 100644 --- a/app/code/Magento/Braintree/Gateway/Request/VaultCaptureDataBuilder.php +++ b/app/code/Magento/Braintree/Gateway/Request/VaultCaptureDataBuilder.php @@ -6,6 +6,7 @@ namespace Magento\Braintree\Gateway\Request; use Magento\Braintree\Gateway\SubjectReader; +use Magento\Payment\Gateway\Command\CommandException; use Magento\Payment\Gateway\Request\BuilderInterface; use Magento\Payment\Helper\Formatter; @@ -31,6 +32,7 @@ public function __construct(SubjectReader $subjectReader) $this->subjectReader = $subjectReader; } + /** * @inheritdoc */ @@ -41,6 +43,9 @@ public function build(array $buildSubject) $payment = $paymentDO->getPayment(); $extensionAttributes = $payment->getExtensionAttributes(); $paymentToken = $extensionAttributes->getVaultPaymentToken(); + if ($paymentToken === null) { + throw new CommandException(__('The Payment Token is not available to perform the request.')); + } return [ 'amount' => $this->formatPrice($this->subjectReader->readAmount($buildSubject)), 'paymentMethodToken' => $paymentToken->getGatewayToken() diff --git a/app/code/Magento/Braintree/Test/Unit/Gateway/Request/VaultCaptureDataBuilderTest.php b/app/code/Magento/Braintree/Test/Unit/Gateway/Request/VaultCaptureDataBuilderTest.php index 5af050002eb2d..80d333db80f0a 100644 --- a/app/code/Magento/Braintree/Test/Unit/Gateway/Request/VaultCaptureDataBuilderTest.php +++ b/app/code/Magento/Braintree/Test/Unit/Gateway/Request/VaultCaptureDataBuilderTest.php @@ -3,16 +3,20 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + namespace Magento\Braintree\Test\Unit\Gateway\Request; -use Magento\Braintree\Gateway\SubjectReader; use Magento\Braintree\Gateway\Request\VaultCaptureDataBuilder; +use Magento\Braintree\Gateway\SubjectReader; use Magento\Payment\Gateway\Data\PaymentDataObjectInterface; use Magento\Sales\Api\Data\OrderPaymentExtension; use Magento\Sales\Model\Order\Payment; use Magento\Vault\Model\PaymentToken; use PHPUnit_Framework_MockObject_MockObject as MockObject; +/** + * Tests VaultCaptureDataBuilder. + */ class VaultCaptureDataBuilderTest extends \PHPUnit\Framework\TestCase { /** @@ -30,7 +34,15 @@ class VaultCaptureDataBuilderTest extends \PHPUnit\Framework\TestCase */ private $payment; - public function setUp() + /** + * @var SubjectReader|MockObject + */ + private $subjectReader; + + /** + * @inheritdoc + */ + protected function setUp() { $this->paymentDO = $this->createMock(PaymentDataObjectInterface::class); $this->payment = $this->getMockBuilder(Payment::class) @@ -39,11 +51,15 @@ public function setUp() $this->paymentDO->method('getPayment') ->willReturn($this->payment); - $this->builder = new VaultCaptureDataBuilder(new SubjectReader()); + $this->subjectReader = $this->getMockBuilder(SubjectReader::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->builder = new VaultCaptureDataBuilder($this->subjectReader); } /** - * \Magento\Braintree\Gateway\Request\VaultCaptureDataBuilder::build + * Checks the result after builder execution. */ public function testBuild() { @@ -51,19 +67,28 @@ public function testBuild() $token = '5tfm4c'; $buildSubject = [ 'payment' => $this->paymentDO, - 'amount' => $amount + 'amount' => $amount, ]; $expected = [ 'amount' => $amount, - 'paymentMethodToken' => $token + 'paymentMethodToken' => $token, ]; + $this->subjectReader->method('readPayment') + ->with($buildSubject) + ->willReturn($this->paymentDO); + $this->subjectReader->method('readAmount') + ->with($buildSubject) + ->willReturn($amount); + + /** @var OrderPaymentExtension|MockObject $paymentExtension */ $paymentExtension = $this->getMockBuilder(OrderPaymentExtension::class) ->setMethods(['getVaultPaymentToken']) ->disableOriginalConstructor() ->getMockForAbstractClass(); + /** @var PaymentToken|MockObject $paymentToken */ $paymentToken = $this->getMockBuilder(PaymentToken::class) ->disableOriginalConstructor() ->getMock(); @@ -79,4 +104,39 @@ public function testBuild() $result = $this->builder->build($buildSubject); self::assertEquals($expected, $result); } + + /** + * Checks a builder execution if Payment Token doesn't exist. + * + * @expectedException \Magento\Payment\Gateway\Command\CommandException + * @expectedExceptionMessage The Payment Token is not available to perform the request. + */ + public function testBuildWithoutPaymentToken(): void + { + $amount = 30.00; + $buildSubject = [ + 'payment' => $this->paymentDO, + 'amount' => $amount, + ]; + + $this->subjectReader->method('readPayment') + ->with($buildSubject) + ->willReturn($this->paymentDO); + $this->subjectReader->method('readAmount') + ->with($buildSubject) + ->willReturn($amount); + + /** @var OrderPaymentExtension|MockObject $paymentExtension */ + $paymentExtension = $this->getMockBuilder(OrderPaymentExtension::class) + ->setMethods(['getVaultPaymentToken']) + ->disableOriginalConstructor() + ->getMockForAbstractClass(); + + $this->payment->method('getExtensionAttributes') + ->willReturn($paymentExtension); + $paymentExtension->method('getVaultPaymentToken') + ->willReturn(null); + + $this->builder->build($buildSubject); + } } diff --git a/app/code/Magento/Braintree/i18n/en_US.csv b/app/code/Magento/Braintree/i18n/en_US.csv index 6bf677151ed0d..7bd305f546dc6 100644 --- a/app/code/Magento/Braintree/i18n/en_US.csv +++ b/app/code/Magento/Braintree/i18n/en_US.csv @@ -192,3 +192,4 @@ Currency,Currency "Too many concurrent attempts to refund this transaction. Try again later.","Too many concurrent attempts to refund this transaction. Try again later." "Too many concurrent attempts to void this transaction. Try again later.","Too many concurrent attempts to void this transaction. Try again later." "Braintree Settlement","Braintree Settlement" +"The Payment Token is not available to perform the request.","The Payment Token is not available to perform the request." From ff68de826d58a04913c6d7db1309946a9e1cfc3a Mon Sep 17 00:00:00 2001 From: Serhiy Yelahin Date: Thu, 11 Oct 2018 09:38:58 +0300 Subject: [PATCH 09/10] MAGETWO-94857: Unable to create invoice for an order via Braintree PayPal --- .../Braintree/Gateway/Request/VaultCaptureDataBuilder.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/code/Magento/Braintree/Gateway/Request/VaultCaptureDataBuilder.php b/app/code/Magento/Braintree/Gateway/Request/VaultCaptureDataBuilder.php index 78753af0ab11b..5b773e03c8870 100644 --- a/app/code/Magento/Braintree/Gateway/Request/VaultCaptureDataBuilder.php +++ b/app/code/Magento/Braintree/Gateway/Request/VaultCaptureDataBuilder.php @@ -32,7 +32,6 @@ public function __construct(SubjectReader $subjectReader) $this->subjectReader = $subjectReader; } - /** * @inheritdoc */ From 5139e6fefc0625e8471b75f44e12ddaf9ef58ea2 Mon Sep 17 00:00:00 2001 From: Mastiuhin Olexandr Date: Thu, 11 Oct 2018 16:23:04 +0300 Subject: [PATCH 10/10] MAGETWO-95585: Can't generate Plugin if object method returns self --- .../Framework/Interception/Code/Generator/Interceptor.php | 4 ++-- .../Magento/Framework/ObjectManager/Code/Generator/Proxy.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/internal/Magento/Framework/Interception/Code/Generator/Interceptor.php b/lib/internal/Magento/Framework/Interception/Code/Generator/Interceptor.php index 59bba2c49631c..05f5f01efb29e 100644 --- a/lib/internal/Magento/Framework/Interception/Code/Generator/Interceptor.php +++ b/lib/internal/Magento/Framework/Interception/Code/Generator/Interceptor.php @@ -195,9 +195,9 @@ private function getReturnTypeValue($returnType) $returnTypeValue = null; if ($returnType) { - $returnTypeValue = ($returnType->getName() === 'self') + $returnTypeValue = ((string)$returnType === 'self') ? $this->getSourceClassName() - : $returnType->getName(); + : (string)$returnType; } return $returnTypeValue; diff --git a/lib/internal/Magento/Framework/ObjectManager/Code/Generator/Proxy.php b/lib/internal/Magento/Framework/ObjectManager/Code/Generator/Proxy.php index a08ff4677b3a9..39cbea74847f9 100644 --- a/lib/internal/Magento/Framework/ObjectManager/Code/Generator/Proxy.php +++ b/lib/internal/Magento/Framework/ObjectManager/Code/Generator/Proxy.php @@ -257,9 +257,9 @@ private function getReturnTypeValue($returnType) $returnTypeValue = null; if ($returnType) { - $returnTypeValue = ($returnType->getName() === 'self') + $returnTypeValue = ((string)$returnType === 'self') ? $this->getSourceClassName() - : $returnType->getName(); + : (string)$returnType; } return $returnTypeValue;