Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
tigerx7 opened this issue Dec 6, 2015 · 72 comments
Labels
bug report Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development

Comments

@tigerx7
Copy link

tigerx7 commented Dec 6, 2015

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"
  5. Flushed all caches plus static files
  6. Navigated to sample data category: http://{domain}/gear/bags
  7. 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?

@sergei-sss
Copy link

The command php bin/magento indexer:reindex changes nothing. In the table 'catalog_url_rewrite_product_category' there are no changes...

@ghost
Copy link

ghost commented Dec 15, 2015

Yes having the same issue "getProductUrl() isn't returning a URL with the category in the path in the catalog pages"

@antonkril antonkril assigned antonkril and unassigned antonkril Dec 16, 2015
@vkholoshenko
Copy link
Contributor

Hi, @tigerx7. Thanks for reporting the issue! We've created an internal ticket MAGETWO-46927 to process it.

@vkholoshenko vkholoshenko added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development MX bug report labels Dec 16, 2015
@tigerx7
Copy link
Author

tigerx7 commented Dec 16, 2015

Good to hear. I've been studying the code trying to get my head around it by following the creation of the URL path in the catalog models. I got as far as getUrl() in \Magento\Catalog\Model\Product\Url. I'm able to confirm the following values in my test setup right before an instance of \Magento\Framework\UrlFactory gets created on the last line:

$storeId = 1, $routePath = "catalog/product/view", $routeParams = ['id' => 39, 's' => 'prod-url-key', 'category' => 7, 'query' => []].

At that point it passes off to \Magento\Framework\UrlFactory and that's as far as I got. The parameters seem right, but I'm fairly new to Magento development and wouldn't consider myself too well versed just yet.

@tigerx7
Copy link
Author

tigerx7 commented Dec 21, 2015

I did a bit more digging. It seems that $product->hasUrlDataObject() is returning false and so is $product->getRequestPath() in function getUrl so UrlRewrite and setRequestPath never kick in.

When ignoring the false check if statement on $product->getRequestPath(), the following $filterData gets formed in my test:

[filterData] => Array
        (
            [entity_id] => 62
            [entity_type] => product
            [store_id] => 1
            [metadata] => Array
                (
                    [category_id] => 5
                )
        )

However, when that gets passed to $this->urlFinder->findOneByData($filterData); it returns NULL

The product id is correct (62) and it is in category_id 5, which is 3 levels deep.

@GeromeF
Copy link

GeromeF commented Dec 22, 2015

I did it working this way
delete FROM url_rewrite WHERE entity_type = 'product'
take a product and change categories for it
bin/magento indexer:reindex
urls are ok for the updated product, others are still wrong
Don't seems to be a bug, maybe someting went wrong during the install of the sample datas.

@tigerx7
Copy link
Author

tigerx7 commented Dec 26, 2015

@GeromeF , I'm experiencing the same issue without sample data loaded. The urlrewrites are correct and I'm able to to load products when typing the rewritten url directly (ie. {domain}/{category}/{subcategory}/{product_url_key}. It brings up the product successfully. However, the url's that are being generated for the products when they're being listed at the category level and use catalog path in url is set to true are failing and generating something like: http://{domain}/catalog/product/view/id/1/s/{product_url_key}/category/4/

Are you able to reproduce successful URL's with the catalog path in the product list anchor tags at the catalog level?

@tigerx7
Copy link
Author

tigerx7 commented Dec 27, 2015

While tracking down another issue, I think I found the problem with the Product URL paths. getProductUrl isn't taking into account categories set as anchors.

So say if a product is only assigned to the last child category: {default category}-->{level 1}->{level 2}->{level 3}. If a visitor navigates all the way to {level 3}, then the path for the product is generated correctly: {domain}/{level 1}/{level 2}/{level 3}/{product url key}.

However, if {level 2} is set as an anchor, and the visitor only navigates that far, then that's when getUrlPath() fails and generates the non-url-rewrite path for a product.

@vkholoshenko
Copy link
Contributor

Sorry for the delay. First issue was fixed in e0f560f. And for issue from your last comment we've created one more internal ticket: MAGETWO-47656
Thanks for your input!

@vkholoshenko vkholoshenko assigned voleye and unassigned vkholoshenko Jan 6, 2016
@tigerx7
Copy link
Author

tigerx7 commented Jan 6, 2016

@vladimir-k Awesome, I was able to confirm that applying e0f560f fixed the issue for category assigned products. It's one step closer, now it's just the anchor categories having the url path issue. Applying the mentioned changes didn't fix the issue in anchor categories.

@rahulanand77
Copy link

Whenever we are creating product from Admin, it is doing an insert in "url_rewrite" table(magento2), after doing reindexing it is working fine on the front end. But when we are importing products from csv, then it is creating wrong url's like "catalog/product/view" etc. Even after reindexing its not working.

Also we have noticed, its not doing any insert in "url_rewrite" when doing reindexing. We have noticed that its only inserting into "url_rewrite" when doing insert from admin. Is it default feature of magento now?

@Tjitse-E
Copy link
Contributor

@pravalitera i've modified the script, according your comment. The strange thing is that the regenerated url's don't have the store view value's, but the main store value's after regeneration. I'm digging in the DB now to find out more...

@pravalitera
Copy link

@Tjitse-E I think it's because when the table "url_rewrite" is already generated, you fall into the "catch" of the insert (duplicate). You have "-" instead of "." when you launch it.

I will repost asap with a proper script that does everything right, i'm dealing with another things right now. I'm just astonished that i have to rewrite something to reindex urls...

@pravalitera
Copy link

pravalitera commented Nov 24, 2016

Ok. Here is a new version that rewrite products and category.

  • @miro91 Your script does not work with url_key by stores. And seems very slow compared to mine, maybe i done something wrong.
  • @Tjitse-E It's maybe because of laking of ->setStoreFilter on collection or ->setStoreId on objects...
  • It will always DELETE old urls. So no redirect of old ones.
  • I will do another version when i can, to handle params such as $product_ids etc...
  • Expect new version soon.
  • You will have to write a plugin... because I think i found a bug in Magento 2.

in Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator;

l.109
$productCategories = $product->getCategoryCollection()
            ->addAttributeToSelect('url_key')
            ->addAttributeToSelect('url_path')
            ->setStore($storeId) // Line to add !!!
        ;

If you don't do that, the product will not have the proper category path by stores...

My new script :

/**
 * CLI Command to Reindex URLS
 *
 *
 * @author RAVALITERA Pol <pol.ravalitera@gmail.com>
 * @author Tjitse <@github>
 * @author kanduvisla <@github>
 * @author miro91 <@github>
 * @version 0.0.1
 */

namespace Afg\Core\Console\UrlRewriting;

use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
use Magento\UrlRewrite\Model\UrlPersistInterface;
use Magento\CatalogUrlRewrite\Model\CategoryUrlRewriteGenerator;

/**
 * Class RegenerateUrls.php
 */
class RegenerateUrls extends Command
{
    /**
     * @var \Magento\Store\Model\StoreManagerInterface
     */
    protected $storeManager;

    /**
     * @var \Magento\UrlRewrite\Model\UrlRewriteFactory
     */
    protected $urlRewriteFactory;

    /**
     * @var CategoryUrlRewriteGenerator
     */
    protected $categoryUrlRewriteGenerator;

    /**
     * @var UrlPersistInterface
     */
    protected $urlPersist;

    /**
     * @var \Magento\Catalog\Model\ResourceModel\Category\CollectionFactory
     */
    protected $categoryCollectionFactory;

    /**
     * @var \Magento\CatalogUrlRewrite\Observer\UrlRewriteHandler
     */
    protected $urlRewriteHandler;


    /**
     * RegenerateUrls constructor.
     * @param \Magento\UrlRewrite\Model\UrlRewriteFactory $urlRewriteFactory
     * @param \Magento\Store\Model\StoreManagerInterface $storeManager
     * @param \Magento\Catalog\Model\ResourceModel\Category\CollectionFactory $categoryCollectionFactory
     * @param string $name
     */
    public function __construct(
        \Magento\UrlRewrite\Model\UrlRewriteFactory $urlRewriteFactory,
        \Magento\Store\Model\StoreManagerInterface $storeManager,
        \Magento\CatalogUrlRewrite\Model\CategoryUrlRewriteGenerator $categoryUrlRewriteGenerator,
        \Magento\Catalog\Model\ResourceModel\Category\CollectionFactory $categoryCollectionFactory,
        UrlPersistInterface $urlPersist,
        \Magento\CatalogUrlRewrite\Observer\UrlRewriteHandler $urlRewriteHandler,
        $name = 'regenerate_urls'
    )
    {
        $this->urlRewriteFactory = $urlRewriteFactory;
        $this->storeManager = $storeManager;

        $this->categoryUrlRewriteGenerator = $categoryUrlRewriteGenerator;
        $this->categoryCollectionFactory = $categoryCollectionFactory;
        $this->urlPersist = $urlPersist;
        $this->urlRewriteHandler = $urlRewriteHandler;

        parent::__construct($name);
    }

    /**
     * Configure the command
     */
    protected function configure()
    {
        $this->setName('afg:rewrite_url:category2');
        $this->setDescription('Regenerate Url\'s for categories');
    }

    /**
     * @param InputInterface $input
     * @param OutputInterface $output
     */
    protected function execute(InputInterface $input, OutputInterface $output)
    {

        if ($this->storeManager->isSingleStoreMode()) {
            $stores = [$this->storeManager->getStore(0)];
        } else {
            $stores = $this->storeManager->getStores();
        }

        $start_time = microtime(true);
        foreach ($stores as $store) {
            $output->writeln($store->getCode().'...');
            $categories = $this->categoryCollectionFactory->create()->setStore($store->getId())
                ->addAttributeToSelect(array('url_key', 'url_path', 'is_anchor'))
                ->addAttributeToFilter('level', 2)
                ->addAttributeToFilter('parent_id',$store->getRootCategoryId())
            ;
            foreach ($categories as $category) {
                if ($category->getUrlKey()) {
                    $category->setStoreId($store->getId());
                    $urlRewrites = array_merge(
                        $this->categoryUrlRewriteGenerator->generate($category, true)
                        , $this->urlRewriteHandler->generateProductUrlRewrites($category)
                    );
                    $this->urlPersist->replace($urlRewrites);
                }
            }
        }
        $output->writeln('[DONE] ' . round(microtime(true) - $start_time,2) .' sec');
    }
    
}

@Gael42
Copy link

Gael42 commented Nov 24, 2016

@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
Copy link

pravalitera commented Nov 24, 2016

@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
Copy link
Contributor

Tjitse-E commented Nov 25, 2016

@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/English_url/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
Copy link

@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
Copy link
Contributor

@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
Copy link

crantron commented Dec 7, 2016

Incase this helps anyone. I truncated the url_rewrite table.

  • I used @iazel module to restore all product rewrites.
  • Then for categories went to each sub root category, and resaved.
  • Then went to SEO and changed everything to not have .html
  • now everything is perfect.

thanks for this thread, helped a bunch. cheers!

@MattDelac
Copy link

@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
Copy link

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

@jarhody
Copy link

jarhody commented Mar 14, 2017

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
Copy link

skast96 commented Jul 18, 2017

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

@Jeordy
Copy link

Jeordy commented Jul 21, 2017

@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
Copy link

skast96 commented Jul 22, 2017

@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
Copy link

Jeordy commented Jul 24, 2017

@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
Copy link
Contributor

@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
Copy link

ghost commented Mar 14, 2018

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.

magento-team pushed a commit that referenced this issue Jun 27, 2018
[TSG] Backporting for 2.2 (pr28) (2.2.5)
@Eddcapone
Copy link

@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
Copy link
Contributor

hostep commented Aug 18, 2020

@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
Copy link

@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
Copy link
Contributor

hostep commented Aug 18, 2020

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
Copy link

ghost commented Aug 24, 2020

@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
Copy link

ivanaugustobd commented May 25, 2021

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
Copy link

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development
Projects
None yet
Development

No branches or pull requests