Skip to content
This repository has been archived by the owner on Apr 29, 2019. It is now read-only.

SKU uniqueness validation #102

Closed
wants to merge 2 commits into from
Closed

SKU uniqueness validation #102

wants to merge 2 commits into from

Conversation

federivo
Copy link
Contributor

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)

#101

Manual testing scenarios

  1. Create product with sku "TEST"
  2. Create another product with sku "TEST"
  3. 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
Copy link
Contributor Author

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-engcom-team pushed a commit that referenced this pull request Apr 13, 2018
Eliminate usage of Zend_Mime from Magento 2 Open Source
@magento-cicd2
Copy link
Contributor

magento-cicd2 commented May 30, 2018

CLA assistant check
All committers have signed the CLA.

@dmanners
Copy link
Contributor

@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
Copy link
Contributor

@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.

magento-engcom-team added a commit that referenced this pull request Jul 19, 2018
 - Merge Pull Request magento/graphql-ce#102 from magento/graphql-ce:graphql-fix-the-category-tree-depth-calculation-issue
 - Merged commits:
   1. 9116d82
@dmanners
Copy link
Contributor

Will close this PR in favor of #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.

@dmanners dmanners closed this Aug 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants