SKU uniqueness validation

magento-engcom / import-export-improvements

Open Software License 3.0

31 stars 29 forks source link

SKU uniqueness validation #102

Closed federivo closed 6 years ago

federivo commented 6 years ago

Description

Removed logic for generating unique SKU when already exists. Throw an exception instead which will be presented to the user.

Fixed Issues (if relevant)

https://github.com/magento-engcom/import-export-improvements/issues/101

Manual testing scenarios

Create product with sku "TEST"
Create another product with sku "TEST"
You should receive an error saying that the sku already exists instead of getting your sku modified.

Contribution checklist

[ ] Pull request has a meaningful description of its purpose
[ ] All commits are accompanied by meaningful commit messages
[ ] All new or changed code is covered with unit/integration tests (if applicable)
[ ] All automated tests passed successfully (all builds on Travis CI are green)

federivo commented 6 years ago

Work in progress

Pending:

Modify existing tests regarding unique sku generation.
Add tests for checking the exception thrown when attempting to repeat a sku.

magento-cicd2 commented 6 years ago

All committers have signed the CLA.

dmanners commented 6 years ago

@federivo @piotrekkaminski

I think what we will have to do here is change the way that products are duplicated as currently they will silently fail when a product is duplicated via the admin.

I would recommend changing the class Catalog/Model/Product/Copier.php to take a new class that will allow you to generate a unique url and sku. I know that this is kind of what we are replacing but for me I think that in the case of admin product duplication then generating a unique sku and url would be fine.

What do you both think?

piotrekkaminski commented 6 years ago

@dmanners so I think we had an automatic naming logic before but it often resulted in proliferation of copies. I think the plan for Magento 2 was to offer meaningful error messages. Can we provide good error messages instead - like product with this SKU already exists, please use a different SKU or edit the original product instead.

dmanners commented 6 years ago

Will close this PR in favor of https://github.com/magento-engcom/import-export-improvements/pull/119 119 takes the starting point of this PR and adds a bit more work to make duplication work so @federivo your commits and code will still be in place.

Thank you for your contribution.