Skip to content

getRewriteByProductStore does not return product URL rewrite #23357

Open
@daniel-ifrim

Description

@daniel-ifrim

Preconditions (*)

  1. Magento 2.2.7 but I think it's on all version Magento 2.3-develop branch too
  2. MySQL 5.7.26

Steps to reproduce (*)

  1. Not much to test unless you make a code construct like this:
/* @var \Magento\Catalog\Model\ResourceModel\Url $this->catalogUrl */
$this->catalogUrl->getRewriteByProductStore([$productId => $storeId])

Expected result (*)

  1. It should return an array with product slug like this:
[4256] => Array
        (
            [store_id] => 9999
            [visibility] => 4
            [url_rewrite] => lorem-ipsum.html
        )

Actual result (*)

  1. It returns product category URL in some cases:
[4256] => Array
        (
            [store_id] => 9999
            [visibility] => 4
            [url_rewrite] => test-category/lorem-ipsum.html
        )

It happens because the SQL query from
\Magento\Catalog\Model\ResourceModel\Url::getRewriteByProductStore
doesn't look good.

This is wrong:

)->joinLeft(
    ['r' => $this->getTable('catalog_url_rewrite_product_category')],
    'u.url_rewrite_id = r.url_rewrite_id AND r.category_id is NULL',
    []
);

To get rows from table url_rewrite not in left joined table catalog_url_rewrite_product_category
we should remove AND r.category_id is NULL from left join and add it in where as:

AND r.url_rewrite_id IS NULL

where r is the alias of table url_rewrite.

Leaving AND r.category_id is NULL' in join condition will create a result with more than one row. I get all rows for that specific product including rows with product category request_path.

Further down in function getRewriteByProductStore
it adds to return variable the last row for a product:

$rowSet = $connection->fetchAll($select, $bind);
foreach ($rowSet as $row) {
    $result[$row['product_id']] = [
        'store_id' => $row['store_id'],
        'visibility' => $row['visibility'],
        'url_rewrite' => $row['request_path'],
    ];
}

In my case the last row is product category request_path and is not product request_path.
The product request_path is in the first row from the result set ($rowSet).

Query done in getRewriteByProductStore

SELECT `i`.`product_id`, 
       `i`.`store_id`, 
       `i`.`visibility`, 
       `u`.`request_path` 
FROM   `catalog_category_product_index_store22` AS `i` 
       LEFT JOIN `url_rewrite` AS `u` 
              ON i.product_id = u.entity_id 
                 AND i.store_id = u.store_id 
                 AND u.entity_type = "product" 
       LEFT JOIN `catalog_url_rewrite_product_category` AS `r` 
              ON u.url_rewrite_id = r.url_rewrite_id 
                 AND r.category_id IS NULL 
WHERE  (( i.product_id = 4256 
          AND i.store_id = 22 
          AND i.category_id = 2 ));

It returns 3 rows.

Query modied:

SELECT `i`.`product_id`, 
       `i`.`store_id`, 
       `i`.`visibility`, 
       `u`.`request_path` 
FROM   `catalog_category_product_index_store22` AS `i` 
       LEFT JOIN `url_rewrite` AS `u` 
              ON i.product_id = u.entity_id 
                 AND i.store_id = u.store_id 
                 AND u.entity_type = "product" 
       LEFT JOIN `catalog_url_rewrite_product_category` AS `r` 
              ON u.url_rewrite_id = r.url_rewrite_id
WHERE  (( i.product_id = 4256 
          AND i.store_id = 22 
          AND i.category_id = 2 )
          AND r.url_rewrite_id IS NULL);

It returns 1 row with product request_path.

UPDATE
The SQL query should include u.redirect_type=0 also because otherwise it can take row with redirect_type=301 (permanent redirects).

This code

            $select = $connection->select()->from(
                ['i' => $this->tableMaintainer->getMainTable($storeId)],
                ['product_id', 'store_id', 'visibility']
            )->joinLeft(
                ['u' => $this->getMainTable()],
                'i.product_id = u.entity_id AND i.store_id = u.store_id'
                . ' AND u.entity_type = "' . ProductUrlRewriteGenerator::ENTITY_TYPE . '"',
                ['request_path']
            )->joinLeft(
                ['r' => $this->getTable('catalog_url_rewrite_product_category')],
                'u.url_rewrite_id = r.url_rewrite_id AND r.category_id is NULL',
                []
            );

            $bind = [];
            foreach ($productIds as $productId) {
                $catId = $this->_storeManager->getStore($storeId)->getRootCategoryId();
                $productBind = 'product_id' . $productId;
                $storeBind = 'store_id' . $storeId;
                $catBind = 'category_id' . $catId;
                $bindArray = [
                    'i.product_id = :' . $productBind,
                    'i.store_id = :' . $storeBind,
                    'i.category_id = :' . $catBind
                ];
                $cond = '(' . implode(' AND ', $bindArray) . ')';
                $bind[$productBind] = $productId;
                $bind[$storeBind] = $storeId;
                $bind[$catBind] = $catId;
                $select->orWhere($cond);
            }

            $rowSet = $connection->fetchAll($select, $bind);
            foreach ($rowSet as $row) {
                $result[$row['product_id']] = [
                    'store_id' => $row['store_id'],
                    'visibility' => $row['visibility'],
                    'url_rewrite' => $row['request_path'],
                ];
            }

should be something like:

            $select = $connection->select()->from(
                ['i' => $this->tableMaintainer->getMainTable($storeId)],
                ['product_id', 'store_id', 'visibility']
            )->joinLeft(
                ['u' => $this->getMainTable()],
                'i.product_id = u.entity_id AND i.store_id = u.store_id'
                . ' AND u.entity_type = "' . ProductUrlRewriteGenerator::ENTITY_TYPE . '"',
                ['request_path']
            )->joinLeft(
                ['r' => $this->getTable('catalog_url_rewrite_product_category')],
                'u.url_rewrite_id = r.url_rewrite_id',
                []
            );

            $bind = [];
            foreach ($productIds as $productId) {
                $catId = $this->_storeManager->getStore($storeId)->getRootCategoryId();
                $productBind = 'product_id' . $productId;
                $storeBind = 'store_id' . $storeId;
                $catBind = 'category_id' . $catId;
                $bindArray = [
                    'i.product_id = :' . $productBind,
                    'i.store_id = :' . $storeBind,
                    'i.category_id = :' . $catBind
                ];
                $cond = '(' . implode(' AND ', $bindArray) . ' AND (r.url_rewrite_id IS NULL)) AND u.redirect_type = 0';
                $bind[$productBind] = $productId;
                $bind[$storeBind] = $storeId;
                $bind[$catBind] = $catId;
                $select->orWhere($cond);
            }

            $rowSet = $connection->fetchAll($select, $bind);
            foreach ($rowSet as $row) {
                $result[$row['product_id']] = [
                    'store_id' => $row['store_id'],
                    'visibility' => $row['visibility'],
                    'url_rewrite' => $row['request_path'],
                ];
            }

Diff (don't copy paste, white-spaces will mess the diff):

Index: ../vendor/magento/module-catalog/Model/ResourceModel/Url.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- ../vendor/magento/module-catalog/Model/ResourceModel/Url.php	(date 1567697855000)
+++ ../vendor/magento/module-catalog/Model/ResourceModel/Url.php	(date 1567697855000)
@@ -680,7 +680,7 @@
                 ['request_path']
             )->joinLeft(
                 ['r' => $this->getTable('catalog_url_rewrite_product_category')],
-                'u.url_rewrite_id = r.url_rewrite_id AND r.category_id is NULL',
+                'u.url_rewrite_id = r.url_rewrite_id',
                 []
             );
 
@@ -695,7 +695,7 @@
                     'i.store_id = :' . $storeBind,
                     'i.category_id = :' . $catBind
                 ];
-                $cond = '(' . implode(' AND ', $bindArray) . ')';
+                $cond = '(' . implode(' AND ', $bindArray) . ' AND (r.url_rewrite_id IS NULL)) AND u.redirect_type = 0';
                 $bind[$productBind] = $productId;
                 $bind[$storeBind] = $storeId;
                 $bind[$catBind] = $catId;

Metadata

Metadata

Assignees

No one assigned

    Labels

    Area: CatalogComponent: CatalogIssue: ConfirmedGate 3 Passed. Manual verification of the issue completed. Issue is confirmedPriority: P2A defect with this priority could have functionality issues which are not to expectations.Progress: ready for devReproduced on 2.3.xThe issue has been reproduced on latest 2.3 releaseSeverity: S2Major restrictions or short-term circumventions are required until a fix is available.Triage: Dev.ExperienceIssue related to Developer Experience and needs help with Triage to Confirm or Reject it

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions