magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.48k stars 9.29k forks source link

Categories Path for Product URLs not working, issue with getProductUrl on Catalog Module? #2619

Closed tigerx7 closed 8 years ago

tigerx7 commented 8 years ago

It seems that getProductUrl() isn't returning a URL with the category in the path in the catalog pages. I'm able to reproduce this with a clean install using my own data and a clean install using the sample data.

Steps: 1) Loaded Magento 2.0.0 via Composer, including sample data 2) Did web based setup wizard. Sample data loaded. 3) Admin -> Stores -> Configuration -> Catalog -> Catalog -> Search Engine Optimization -> "Category URL Suffix" set to "", "Use Categories Path for Product URLs" set to "Yes" 4) Admin -> Products -> Catalog -> SKU "24-MB01" -> Search Engine Optimization -> Verified that URL Key was set to "joust-duffle-bag" 4) Flushed all caches plus static files 5) Navigated to sample data category: http://{domain}/gear/bags 6) Link on first product "Joust Duffle Bag " is to: "http://{domain}/catalog/product/view/id/1/s/joust-duffle-bag/category/4/" instead of the expected: "http://{domain}/gear/bags/joust-duffle-bag.html"

URL Rewrite is functioning since "gear/bags/joust-duffle-bag.html" is in the url_rewrite DB table and http:///gear/bags/joust-duffle-bag.html successfully pulls up the product page. It just seems there's an issue with getProductUrl() on the catalog module inserting the category correctly; unless I have a misunderstanding on something?

Gael42 commented 7 years ago

@Tristan-N there is the code I use in a custom admin route It's almost @iazel code plus the $this->_storeManager->setCurrentStore($store_id); line

    public function rewrite_regen()
    {
        $store_id = 5; // Store id - mandatory
        $pids = [13879]; // Product id list, if empty all products are processed
        //$pids = [];

        echo "regen urls for store id #".$store_id.':<br>';
        $this->_storeManager->setCurrentStore($store_id);
        $collection = $this->_productCollectionFactory->create();
        $collection->addStoreFilter($store_id)->setStoreId($store_id);

        if( !empty($pids) )
            $collection->addIdFilter($pids);

        $collection->addAttributeToSelect(['url_path', 'url_key']);
        $list = $collection->load();
        foreach($list as $product)
        {
            if($store_id === \Magento\Store\Model\Store::DEFAULT_STORE_ID)
                $product->setStoreId($store_id);

            $this->_urlPersist->deleteByData([
                \Magento\UrlRewrite\Service\V1\Data\UrlRewrite::ENTITY_ID => $product->getId(),
                \Magento\UrlRewrite\Service\V1\Data\UrlRewrite::ENTITY_TYPE => \Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator::ENTITY_TYPE,
                \Magento\UrlRewrite\Service\V1\Data\UrlRewrite::REDIRECT_TYPE => 0,
                \Magento\UrlRewrite\Service\V1\Data\UrlRewrite::STORE_ID => $store_id
            ]);
            try {
                $this->_urlPersist->replace(
                    $this->_productUrlRewriteGenerator->generate($product)
                );
            }
            catch(\Exception $e) {
                echo "error: product id #".$product->getId().': '.$e->getMessage();
            }
        }
    }

And you need to inject \Magento\Catalog\Model\ResourceModel\Product\CollectionFactory and get StoreManager from context in the constructor of your Block class :

    protected $_productCollectionFactory;
    protected $_storeManager;

    public function __construct(
        \Magento\Catalog\Model\ResourceModel\Product\CollectionFactory $productCollectionFactory
    )
    {

        $this->_productCollectionFactory = $productCollectionFactory;
        $this->_storeManager = $context->getStoreManager();

        parent::__construct($context);
    }
pravalitera commented 7 years ago

@Gael42 The $storeManager->setCurrentStore() handle the bug i speak about just over, in Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator Still a bug to me, but it handles it, thx.

I think your code will just be impossible to run on my website: i have more than 100 000 products ! If it's like in Magento 1, the collection will make the script go out of memory :(

i'll try !

And i don't understand the "delete" part: the replace already does that. (Edit: ah ok, to delete even if we moved a product)

@iazel code does not work out of the box : when i do the setup:upgrade , i have:

[Magento\Framework\Exception\LocalizedException]
Area code is already set

I had to comment the $state->setAreaCode('adminhtml'); to setup, then decomment it after... Weird.

I imported my 100 000 products with a modified Magmi i have done... A shame that there is no more url indexer...

Tjitse-E commented 7 years ago

@pravalitera @Iazel code works on my store without problems. Although i'm using a slightly older version of https://github.com/Iazel/magento2-regenurl. All the product url's are generated perfectly on each store view (i've tested with 12 store views).

Thanks for your efforts!

@pravalitera i've tested your last generation code (including the fix in Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator), on a multistore with 10 store-views in multiple languages. But the RegenerateUrls script seems to mess up the new category rewrites. The store value's just seem random. I've got category URL's like: www.mainsite.com/Englishurl/Url Francais/. Do you have the same problems?

Steps that i've taken to test the script:

  1. Remove category rewrites in DB DELETE FROM `url_rewrite` WHERE `entity_type` LIKE 'category';
  2. Regenerate catetegory rewrites n98-magerun2 gb:regenerate_urls
  3. Reindex/Flush cache.
pravalitera commented 7 years ago

@Tjitse-E It seems ok on my side... i dunno why... The fix in ProductUrlRewriteGenerator is not mandatory if you put $this->storeManager->setCurrentStore($store->getId()); in the "foreach($store)

My scripts does not handle product that are not in any categories, that's why @lazel's script was slower than mine... Stupid i am ^^

I just have a question: How many products do you have in your base ? Because making a collection of 100 000 products is a big deal i think.

And the big flaw that we have is that it does not handle 301 redirection for old url... But i think I know how to make it...

Tjitse-E commented 7 years ago

@pravalitera we only have 300 products, 60 categories and 10 store views in different languages, so no big deal there. We've corrected the category rewrites manually, so the problem for this client is resolved. But it would be nice if Magento solves this in 2.2...

crantron commented 7 years ago

Incase this helps anyone. I truncated the url_rewrite table.

thanks for this thread, helped a bunch. cheers!

MattDelac commented 7 years ago

@pravalitera , you said "And the big flaw that we have is that it does not handle 301 redirection for old url... But i think I know how to make it..."

Did you find how to do it ? This would be a nice feature !

Thank you :)

pixiemediaweb commented 7 years ago

juat wanted to stop by and say thanks @Iazel you sir are a life saver.

jarhody commented 7 years ago

I have experienced a similar issue on 2.1.5.

All migrated products work fine and creating new products works fine (URLs correctly formatted) but, if I use the duplicate feature the product url is formatted incorrectly.

No imports. Migrated from 1.9 to 2.1.2.

Correct site URLS site.com/top-category.html site.com/top-category/sub-category.html Use category paths for product URLs = NO Correct product URL (regardless of category) site.com/product-url-key.html

Incorrect product URL (made by using Save & Duplicate) site.com/catalog/product/view/id/2786/s/product-url-key/category/208/

If I manually type in: site.com/product-url-key.html ...results in a 404.

Admin settings Use Flat Catalog Category = NO Use Flat Catalog Product = YES Product URL suffix = .html Category URL suffix = .html Use category paths for product URLs = NO Create permanent redirects on key change = YES

skast96 commented 7 years ago

Is this issue still there in Version 2.1.7 , cause it is pretty annoying.

Jeordy commented 7 years ago

@skast96 That's correct. However we turned off 'Use Categories Path for Product URLs' and now the canonical url is used. That works, but the strange thing is that half of the products have correct url's and the other half don't when enabling the option again. All our products are inserted by hand, so there should be no problem creating url keys.

skast96 commented 7 years ago

@Jeordy thank you for your answer, I have been thinking about this issue quiet a while, and came to the conclusion that i just load a collection with one particular product in it and call the addRewriteUrl function on it. It's the easiest way I thought of and it works really well. The best way would be adding this method to the product itself if someone, like me, just needs one particular product url.

Jeordy commented 7 years ago

@skast96 We've solved the issue with some help. It seems we changed the dbStorage with a Github fix that isn't a fix. We reverted this 'fix' and used a module called: URL Rewrite Regeneration from ThLassche (https://marketplace.magento.com/thlassche-regeneraterewrites.html). Great module and I advise you to buy and use it. Great support too from ThLassche himself.

duckchip commented 6 years ago

@magento-engcom-team could you provide the pull request in where this is fixed so I can create a back-port for 2.1 and 2.2 branches. Or provide me the back-port if it already exists :) thx

ghost commented 6 years ago

Since it is magento and version 2.3 has been released there is still no solution for this matter? Why doesn't magento-engcom-team move this issue forward in TODO in branch version 6.8-develop so we know it will be repaired in a few years from now. Having this problem since version 2.1.4. I refuse to pay for a matter that is well known and there should be a decent basic and free (still open-source) solution for it already. Hoping for a Miracle or Magento to get there software right.

Eddcapone commented 4 years ago

@koopjesboom We are using Enterprise 2.3.5-p2 and the problem is still not fixed. If I execute php bin/magento indexer:reindex, then the url_rewrite Table does not change.

hostep commented 4 years ago

@Eddcapone: Magento 2 doesn't generate url rewrites with indexers, Magento 1 did that, but Magento 2 has no official way to regenerate url rewrites. If you want to regenerate them, you need to use an external module (be sure to test one of these thoroughly before running it on production, it could destroy your SEO ranking):

This module might also be useful to find incorrect data in the url data of your catalog btw:

Eddcapone commented 4 years ago

@hostep, Thank you for the info. Do you know why they removed the feature from M2? How are we officialy supposed to auto generate URL Rewrites now without using a 3rd party extension? I thought M2 is supposed to be better than M1.

I tried https://github.com/olegkoval/magento2-regenerate_url_rewrites but it does not generate category urls. I will try the others though. Thanks!

hostep commented 4 years ago

It's bad for SEO if you regenerate a bunch of url rewrites without then also generating 301 redirects from the old to the new url. I think that was the main thought when they rebuild the system for M2. But never saw such an official statement being made, this is only guessing here.

I do agree that it's a feature missing in M2 in certain cases, for example when you aren't in production yet and are building your store and tweaking settings/data to try out various ways of setting up url's. Then it would be handy if you could regenerate all url rewrites to make them consistent. Unfortunately you need 3rd party modules to do this.

In theory all changes should always cause correct url rewrites to be generated (including 301 redirects if you ask for it), but there are way too many bugs in M2 where this fails, from time to time such a bug gets fixed but it goes very slowly and there are still a bunch of bugs open I believe.

Good luck with your endeavours! The url rewrite problems in M2 are probably one of the most frustrating things I had to deal with so far.

ghost commented 4 years ago

@hostep, Thank you for the info. Do you know why they removed the feature from M2? How are we officialy supposed to auto generate URL Rewrites now without using a 3rd party extension? I thought M2 is supposed to be better than M1.

I tried https://github.com/olegkoval/magento2-regenerate_url_rewrites but it does not generate category urls. I will try the others though. Thanks!

I have used this Oleg Koval Extension and yes it generates the URL's for the category urls.

If not use separate commands for products and categories: $> php bin/magento ok:urlrewrites:regenerate --entity-type=category

All the commands you can find here: https://github.com/olegkoval/magento2-regenerate_url_rewrites

I do not know if there is dry run in this olegkoval extension, if not this should a default feature.

ivanaugustobd commented 3 years ago

I'm seriously starting to see M1 as better than M2 to develop for. Current Magento introduce a lot of unnecessary complexity (specially at front-end level, KnockoutJS + RequireJS absolutely sucks) and now even removes widely used features such as this.

I seriously hope that M3 will came and solve this issues, being a powerful e-commerce platform that the developers could actually developer for it instead of losing a bunch of time with such trivial things or fighting against it. 🤦‍♂️

Eddcapone commented 3 years ago

@ivanaugustobd I understand you, I was also at this point. Even extremly simple customisations can take hours and sometimes days because you have to figure everything out and the documentation is very bad and misses beginner examples. But the longer you work with it, the more you will understand its benefits.