magento-engcom/import-export-improvements#101 Remove sku autogeneration on new products, keep on duplication

pogster commented 6 years ago

Description

This PR may replace #102. In contrast to the original PR I restored the SKU autogeneration in case a product is duplicated. The product copier will copy the original SKU and save the duplicate in a loop until a unique url key is found - if no SKU is autogenerated and no valid SKU is supplied it simply won't save the product.

Since the copier sets "isDuplicate" = true I thought it would be good simple solution to check for that entry before autogenerating a SKU.

Fixed Issues (if relevant)

magento-engcom/import-export-improvements#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.
"Save & Duplicate" the product with sku "TEST"
The duplicate receives the SKU "TEST-1" and will already be saved.

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)

dmanners commented 6 years ago

Thanks for the PR @pogster for me I think it would be fine to generate the sku on duplication. Feel free to reach out to me if you need help with the tests.

dmanners commented 5 years ago

On you branch we have the following testing scenario:

Create product with SKU "TEST"
Click Edit product with SKU "TEST"
Click "Save & Duplicate".
New product is created with SKU="TEST-1" and URL Key ="test-1".
Click Edit product with SKU "TEST"
Click "Save & Duplicate".
Validation message "The value specified in the URL Key field would generate a URL that already exists."

But on 2.3-develop we get the following on step 7:

New product is created with SKU="TEST-2" and URL Key ="test-2".

My thought is that it should work as it does on 2.3-develop with a new product created with a unique sku and url. What do you think @pogster

pogster commented 5 years ago

I'm sorry but I fail to reproduce that behaviour on my branch pogster:sku-uniqueness. I don't see a difference to the 2.3-develop behaviour and I don't get that error message.

magento-cicd2 commented 5 years ago

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 3 committers have signed the CLA.

:white_check_mark: dmanners
:x: carstenpfeifer
:x: okorshenko

carstenpfeifer seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.

dmanners commented 5 years ago

No update here as yet from the QA team but I have asked for further details for you.

pogster commented 5 years ago

Looking at the code I can not imagine how the described behaviour should occur. For me it sounds like it was tested on a wrong/incomplete/old branch.

Am Fr., 16. Nov. 2018, 13:57 hat David Manners notifications@github.com geschrieben:

No update here as yet from the QA team but I have asked for further details for you.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/magento-engcom/import-export-improvements/pull/119#issuecomment-439385052, or mute the thread https://github.com/notifications/unsubscribe-auth/AHPlIH3TeB_SO-WJlIRQT3Bsu2XDeCEHks5uvrYcgaJpZM4V5UnB .

VasylShvorak commented 5 years ago

@pogster @dmanners Sorry for delay. I recorded video according steps from testing scenario. In general, we cannot create duplicate for same product more than once. test UPD: Added file with better resolution

dmanners commented 5 years ago

Thanks for the information @VasylShvorak does that help you out @pogster?

federivo commented 5 years ago

Per request from @dmanners I ran a few test scenarios:

Testing scenarios: A) 1- Create new product with sku "ABC" 2- Go back to grid, re-enter in new product page and create new product with sku "ABC" Expected behavior: Magento throws error message saying the sku is already in use. Result: Passed.

B) 1- Edit an existing product, for example sku "ABC" 2- "Save and duplicate" from edit page. Expected behavior: Magento duplicates the product. Autogenerates sku with a dash + sequence number. In the example: "ABC-1" Result: Passed.

C) 1- Edit an existing product, for example sku "ABC" 2- "Save and duplicate" from edit page. 3- Edit "ABC" again and "Save and duplicate" once more. Expected behavior: Magento duplicates the product. Autogenerates sku with a dash + sequence number. In the example: "ABC-2" since "ABC-1" already exists. Result: Passed.

In line with @pogster report, I also cannot reproduce the duplication issue. I have successfully duplicated more than 1 products from the original product.

sivaschenko commented 5 years ago

@magento-engcom-team give me test instance

orlangur commented 5 years ago

Please reopen PR ^ when this one is merged.

okorshenko commented 5 years ago

Closing this PR. We will move this PR to magento/magento2 repository

ghost commented 5 years ago

Hi @pogster, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.