delete url rewrites for all products of an attribute set when said at…

[patrikpihlstrom]

…tribute set is deleted

Description

This PR adds an observer and dispatches an event when an attribute set is deleted. The observer then deletes any url rewrites for products that belonged to the deleted attribute set.

Fixed Issues (if relevant)

1. Delete Attribute Set is not Deleting product url key in url rewrite #10764: Delete Attribute Set is not Deleting product url key in url rewrite

Manual testing scenarios

1. Create an attribute set
2. Create a product from the newly created attribute set and insure that a url key is specified
3. Delete the attribute set
4. Create a new product from the default attribute set and apply the same url key
5. Hopefully you won't get an error when you save the product

[patrikpihlstrom]

I'm having some trouble writing the tests for this one. I'm unable to call getAllIds on the product collection I'm passing to the AttributeSetRepository. This is how I'm creating the mock:

$this->productCollectionMock = $this->getMockBuilder(\Magento\Catalog\Model\ResourceModel\Product\Collection::class)
            ->setMethods(['addFieldToFilter', 'getAllIds'])
            ->disableOriginalConstructor()
            ->getMock();

Would it be better to dispatch an event before the deletion, and register the necessary product ids for later use after the attribute set is deleted? This would negate the need for a product collection to be passed to the AttributeSetRepository.

[kandy]

Can you describe why it need to delete URL rewrites in this case? I don't see the reason for this at all.

[patrikpihlstrom]

The referenced issue describes this fairly well. If you attempt to create a product with the same url key as a product that has been deleted as a result of deleting the attribute set said product belonged to, you'll get an error.

[orlangur]

On 2.2.0 branch products are removed successfully but URL Rewrites for deleted products remain orphaned in database?

[patrikpihlstrom]

That's correct. The 'catalog_product_delete_after' event isn't dispatched when the attribute set is deleted. Otherwise I would've observed it and deleted the corresponding URL rewrite.

[orlangur]

Probably current implementation is workable but it is not appropriate.

Please change implementation in such a way that it is placed in CatalogUrlRewrite module and URL Rewrites removal is triggered by corresponding products removal and not attribute set removal. Most likely it will be plugin-interceptor for an appropriate resource model or repository.

After all changes are made and all builds are green, please squash changes into a single commit.

[orlangur]

\Magento\UrlRewrite\Model\UrlPersistInterface::deleteByData must be used instead.

[patrikpihlstrom]

Yeah, I was looking for a more elegant way of deleting the entries.

[orlangur]

Catalog must not be referred in Eav module. New events must not be introduced when it's enough extension points to create a plugin-interceptor.

[patrikpihlstrom]

You're right! I'll get on this later today 👍

[patrikpihlstrom]

I don't think that the removal of an attribute set calls any delete function on a per product level, at least as far as I can tell.

[orlangur]

Products are dropped due to foreign keys? Maybe similar constraints should be added for product URL Rewrites?

[patrikpihlstrom]

That isn't possible since there are other url rewrites in the table that relate to different entity types. Or am I just not seeing something that's super obvious?

[orlangur]

You are right https://stackoverflow.com/a/28322144/8640867 :) That's our case, needs to be handled on application side, please implement as an interceptor attached to attribute set entity removal.

[patrikpihlstrom]

I've created a new pull request for this #11391