Prerequisites:
- Magento 2.3
- PHP 7.2
composer require techdivision/import-cli-simple:3.1.0-beta9
Description:
In TechDivision\Import\Category\Observers\UrlRewriteUpdateObserver the process() method has a number of problems:
The lines
foreach ($this->existingUrlRewrites as $existingUrlRewrites) {
foreach ($existingUrlRewrites as $existingUrlRewrite) {
first loop over the URL rewrites rows and then over the columns.
Within the loop the rewrite data does not set the is_autogenerated field:
$attr = array(
MemberNames::REDIRECT_TYPE => 301,
MemberNames::TARGET_PATH => $targetPath,
);
The problem here is that the Magento (at least as of EE 2.3) loads category collections by joining the URL rewrites table and filtering by is_autogenerated = 1 instead of filterung by the redirect type. So when a 301-redirect is created that has is_autogenerated = 1 set, the collection loads the same category twice with two different URL rewrites joined. An exception is thrown. Something like "Item with ID ?? alread exists in collection".
I created a patch for this.
bugfix-[5.x]-fix-url-rewrite-update.diff.zip
Functionality
There is also the problem with thUrlRewriteUpdateObserver handles rewrites. It doesn't resolve transitive URL rewrites. And as such it does not handle rewrite loops correctly. I modified the implementation of the UrlRewriteUpdateObserver to handle this.
class UrlRewriteUpdateObserver extends \TechDivision\Import\Category\Observers\UrlRewriteUpdateObserver
{
protected function process()
{
if ($this->getSubject()->getConfiguration()->hasParam(\TechDivision\Import\Category\Utils\ConfigurationKeys::CLEAN_UP_URL_REWRITES) &&
!$this->getSubject()->getCoreConfigData(\TechDivision\Import\Category\Utils\CoreConfigDataKeys::CATALOG_SEO_SAVE_REWRITES_HISTORY, true))
{
// in our client's case we don't have to deal with this case
parent::process();
return;
}
// since we have our own implementation for this case, we skip the implementation in the immediate
// parent class
\TechDivision\Import\Category\Observers\UrlRewriteObserver::process();
// get new target path for the current category
$newTargetPath = $this->prepareRequestPath();
// create redirect URL rewrites for the existing URL rewrites and save the old target path, if any
$oldTargetPath = null;
foreach ($this->existingUrlRewrites as $existingUrlRewrite)
{
if ($existingUrlRewrite[MemberNames::REDIRECT_TYPE] == 0)
{
// in this case, this is the previous main rewrite with
$oldTargetPath = $existingUrlRewrite[MemberNames::REQUEST_PATH];
}
else
{
// nothing do to for redirect_type!=0 if the target path is alredy correct
if ($existingUrlRewrite[MemberNames::TARGET_PATH] === $newTargetPath)
continue;
}
// if updating the existing URL rewrite would create an identity rewrite, the existing URL rewrite
// has to be deleted
if ($existingUrlRewrite[MemberNames::REQUEST_PATH] === $newTargetPath)
{
$this->deleteUrlRewrite([
'url_rewrite_id' => $existingUrlRewrite[MemberNames::URL_REWRITE_ID]
]);
continue;
}
// override data with the 301 configuration
$attr = array(
MemberNames::REDIRECT_TYPE => 301,
MemberNames::TARGET_PATH => $newTargetPath,
MemberNames::IS_AUTOGENERATED => 0
);
// is_autogenerated=0 is necessary, since Magento 2 handles category collections in a weird way:
// instead of searching for redirect_type=0 it filters by is_autogenerated=1
// merge and return the prepared URL rewrite
$existingUrlRewrite = $this->mergeEntity($existingUrlRewrite, $attr);
// create the URL rewrite
$this->persistUrlRewrite($existingUrlRewrite);
}
}
protected function initializeUrlRewrite(array $attr)
{
// iterate over the available URL rewrites to find the one that matches the request path, if any
foreach ($this->existingUrlRewrites as $urlRewrite)
{
// if the request path matches, this rewrite has to be the new main rewrite (i.e. redirect_type=0)
if ($urlRewrite[MemberNames::REQUEST_PATH] === $attr[MemberNames::REQUEST_PATH])
{
// make sure it is not processed any further after inserting
$this->removeExistingUrlRewrite($urlRewrite);
return $this->mergeEntity($urlRewrite, $attr);
}
}
// simple return the attributes, so that a new rewrite is created
return $attr;
}
}
Prerequisites:
composer require techdivision/import-cli-simple:3.1.0-beta9Description:
In
TechDivision\Import\Category\Observers\UrlRewriteUpdateObservertheprocess()method has a number of problems:The lines
first loop over the URL rewrites rows and then over the columns.
Within the loop the rewrite data does not set the
is_autogeneratedfield:The problem here is that the Magento (at least as of EE 2.3) loads category collections by joining the URL rewrites table and filtering by
is_autogenerated = 1instead of filterung by the redirect type. So when a 301-redirect is created that hasis_autogenerated = 1set, the collection loads the same category twice with two different URL rewrites joined. An exception is thrown. Something like "Item with ID ?? alread exists in collection".I created a patch for this.
bugfix-[5.x]-fix-url-rewrite-update.diff.zip
Functionality
There is also the problem with th
UrlRewriteUpdateObserverhandles rewrites. It doesn't resolve transitive URL rewrites. And as such it does not handle rewrite loops correctly. I modified the implementation of theUrlRewriteUpdateObserverto handle this.