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

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

Closed
wants to merge 12 commits into from

Conversation

pogster
Copy link
Contributor

@pogster pogster commented Aug 11, 2018

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)

  1. Magento modifies the SKU when creating products and SKU is already used #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
Copy link
Contributor

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.

Clean up the header of the class to fit with the static-tests.
paliarush
paliarush previously approved these changes Oct 24, 2018
@dmanners
Copy link
Contributor

dmanners commented Nov 9, 2018

On you branch we have the following testing scenario:

  1. Create product with SKU "TEST"
  2. Click Edit product with SKU "TEST"
  3. Click "Save & Duplicate".
  4. New product is created with SKU="TEST-1" and URL Key ="test-1".
  5. Click Edit product with SKU "TEST"
  6. Click "Save & Duplicate".
  7. 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:

  1. 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
Copy link
Contributor Author

pogster commented Nov 9, 2018

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

magento-cicd2 commented Nov 9, 2018

CLA assistant check
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.

✅ dmanners
❌ carstenpfeifer
❌ 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
Copy link
Contributor

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

@pogster
Copy link
Contributor Author

pogster commented Nov 16, 2018 via email

@VasylShvorak
Copy link
Contributor

VasylShvorak commented Nov 19, 2018

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

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

@federivo
Copy link
Contributor

federivo commented Dec 4, 2018

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
Copy link
Member

@magento-engcom-team give me test instance

@orlangur
Copy link
Contributor

Please reopen PR ^ when this one is merged.

@okorshenko
Copy link
Contributor

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

@okorshenko okorshenko closed this Apr 29, 2019
@ghost
Copy link

ghost commented Apr 29, 2019

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.

@dmanners dmanners mentioned this pull request May 6, 2019
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants