Once #3042257: Allow stores, promotions to be duplicated is done, we can proceed to products.

The simplest implementation would remove all referenced variations in createDuplicate(), since variations can't be shared between products and they have unique SKUs. However, we can implement an optimization for single-variation products ($product_type->allowMultipleVariations() is FALSE), and clone the variation in that case (since we know that the SKU field will be shown along with the product form).

Also, feedback suggests we need a way to duplicate variations as well. If we proceed without it, we should open a followup.

Issue fork commerce-3042258

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bojanz created an issue. See original summary.

bojanz’s picture

Status: Active » Needs review
FileSize
4.48 KB

Initial work.

Lukas von Blarer’s picture

Status: Needs review » Needs work

Works perfectly as described.

In my use case it is crucial to be able to clone the product variations as well, since they cause the main work for the editors. How could we solve this?

zenimagine’s picture

I applied the patch and it works for store owners.

But there is a problem with the module :

https://www.drupal.org/project/commerce_autosku

If the module is configured on "Automatically generate the SKU and hide the label field " the product variation which is duplicated displays an error of an empty SKU field.

If anyone knows an alternative to the Auto SKU module I am a taker. Because it's been 2 years since it had no update and it's been a long time since it is no longer compatible with commerce.

liquidcms’s picture

Having this as an action would be great.

Although, would it be ok to leave variation info blank (title, sku, price) or clone those from original (as Lukas suggests). Is it even allowed to save a product with a variation with an sku value already in use?

abx’s picture

From this issue -> https://www.drupal.org/project/commerce/issues/2986656

The patch #8 works great with Entity Clone module. Just tested it. The only thing changed after cloned is random SKU which is ok in my case since it's better than having to fill out all information in product variation again.

Also, attach the patch that should work with current Commerce Dev.

matthiasm11’s picture

Duplicating the product variations suits my situations the best too.

Fixed a warning regarding Random::string() should not be called statically, patch in attachment.

Instead of creating a random SKU, we should probably just append _cloned to the SKU, just like we do with the product label. If my_product_variation_cloned already exists, a _1, _2 etc should be appended I think. An example can be found in WebformHandlerFormBase::getUniqueMachineName() in the Webform module.

I agree with bojanz in https://www.drupal.org/project/commerce/issues/2986656#comment-13033975, regarding duplicating product variations may not be suitable for everyone. I suggest two possible solutions:

  • Make the "Product variation" checkbox on /admin/config/system/entity-clone work. This means a checkbox for the product variation field should be added to /entity_clone/commerce_product/{product_variation_id}.
  • We add a setting to the Commerce config like "Duplicate product variations when duplicating products (yes/no)" and let the code in createDuplicate() check this setting.

The first suggestion seems the most flexibel and most correct one to me. (use the power of entity_clone instead of programming something Commerce specific)

abx’s picture

Another patch, instead of random the sku. Use the original sku with "_clone" after it. Also, if it's duplicated, then, add number in the end.

matthiasm11’s picture

  • Fixed unexisting variable error.
  • Prevent generating my-sku_clone_0_1, if my-sku_clone_0 already exists, it should become my-sku_clone_1.
  • Clean up code to match the coding standards.
jsacksick’s picture

Status: Needs review » Needs work

I don't really understand what happened with the patches (the initial patch from comment #2 had tests coverage), the latest patch has none and duplicated logic from the parent createDuplicate() method.

If we end up adding logic to support duplicating variations, I believe we should simply implement a createDuplicate() method from ProductVariation that clears out the variation SKU.

Then I'd expect the Commerce AUTOSKU module to automatically generate a SKU if configured to do so. If the module isn't present, I wonder what would happen with empty skus though :p.

igorbarato’s picture

I added support to Commerce AutoSKU module when it's enabled.

When module is not installed, we are using the same logic but I removed ' _clone' after SKU, we only add a count when the SKU already exist because this would be confuse when we re-clone a entity 'my-sku_clone_0_clone_0'.

igorbarato’s picture

Fixed a wrong logic that is cloning the variations and set the old product value.

igorbarato’s picture

Component: User experience » Product
Status: Needs work » Needs review
s.messaris’s picture

+1 for this functionality, will test soon and provide feedback.

s.messaris’s picture

Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs tests
FileSize
6.25 KB
2.3 KB

My review:
- #2 works to duplicate without variations.
- the patch in #13 adds a duplicate logic for variations that works, but misses everything else in #2, such as UI buttons and tests.

The derailing of this issue suggests we need to add a way to include cloning variations with the product cloning. If we proceed without it, we should open a followup, but I believe we better include the duplication of variations here.

I am adding a patch based on #2, with the duplicate variation logic from #13.

It works for me for now, but setting to needs work, as:
- the tests need to be updated and
- I am not sure the way to duplicate variations and set skus is the best we can do. For example the way added to support commerce_autosku, might be better done with some event / hook that commerce_autosku can subscribe to.

abx’s picture

Just did a quick tested.

Patch #15 does work only with Product with multiple variations. In case we have single variation, there is no way we can clone that product. (Uncheck "Allow each product to have multiple variations." in "Product Types") We have to install Entity Clone Module. But then, when, we cloned with Entity Clone module - Body, Price and SKU are linked to the Original product. Any edit in those field will also change content in the original product.

Patch #12 works great with single variation and Entity Clone module. Once cloned, both product are completely separate from each other.

simgui8’s picture

#2 is in line with the initial feature request: duplicate products.
Since products often hold a lot of marketing information that can take some time to create, staff in charge of building those products will immediately benefit from it.

I would prefer to have #2 now than wait for all the bells and whistles of duplicated variations sometime later.

rokzabukovec’s picture

FileSize
7.45 KB

I just added the event before the opening the form so we can modify the duplicated entity before the user sees it. I used patch from #15 as a base. Which was working ok for me.

garmoza’s picture

As said abx,

Once cloned, both product are completely separate from each other.

This is exactly what I needed.

Patch #18 works for me. Thanks!

WalkingDexter’s picture

Status: Needs work » Needs review
FileSize
4.74 KB

I simplified the patch and removed its main drawback - creating variations before creating a product. I tested the patch with different number of variations and the duplication works as expected.

@rokzabukovec You can use hook_ENTITY_TYPE_prepare_form to modify the duplicated entity.
https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Entity%21...

P.S. The tests still need work.

WalkingDexter’s picture

abx’s picture

Tested #21 and it works for both Product and Product Variation cloning. However, after click at Duplicate product item, there is a button "Save and add variations" but it's actually "Save and go to variations tab" to allow us to add/edit/delete variation. (Which is good since SKU is randomly generated and we usually has to go into variation to change to what we want to.)

Thanks WalkingDexter,

dunot’s picture

Patch #12 for 1 product = 1 variation on D9.4.5 DC2.31:
variation` title stucks until 2-nd Product save. It results in "- Cloned" products in Cart and in orders.
I tried to change variation title on hook_entity_presave/update/postsave, but unsuccessfully.

use Drupal\commerce_product\Entity\ProductVariationInterface;
// function mymod_entity_presave(Drupal\Core\Entity\EntityInterface $entity){
function mymod_entity_postsave(Drupal\Core\Entity\EntityInterface $entity){
// function mymod_entity_update(Drupal\Core\Entity\EntityInterface $entity){
    
  // entity_clone bug: we set variation title same as Product`s title
  if ($entity -> getEntityTypeId() == 'commerce_product_variation'){    
    $variation = $entity;    
    $langcodes = array_keys(\Drupal::languageManager()->getLanguages());
    foreach ($langcodes as $langCode) {
      $variation_translation = $variation->getTranslation($langCode);
      // if ($variation->hasTranslation($langCode) && is_int(strpos($variation_translation->getTitle(),'Cloned')) )  { // all variations should have product titles if product titles has been changed
      if ($variation->hasTranslation($langCode))  {
        $variation_translation_product = $variation_translation->getProduct();
        $variation_translation->setTitle($variation_translation_product->getTitle());
        $variation_translation->save();
      }
    }
  }
}

Edited: aaa, no, it's dc bug

Marios Anagnostopoulos’s picture

Attaching a patch based on #24 for 2.37

jsacksick’s picture

Main thing I'm worried about here is duplicating a product with thousands of variations... Even though that concern is also valid for the Product::postSave() logic as we loop over all variations there... This potentially has to be improved btw...

Marios Anagnostopoulos’s picture

Main thing I'm worried about here is duplicating a product with thousands of variations

This probably is a more general issue, as you mentioned, and maybe it's best to be tackled on a separate issue.

This potentially has to be improved btw

+1

rhovland changed the visibility of the branch 3042258-allow-products-to to hidden.

rhovland changed the visibility of the branch 3042258-allow-products-to to active.