Skip to content

Commit 6f4f511

Browse files
author
Oleksandr Dubovyk
committed
MAGETWO-98729: [2.3] [Magento Cloud] Translated Arabic URL's are returning 404's
- fixed - modified tests - reduced nesting level - eliminated cyclomatic complexity - eliminated npath complexity - eliminated long function
1 parent 645941c commit 6f4f511

File tree

7 files changed

+431
-179
lines changed

7 files changed

+431
-179
lines changed

‎app/code/Magento/CatalogImportExport/Model/Import/Product.php

Lines changed: 197 additions & 146 deletions
Large diffs are not rendered by default.

‎app/code/Magento/CatalogUrlRewrite/Model/ProductUrlPathGenerator.php

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,21 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
67
namespace Magento\CatalogUrlRewrite\Model;
78

9+
use Magento\Store\Model\Store;
10+
use Magento\Catalog\Api\ProductRepositoryInterface;
11+
use Magento\Catalog\Model\Category;
12+
use Magento\Catalog\Model\Product;
13+
use Magento\CatalogUrlRewrite\Model\CategoryUrlPathGenerator;
14+
use Magento\Framework\App\Config\ScopeConfigInterface;
15+
use Magento\Store\Model\ScopeInterface;
16+
use Magento\Store\Model\StoreManagerInterface;
17+
18+
/**
19+
* Class ProductUrlPathGenerator
20+
*/
821
class ProductUrlPathGenerator
922
{
1023
const XML_PATH_PRODUCT_URL_SUFFIX = 'catalog/seo/product_url_suffix';
@@ -17,36 +30,36 @@ class ProductUrlPathGenerator
1730
protected $productUrlSuffix = [];
1831

1932
/**
20-
* @var \Magento\Store\Model\StoreManagerInterface
33+
* @var StoreManagerInterface
2134
*/
2235
protected $storeManager;
2336

2437
/**
25-
* @var \Magento\Framework\App\Config\ScopeConfigInterface
38+
* @var ScopeConfigInterface
2639
*/
2740
protected $scopeConfig;
2841

2942
/**
30-
* @var \Magento\CatalogUrlRewrite\Model\CategoryUrlPathGenerator
43+
* @var CategoryUrlPathGenerator
3144
*/
3245
protected $categoryUrlPathGenerator;
3346

3447
/**
35-
* @var \Magento\Catalog\Api\ProductRepositoryInterface
48+
* @var ProductRepositoryInterface
3649
*/
3750
protected $productRepository;
3851

3952
/**
40-
* @param \Magento\Store\Model\StoreManagerInterface $storeManager
41-
* @param \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig
53+
* @param StoreManagerInterface $storeManager
54+
* @param ScopeConfigInterface $scopeConfig
4255
* @param CategoryUrlPathGenerator $categoryUrlPathGenerator
43-
* @param \Magento\Catalog\Api\ProductRepositoryInterface $productRepository
56+
* @param ProductRepositoryInterface $productRepository
4457
*/
4558
public function __construct(
46-
\Magento\Store\Model\StoreManagerInterface $storeManager,
47-
\Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig,
48-
\Magento\CatalogUrlRewrite\Model\CategoryUrlPathGenerator $categoryUrlPathGenerator,
49-
\Magento\Catalog\Api\ProductRepositoryInterface $productRepository
59+
StoreManagerInterface $storeManager,
60+
ScopeConfigInterface $scopeConfig,
61+
CategoryUrlPathGenerator $categoryUrlPathGenerator,
62+
ProductRepositoryInterface $productRepository
5063
) {
5164
$this->storeManager = $storeManager;
5265
$this->scopeConfig = $scopeConfig;
@@ -57,8 +70,8 @@ public function __construct(
5770
/**
5871
* Retrieve Product Url path (with category if exists)
5972
*
60-
* @param \Magento\Catalog\Model\Product $product
61-
* @param \Magento\Catalog\Model\Category $category
73+
* @param Product $product
74+
* @param Category $category
6275
*
6376
* @return string
6477
*/
@@ -78,10 +91,10 @@ public function getUrlPath($product, $category = null)
7891
/**
7992
* Prepare URL Key with stored product data (fallback for "Use Default Value" logic)
8093
*
81-
* @param \Magento\Catalog\Model\Product $product
94+
* @param Product $product
8295
* @return string
8396
*/
84-
protected function prepareProductDefaultUrlKey(\Magento\Catalog\Model\Product $product)
97+
protected function prepareProductDefaultUrlKey(Product $product)
8598
{
8699
$storedProduct = $this->productRepository->getById($product->getId());
87100
$storedUrlKey = $storedProduct->getUrlKey();
@@ -91,9 +104,9 @@ protected function prepareProductDefaultUrlKey(\Magento\Catalog\Model\Product $p
91104
/**
92105
* Retrieve Product Url path with suffix
93106
*
94-
* @param \Magento\Catalog\Model\Product $product
107+
* @param Product $product
95108
* @param int $storeId
96-
* @param \Magento\Catalog\Model\Category $category
109+
* @param Category $category
97110
* @return string
98111
*/
99112
public function getUrlPathWithSuffix($product, $storeId, $category = null)
@@ -104,8 +117,8 @@ public function getUrlPathWithSuffix($product, $storeId, $category = null)
104117
/**
105118
* Get canonical product url path
106119
*
107-
* @param \Magento\Catalog\Model\Product $product
108-
* @param \Magento\Catalog\Model\Category|null $category
120+
* @param Product $product
121+
* @param Category|null $category
109122
* @return string
110123
*/
111124
public function getCanonicalUrlPath($product, $category = null)
@@ -117,7 +130,7 @@ public function getCanonicalUrlPath($product, $category = null)
117130
/**
118131
* Generate product url key based on url_key entered by merchant or product name
119132
*
120-
* @param \Magento\Catalog\Model\Product $product
133+
* @param Product $product
121134
* @return string|null
122135
*/
123136
public function getUrlKey($product)
@@ -129,13 +142,15 @@ public function getUrlKey($product)
129142
/**
130143
* Prepare url key for product
131144
*
132-
* @param \Magento\Catalog\Model\Product $product
145+
* @param Product $product
133146
* @return string
134147
*/
135-
protected function prepareProductUrlKey(\Magento\Catalog\Model\Product $product)
148+
protected function prepareProductUrlKey(Product $product)
136149
{
137-
$urlKey = $product->getUrlKey();
138-
return $product->formatUrlKey($urlKey === '' || $urlKey === null ? $product->getName() : $urlKey);
150+
$urlKey = (string)$product->getUrlKey();
151+
$urlKey = trim(strtolower($urlKey));
152+
153+
return $urlKey ?: $product->formatUrlKey($product->getName());
139154
}
140155

141156
/**
@@ -153,7 +168,7 @@ protected function getProductUrlSuffix($storeId = null)
153168
if (!isset($this->productUrlSuffix[$storeId])) {
154169
$this->productUrlSuffix[$storeId] = $this->scopeConfig->getValue(
155170
self::XML_PATH_PRODUCT_URL_SUFFIX,
156-
\Magento\Store\Model\ScopeInterface::SCOPE_STORE,
171+
ScopeInterface::SCOPE_STORE,
157172
$storeId
158173
);
159174
}

‎app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/ProductUrlPathGeneratorTest.php

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
1212
use Magento\Store\Model\ScopeInterface;
1313

14+
/**
15+
* Class ProductUrlPathGeneratorTest
16+
*/
1417
class ProductUrlPathGeneratorTest extends \PHPUnit\Framework\TestCase
1518
{
1619
/** @var \Magento\CatalogUrlRewrite\Model\ProductUrlPathGenerator */
@@ -77,27 +80,30 @@ protected function setUp(): void
7780
public function getUrlPathDataProvider(): array
7881
{
7982
return [
80-
'path based on url key' => ['url-key', null, 'url-key'],
81-
'path based on product name 1' => ['', 'product-name', 'product-name'],
82-
'path based on product name 2' => [null, 'product-name', 'product-name'],
83-
'path based on product name 3' => [false, 'product-name', 'product-name']
83+
'path based on url key uppercase' => ['Url-Key', null, 0, 'url-key'],
84+
'path based on url key' => ['url-key', null, 0, 'url-key'],
85+
'path based on product name 1' => ['', 'product-name', 1, 'product-name'],
86+
'path based on product name 2' => [null, 'product-name', 1, 'product-name'],
87+
'path based on product name 3' => [false, 'product-name', 1, 'product-name']
8488
];
8589
}
8690

8791
/**
8892
* @dataProvider getUrlPathDataProvider
8993
* @param string|null|bool $urlKey
9094
* @param string|null|bool $productName
95+
* @param int $formatterCalled
9196
* @param string $result
9297
* @return void
9398
*/
94-
public function testGetUrlPath($urlKey, $productName, $result): void
99+
public function testGetUrlPath($urlKey, $productName, $formatterCalled, $result): void
95100
{
96101
$this->product->expects($this->once())->method('getData')->with('url_path')
97102
->will($this->returnValue(null));
98103
$this->product->expects($this->any())->method('getUrlKey')->will($this->returnValue($urlKey));
99104
$this->product->expects($this->any())->method('getName')->will($this->returnValue($productName));
100-
$this->product->expects($this->once())->method('formatUrlKey')->will($this->returnArgument(0));
105+
$this->product->expects($this->exactly($formatterCalled))
106+
->method('formatUrlKey')->will($this->returnArgument(0));
101107

102108
$this->assertEquals($result, $this->productUrlPathGenerator->getUrlPath($this->product, null));
103109
}

‎dev/tests/integration/testsuite/Magento/Catalog/_files/product_simple_with_non_latin_url_key.php

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
use Magento\Catalog\Api\ProductRepositoryInterface;
8+
use Magento\Catalog\Api\Data\ProductInterface;
9+
use Magento\Catalog\Model\Product\Attribute\Source\Status;
10+
use Magento\Catalog\Model\Product\Type;
11+
use Magento\Catalog\Model\Product\Visibility;
12+
use Magento\Framework\ObjectManagerInterface;
13+
use Magento\TestFramework\Helper\Bootstrap;
14+
15+
$stockDataConfig = [
16+
'use_config_manage_stock' => 1,
17+
'qty' => 100,
18+
'is_qty_decimal' => 0,
19+
'is_in_stock' => 1
20+
];
21+
22+
/** @var ObjectManagerInterface $objectManager */
23+
$objectManager = Bootstrap::getObjectManager();
24+
25+
/** @var ProductRepositoryInterface $productRepository */
26+
$productRepository = $objectManager->create(ProductRepositoryInterface::class);
27+
28+
/** @var ProductInterface $product */
29+
$product = $objectManager->create(ProductInterface::class);
30+
$product->setTypeId(Type::TYPE_SIMPLE)
31+
->setAttributeSetId(4)
32+
->setWebsiteIds([1])
33+
->setName('Чудовий продукт без Url Key')
34+
->setSku('ukrainian-without-url-key')
35+
->setPrice(10)
36+
->setVisibility(Visibility::VISIBILITY_BOTH)
37+
->setStatus(Status::STATUS_ENABLED)
38+
->setCategoryIds([2])
39+
->setStockData($stockDataConfig);
40+
try {
41+
$productRepository->save($product);
42+
} catch (\Exception $e) {
43+
// problems during save
44+
};
45+
46+
/** @var ProductInterface $product */
47+
$product = $objectManager->create(ProductInterface::class);
48+
$product->setTypeId(Type::TYPE_SIMPLE)
49+
->setAttributeSetId(4)
50+
->setWebsiteIds([1])
51+
->setName('Надзвичайний продукт з Url Key')
52+
->setSku('ukrainian-with-url-key')
53+
->setPrice(10)
54+
->setVisibility(Visibility::VISIBILITY_BOTH)
55+
->setStatus(Status::STATUS_ENABLED)
56+
->setCategoryIds([2])
57+
->setStockData($stockDataConfig)
58+
->setUrlKey('надзвичайний продукт на кожен день');
59+
try {
60+
$productRepository->save($product);
61+
} catch (\Exception $e) {
62+
// problems during save
63+
};

‎dev/tests/integration/testsuite/Magento/Catalog/_files/product_simple_with_non_latin_url_key_rollback.php

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
use Magento\Catalog\Api\ProductRepositoryInterface;
8+
use Magento\Framework\Exception\NoSuchEntityException;
9+
use Magento\Framework\Registry;
10+
use Magento\TestFramework\Helper\Bootstrap;
11+
12+
Bootstrap::getInstance()->getInstance()->reinitialize();
13+
14+
/** @var Registry $registry */
15+
$registry = Bootstrap::getObjectManager()->get(Registry::class);
16+
$registry->unregister('isSecureArea');
17+
$registry->register('isSecureArea', true);
18+
19+
/** @var ProductRepositoryInterface $productRepository */
20+
$productRepository = Bootstrap::getObjectManager()
21+
->get(ProductRepositoryInterface::class);
22+
23+
$productSkus = [
24+
'ukrainian-with-url-key',
25+
'ukrainian-without-url-key',
26+
];
27+
try {
28+
foreach ($productSkus as $sku) {
29+
$product = $productRepository->get($sku, false, null, true);
30+
$productRepository->delete($product);
31+
}
32+
} catch (NoSuchEntityException $e) {
33+
// nothing to delete
34+
}
35+
36+
$registry->unregister('isSecureArea');
37+
$registry->register('isSecureArea', false);

0 commit comments

Comments
 (0)