From 574f032d7facd5d9f16d647a5c6d9392a60dd83b Mon Sep 17 00:00:00 2001 From: Oleksandr Gorkun Date: Thu, 16 Nov 2017 11:56:41 +0200 Subject: [PATCH 1/9] MAGETWO-75217: SKU parameter in capital letters on CSV ignored by database in Import but OK'd by validator --- .../Import/Product/Type/Configurable.php | 45 ++++++++++++++++--- .../Import/Product/Type/ConfigurableTest.php | 41 +++++++++++++++++ 2 files changed, 79 insertions(+), 7 deletions(-) diff --git a/app/code/Magento/ConfigurableImportExport/Model/Import/Product/Type/Configurable.php b/app/code/Magento/ConfigurableImportExport/Model/Import/Product/Type/Configurable.php index 718a7dba73eb2..2e37c7dfd615d 100644 --- a/app/code/Magento/ConfigurableImportExport/Model/Import/Product/Type/Configurable.php +++ b/app/code/Magento/ConfigurableImportExport/Model/Import/Product/Type/Configurable.php @@ -13,6 +13,7 @@ use Magento\Catalog\Api\Data\ProductInterface; use Magento\CatalogImportExport\Model\Import\Product as ImportProduct; use Magento\Framework\EntityManager\MetadataPool; +use Magento\Framework\Exception\LocalizedException; /** * Importing configurable products @@ -34,16 +35,24 @@ class Configurable extends \Magento\CatalogImportExport\Model\Import\Product\Typ const ERROR_DUPLICATED_VARIATIONS = 'duplicatedVariations'; + const ERROR_UNIDENTIFIABLE_VARIATION = 'unidentifiableVariation'; + /** * Validation failure message template definitions * * @var array */ protected $_messageTemplates = [ - self::ERROR_ATTRIBUTE_CODE_IS_NOT_SUPER => 'Attribute with code "%s" is not super', - self::ERROR_INVALID_OPTION_VALUE => 'Invalid option value for attribute "%s"', - self::ERROR_INVALID_WEBSITE => 'Invalid website code for super attribute', - self::ERROR_DUPLICATED_VARIATIONS => 'SKU %s contains duplicated variations', + self::ERROR_ATTRIBUTE_CODE_IS_NOT_SUPER => + 'Attribute with code "%s" is not super', + self::ERROR_INVALID_OPTION_VALUE => + 'Invalid option value for attribute "%s"', + self::ERROR_INVALID_WEBSITE => + 'Invalid website code for super attribute', + self::ERROR_DUPLICATED_VARIATIONS => + 'SKU %s contains duplicated variations', + self::ERROR_UNIDENTIFIABLE_VARIATION => + 'Configurable variation "%s" is unidentifiable', ]; /** @@ -471,6 +480,7 @@ protected function _processSuperData() * @param array $rowData * * @return array + * @throws LocalizedException * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) */ @@ -489,8 +499,10 @@ protected function _parseVariations($rowData) foreach ($fieldAndValuePairsText as $nameAndValue) { $nameAndValue = explode(ImportProduct::PAIR_NAME_VALUE_SEPARATOR, $nameAndValue); if (!empty($nameAndValue)) { - $value = isset($nameAndValue[1]) ? trim($nameAndValue[1]) : ''; - $fieldName = trim($nameAndValue[0]); + $value = isset($nameAndValue[1]) ? + trim($nameAndValue[1]) : ''; + //Ignoring field names' case. + $fieldName = mb_strtolower(trim($nameAndValue[0])); if ($fieldName) { $fieldAndValuePairs[$fieldName] = $value; } @@ -511,8 +523,19 @@ protected function _parseVariations($rowData) $additionalRow = []; $position += 1; } + } else { + $errorCode = self::ERROR_UNIDENTIFIABLE_VARIATION; + throw new LocalizedException( + __( + sprintf( + $this->_messageTemplates[$errorCode], + $variation + ) + ) + ); } } + return $additionalRows; } @@ -823,7 +846,14 @@ protected function configurableInBunch($bunch) public function isRowValid(array $rowData, $rowNum, $isNewProduct = true) { $error = false; - $dataWithExtraVirtualRows = $this->_parseVariations($rowData); + try { + $dataWithExtraVirtualRows = $this->_parseVariations($rowData); + } catch (LocalizedException $exception) { + $this->_entityModel->addRowError($exception->getMessage(), $rowNum); + + return false; + } + $skus = []; $rowData['price'] = isset($rowData['price']) && $rowData['price'] ? $rowData['price'] : '0.00'; if (!empty($dataWithExtraVirtualRows)) { @@ -841,6 +871,7 @@ public function isRowValid(array $rowData, $rowNum, $isNewProduct = true) } $error |= !parent::isRowValid($option, $rowNum, $isNewProduct); } + return !$error; } diff --git a/app/code/Magento/ConfigurableImportExport/Test/Unit/Model/Import/Product/Type/ConfigurableTest.php b/app/code/Magento/ConfigurableImportExport/Test/Unit/Model/Import/Product/Type/ConfigurableTest.php index f6912fe8b6d6c..54552364de13b 100644 --- a/app/code/Magento/ConfigurableImportExport/Test/Unit/Model/Import/Product/Type/ConfigurableTest.php +++ b/app/code/Magento/ConfigurableImportExport/Test/Unit/Model/Import/Product/Type/ConfigurableTest.php @@ -560,15 +560,56 @@ public function testIsRowValid() '_type' => 'configurable', '_product_websites' => 'website_1', ]; + //Checking that variations' field names are case-insensitive with this + //product. + $caseInsensitiveSKU = 'configurableskuI22CaseInsensitive'; + $caseInsensitiveProduct = [ + 'sku' => $caseInsensitiveSKU, + 'store_view_code' => null, + 'attribute_set_code' => 'Default', + 'product_type' => 'configurable', + 'name' => 'Configurable Product 21', + 'product_websites' => 'website_1', + 'configurable_variation_labels' => 'testattr2=Select Color, testattr3=Select Size', + 'configurable_variations' => 'SKU=testconf2-attr2val1-testattr3v1,' + . 'testattr2=attr2val1,' + . 'testattr3=testattr3v1,' + . 'display=1|sku=testconf2-attr2val1-testattr3v2,' + . 'testattr2=attr2val1,' + . 'testattr3=testattr3v2,' + . 'display=0', + '_store' => null, + '_attribute_set' => 'Default', + '_type' => 'configurable', + '_product_websites' => 'website_1', + ]; $bunch[] = $badProduct; + $bunch[] = $caseInsensitiveProduct; // Set _attributes to avoid error in Magento\CatalogImportExport\Model\Import\Product\Type\AbstractType. $this->setPropertyValue($this->configurable, '_attributes', [ $badProduct[\Magento\CatalogImportExport\Model\Import\Product::COL_ATTR_SET] => [], ]); + //Avoiding errors about attributes not being super + $this->setPropertyValue( + $this->configurable, + '_superAttributes', + [ + 'testattr2' => ['options' => ['attr2val1' => 1]], + 'testattr3' => [ + 'options' => [ + 'testattr3v2' => 1, + 'testattr3v1' => 1, + ], + ], + ] + ); foreach ($bunch as $rowData) { $result = $this->configurable->isRowValid($rowData, 0, !isset($this->_oldSku[$rowData['sku']])); $this->assertNotNull($result); + if ($rowData['sku'] === $caseInsensitiveSKU) { + $this->assertTrue($result); + } } } From a796fe6e73ab03eedad571916ff128471fcf1eaa Mon Sep 17 00:00:00 2001 From: Oleksandr Gorkun Date: Thu, 16 Nov 2017 12:50:00 +0200 Subject: [PATCH 2/9] MAGETWO-75217: SKU parameter in capital letters on CSV ignored by database in Import but OK'd by validator --- .../Model/Import/Product/Type/Configurable.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/ConfigurableImportExport/Model/Import/Product/Type/Configurable.php b/app/code/Magento/ConfigurableImportExport/Model/Import/Product/Type/Configurable.php index 2e37c7dfd615d..0e7a026c526c9 100644 --- a/app/code/Magento/ConfigurableImportExport/Model/Import/Product/Type/Configurable.php +++ b/app/code/Magento/ConfigurableImportExport/Model/Import/Product/Type/Configurable.php @@ -487,7 +487,7 @@ protected function _processSuperData() protected function _parseVariations($rowData) { $additionalRows = []; - if (!isset($rowData['configurable_variations'])) { + if (empty($rowData['configurable_variations'])) { return $additionalRows; } $variations = explode(ImportProduct::PSEUDO_MULTI_LINE_SEPARATOR, $rowData['configurable_variations']); From 21d31537c14a63d91cb3ad53b1f40a43c32a6bff Mon Sep 17 00:00:00 2001 From: Oleksandr Gorkun Date: Fri, 17 Nov 2017 15:57:13 +0200 Subject: [PATCH 3/9] MAGETWO-75217: SKU parameter in capital letters on CSV ignored by database in Import but OK'd by validator --- .../Model/Import/Product/Type/Configurable.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/ConfigurableImportExport/Model/Import/Product/Type/Configurable.php b/app/code/Magento/ConfigurableImportExport/Model/Import/Product/Type/Configurable.php index 0e7a026c526c9..4ca21d554ccd9 100644 --- a/app/code/Magento/ConfigurableImportExport/Model/Import/Product/Type/Configurable.php +++ b/app/code/Magento/ConfigurableImportExport/Model/Import/Product/Type/Configurable.php @@ -502,7 +502,7 @@ protected function _parseVariations($rowData) $value = isset($nameAndValue[1]) ? trim($nameAndValue[1]) : ''; //Ignoring field names' case. - $fieldName = mb_strtolower(trim($nameAndValue[0])); + $fieldName = strtolower(trim($nameAndValue[0])); if ($fieldName) { $fieldAndValuePairs[$fieldName] = $value; } From 2158987fe15d22e76f0c0e7555e3d2c94d64bc4c Mon Sep 17 00:00:00 2001 From: Dmytro Voskoboinikov Date: Mon, 20 Nov 2017 16:25:54 +0200 Subject: [PATCH 4/9] MAGETWO-77840: [2.2.x] - Special/lowest price in child of a Configurable Product causes the entire product to show that price --- .../templates/product/price/final_price.phtml | 29 ++++++------ .../view/frontend/web/js/configurable.js | 35 ++++++++++++++- .../view/frontend/web/js/swatch-renderer.js | 21 ++++++++- .../Block/Product/Compare/ListCompare.php | 11 ++++- .../Catalog/Test/Block/Product/View.php | 15 ++++++- .../Constraint/AssertProductComparePage.php | 25 ++++++++--- .../Test/Constraint/AssertProductPage.php | 2 +- ...AssertProductSpecialPriceOnProductPage.php | 2 +- .../Product/AddCompareProductsTest.xml | 4 +- .../Test/Block/Product/Price.php | 45 +++++++++++++++++++ .../Test/Block/Product/View.php | 20 ++++++++- .../AssertConfigurableProductPage.php | 37 ++++++++++++++- .../Test/Repository/ConfigurableProduct.xml | 31 +++++++++++++ .../RenderingBasedOnIsProductListFlagTest.php | 8 ++-- 14 files changed, 246 insertions(+), 39 deletions(-) create mode 100644 dev/tests/functional/tests/app/Magento/ConfigurableProduct/Test/Block/Product/Price.php diff --git a/app/code/Magento/ConfigurableProduct/view/base/templates/product/price/final_price.phtml b/app/code/Magento/ConfigurableProduct/view/base/templates/product/price/final_price.phtml index 98abb906f69d6..df05fb22104a5 100644 --- a/app/code/Magento/ConfigurableProduct/view/base/templates/product/price/final_price.phtml +++ b/app/code/Magento/ConfigurableProduct/view/base/templates/product/price/final_price.phtml @@ -19,16 +19,22 @@ $finalPriceModel = $block->getPriceType('final_price'); $idSuffix = $block->getIdSuffix() ? $block->getIdSuffix() : ''; $schema = ($block->getZone() == 'item_view') ? true : false; ?> -isProductList() && $block->hasSpecialPrice()): ?> - - renderAmount($finalPriceModel->getAmount(), [ - 'display_label' => __('Special Price'), - 'price_id' => $block->getPriceId('product-price-' . $idSuffix), - 'price_type' => 'finalPrice', + + + __('As low as'), + 'price_id' => $block->getPriceId('product-price-' . $idSuffix), + 'price_type' => 'finalPrice', 'include_container' => true, 'schema' => $schema - ]); ?> - + ]; + + /* @escapeNotVerified */ echo $block->renderAmount($finalPriceModel->getAmount(), $arguments); + ?> + + +isProductList() && $block->hasSpecialPrice()): ?> renderAmount($priceModel->getAmount(), [ 'display_label' => __('Regular Price'), @@ -38,13 +44,6 @@ $schema = ($block->getZone() == 'item_view') ? true : false; 'skip_adjustments' => true ]); ?> - - renderAmount($finalPriceModel->getAmount(), [ - 'price_id' => $block->getPriceId('product-price-' . $idSuffix), - 'price_type' => 'finalPrice', - 'include_container' => true, - 'schema' => $schema - ]); ?> showMinimalPrice()): ?> diff --git a/app/code/Magento/ConfigurableProduct/view/frontend/web/js/configurable.js b/app/code/Magento/ConfigurableProduct/view/frontend/web/js/configurable.js index 545887d04c965..8cabe71c17504 100644 --- a/app/code/Magento/ConfigurableProduct/view/frontend/web/js/configurable.js +++ b/app/code/Magento/ConfigurableProduct/view/frontend/web/js/configurable.js @@ -32,6 +32,7 @@ define([ mediaGallerySelector: '[data-gallery-role=gallery-placeholder]', mediaGalleryInitial: null, slyOldPriceSelector: '.sly-old-price', + normalPriceLabelSelector: '.normal-price .price-label', /** * Defines the mechanism of how images of a gallery should be @@ -269,6 +270,7 @@ define([ this._reloadPrice(); this._displayRegularPriceBlock(this.simpleProduct); this._displayTierPriceBlock(this.simpleProduct); + this._displayNormalPriceLabel(); this._changeProductImage(); }, @@ -527,8 +529,16 @@ define([ * @private */ _displayRegularPriceBlock: function (optionId) { - if (typeof optionId != 'undefined' && - this.options.spConfig.optionPrices[optionId].oldPrice.amount != //eslint-disable-line eqeqeq + var shouldBeShown = true; + + _.each(this.options.settings, function (element) { + if (element.value === '') { + shouldBeShown = false; + } + }); + + if (shouldBeShown && + this.options.spConfig.optionPrices[optionId].oldPrice.amount !== this.options.spConfig.optionPrices[optionId].finalPrice.amount ) { $(this.options.slyOldPriceSelector).show(); @@ -537,6 +547,27 @@ define([ } }, + /** + * Show or hide normal price label + * + * @private + */ + _displayNormalPriceLabel: function () { + var shouldBeShown = false; + + _.each(this.options.settings, function (element) { + if (element.value === '') { + shouldBeShown = true; + } + }); + + if (shouldBeShown) { + $(this.options.normalPriceLabelSelector).show(); + } else { + $(this.options.normalPriceLabelSelector).hide(); + } + }, + /** * Callback which fired after gallery gets initialized. * diff --git a/app/code/Magento/Swatches/view/frontend/web/js/swatch-renderer.js b/app/code/Magento/Swatches/view/frontend/web/js/swatch-renderer.js index c09c17f0ad6ba..df2345b741ea1 100644 --- a/app/code/Magento/Swatches/view/frontend/web/js/swatch-renderer.js +++ b/app/code/Magento/Swatches/view/frontend/web/js/swatch-renderer.js @@ -268,8 +268,11 @@ define([ // tier prise selectors start tierPriceTemplateSelector: '#tier-prices-template', tierPriceBlockSelector: '[data-role="tier-price-block"]', - tierPriceTemplate: '' + tierPriceTemplate: '', // tier prise selectors end + + // A price label selector + normalPriceLabelSelector: '.normal-price .price-label' }, /** @@ -930,6 +933,22 @@ define([ } else { $(this.options.tierPriceBlockSelector).hide(); } + + $(this.options.normalPriceLabelSelector).hide(); + + _.each($('.' + this.options.classes.attributeOptionsWrapper), function (attribute) { + if ($(attribute).find('.' + this.options.classes.optionClass + '.selected').length === 0) { + if ($(attribute).find('.' + this.options.classes.selectClass).length > 0) { + _.each($(attribute).find('.' + this.options.classes.selectClass), function (dropdown) { + if ($(dropdown).val() === '0') { + $(this.options.normalPriceLabelSelector).show(); + } + }.bind(this)); + } else { + $(this.options.normalPriceLabelSelector).show(); + } + } + }.bind(this)); }, /** diff --git a/dev/tests/functional/tests/app/Magento/Catalog/Test/Block/Product/Compare/ListCompare.php b/dev/tests/functional/tests/app/Magento/Catalog/Test/Block/Product/Compare/ListCompare.php index 0a19fb73c3101..7082eb5ffaec4 100644 --- a/dev/tests/functional/tests/app/Magento/Catalog/Test/Block/Product/Compare/ListCompare.php +++ b/dev/tests/functional/tests/app/Magento/Catalog/Test/Block/Product/Compare/ListCompare.php @@ -16,6 +16,13 @@ */ class ListCompare extends Block { + /** + * Price displaying format. + * + * @var int + */ + protected $priceFormat = 2; + /** * Selector by product info. * @@ -94,12 +101,12 @@ class ListCompare extends Block protected $confirmModal = '.confirm._show[data-role=modal]'; /** - * Get product info. + * Get Product info. * * @param int $index * @param string $attributeKey * @param string $currency - * @return string + * @return string|array */ public function getProductInfo($index, $attributeKey, $currency = ' $') { diff --git a/dev/tests/functional/tests/app/Magento/Catalog/Test/Block/Product/View.php b/dev/tests/functional/tests/app/Magento/Catalog/Test/Block/Product/View.php index 92f3a2102e85e..955731e8209b3 100644 --- a/dev/tests/functional/tests/app/Magento/Catalog/Test/Block/Product/View.php +++ b/dev/tests/functional/tests/app/Magento/Catalog/Test/Block/Product/View.php @@ -243,10 +243,23 @@ public function getThresholdMessage() /** * Get block price. * + * @param FixtureInterface|null $product + * * @return Price */ - public function getPriceBlock() + public function getPriceBlock(FixtureInterface $product = null) { + $typeId = null; + + if ($product) { + $dataConfig = $product->getDataConfig(); + $typeId = isset($dataConfig['type_id']) ? $dataConfig['type_id'] : null; + } + + if ($this->hasRender($typeId)) { + return $this->callRender($typeId, 'getPriceBlock'); + } + return $this->blockFactory->create( \Magento\Catalog\Test\Block\Product\Price::class, ['element' => $this->_rootElement->find($this->priceBlock, Locator::SELECTOR_XPATH)] diff --git a/dev/tests/functional/tests/app/Magento/Catalog/Test/Constraint/AssertProductComparePage.php b/dev/tests/functional/tests/app/Magento/Catalog/Test/Constraint/AssertProductComparePage.php index d7dd6fbdaafd4..9831328fffbfe 100644 --- a/dev/tests/functional/tests/app/Magento/Catalog/Test/Constraint/AssertProductComparePage.php +++ b/dev/tests/functional/tests/app/Magento/Catalog/Test/Constraint/AssertProductComparePage.php @@ -16,6 +16,13 @@ */ class AssertProductComparePage extends AbstractConstraint { + /** + * Price displaying format. + * + * @var int + */ + protected $priceFormat = 2; + /** * Product attribute on compare product page * @@ -30,8 +37,8 @@ class AssertProductComparePage extends AbstractConstraint ]; /** - * Assert that "Compare Product" page contains product(s) that was added - * - Product name + * Assert that "Compare Product" Storefront page contains added Products with expected Attribute values: + * - Name * - Price * - SKU * - Description (if exists, else text "No") @@ -56,19 +63,23 @@ public function processAssert( $value = $attribute; $attribute = is_numeric($attributeKey) ? $attribute : $attributeKey; - $attributeValue = $attribute != 'price' + $expectedAttributeValue = $attribute != 'price' ? ($product->hasData($attribute) ? $product->getData($attribute) : 'N/A') : ($product->getDataFieldConfig('price')['source']->getPriceData() !== null ? $product->getDataFieldConfig('price')['source']->getPriceData()['compare_price'] - : number_format($product->getPrice(), 2)); + : number_format($product->getPrice(), $this->priceFormat)); $attribute = is_numeric($attributeKey) ? 'info' : 'attribute'; + $attribute = ucfirst($attribute); + $actualAttributeValue = + $comparePage->getCompareProductsBlock()->{'getProduct' . $attribute}($key + 1, $value); + \PHPUnit_Framework_Assert::assertEquals( - $attributeValue, - $comparePage->getCompareProductsBlock()->{'getProduct' . ucfirst($attribute)}($key + 1, $value), - 'Product "' . $product->getName() . '" is\'n equals with data from fixture.' + $expectedAttributeValue, + $actualAttributeValue, + 'Product "' . $product->getName() . '" has "' . $attribute . '" value different from fixture one.' ); } } diff --git a/dev/tests/functional/tests/app/Magento/Catalog/Test/Constraint/AssertProductPage.php b/dev/tests/functional/tests/app/Magento/Catalog/Test/Constraint/AssertProductPage.php index 049ecaf053eea..cf9f608b9353e 100644 --- a/dev/tests/functional/tests/app/Magento/Catalog/Test/Constraint/AssertProductPage.php +++ b/dev/tests/functional/tests/app/Magento/Catalog/Test/Constraint/AssertProductPage.php @@ -145,7 +145,7 @@ protected function verifySpecialPrice() } $expectedSpecialPrice = $this->product->getSpecialPrice(); $expectedSpecialPrice = number_format($expectedSpecialPrice, 2); - $priceBlock = $this->productView->getPriceBlock(); + $priceBlock = $this->productView->getPriceBlock($this->product); if (!$priceBlock->isVisible()) { return "Price block for '{$this->product->getName()}' product' is not visible."; } diff --git a/dev/tests/functional/tests/app/Magento/Catalog/Test/Constraint/AssertProductSpecialPriceOnProductPage.php b/dev/tests/functional/tests/app/Magento/Catalog/Test/Constraint/AssertProductSpecialPriceOnProductPage.php index 0d8ebd5ac337a..17607ffad5912 100644 --- a/dev/tests/functional/tests/app/Magento/Catalog/Test/Constraint/AssertProductSpecialPriceOnProductPage.php +++ b/dev/tests/functional/tests/app/Magento/Catalog/Test/Constraint/AssertProductSpecialPriceOnProductPage.php @@ -64,7 +64,7 @@ public function setErrorMessage($errorMessage) public function assertPrice(FixtureInterface $product, View $productViewBlock) { $fields = $product->getData(); - $specialPrice = $productViewBlock->getPriceBlock()->getSpecialPrice(); + $specialPrice = $productViewBlock->getPriceBlock($product)->getSpecialPrice(); if (isset($fields['special_price'])) { \PHPUnit_Framework_Assert::assertEquals( number_format($fields['special_price'], 2), diff --git a/dev/tests/functional/tests/app/Magento/Catalog/Test/TestCase/Product/AddCompareProductsTest.xml b/dev/tests/functional/tests/app/Magento/Catalog/Test/TestCase/Product/AddCompareProductsTest.xml index 2ff67cdeb4d2a..1827292303174 100644 --- a/dev/tests/functional/tests/app/Magento/Catalog/Test/TestCase/Product/AddCompareProductsTest.xml +++ b/dev/tests/functional/tests/app/Magento/Catalog/Test/TestCase/Product/AddCompareProductsTest.xml @@ -21,14 +21,14 @@ - configurableProduct::configurable_with_qty_1 + configurableProduct::configurable_as_low_as Yes - catalogProductSimple::simple_for_composite_products,catalogProductVirtual::default,downloadableProduct::default,groupedProduct::grouped_product_with_price,configurableProduct::configurable_with_qty_1,bundleProduct::bundle_dynamic_product,bundleProduct::bundle_fixed_product + catalogProductSimple::simple_for_composite_products,catalogProductVirtual::default,downloadableProduct::default,groupedProduct::grouped_product_with_price,configurableProduct::configurable_as_low_as,bundleProduct::bundle_dynamic_product,bundleProduct::bundle_fixed_product Yes diff --git a/dev/tests/functional/tests/app/Magento/ConfigurableProduct/Test/Block/Product/Price.php b/dev/tests/functional/tests/app/Magento/ConfigurableProduct/Test/Block/Product/Price.php new file mode 100644 index 0000000000000..2facfafbb83b0 --- /dev/null +++ b/dev/tests/functional/tests/app/Magento/ConfigurableProduct/Test/Block/Product/Price.php @@ -0,0 +1,45 @@ + [ + 'selector' => '.normal-price .price' + ] + ]; + + /** + * This method returns the price represented by the block. + * + * @return SimpleElement + */ + public function getPriceLabel() + { + return $this->_rootElement->find($this->priceLabel); + } +} diff --git a/dev/tests/functional/tests/app/Magento/ConfigurableProduct/Test/Block/Product/View.php b/dev/tests/functional/tests/app/Magento/ConfigurableProduct/Test/Block/Product/View.php index a1f64822a6ea6..6092983c7e86d 100644 --- a/dev/tests/functional/tests/app/Magento/ConfigurableProduct/Test/Block/Product/View.php +++ b/dev/tests/functional/tests/app/Magento/ConfigurableProduct/Test/Block/Product/View.php @@ -3,11 +3,10 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ - namespace Magento\ConfigurableProduct\Test\Block\Product; use Magento\ConfigurableProduct\Test\Block\Product\View\ConfigurableOptions; -use Magento\ConfigurableProduct\Test\Fixture\ConfigurableProduct; +use Magento\Mtf\Client\Locator; use Magento\Mtf\Fixture\FixtureInterface; use Magento\Mtf\Fixture\InjectableFixture; @@ -17,6 +16,23 @@ */ class View extends \Magento\Catalog\Test\Block\Product\View { + /** + * Gets a configurable product price block. + * + * @param FixtureInterface|null $product + * + * @return Price + * + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ + public function getPriceBlock(FixtureInterface $product = null) + { + return $this->blockFactory->create( + 'Magento\ConfigurableProduct\Test\Block\Product\Price', + ['element' => $this->_rootElement->find($this->priceBlock, Locator::SELECTOR_XPATH)] + ); + } + /** * Get configurable options block * diff --git a/dev/tests/functional/tests/app/Magento/ConfigurableProduct/Test/Constraint/AssertConfigurableProductPage.php b/dev/tests/functional/tests/app/Magento/ConfigurableProduct/Test/Constraint/AssertConfigurableProductPage.php index 580aff912104b..e582be6337920 100644 --- a/dev/tests/functional/tests/app/Magento/ConfigurableProduct/Test/Constraint/AssertConfigurableProductPage.php +++ b/dev/tests/functional/tests/app/Magento/ConfigurableProduct/Test/Constraint/AssertConfigurableProductPage.php @@ -14,6 +14,14 @@ */ class AssertConfigurableProductPage extends AssertProductPage { + + /** + * Price format. + * + * @var int + */ + protected $priceFormat = 2; + /** * Verify displayed product data on product page(front-end) equals passed from fixture: * 1. Product Name @@ -28,6 +36,7 @@ class AssertConfigurableProductPage extends AssertProductPage protected function verify() { $errors = parent::verify(); + $errors[] = $this->verifyPriceLabel(); $errors[] = $this->verifyAttributes(); return array_filter($errors); @@ -47,7 +56,7 @@ protected function verifyPrice() $formPrice = $priceBlock->isOldPriceVisible() ? $priceBlock->getOldPrice() : $priceBlock->getPrice(); $fixturePrice = $this->getLowestConfigurablePrice(); - if ($fixturePrice != $formPrice) { + if ($fixturePrice != number_format($formPrice, $this->priceFormat)) { return "Displayed product price on product page(front-end) not equals passed from fixture. " . "Actual: {$formPrice}, expected: {$fixturePrice}."; } @@ -144,4 +153,30 @@ protected function getLowestConfigurablePrice() } return $price; } + + /** + * Verifies displayed product price label on a product page (front-end) + * equals passed from the fixture. + * + * @return string|null + */ + protected function verifyPriceLabel() + { + /** @var \Magento\ConfigurableProduct\Test\Block\Product\Price $priceBlock */ + $priceBlock = $this->productView->getPriceBlock($this->product); + + if (!$priceBlock->getPriceLabel()->isVisible()) { + return "Product price label should be displayed."; + } else { + $expectedPriceLabel = 'As low as'; + $actualPriceLabel = $priceBlock->getPriceLabel()->getText(); + + if ($expectedPriceLabel !== $actualPriceLabel) { + return "Displayed product price label on product page (front-end) not equals passed from fixture. " + . "Actual: {$actualPriceLabel}, expected: {$expectedPriceLabel}."; + } + } + + return null; + } } diff --git a/dev/tests/functional/tests/app/Magento/ConfigurableProduct/Test/Repository/ConfigurableProduct.xml b/dev/tests/functional/tests/app/Magento/ConfigurableProduct/Test/Repository/ConfigurableProduct.xml index a524139d3f3ad..61ae2a4dfadd1 100644 --- a/dev/tests/functional/tests/app/Magento/ConfigurableProduct/Test/Repository/ConfigurableProduct.xml +++ b/dev/tests/functional/tests/app/Magento/ConfigurableProduct/Test/Repository/ConfigurableProduct.xml @@ -176,6 +176,37 @@ + + Test configurable product %isolation% + sku_test_configurable_product_%isolation% + + price_as-40.00 + + This item has weight + 30 + Yes + Catalog, Search + + taxable_goods + + configurable-product-%isolation% + + default + + + In Stock + + + Main Website + + + default + + + configurable_options_with_qty_1 + + + Test configurable product %isolation% sku_test_configurable_product_%isolation% diff --git a/dev/tests/integration/testsuite/Magento/ConfigurableProduct/Pricing/Render/FinalPriceBox/RenderingBasedOnIsProductListFlagTest.php b/dev/tests/integration/testsuite/Magento/ConfigurableProduct/Pricing/Render/FinalPriceBox/RenderingBasedOnIsProductListFlagTest.php index 379e61e64c788..076ed34262711 100644 --- a/dev/tests/integration/testsuite/Magento/ConfigurableProduct/Pricing/Render/FinalPriceBox/RenderingBasedOnIsProductListFlagTest.php +++ b/dev/tests/integration/testsuite/Magento/ConfigurableProduct/Pricing/Render/FinalPriceBox/RenderingBasedOnIsProductListFlagTest.php @@ -83,7 +83,7 @@ public function testRenderingByDefault() $this->assertGreaterThanOrEqual( 1, \Magento\TestFramework\Helper\Xpath::getElementsCountForXpath( - '//*[contains(@class,"special-price")]', + '//*[contains(@class,"normal-price")]', $html ) ); @@ -116,9 +116,9 @@ public function testRenderingAccordingToIsProductListFlag($flag, $count) $html = $this->finalPriceBox->toHtml(); self::assertContains('5.99', $html); $this->assertEquals( - $count, + 1, \Magento\TestFramework\Helper\Xpath::getElementsCountForXpath( - '//*[contains(@class,"special-price")]', + '//*[contains(@class,"normal-price")]', $html ) ); @@ -137,7 +137,7 @@ public function testRenderingAccordingToIsProductListFlag($flag, $count) public function isProductListDataProvider() { return [ - 'is_not_product_list' => [false, true], + 'is_not_product_list' => [false, 1], 'is_product_list' => [true, 0], ]; } From 4f3625b40c1e20c93391afb003ad82dd98cd7fb0 Mon Sep 17 00:00:00 2001 From: Dmytro Vilchynskyi Date: Mon, 20 Nov 2017 19:36:10 +0200 Subject: [PATCH 5/9] MAGETWO-70725: Admin token does not expire after 'Admin Token Lifetime (hours)' - fix --- .../Model/Authorization/TokenUserContext.php | 56 +++- .../Authorization/TokenUserContextTest.php | 269 +++++++++++++++++- 2 files changed, 312 insertions(+), 13 deletions(-) diff --git a/app/code/Magento/Webapi/Model/Authorization/TokenUserContext.php b/app/code/Magento/Webapi/Model/Authorization/TokenUserContext.php index eaf39549b2c2a..afa5889fe55e8 100644 --- a/app/code/Magento/Webapi/Model/Authorization/TokenUserContext.php +++ b/app/code/Magento/Webapi/Model/Authorization/TokenUserContext.php @@ -7,6 +7,8 @@ namespace Magento\Webapi\Model\Authorization; use Magento\Authorization\Model\UserContextInterface; +use Magento\Framework\App\ObjectManager; +use Magento\Framework\Oauth\Exception; use Magento\Integration\Model\Oauth\Token; use Magento\Integration\Model\Oauth\TokenFactory; use Magento\Integration\Api\IntegrationServiceInterface; @@ -47,6 +49,21 @@ class TokenUserContext implements UserContextInterface */ protected $integrationService; + /** + * @var \Magento\Framework\Stdlib\DateTime + */ + private $dateTime; + + /** + * @var \Magento\Framework\Stdlib\DateTime\DateTime + */ + private $date; + + /** + * @var \Magento\Integration\Helper\Oauth\Data + */ + private $oauthHelper; + /** * Initialize dependencies. * @@ -57,11 +74,23 @@ class TokenUserContext implements UserContextInterface public function __construct( Request $request, TokenFactory $tokenFactory, - IntegrationServiceInterface $integrationService + IntegrationServiceInterface $integrationService, + \Magento\Framework\Stdlib\DateTime $dateTime = null, + \Magento\Framework\Stdlib\DateTime\DateTime $date = null, + \Magento\Integration\Helper\Oauth\Data $oauthHelper = null ) { $this->request = $request; $this->tokenFactory = $tokenFactory; $this->integrationService = $integrationService; + $this->dateTime = $dateTime ?: ObjectManager::getInstance()->get( + \Magento\Framework\Stdlib\DateTime::class + ); + $this->date = $date ?: ObjectManager::getInstance()->get( + \Magento\Framework\Stdlib\DateTime\DateTime::class + ); + $this->oauthHelper = $oauthHelper ?: ObjectManager::getInstance()->get( + \Magento\Integration\Helper\Oauth\Data::class + ); } /** @@ -82,6 +111,29 @@ public function getUserType() return $this->userType; } + /** + * Check if token is expired. + * + * @param Token $token + * + * @return bool + */ + private function isTokenExpired(Token $token) + { + if ($token->getUserType() == \Magento\Authorization\Model\UserContextInterface::USER_TYPE_ADMIN) { + $tokenTtl = $this->oauthHelper->getAdminTokenLifetime(); + } elseif ($token->getUserType() == \Magento\Authorization\Model\UserContextInterface::USER_TYPE_CUSTOMER) { + $tokenTtl = $this->oauthHelper->getCustomerTokenLifetime(); + } else { + // other user-type tokens are considered always valid + return false; + } + if ($this->dateTime->strToTime($token->getCreatedAt()) < ($this->date->gmtTimestamp() - $tokenTtl * 3600)) { + return true; + } + return false; + } + /** * Finds the bearer token and looks up the value. * @@ -114,7 +166,7 @@ protected function processRequest() $bearerToken = $headerPieces[1]; $token = $this->tokenFactory->create()->loadByToken($bearerToken); - if (!$token->getId() || $token->getRevoked()) { + if (!$token->getId() || $token->getRevoked() || $this->isTokenExpired($token)) { $this->isRequestProcessed = true; return; } diff --git a/app/code/Magento/Webapi/Test/Unit/Model/Authorization/TokenUserContextTest.php b/app/code/Magento/Webapi/Test/Unit/Model/Authorization/TokenUserContextTest.php index 45ea678bdf859..f7ab848083dcc 100644 --- a/app/code/Magento/Webapi/Test/Unit/Model/Authorization/TokenUserContextTest.php +++ b/app/code/Magento/Webapi/Test/Unit/Model/Authorization/TokenUserContextTest.php @@ -24,20 +24,35 @@ class TokenUserContextTest extends \PHPUnit\Framework\TestCase protected $tokenUserContext; /** - * @var \Magento\Integration\Model\Oauth\TokenFactory + * @var \Magento\Integration\Model\Oauth\TokenFactory|\PHPUnit_Framework_MockObject_MockObject */ protected $tokenFactory; /** - * @var \Magento\Integration\Api\IntegrationServiceInterface + * @var \Magento\Integration\Api\IntegrationServiceInterface|\PHPUnit_Framework_MockObject_MockObject */ protected $integrationService; /** - * @var \Magento\Framework\Webapi\Request + * @var \Magento\Framework\Webapi\Request|\PHPUnit_Framework_MockObject_MockObject */ protected $request; + /** + * @var \Magento\Integration\Helper\Oauth\Data|\PHPUnit_Framework_MockObject_MockObject + */ + private $oauthHelperMock; + + /** + * @var \Magento\Framework\Stdlib\DateTime\DateTime|\PHPUnit_Framework_MockObject_MockObject + */ + private $dateMock; + + /** + * @var \Magento\Framework\Stdlib\DateTime|\PHPUnit_Framework_MockObject_MockObject + */ + private $dateTimeMock; + protected function setUp() { $this->objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); @@ -68,12 +83,39 @@ protected function setUp() ) ->getMock(); + $this->oauthHelperMock = $this->getMockBuilder(\Magento\Integration\Helper\Oauth\Data::class) + ->disableOriginalConstructor() + ->setMethods(['getAdminTokenLifetime', 'getCustomerTokenLifetime']) + ->getMock(); + + $this->dateMock = $this->getMockBuilder(\Magento\Framework\Stdlib\DateTime\DateTime::class) + ->disableOriginalConstructor() + ->setMethods(['gmtTimestamp']) + ->getMock(); + + $this->dateTimeMock = $this->getMockBuilder(\Magento\Framework\Stdlib\DateTime::class) + ->disableOriginalConstructor() + ->setMethods(['strToTime']) + ->getMock(); + + $this->dateTimeMock->expects($this->any()) + ->method('strToTime') + ->will($this->returnCallback( + function ($str) { + return strtotime($str); + } + ) + ); + $this->tokenUserContext = $this->objectManager->getObject( \Magento\Webapi\Model\Authorization\TokenUserContext::class, [ 'request' => $this->request, 'tokenFactory' => $this->tokenFactory, - 'integrationService' => $this->integrationService + 'integrationService' => $this->integrationService, + 'oauthHelper' => $this->oauthHelperMock, + 'date' => $this->dateMock, + 'dateTime' => $this->dateTimeMock, ] ); } @@ -181,8 +223,17 @@ public function testValidToken($userType, $userId, $expectedUserType, $expectedU $token = $this->getMockBuilder(\Magento\Integration\Model\Oauth\Token::class) ->disableOriginalConstructor() - ->setMethods(['loadByToken', 'getId', 'getUserType', 'getCustomerId', 'getAdminId', '__wakeup']) - ->getMock(); + ->setMethods( + [ + 'loadByToken', + 'getId', + 'getUserType', + 'getCustomerId', + 'getAdminId', + '__wakeup', + 'getCreatedAt' + ] + )->getMock(); $this->tokenFactory->expects($this->once()) ->method('create') ->will($this->returnValue($token)); @@ -193,17 +244,21 @@ public function testValidToken($userType, $userId, $expectedUserType, $expectedU $token->expects($this->once()) ->method('getId') ->will($this->returnValue(1)); - $token->expects($this->once()) + $token->expects($this->any()) ->method('getUserType') ->will($this->returnValue($userType)); - $integration = $this->getMockBuilder(\Magento\Integration\Model\Integration::class) - ->disableOriginalConstructor() - ->setMethods(['getId', '__wakeup']) - ->getMock(); + $token->expects($this->any()) + ->method('getCreatedAt') + ->willReturn(date('Y-m-d H:i:s', time())); switch ($userType) { case UserContextInterface::USER_TYPE_INTEGRATION: + $integration = $this->getMockBuilder(\Magento\Integration\Model\Integration::class) + ->disableOriginalConstructor() + ->setMethods(['getId', '__wakeup']) + ->getMock(); + $integration->expects($this->once()) ->method('getId') ->will($this->returnValue($userId)); @@ -260,4 +315,196 @@ public function getValidTokenData() ] ]; } + + /** + * @dataProvider getExpiredTestTokenData + */ + public function testExpiredToken($tokenData, $tokenTtl, $currentTime, $expectedUserType, $expectedUserId) + { + $bearerToken = 'bearer1234'; + + $this->dateMock->expects($this->any()) + ->method('gmtTimestamp') + ->willReturn($currentTime); + + $this->request->expects($this->once()) + ->method('getHeader') + ->with('Authorization') + ->will($this->returnValue("Bearer {$bearerToken}")); + + $token = $this->getMockBuilder(\Magento\Integration\Model\Oauth\Token::class) + ->disableOriginalConstructor() + ->setMethods( + [ + 'loadByToken', + 'getCreatedAt', + 'getId', + 'getUserType', + 'getCustomerId', + 'getAdminId', + '__wakeup', + ] + )->getMock(); + + $token->expects($this->once()) + ->method('loadByToken') + ->with($bearerToken) + ->will($this->returnSelf()); + + $token->expects($this->any()) + ->method('getId') + ->will($this->returnValue(1)); + + $token->expects($this->any()) + ->method('getUserType') + ->will($this->returnValue($tokenData['user_type'])); + + $token->expects($this->any()) + ->method('getCreatedAt') + ->willReturn($tokenData['created_at']); + + $this->tokenFactory->expects($this->once()) + ->method('create') + ->will($this->returnValue($token)); + + $this->oauthHelperMock->expects($this->any()) + ->method('getAdminTokenLifetime') + ->willReturn($tokenTtl); + + $this->oauthHelperMock->expects($this->any()) + ->method('getCustomerTokenLifetime') + ->willReturn($tokenTtl); + + switch ($tokenData['user_type']) { + case UserContextInterface::USER_TYPE_INTEGRATION: + $integration = $this->getMockBuilder(\Magento\Integration\Model\Integration::class) + ->disableOriginalConstructor() + ->setMethods(['getId', '__wakeup']) + ->getMock(); + $integration->expects($this->any()) + ->method('getId') + ->will($this->returnValue($tokenData['user_id'])); + + $this->integrationService->expects($this->any()) + ->method('findByConsumerId') + ->will($this->returnValue($integration)); + break; + case UserContextInterface::USER_TYPE_ADMIN: + $token->expects($this->any()) + ->method('getAdminId') + ->will($this->returnValue($tokenData['user_id'])); + break; + case UserContextInterface::USER_TYPE_CUSTOMER: + $token->expects($this->any()) + ->method('getCustomerId') + ->will($this->returnValue($tokenData['user_id'])); + break; + } + + $this->assertEquals($expectedUserType, $this->tokenUserContext->getUserType()); + $this->assertEquals($expectedUserId, $this->tokenUserContext->getUserId()); + + /* check again to make sure that the above method loadByToken in only called once */ + $this->assertEquals($expectedUserType, $this->tokenUserContext->getUserType()); + $this->assertEquals($expectedUserId, $this->tokenUserContext->getUserId()); + } + + /** + * Data provider for expired token test + * @return array + */ + public function getExpiredTestTokenData() + { + $time = time(); + return [ + 'token_expired_admin' => [ + 'tokenData' => [ + 'user_type' => UserContextInterface::USER_TYPE_ADMIN, + 'user_id' => 1234, + 'created_at' => date('Y-m-d H:i:s', $time - 3600 - 400), + ], + 'tokenTtl' => 1, + 'currentTime' => $time, + 'expedtedUserType' => null, + 'expectedUserId' => null, + ], + 'token_vigent_admin' => [ + 'tokenData' => [ + 'user_type' => UserContextInterface::USER_TYPE_ADMIN, + 'user_id' => 1234, + 'created_at' => date('Y-m-d H:i:s', $time - 400), + ], + 'tokenTtl' => 1, + 'currentTime' => $time, + 'expedtedUserType' => UserContextInterface::USER_TYPE_ADMIN, + 'expectedUserId' => 1234, + ], + 'token_expired_customer' => [ + 'tokenData' => [ + 'user_type' => UserContextInterface::USER_TYPE_CUSTOMER, + 'user_id' => 1234, + 'created_at' => date('Y-m-d H:i:s', $time - 3600 - 400), + ], + 'tokenTtl' => 1, + 'currentTime' => $time, + 'expedtedUserType' => null, + 'expectedUserId' => null, + ], + 'token_vigent_customer' => [ + 'tokenData' => [ + 'user_type' => UserContextInterface::USER_TYPE_CUSTOMER, + 'user_id' => 1234, + 'created_at' => date('Y-m-d H:i:s', $time - 400), + ], + 'tokenTtl' => 1, + 'currentTime' => $time, + 'expedtedUserType' => UserContextInterface::USER_TYPE_CUSTOMER, + 'expectedUserId' => 1234, + ], + 'token_expired_integration' => [ + 'tokenData' => [ + 'user_type' => UserContextInterface::USER_TYPE_INTEGRATION, + 'user_id' => 1234, + 'created_at' => date('Y-m-d H:i:s', $time - 3600 - 400), + ], + 'tokenTtl' => 1, + 'currentTime' => $time, + 'expectedUserType' => UserContextInterface::USER_TYPE_INTEGRATION, + 'expectedUserId' => 1234, + ], + 'token_vigent_integration' => [ + 'tokenData' => [ + 'user_type' => UserContextInterface::USER_TYPE_INTEGRATION, + 'user_id' => 1234, + 'created_at' => date('Y-m-d H:i:s', $time - 400), + ], + 'tokenTtl' => 1, + 'currentTime' => $time, + 'expedtedUserType' => UserContextInterface::USER_TYPE_INTEGRATION, + 'expectedUserId' => 1234, + ], + 'token_expired_guest' => [ + 'tokenData' => [ + 'user_type' => UserContextInterface::USER_TYPE_GUEST, + 'user_id' => 1234, + 'created_at' => date('Y-m-d H:i:s', $time - 3600 - 400), + ], + 'tokenTtl' => 1, + 'currentTime' => $time, + 'expedtedUserType' => null, + 'expectedUserId' => null, + ], + 'token_vigent_guest' => [ + 'tokenData' => [ + 'user_type' => UserContextInterface::USER_TYPE_GUEST, + 'user_id' => 1234, + 'created_at' => date('Y-m-d H:i:s', $time - 400), + ], + 'tokenTtl' => 1, + 'currentTime' => $time, + 'expedtedUserType' => null, + 'expectedUserId' => null, + ], + ]; + } } From 813ff0a2994f298952846a732ddd986747cdba3a Mon Sep 17 00:00:00 2001 From: Dmytro Voskoboinikov Date: Tue, 21 Nov 2017 12:35:48 +0200 Subject: [PATCH 6/9] MAGETWO-77840: [2.2.x] - Special/lowest price in child of a Configurable Product causes the entire product to show that price --- .../view/base/templates/product/price/final_price.phtml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/ConfigurableProduct/view/base/templates/product/price/final_price.phtml b/app/code/Magento/ConfigurableProduct/view/base/templates/product/price/final_price.phtml index df05fb22104a5..97238d0813b09 100644 --- a/app/code/Magento/ConfigurableProduct/view/base/templates/product/price/final_price.phtml +++ b/app/code/Magento/ConfigurableProduct/view/base/templates/product/price/final_price.phtml @@ -30,7 +30,7 @@ $schema = ($block->getZone() == 'item_view') ? true : false; 'schema' => $schema ]; - /* @escapeNotVerified */ echo $block->renderAmount($finalPriceModel->getAmount(), $arguments); + /* @noEscape */ echo $block->renderAmount($finalPriceModel->getAmount(), $arguments); ?> From bc727b159a4593e693e89dac7360f7da5c50962a Mon Sep 17 00:00:00 2001 From: Dmytro Vilchynskyi Date: Tue, 21 Nov 2017 13:26:34 +0200 Subject: [PATCH 7/9] MAGETWO-70725: Admin token does not expire after 'Admin Token Lifetime (hours)' - fix static tests --- .../Unit/Model/Authorization/TokenUserContextTest.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/app/code/Magento/Webapi/Test/Unit/Model/Authorization/TokenUserContextTest.php b/app/code/Magento/Webapi/Test/Unit/Model/Authorization/TokenUserContextTest.php index f7ab848083dcc..3fca984725757 100644 --- a/app/code/Magento/Webapi/Test/Unit/Model/Authorization/TokenUserContextTest.php +++ b/app/code/Magento/Webapi/Test/Unit/Model/Authorization/TokenUserContextTest.php @@ -100,11 +100,12 @@ protected function setUp() $this->dateTimeMock->expects($this->any()) ->method('strToTime') - ->will($this->returnCallback( - function ($str) { - return strtotime($str); - } - ) + ->will( + $this->returnCallback( + function ($str) { + return strtotime($str); + } + ) ); $this->tokenUserContext = $this->objectManager->getObject( From 6fc27357f240786b1a3bb0319fb8438bc6a18f40 Mon Sep 17 00:00:00 2001 From: Dmytro Vilchynskyi Date: Tue, 21 Nov 2017 13:58:19 +0200 Subject: [PATCH 8/9] MAGETWO-70725: Admin token does not expire after 'Admin Token Lifetime (hours)' - fix static tests --- .../Model/Authorization/TokenUserContext.php | 33 +++++---- .../Authorization/TokenUserContextTest.php | 73 +++++++++++-------- 2 files changed, 61 insertions(+), 45 deletions(-) diff --git a/app/code/Magento/Webapi/Model/Authorization/TokenUserContext.php b/app/code/Magento/Webapi/Model/Authorization/TokenUserContext.php index afa5889fe55e8..110191360acfd 100644 --- a/app/code/Magento/Webapi/Model/Authorization/TokenUserContext.php +++ b/app/code/Magento/Webapi/Model/Authorization/TokenUserContext.php @@ -8,11 +8,13 @@ use Magento\Authorization\Model\UserContextInterface; use Magento\Framework\App\ObjectManager; -use Magento\Framework\Oauth\Exception; use Magento\Integration\Model\Oauth\Token; use Magento\Integration\Model\Oauth\TokenFactory; use Magento\Integration\Api\IntegrationServiceInterface; use Magento\Framework\Webapi\Request; +use Magento\Framework\Stdlib\DateTime\DateTime as Date; +use Magento\Framework\Stdlib\DateTime; +use Magento\Integration\Helper\Oauth\Data as OauthHelper; /** * A user context determined by tokens in a HTTP request Authorization header. @@ -50,46 +52,50 @@ class TokenUserContext implements UserContextInterface protected $integrationService; /** - * @var \Magento\Framework\Stdlib\DateTime + * @var DateTime */ private $dateTime; /** - * @var \Magento\Framework\Stdlib\DateTime\DateTime + * @var Date */ private $date; /** - * @var \Magento\Integration\Helper\Oauth\Data + * @var OauthHelper */ private $oauthHelper; /** * Initialize dependencies. * + * TokenUserContext constructor. * @param Request $request * @param TokenFactory $tokenFactory * @param IntegrationServiceInterface $integrationService + * @param DateTime|null $dateTime + * @param Date|null $date + * @param OauthHelper|null $oauthHelper */ public function __construct( Request $request, TokenFactory $tokenFactory, IntegrationServiceInterface $integrationService, - \Magento\Framework\Stdlib\DateTime $dateTime = null, - \Magento\Framework\Stdlib\DateTime\DateTime $date = null, - \Magento\Integration\Helper\Oauth\Data $oauthHelper = null + DateTime $dateTime = null, + Date $date = null, + OauthHelper $oauthHelper = null ) { $this->request = $request; $this->tokenFactory = $tokenFactory; $this->integrationService = $integrationService; $this->dateTime = $dateTime ?: ObjectManager::getInstance()->get( - \Magento\Framework\Stdlib\DateTime::class + DateTime::class ); $this->date = $date ?: ObjectManager::getInstance()->get( - \Magento\Framework\Stdlib\DateTime\DateTime::class + Date::class ); $this->oauthHelper = $oauthHelper ?: ObjectManager::getInstance()->get( - \Magento\Integration\Helper\Oauth\Data::class + OauthHelper::class ); } @@ -115,14 +121,13 @@ public function getUserType() * Check if token is expired. * * @param Token $token - * * @return bool */ - private function isTokenExpired(Token $token) + private function isTokenExpired(Token $token): bool { - if ($token->getUserType() == \Magento\Authorization\Model\UserContextInterface::USER_TYPE_ADMIN) { + if ($token->getUserType() == UserContextInterface::USER_TYPE_ADMIN) { $tokenTtl = $this->oauthHelper->getAdminTokenLifetime(); - } elseif ($token->getUserType() == \Magento\Authorization\Model\UserContextInterface::USER_TYPE_CUSTOMER) { + } elseif ($token->getUserType() == UserContextInterface::USER_TYPE_CUSTOMER) { $tokenTtl = $this->oauthHelper->getCustomerTokenLifetime(); } else { // other user-type tokens are considered always valid diff --git a/app/code/Magento/Webapi/Test/Unit/Model/Authorization/TokenUserContextTest.php b/app/code/Magento/Webapi/Test/Unit/Model/Authorization/TokenUserContextTest.php index 3fca984725757..c7272e3e5a2b2 100644 --- a/app/code/Magento/Webapi/Test/Unit/Model/Authorization/TokenUserContextTest.php +++ b/app/code/Magento/Webapi/Test/Unit/Model/Authorization/TokenUserContextTest.php @@ -6,68 +6,78 @@ namespace Magento\Webapi\Test\Unit\Model\Authorization; +use Magento\Webapi\Model\Authorization\TokenUserContext; +use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; use Magento\Authorization\Model\UserContextInterface; +use Magento\Integration\Model\Oauth\TokenFactory; +use Magento\Integration\Model\Oauth\Token; +use Magento\Integration\Api\IntegrationServiceInterface; +use Magento\Framework\Webapi\Request; +use Magento\Integration\Helper\Oauth\Data as OauthHelper; +use Magento\Framework\Stdlib\DateTime\DateTime as Date; +use Magento\Framework\Stdlib\DateTime; +use Magento\Integration\Model\Integration; /** - * Tests \Magento\Webapi\Model\Authorization\TokenUserContext + * Tests TokenUserContext */ class TokenUserContextTest extends \PHPUnit\Framework\TestCase { /** - * @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager + * @var ObjectManager */ protected $objectManager; /** - * @var \Magento\Webapi\Model\Authorization\TokenUserContext + * @var TokenUserContext */ protected $tokenUserContext; /** - * @var \Magento\Integration\Model\Oauth\TokenFactory|\PHPUnit_Framework_MockObject_MockObject + * @var TokenFactory|\PHPUnit_Framework_MockObject_MockObject */ protected $tokenFactory; /** - * @var \Magento\Integration\Api\IntegrationServiceInterface|\PHPUnit_Framework_MockObject_MockObject + * @var IntegrationServiceInterface|\PHPUnit_Framework_MockObject_MockObject */ protected $integrationService; /** - * @var \Magento\Framework\Webapi\Request|\PHPUnit_Framework_MockObject_MockObject + * @var Request|\PHPUnit_Framework_MockObject_MockObject */ protected $request; /** - * @var \Magento\Integration\Helper\Oauth\Data|\PHPUnit_Framework_MockObject_MockObject + * @var OauthHelper|\PHPUnit_Framework_MockObject_MockObject */ private $oauthHelperMock; /** - * @var \Magento\Framework\Stdlib\DateTime\DateTime|\PHPUnit_Framework_MockObject_MockObject + * @var Date|\PHPUnit_Framework_MockObject_MockObject */ private $dateMock; /** - * @var \Magento\Framework\Stdlib\DateTime|\PHPUnit_Framework_MockObject_MockObject + * @var DateTime|\PHPUnit_Framework_MockObject_MockObject */ private $dateTimeMock; protected function setUp() { - $this->objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this); + $this->objectManager = new ObjectManager($this); - $this->request = $this->getMockBuilder(\Magento\Framework\Webapi\Request::class) + $this->request = $this->getMockBuilder(Request::class) ->disableOriginalConstructor() ->setMethods(['getHeader']) ->getMock(); - $this->tokenFactory = $this->getMockBuilder(\Magento\Integration\Model\Oauth\TokenFactory::class) + $this->tokenFactory = $this->getMockBuilder(TokenFactory::class) ->disableOriginalConstructor() ->setMethods(['create']) ->getMock(); - $this->integrationService = $this->getMockBuilder(\Magento\Integration\Api\IntegrationServiceInterface::class) + $this->integrationService = $this->getMockBuilder(IntegrationServiceInterface::class) ->disableOriginalConstructor() ->setMethods( [ @@ -83,17 +93,17 @@ protected function setUp() ) ->getMock(); - $this->oauthHelperMock = $this->getMockBuilder(\Magento\Integration\Helper\Oauth\Data::class) + $this->oauthHelperMock = $this->getMockBuilder(OauthHelper::class) ->disableOriginalConstructor() ->setMethods(['getAdminTokenLifetime', 'getCustomerTokenLifetime']) ->getMock(); - $this->dateMock = $this->getMockBuilder(\Magento\Framework\Stdlib\DateTime\DateTime::class) + $this->dateMock = $this->getMockBuilder(Date::class) ->disableOriginalConstructor() ->setMethods(['gmtTimestamp']) ->getMock(); - $this->dateTimeMock = $this->getMockBuilder(\Magento\Framework\Stdlib\DateTime::class) + $this->dateTimeMock = $this->getMockBuilder(DateTime::class) ->disableOriginalConstructor() ->setMethods(['strToTime']) ->getMock(); @@ -109,7 +119,7 @@ function ($str) { ); $this->tokenUserContext = $this->objectManager->getObject( - \Magento\Webapi\Model\Authorization\TokenUserContext::class, + TokenUserContext::class, [ 'request' => $this->request, 'tokenFactory' => $this->tokenFactory, @@ -160,7 +170,7 @@ public function testNoTokenInDatabase() ->with('Authorization') ->will($this->returnValue("Bearer {$bearerToken}")); - $token = $this->getMockBuilder(\Magento\Integration\Model\Oauth\Token::class) + $token = $this->getMockBuilder(Token::class) ->disableOriginalConstructor() ->setMethods(['loadByToken', 'getId', '__wakeup']) ->getMock(); @@ -188,7 +198,7 @@ public function testRevokedToken() ->with('Authorization') ->will($this->returnValue("Bearer {$bearerToken}")); - $token = $this->getMockBuilder(\Magento\Integration\Model\Oauth\Token::class) + $token = $this->getMockBuilder(Token::class) ->disableOriginalConstructor() ->setMethods(['loadByToken', 'getId', 'getRevoked', '__wakeup']) ->getMock(); @@ -222,7 +232,7 @@ public function testValidToken($userType, $userId, $expectedUserType, $expectedU ->with('Authorization') ->will($this->returnValue("Bearer {$bearerToken}")); - $token = $this->getMockBuilder(\Magento\Integration\Model\Oauth\Token::class) + $token = $this->getMockBuilder(Token::class) ->disableOriginalConstructor() ->setMethods( [ @@ -255,7 +265,7 @@ public function testValidToken($userType, $userId, $expectedUserType, $expectedU switch ($userType) { case UserContextInterface::USER_TYPE_INTEGRATION: - $integration = $this->getMockBuilder(\Magento\Integration\Model\Integration::class) + $integration = $this->getMockBuilder(Integration::class) ->disableOriginalConstructor() ->setMethods(['getId', '__wakeup']) ->getMock(); @@ -333,7 +343,7 @@ public function testExpiredToken($tokenData, $tokenTtl, $currentTime, $expectedU ->with('Authorization') ->will($this->returnValue("Bearer {$bearerToken}")); - $token = $this->getMockBuilder(\Magento\Integration\Model\Oauth\Token::class) + $token = $this->getMockBuilder(Token::class) ->disableOriginalConstructor() ->setMethods( [ @@ -378,7 +388,7 @@ public function testExpiredToken($tokenData, $tokenTtl, $currentTime, $expectedU switch ($tokenData['user_type']) { case UserContextInterface::USER_TYPE_INTEGRATION: - $integration = $this->getMockBuilder(\Magento\Integration\Model\Integration::class) + $integration = $this->getMockBuilder(Integration::class) ->disableOriginalConstructor() ->setMethods(['getId', '__wakeup']) ->getMock(); @@ -411,7 +421,8 @@ public function testExpiredToken($tokenData, $tokenTtl, $currentTime, $expectedU } /** - * Data provider for expired token test + * Data provider for expired token test. + * * @return array */ public function getExpiredTestTokenData() @@ -426,7 +437,7 @@ public function getExpiredTestTokenData() ], 'tokenTtl' => 1, 'currentTime' => $time, - 'expedtedUserType' => null, + 'expectedUserType' => null, 'expectedUserId' => null, ], 'token_vigent_admin' => [ @@ -437,7 +448,7 @@ public function getExpiredTestTokenData() ], 'tokenTtl' => 1, 'currentTime' => $time, - 'expedtedUserType' => UserContextInterface::USER_TYPE_ADMIN, + 'expectedUserType' => UserContextInterface::USER_TYPE_ADMIN, 'expectedUserId' => 1234, ], 'token_expired_customer' => [ @@ -448,7 +459,7 @@ public function getExpiredTestTokenData() ], 'tokenTtl' => 1, 'currentTime' => $time, - 'expedtedUserType' => null, + 'expectedUserType' => null, 'expectedUserId' => null, ], 'token_vigent_customer' => [ @@ -459,7 +470,7 @@ public function getExpiredTestTokenData() ], 'tokenTtl' => 1, 'currentTime' => $time, - 'expedtedUserType' => UserContextInterface::USER_TYPE_CUSTOMER, + 'expectedUserType' => UserContextInterface::USER_TYPE_CUSTOMER, 'expectedUserId' => 1234, ], 'token_expired_integration' => [ @@ -481,7 +492,7 @@ public function getExpiredTestTokenData() ], 'tokenTtl' => 1, 'currentTime' => $time, - 'expedtedUserType' => UserContextInterface::USER_TYPE_INTEGRATION, + 'expectedUserType' => UserContextInterface::USER_TYPE_INTEGRATION, 'expectedUserId' => 1234, ], 'token_expired_guest' => [ @@ -492,7 +503,7 @@ public function getExpiredTestTokenData() ], 'tokenTtl' => 1, 'currentTime' => $time, - 'expedtedUserType' => null, + 'expectedUserType' => null, 'expectedUserId' => null, ], 'token_vigent_guest' => [ @@ -503,7 +514,7 @@ public function getExpiredTestTokenData() ], 'tokenTtl' => 1, 'currentTime' => $time, - 'expedtedUserType' => null, + 'expectedUserType' => null, 'expectedUserId' => null, ], ]; From 5e6f1cb064ab1b0181d4cbe1f09addda25a5f3b6 Mon Sep 17 00:00:00 2001 From: Dmytro Vilchynskyi Date: Thu, 23 Nov 2017 18:07:47 +0200 Subject: [PATCH 9/9] MAGETWO-77840: [2.2.x] - Special/lowest price in child of a Configurable Product causes the entire product to show that price - fixing FAT --- .../AssertCatalogPriceRuleAppliedProductPage.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/dev/tests/functional/tests/app/Magento/CatalogRule/Test/Constraint/AssertCatalogPriceRuleAppliedProductPage.php b/dev/tests/functional/tests/app/Magento/CatalogRule/Test/Constraint/AssertCatalogPriceRuleAppliedProductPage.php index ce0f7a73006af..b2cc7e1297ec2 100644 --- a/dev/tests/functional/tests/app/Magento/CatalogRule/Test/Constraint/AssertCatalogPriceRuleAppliedProductPage.php +++ b/dev/tests/functional/tests/app/Magento/CatalogRule/Test/Constraint/AssertCatalogPriceRuleAppliedProductPage.php @@ -11,6 +11,8 @@ use Magento\Mtf\Constraint\AbstractConstraint; use Magento\Catalog\Test\Page\Product\CatalogProductView; use Magento\Catalog\Test\Page\Category\CatalogCategoryView; +use Magento\Customer\Test\TestStep\LoginCustomerOnFrontendStep; +use Magento\Customer\Test\TestStep\LogoutCustomerOnFrontendStep; /** * Assert that Catalog Price Rule is applied on Product page. @@ -38,11 +40,11 @@ public function processAssert( ) { if ($customer !== null) { $this->objectManager->create( - \Magento\Customer\Test\TestStep\LoginCustomerOnFrontendStep::class, + LoginCustomerOnFrontendStep::class, ['customer' => $customer] )->run(); } else { - $this->objectManager->create(\Magento\Customer\Test\TestStep\LogoutCustomerOnFrontendStep::class)->run(); + $this->objectManager->create(LogoutCustomerOnFrontendStep::class)->run(); } $cmsIndexPage->open(); @@ -52,7 +54,7 @@ public function processAssert( $catalogCategoryViewPage->getListProductBlock()->getProductItem($product)->open(); $catalogProductViewPage->getViewBlock()->waitLoader(); - $productPriceBlock = $catalogProductViewPage->getViewBlock()->getPriceBlock(); + $productPriceBlock = $catalogProductViewPage->getViewBlock()->getPriceBlock($product); $actualPrice['special'] = $productPriceBlock->getSpecialPrice(); if ($productPrice[$key]['regular'] !== 'No') { $actualPrice['regular'] = $productPriceBlock->getOldPrice();