-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
12110: Missing cascade into attribute set deletion. #12167
Conversation
Hi @nmalevanec, you in a right way, but problem deeper than an issue with URL rewrites. The main reason why URL rewrites still present in db is foreign key (relations between catalog_product_entity and eav_attribute_set tables). You fix problem with rewrites, but we still has problems with other product artifacts that should be deleted in before, after methods, plugins, observers etc. |
* See COPYING.txt for license details. | ||
*/ | ||
|
||
namespace Magento\CatalogUrlRewrite\Plugin\Eav\AttributeSetRepository; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move plugin to Catalog module
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
/** | ||
* Remove url rewrites for products with given attribute set. | ||
*/ | ||
class RemoveProductUrlRewrite |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename plugin to RemoveProducts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
* | ||
* @SuppressWarnings(PHPMD.UnusedFormalParameter) | ||
*/ | ||
public function aroundDelete( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use after plugin instead of around
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, may be I have missed something. But, if I'll use after plugin I won't have attribute set id in results, only bool true and instance of AttributeSetRepository. And I need attribute set id in order to filter product collection.
if (!empty($productIds)) { | ||
$productIds = array_chunk($productIds, $this->chunkSize); | ||
foreach ($productIds as $ids) { | ||
$this->urlPersist->deleteByData( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove products from db
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
* | ||
* @SuppressWarnings(PHPMD.UnusedFormalParameter) | ||
*/ | ||
public function aroundDelete( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, use after plugin.
Usage described in http://devdocs.magento.com/guides/v2.2/extension-dev-guide/plugins.html#after-methods
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I wrote above, in after plugin I do not have attribute set id for filtering product collection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After plugins provide access to all method arguments.
public function afterDelete(
AttributeSetRepositoryInterface $subject,
$result,
AttributeSetInterface $attributeSet
) {
// use $attributeSet->getId()
...
return $result;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Sorry, for not paying enough attention to devdocs before.
Hi @nmalevanec , consider to port this solution to 2.3 version |
Fixed static test (suppress cyclomatic complexity for \Magento\Catalog\Setup\UpgradeSchema::upgrade) |
# Conflicts: # app/code/Magento/Catalog/Setup/UpgradeSchema.php
[EngCom] Public Pull Requests - 2.2-develop - MAGETWO-84981: Trying to get data from non existent products #12539 - MAGETWO-84979: [Backport 2.2-develop] Fix swagger-ui on instances of Magento running on a non-standard port #12541 - MAGETWO-84903: Added namespace to product videos fotorama events #12469 - MAGETWO-84862: [Backport 2.2-develop] #11409: Too many password reset requests even when disabled in settings #11435 - MAGETWO-84856: Issue 12506: Fixup typo getDispretionPath -> getDispersionPath #12507 - MAGETWO-84808: 12110: Missing cascade into attribute set deletion. #12167 - MAGETWO-83503: [2.2] - Add command to view mview state and queue #12122 - MAGETWO-80223: Fix syntax of expectException() calls #11099
Description
Fix for: Missing cascade into attribute set deletion.
Fixed Issues (if relevant)
Manual testing scenarios
Contribution checklist