Product duplication of imported products "fails" due to url rewrites

fwolfst commented 6 years ago

Preconditions

Magento 2.4-develop

Steps to reproduce

Save and duplicate product A, creating a duplicate product B
Edit product B, change the URL Key under "Search Engine Optimization" while leaving "Create Permanent Redirect for old URL" checked and save the product
Open product A
Save and duplicate product A again in an attempt to create product C

Expected result

A workflow that lets me duplicate the product by e.g. changing the URL Key.

Actual result

Warning The value specified in the URL Key field would generate a URL that already exists.
Products don't have url key product-a-1 but product A cannot be duplicated again

Original descirption

Preconditions

Environment
- php7.0 7.0.25-1+ubuntu16.04.1+deb.sury.org+1
- mysql-server-5.7 5.7.20-0ubuntu0.16.04.1
- magento 2.2.1 with (some) data imported from m1.6.2 (with earlier m2 version)
Some categories of which some are imported from m1.6.2 (with earlier m2 version)
URLRewrites for these products and categories

Steps to reproduce

Open an old product, click on "Save and duplicate"

Expected result

A workflow that lets me duplicate the product by e.g. changing the URL Key.

Actual result

I see a warning

The value specified in the URL Key field would generate a URL that already exists.

To resolve this conflict, you can either change the value of the URL Key field (located in the Search Engine Optimization section) to a unique value, or change the Request Path fields in all locations listed below:

- PRODURLKEY.html
- CATURLKEY/SUBCATURLKEY/PRODURLKEY.html
[... modified for readability ...]
You saved the product.

The product is then not duplicated.

I assume the error/warning message changed in Magento 2.2.1 (might have been "URL key for specified store already exists." before). There is a variety of other issues, none of which addresses the implications of this usecase (extremely inconvient product duplication workflow).

Also, the solution proposed by the warning (changing the URL Key and hitting "Save and duplicate") does not work.

I think it might have something to do with the attributes (url_key, url_path) being assigned to different stores (store_id).

I understand that developers favor NOT to use magento1-stlye and just append something to the url_key to prevent duplicates to avoid what some user called a SEO-mess. However, it should be easy to duplicate a given product (and e.g. manually assign a new url-key).

I will update this issue with a list of related issues as they cross my way.

magento-engcom-team commented 6 years ago

@fwolfst, thank you for your report. We've created internal ticket(s) MAGETWO-83895 to track progress on the issue.

swissdeveloper commented 6 years ago

@magento-engcom-team Any updates or fixes on this issue?

Kreativsoehne commented 6 years ago

Same problem here. Got a very angry customer and all can tell them is that I have no Idea what's causing the problem.

PHP 7.1.13-1+ubuntu16.04.1+deb.sury.org+1 MariaDB 10.0 SQL Magento 2.2.2, fresh install with products and categories imported by an external software. But the issue also appears on manually created products :(

allisonlawrencels commented 6 years ago

I am also in need of a solution for this.

PHP 7.0.22-0ubuntu0.16.04.1 MySQL 5.7.20-0ubuntu0.16.04.1-log Magento 2.2.1

allisonlawrencels commented 6 years ago

It looks like this has something to do with the combo of the way that \Magento\CatalogUrlRewrite\Model\ProductUrlPathGenerator::getUrlPath() first looks for a url_path, then falls back to a url_key:

public function getUrlPath($product, $category = null)
    {
        $path = $product->getData('url_path');
        if ($path === null) {
            $path = $product->getUrlKey()
                ? $this->prepareProductUrlKey($product)
                : $this->prepareProductDefaultUrlKey($product);
        }
        return $category === null
            ? $path
            : $this->categoryUrlPathGenerator->getUrlPath($category) . '/' . $path;
    }

And the product copier function (\Magento\Catalog\Model\Product\Copier::copy()) only updates the url_key on duplication, leaving the url_path in tact from the old product:

public function copy(\Magento\Catalog\Model\Product $product)
    {
        .
        .
        .
        $isDuplicateSaved = false;
        do {
            $urlKey = $duplicate->getUrlKey();
            $urlKey = preg_match('/(.*)-(\d+)$/', $urlKey, $matches)
                ? $matches[1] . '-' . ($matches[2] + 1)
                : $urlKey . '-1';
            $duplicate->setUrlKey($urlKey);
            try {
                $duplicate->save();
                $isDuplicateSaved = true;
            } catch (\Magento\Framework\Exception\AlreadyExistsException $e) {
            }
        } while (!$isDuplicateSaved);
        .
        .
        .
        return $duplicate;
    }

I was able to force the url_path to reset on duplication, however this isn't a complete fix because I'm unable to update the URL key on the duplicated product after that, and the url_path in the database for the new product is still the original product's url_path.

Just a start, hope this helps the Magento team diagnose this issue.

Kreativsoehne commented 6 years ago

I found out whats’s the problem. The error is caused by a non-existing url_key value. This happend because of the product import which lead to sort of half way stored product entries. Some values were missing. Saving a product generates these missing values. But somerhing different happens when duplicating a product. The url_key seems not to be generated, which leads to the error. This seems also to lead to broken database entries.

Long story short: Simply save a product BEFORE you want to duplicate it. This triggers Magento to generate all the missing values including the url_key. I have tried some other things during debugging. So I had to manually clean up the database. I have developed a script which automatically saves every product and triggers these value-generating processes. And that‘s what made it working again.

Please let me know if you are interested in this script. I‘ll post it here if I am allowed.

allisonlawrencels commented 6 years ago

Thanks @Kreativsoehne - I was not able to find the same success as you with saving products but it looks like my problem is actually stemming from migrating too much data. The url_path attribute does not seem to get filled out when creating new products, so I tried deleting all values of url_path from products, (I didn't see any place in core that actually updated it ever) and my problems seem to be solved now.

Magento team, can you please verify this for others trying to migrate from M1 to M2? Is url_path on products required anymore?

xpedicion commented 6 years ago

Yeah, this one is a pain. For the time being, I "fixed" it by extending the ProductUrlPathGenerator class in my own module and forcing the getUrlPath function to always just use url_key and ignore url_path altogether until this gets officially fixed.

luckyduck commented 6 years ago

@xpedicion Can you post your solution? Would be awesome. Because with my approach, the problem persists.

xpedicion commented 6 years ago

@luckyduck Here's my solution: app/code/<VENDOR>/<MODULE>/CatalogUrlRewrite/Model/ProductUrlPathGenerator.php:

namespace <VENDOR>\<MODULE>\CatalogUrlRewrite\Model;

class ProductUrlPathGenerator extends \Magento\CatalogUrlRewrite\Model\ProductUrlPathGenerator
{
    /**
     * Ignore attribute url_path. Always use url_key instead.
     *
     * @inheritdoc
     */
    public function getUrlPath($product, $category = null)
    {
        $path = $product->getUrlKey()
            ? $this->prepareProductUrlKey($product)
            : $this->prepareProductDefaultUrlKey($product);
        return $category === null
            ? $path
            : $this->categoryUrlPathGenerator->getUrlPath($category) . '/' . $path;
    }
}

Tie this in with app/code/<VENDOR>/<MODULE>/etc/di.xml:

<?xml version="1.0"?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd">
    <preference for="Magento\CatalogUrlRewrite\Model\ProductUrlPathGenerator"
                type="<VENDOR>\<MODULE>\CatalogUrlRewrite\Model\ProductUrlPathGenerator" />
</config>

Hope this helps.

rvm-pmis commented 6 years ago

I can reproduce this as follows:

Save and duplicate product A, creating a duplicate product B
Edit product B, change the URL Key under "Search Engine Optimization" while leaving "Create Permanent Redirect for old URL" checked and save the product
Save and duplicate product A again in an attempt to create product C

jacobchat commented 6 years ago

A quick work around until this is fixed...

For product you want to duplicate...

Before clicking 'Save and Duplicate'.. Uncheck all Websites from 'Products in Websites' tab.. then Click 'Save and Duplicate' Make required changes to duplicated product (making sure to change the URL key under 'Search Engine Optimization') Don't forget to add products to Websites under 'Products in Websites' tab for old and new products as needed

MathRat commented 6 years ago

Same problem here. is there a solution coming or can you recommend one of the users solution?

intelsteel commented 6 years ago

@jacobchat

Your Solution is working. Is there any permanent solution ?

xpedicion commented 6 years ago

@intelsteel Well, mine is. :)

chattertech commented 6 years ago

@expedicion

Is it just a matter of creating those files in the folders you mentioned?

I am not having any module show after adding the files and folders.

Am i missing something?

Thanks Jacob

xpedicion commented 6 years ago

@chattertech Yes, indeed. You will need more files to have a complete module. I'll attach a full one here. You need to extract the zip to app/code or upload the content there, respectively. It includes the needed subfolders. You should delete the files and folders that you created, though. UrlPathFix.zip

coutes commented 6 years ago

1 year later and still no official fix from Magento team?

Erchiragdodia commented 5 years ago

I'm facing the same problem. @xpedicion your solution working but I afraid your solution breaking any other functionality.

StarDev-it commented 5 years ago

Hi all, I'm found the some problem. In do {} while() block, the catched exception was wrong, and there is a problem in resource data duplication. I'm create a simple patch in order to fix the problem:

--- repository/vendor/magento/module-catalog/Model/Product/Copier.php   (date 1557317256000)
+++ repository/vendor/magento/module-catalog/Model/Product/Copier.php   (date 1557317256000)
@@ -26,6 +26,11 @@
      */
     protected $productFactory;

+    /**
+     * @var \Magento\Store\Model\StoreManagerInterface
+     */
+    protected $storeManager;
+
     /**
      * @var \Magento\Framework\EntityManager\MetadataPool
      */
@@ -37,10 +42,12 @@
      */
     public function __construct(
         CopyConstructorInterface $copyConstructor,
-        \Magento\Catalog\Model\ProductFactory $productFactory
+        \Magento\Catalog\Model\ProductFactory $productFactory,
+        \Magento\Store\Model\StoreManagerInterface $storeManager
     ) {
         $this->productFactory = $productFactory;
         $this->copyConstructor = $copyConstructor;
+        $this->storeManager = $storeManager;
     }

     /**
@@ -79,17 +86,30 @@
                 ? $matches[1] . '-' . ($matches[2] + 1)
                 : $urlKey . '-1';
             $duplicate->setUrlKey($urlKey);
+            $duplicate->setUrlPath($urlKey);
             try {
                 $duplicate->save();
                 $isDuplicateSaved = true;
+            } catch (\Magento\UrlRewrite\Model\Exception\UrlAlreadyExistsException $e) {
             } catch (\Magento\Framework\Exception\AlreadyExistsException $e) {
             }
         } while (!$isDuplicateSaved);
+
         $this->getOptionRepository()->duplicate($product, $duplicate);
         $product->getResource()->duplicate(
             $product->getData($metadata->getLinkField()),
             $duplicate->getData($metadata->getLinkField())
         );
+
+        // Reset wrong duplicated url_path like url_key value
+        foreach($this->storeManager->getStores() as $store) {
+            $duplicate->addAttributeUpdate(
+                'url_path',
+                $urlKey,
+                $store->getId()
+            );
+        }
+
         return $duplicate;
     }

djamps commented 5 years ago

There are more issues with duplicate function than just url/path. For example, the following attributes are copied:

image_label
small_image_label
thumbnail_label

With their values = the name of the original product. You cannot see or edit these attributes in admin.

Now, when you search for the original product by name, the newly created products will show along side the original product in your admin product grid. because when filtering by name in admin, it scans all attributes, including these new hidden ones.

SELECT e.*, at_qty.qty FROM catalog_product_entity AS e
LEFT JOIN cataloginventory_stock_item AS at_qty ON (at_qty.product_id=e.entity_id) AND (at_qty.stock_id=1)
WHERE (EXISTS (SELECT 1 FROM catalog_product_entity_varchar
WHERE ((catalog_product_entity_varchar.entity_id = e.entity_id)
AND (catalog_product_entity_varchar.value LIKE '%original product name%')))) ORDER BY e.entity_id ASC
LIMIT 20

So it seems this duplicate function creates a bigger mess than we expected.

cralls commented 5 years ago

The problem for me was the url key was not being used as the new url path on a duplicated product. I want Magento to use the url key that I specify with .html on the end if I change my url key on a duplicated product so to fix this issue you can override Magento\CatalogUrlRewrite\Model\Product\CanonicalUrlRewriteGenerator and update the generate function as follows:

public function generate($storeId, Product $product)
    {
        return [
            $this->urlRewriteFactory->create()
                ->setEntityType(ProductUrlRewriteGenerator::ENTITY_TYPE)
                ->setEntityId($product->getId())
                ->setRequestPath($product->getUrlKey().".html")
                ->setTargetPath($this->productUrlPathGenerator->getCanonicalUrlPath($product))
                ->setStoreId($storeId)
        ];
   }

Purushottam-saini commented 4 years ago

I can reproduce this as follows:

Save and duplicate product A, creating a duplicate product B

Edit product B, change the URL Key under "Search Engine Optimization" while leaving "Create Permanent Redirect for old URL" checked and save the product

Save and duplicate product A again in an attempt to create product C

Hello , It creates a duplicate product in admin. but when i am check "main website" showing same issue.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 14 days if no further activity occurs. Thank you for your contributions.

m2-assistant[bot] commented 3 years ago

Hi @engcom-Delta. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

[ ] 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).
Details
If the issue has a valid description, the label Issue: Format is valid will be added to the issue automatically. Please, edit issue description if needed, until label Issue: Format is valid appears.
[ ] 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue. If the report is valid, add Issue: Clear Description label to the issue by yourself.
[ ] 3. Add Component: XXXXX label(s) to the ticket, indicating the components it may be related to.
[ ] 4. Verify that the issue is reproducible on 2.4-develop branch
Details
- Add the comment @magento give me 2.4-develop instance to deploy test instance on Magento infrastructure.
- If the issue is reproducible on 2.4-develop branch, please, add the label Reproduced on 2.4.x.
- If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!
[ ] 5. Add label Issue: Confirmed once verification is complete.
[ ] 6. Make sure that automatic system confirms that report has been added to the backlog.

magento-engcom-team commented 3 years ago

:white_check_mark: Confirmed by @engcom-Delta Thank you for verifying the issue. Based on the provided information internal tickets MC-40347 were created

Issue Available: @engcom-Delta, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

m2-assistant[bot] commented 3 years ago

Hi @engcom-Charlie. Thank you for working on this issue. Looks like this issue is already verified and confirmed. But if you want to validate it one more time, please, go though the following instruction:

1. Add/Edit Component: XXXXX label(s) to the ticket, indicating the components it may be related to.
1. Verify that the issue is reproducible on 2.4-develop branch
  Details
  - Add the comment @magento give me 2.4-develop instance to deploy test instance on Magento infrastructure.
  - If the issue is reproducible on 2.4-develop branch, please, add the label Reproduced on 2.4.x.
  - If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!
1. If the issue is not relevant or is not reproducible any more, feel free to close it.

Join Magento Community Engineering Slack and ask your questions in #github channel.

magefast commented 2 years ago

2k22 but issue still exist in Magento 2.4 :(

My solution for fix bug.

etc/di.xml

<preference for="Magento\CatalogUrlRewrite\Model\ProductUrlPathGenerator"
                type="Jg\Fix\CatalogUrlRewrite\Model\ProductUrlPathGenerator" />

CatalogUrlRewrite/Model/ProductUrlPathGenerator.phpProductUrlPathGenerator.php

namespace Jg\Fix\CatalogUrlRewrite\Model;

use Magento\Catalog\Api\ProductRepositoryInterface;
use Magento\CatalogUrlRewrite\Model\CategoryUrlPathGenerator;
use Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator;
use Magento\Framework\App\Config\ScopeConfigInterface;
use Magento\Store\Model\StoreManagerInterface;
use Magento\UrlRewrite\Model\UrlFinderInterface;
use Magento\UrlRewrite\Service\V1\Data\UrlRewrite;

class ProductUrlPathGenerator extends \Magento\CatalogUrlRewrite\Model\ProductUrlPathGenerator
{
    /**
     * @var UrlFinderInterface
     */
    private $urlFinder;

    /**
     * @param StoreManagerInterface $storeManager
     * @param ScopeConfigInterface $scopeConfig
     * @param CategoryUrlPathGenerator $categoryUrlPathGenerator
     * @param ProductRepositoryInterface $productRepository
     * @param UrlFinderInterface $urlFinder
     */
    public function __construct(
        StoreManagerInterface      $storeManager,
        ScopeConfigInterface       $scopeConfig,
        CategoryUrlPathGenerator   $categoryUrlPathGenerator,
        ProductRepositoryInterface $productRepository,
        UrlFinderInterface         $urlFinder)
    {
        parent::__construct($storeManager, $scopeConfig, $categoryUrlPathGenerator, $productRepository);

        $this->urlFinder = $urlFinder;
    }

    /**
     * @inheritdoc
     */
    public function getUrlPath($product, $category = null)
    {
        if ($product->getId() && !$product->dataHasChangedFor('url_key')) {
            $searchData = [
                UrlRewrite::ENTITY_TYPE => ProductUrlRewriteGenerator::ENTITY_TYPE,
                UrlRewrite::ENTITY_ID => $product->getId()
            ];

            $urlRewrite = $this->urlFinder->findOneByData($searchData);

            if ($urlRewrite != null && $urlRewrite->getRequestPath()) {
                $path = $urlRewrite->getRequestPath();
            } else {
                $path = $this->getUrlPathAsStandard($product);
            }
        } else {
            $path = $this->getUrlPathAsStandard($product);
        }

        return $category === null
            ? $path
            : $this->categoryUrlPathGenerator->getUrlPath($category) . '/' . $path;
    }

    private function getUrlPathAsStandard($product)
    {
        $path = $product->getUrlKey()
            ? $this->prepareProductUrlKey($product)
            : $this->prepareProductDefaultUrlKey($product);

        return $path;
    }
}

engcom-Bravo commented 3 months ago

Hi @fwolfst,

Thanks for your reporting and collaboration.

We have verified the issue in Latest 2.4-develop instance and the issue is reproducible.Kindly refer the screenshots.

We are getting error The value specified in the URL Key field would generate a URL that already exists.

Hence confirming the issue.

Thanks.

github-jira-sync-bot commented 3 months ago

:white_check_mark: Jira issue https://jira.corp.adobe.com/browse/AC-12649 is successfully created for this GitHub issue.

m2-assistant[bot] commented 3 months ago

:white_check_mark: Confirmed by @engcom-Bravo. Thank you for verifying the issue.
Issue Available: @engcom-Bravo, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

magento / magento2