Support from Acquia helps fund testing for Drupal Acquia logo

Comments

leanderl’s picture

FileSize
0 bytes

First: thank you to the guys who did this great module. It completely changes the usability of commerce and has made me (and hopefully soon my client) very happy.

A clone button is sorely needed. It is the number one request I hear from all editor users.
And perhaps after that a really smooth solution for adding existing product entities (other than SKU). Somehting similar to choosing images with Media Modules Media Library -- browsing product entities a selecting which ones should be included. Then again perhaps that should be compartmentalized into a submodule of IEF.

This is a clone button that works for my project that I did together with OlleOlleOlle. Hopefully it moves things forward... I'm sorry if the syntax of code and commenting should in some instances not adhere the drupal guidelines.

Big problem:
There seems to be something fishy about "add variation"->"cancel add variation" and then adding/editing again. As far as I could tell this problem is already in the module for "add variation" before this patch.
I would sure appreciate some help with this, so that you can choose any action (edit, clone, add variation), then cancel the form and use the actions again. Until that works it isn't really ripe for usage by "laymen", which is what we are trying to accomplish with this module.

Nota bene:
This patch also includes the reordering of the "abort" statements to allow changing weight between entities mentioned in this issue: http://drupal.org/node/1557966

leanderl’s picture

FileSize
14.51 KB

Hopefully this time the patch is there... (previous attachment empty). (By the way it is still work in progress hence the stupid comments in some places...)

leanderl’s picture

Status: Active » Needs review
FileSize
14.65 KB

Here's a cleaned up version of the patch. Seems to be working well. Needs review.

leanderl’s picture

Title: need a "Duplicate" button for product variation » Clone button + add existing
FileSize
26.52 KB

New patch which also includes andyg5000's "select_existing_product_entities" from issue http://drupal.org/node/1526084

leanderl’s picture

FileSize
26.4 KB

Found a bug with the "reordering" of weights part that prevented the entire form to be saved. This patch should hopefully do the trick. Needs community review...

bojanz’s picture

Title: Clone button + add existing » Add a "clone" button
Version: 7.x-1.0-beta1 » 7.x-1.x-dev
Status: Needs review » Needs work

The latest commits broke all patches in the queue (the code has changed drastically, sorry!), so this will need to be rerolled.
Provide only a patch for the clone button, the "reference existing" functionality already has its own issue and its own patch, no reason to merge it.

Anonymous’s picture

+1 for this functionality - this is exactly what I need. Any chance you could re-roll the patch @leanderl? Would be super appreciated!

leanderl’s picture

Hi Jackocnr, I was actually working on a reroll yesterday, but there have been drastic changes to the code, just like bojanz mentioned, so it is kind of hard to get all the way. But hopefully there'll be a patch for "clone-button" soon. I'm really hoping bojanz and the others can help polish it up to be included in the regular module, because I really love what IEF does and it is a major leap in usability.

leanderl’s picture

Hello, so here is a rerolled Clone button. It is quite rough around the edges and probably needs improvement, but it is a starting point. @bojanz I'm att DrupalCon Munich all week including all of friday. Would be great if we could get together and polish this up. Perhaps one would need a setting to enable/disable clone button, so as not to clutter the interface when a clone button is not desirable...

petu’s picture

Hello @leanderl!
Thank you for the necessary functionality patch!
I've tried to apply it to inline_entity_form-7.x-1.x-dev version (from 2012-Oct-12). Here is log:

sites/all/modules/inline_entity_form $ patch -p1 < \[inline_entity_form\]-\[clone\ button\]-\[1590146\].patch 
patching file inline_entity_form.module
Hunk #1 succeeded at 465 (offset 120 lines).
Hunk #2 succeeded at 495 (offset 120 lines).
Hunk #3 succeeded at 577 with fuzz 2 (offset 107 lines).
Hunk #4 succeeded at 696 (offset 160 lines).
Hunk #5 succeeded at 747 (offset 160 lines).
Hunk #6 succeeded at 787 (offset 160 lines).
Hunk #7 succeeded at 817 with fuzz 1 (offset 158 lines).
Hunk #8 succeeded at 1068 (offset 297 lines).

But have no luck :(.

Duplicate button appears but form is absent:
clone button

Any ideas how to make it works?

leanderl’s picture

Here's a clone button patch as of todays dev version of Inline Entity Form.

The clone button is enabled through "manage fields->product reference field" of the product display node.
There might be some more work needed on this, it has not been thouroghly tested, and I would be grateful for any help or feedback.

bojanz’s picture

This is high on my IEF todo list. Haven't forgotten about it. Thank you for your efforts so far.

vasike’s picture

Status: Needs work » Needs review
FileSize
9.76 KB

there were some issues with the previous patch

here is a reworked patch that make some changes:
- use the same form function(s) as add/edit
- it's supposed to work for every entity type, no only commerce product
the entity cloning preparation takes place in the "entity type" form controllers.

leanderl’s picture

@vasike - what a hero you are! I look forward to trying your patch... Thanks for the effort!

gynekolog’s picture

Thanks for your work.
I have one problem. I have a taxonomy field "maker" in product. Widget type is autocomplete. When I'm creating variation by IEF and write new term to "maker" and use clone button, cloned variation have maker field empty.
If i write manually the same maker like in first variation and save product display, I have 2 same term in taxonomy.

All is OK if I save product display after creation first variation. But it resides. Is here some better solution?

Excuse my english, I'm trying to.

edit: My bad. Solution -> http://drupal.org/node/1723864#comment-6337706

pyrello’s picture

Patch in #13 worked for me. I have a fairly complex product entity and it worked very nicely. This saved my day! Thanks to everyone who has been working on this thread.

cocoshogo’s picture

Needed this functionality as well. Applied patch #13 and I am getting this error :

Notice: Undefined index: clone_button EntityInlineEntityFormController->getSetting() (sites/my.site.com/modules/inline_entity_form/includes/entity.inline_entity_form.inc ファイル 124行).

(*Japanese is default language im using)

Any one know why im getting this. Im using the supported version of IEF; 7.x-1.1

vasike’s picture

@cocoshogo : indeed. thank you Sir.
This is because the Clone settings for the IEF field wasn't defined and there's no default one.

here is the new patch that adds the Default Clone button settings for IEF widgets.

kevinsiji’s picture

Status: Needs review » Reviewed & tested by the community

Patch in #18 worked for me. It smoothly clones my products which are also added with "Commerce price table", and many list attributes.

Dave Reid’s picture

Status: Reviewed & tested by the community » Needs review

This works really great, but I actually think that the Clone button should be disabled by default, and not enabled by default. I found this very frustrating when applying the patch, and having to turn off the Clone button on several already production-ready fields.

cocoshogo’s picture

Yes Patch in #18 worked :) thanx so much !

bojanz’s picture

Status: Needs review » Needs work

I agree with #20.
I'd also like to explore making the cloning more generic, we know how to do the initial step, just like Entity API does:

function entity_ui_clone_entity($entity_type, $entity) {
  // Clone the entity and make sure it will get saved as a new entity.
  $entity = clone $entity;

  $entity_info = entity_get_info($entity_type);
  $entity->{$entity_info['entity keys']['id']} = FALSE;
  if (!empty($entity_info['entity keys']['name'])) {
    $entity->{$entity_info['entity keys']['name']} = FALSE;
  }
  $entity->is_new = TRUE;

  // Make sure the status of a cloned exportable is custom.
  if (!empty($entity_info['exportable'])) {
    $status_key = isset($entity_info['entity keys']['status']) ? $entity_info['entity keys']['status'] : 'status';
    $entity->$status_key = ENTITY_CUSTOM;
  }
  return $entity;
}

The controllers could then reset any additional fields they'd want to (altering the sku, and so on).

I can do this prior to the commit, which will be sometime next week.

Dave Reid’s picture

Here's a re-rolled version of #18 that is off by default for make files. Still doesn't account for the request in #22 so leaving as needs work.

vasike’s picture

Status: Needs work » Needs review
FileSize
8.41 KB

here is a new patch that uses the entity_ui_clone_entity() function.

wangxb07’s picture

https://drupal.org/sandbox/wangxb07/2036977
look this. Reference vasike patches, packaged into modules

sibiru’s picture

Status: Needs review » Reviewed & tested by the community

#24
Thanks for creating this patch, it works on my project..
Are this patch applied on version & 7.1.2 ?

#25
Wow this is so good, are this module support generic entity? as @bojanz said in #22

tmsimont’s picture

Status: Reviewed & tested by the community » Needs work

this causes AJAX errors to occur if there is an element on the clone form that has an AJAX callback.

In the patch in #18, the error occurs on this line in inline_entity_form.module:

$cloned_entity = clone $form_state['inline_entity_form'][$delta]['entities'][$ief_row_delta]['entity'];

If using the patch in #25, this line in entity.ui.inc:

  $entity = clone $entity;

The error is:

An AJAX HTTP request terminated abnormally.
Fatal error: __clone method called on non-object in [depends on which patch you're using]

tmsimont’s picture

image fields trigger this error

tmsimont’s picture

this error comes from this part of the patches

// inline_entity_form_field_widget_form() ....
     //Display clone form here
      elseif ($form_state['inline_entity_form'][$ief_id]['form'] == 'clone') {
        $element['form']['#op'] = 'clone';
        $element['form']['#ief_row_delta'] = $form_state['triggering_element']['#ief_row_delta'];
        $element['form'] += inline_entity_form_entity_form($controller, $element['form'], $form_state);
      }

The assumption is that 'triggering element' will always be a trigger for a new cloned entity. In the case of image field uploads, or any other AJAX trigger, this assumption is not valid. If a field inside the clone form becomes a trigger during the form build operation, then Notice: #ief_row_delta is an undefined index in inline_entity_form_field_widget_form()

tmsimont’s picture

Status: Needs work » Needs review
FileSize
9.48 KB

patch in 24 re-rolled to latest dev, and fix for issue brought up in 27-29 attached.

vasike’s picture

here is a new patch that extends "Reference cloning" with Node clone module integration for "Inline entity form - Single value".
I implemented here and not in other issue, as it's built on the same code.

ex. 1 product to 1 product display
- similar products with different brand and price.
node clone could save a lot of time adding new product/products display if there a lot of other identical data in their structure.

also removed a trailing whitespace

@tmsimont : of course, thank you Sir.

vasike’s picture

Title: Add a "clone" button » Clone referenced entities

Retitling

kevinsiji’s picture

Applied patch in #30.

Working fine along with "commerce_pricing_attributes" also.

tmsimont’s picture

i found another problem..

if you have an image field,
1) create a new "parent" entity (like a product display)
2) Create a new "child referenced entity that is clonable" (like a product variation)
3) Enter fields and upload an image to first child (product variation)
4) Clone first child (product variation)
5) remove image from cloned child (new product variation)
6) save cloned child
7) save parent (product display)

4 errors then crash your site:
Notice: Trying to get property of non-object in file_field_presave() (line 220 of ../file.field.inc)
Notice Undefined property: stdClass$uri in file_save() (line 556 of ... file.inc)
and then 2 of these:
PDO Exception .... Duplicate entry for key 'uri' into {file_managed}

It seems like the crash is related to this: #1443158: file_field_presave assumes that a file object has been loaded

But even when you apply patch in #17 in that issue, there's still a problem. The crash is averted but then the first child (product variation) loses its image.

tmsimont’s picture

Status: Needs review » Needs work
vasike’s picture

Status: Needs work » Needs review
FileSize
11.01 KB

for #34 issue with cloning new (unsaved) entities:
here is a new patch that saves the original entity if is new before cloning.

about core issue and its patch #1443158-17: file_field_presave assumes that a file object has been loaded it does get rid of the errors. but the image field of the original entity it will be empty as for the clone.

another approach (to avoid this issue) is to have the clone button only for already saved entities.

airb’s picture

hi! here is another project dealing good with previously uploaded files (so filefield can handle already existing files) https://drupal.org/project/imce_filefield - maybe helps.
how is it working now with filefields? i have a lot of fields on a production site, would be hard to test now... ill find another way if it's not ready for testing... thanks!

eneko1907’s picture

Issue summary: View changes
Status: Needs review » Needs work
FileSize
9.94 KB

After version 1.3, the patches at #18 and #36 cannot be applied. For 1.3, those still work, I ran basic tests (clone an existing entity), that's OK.

However, for versions 1.4 onwards the patches do not work. I adapted them for 1.5, with limited success: my patches apply fine, and the clone button appears. After I click the clone button, this produces the result I expect: a new pre-populated entity form appears with the cloned content used to pre-populate fields. Problem is the "Save" button does not save the cloned entity. Boo. The thing I cannot adapt is the changes for the 1.3 hook "inline_entity_form_process_entity_form", which disappeared in versions 1.4 onwards. In any way, it would be great to see the ability to clone and existing entity back again, sorry I couldnt get it to work for recent versions. For what is worth, here is my attempt to re-take this work. http://drupal.org/files/issues/ief_clone_button-1590146-38.patch (just to insist, it is not working)

Poieo’s picture

This is really close. It actually works if you don't save the cloned variation and just save the entire product display.

Would love to get this working. I'll post back if I figure anything out.

dgastudio’s picture

any update?

zengenuity’s picture

Status: Needs work » Needs review
FileSize
10.63 KB
2.02 KB

I've made a few changes to the patch in #38 to make cloning save properly. This is working for me with commerce products on a commerce kickstart site. Have not tested in other situations, but the primary use case of most people on this thread (not having to re-upload the images for additional product variations) is working for me now on IEF 1.5 and head.

eneko1907’s picture

Awesome, I will test it as soon as I can. BTW, we do not use this ief-clone-feature on the commerce kickstart.

eneko1907’s picture

#41 seems to be working on some tests I ran outside the scope of the commerce -- thank you so much

yenidem’s picture

#41 worked for me, I use drupal commerce, it is not commerce kickstarter. Thank you very much for this feature.

annrockio’s picture

#41 works for me too, in commerce kickstart. Thanks!

radimklaska’s picture

I tested #41 and it worked great. Our content person is happy again :-) Thanks!

Poieo’s picture

Status: Needs review » Reviewed & tested by the community

Marking this RTBC so we can hopefully get it committed. Thanks @zengenuity!

danreb’s picture

#41 works for me too in dev version of Inline Entity form.. worked great..

Thanks

Chithra K’s picture

I tested this in Commerce Kicstart2 which uses the 7.x-1.5 version of Inline entity form and #41 just works fine! Thank you.

yannickoo’s picture

Status: Reviewed & tested by the community » Needs work
  1. +++ b/inline_entity_form.module
    @@ -767,6 +799,21 @@ function inline_entity_form_field_widget_form(&$form, &$form_state, $field, $ins
    +      //Display clone form here
    

    Here should be a space after //.

  2. +++ b/inline_entity_form.module
    @@ -767,6 +799,21 @@ function inline_entity_form_field_widget_form(&$form, &$form_state, $field, $ins
    +        } elseif (isset($form_state['ief_last_ief_row_delta'])) {
    

    This elseif should start in a new line.

danreb’s picture

Status: Needs work » Needs review
FileSize
10.64 KB
10.64 KB

code from #41 + the cleanup suggestion from yannickoo. This is a very nice feature so hoping to be included in IEF

Thanks

danreb’s picture

danreb’s picture

Ok just to make sure.. don't know why the attachement doubled up... here it is again

yannickoo’s picture

Amazing! Thank you so much for this functionality Adolfo, the patch works fine for nodes. We need to check product and custom entities support.

slybud’s picture

Well done guys and thank you for the work. The patch provided in #53 works fine for my use case

  • big drupal commerce website
  • one product display node with one product entity, using inline entity form
  • node_clone module installed
  • Using the "clone content" link on the node/xxxx product display page

It works with special fields like files and images + it empties the SKU field of the duplicated entity => really nice in my case
I've also tested deletion of cloned product to make sure.

So thank you, I think we need one ore two more feedbacks to mark this as RTBC

nicktr’s picture

#53 works great!! Thank you Danreb and all other contributors!

haggins’s picture

Status: Needs review » Reviewed & tested by the community

#53 works. Images are also cloned. Thank you!

Setting to RTBC due to #54 - #57

djdevin’s picture

We've been using this in production for about 3 months now, works as expected.

- Node with an IEF field, referencing a single instance of an Entity type with a bunch more fields
- Cloning the node, also clones the referenced Entity

bojanz’s picture

Status: Reviewed & tested by the community » Needs work

Good job everyone, this is almost done.

+    // Clone product
+    if (isset($entity_form['#cloned_entity'])) {
+      $product->revision_id = NULL;
+      $product->sku = NULL;
+      $entity_form['#entity'] = $product;
+    }

Let's create a clone() method on the controller, use it for this purpose, call it from inline_entity_form_clone_entity(). It will make the code cleaner.

+  // Save the original entity if is new before cloning.
+  // See this comment: https://drupal.org/node/1590146#comment-7769657.
+  if (!empty($entity->is_new)) {
+    entity_save($entity_type, $entity);
+  }

No, we can't do this. Only saving the parent form can trigger other saves.
There's no reason why the cloned unsaved entity shouldn't work just like the original unsaved entity does, feels like an error on our side?
Let's retest without the workaround, since it's 2 years old, and both Core and Entity API have changed since.

+      '#title' => t('Enable Clone button'),

Let's make this title more user friendly. t('Allow users to clone @label.', array('@label' => $labels['plural']))

+      // Node clone integration - clone reference.
+      if ($node_clone) {
+        $element['form']['#op'] = 'clone';
+        // Unset original node reference field data to be cloned.
+        unset($entity_form['#ief_row_delta']);
+        $form_state['inline_entity_form'][$ief_id]['entities'] = array();
+        $form['#node']->{$element['#field_name']} = NULL;
+      }

Let's remove the node_clone integration. This code won't work with nested IEF widgets, and the "remove references on clone" logic doesn't belong in a field widget.

I'm working on fixing this.

bojanz’s picture

Status: Needs work » Needs review
FileSize
11.08 KB

Here's a new patch. It addresses my feedback, performs additional cleanup, checks whether cardinality has been reached and entity access is allowed before showing the Clone button.

Please give it a test, paying special attention to the previously mentioned ajax problems.

njbarrett’s picture

Status: Needs review » Needs work

I manually tested the patch and had a few usability issues. I noticed that on a field with cardinality of 1 using the multiple value widget, the clone button is hidden because the field can only have 1 value. Is there anyway I can clone an existing entity in this case? There's no indication that the feature doesn't work on fields with a cardinality of 1, as it's still enabled on the single value widget as an option that does nothing.

I would love to be able to have a button next to "add existing entity" that says "clone existing entity" which has an autocomplete text field just like the "add existing entity" button but instead of just adding it, it clones it first and then adds it. Seems like then it could work even on fields with a cardinality of 1, and also when #2134035: Allow to add existing entities using the single value field widget gets committed it could work on on the single value widget as well.

bojanz’s picture

I manually tested the patch and had a few usability issues. I noticed that on a field with cardinality of 1 using the multiple value widget, the clone button is hidden because the field can only have 1 value. Is there anyway I can clone an existing entity in this case?

No, because you've reached the maximum number of values the underlying reference field can accept.
There's no workaround for that.

I would love to be able to have a button next to "add existing entity" that says "clone existing entity" which has an autocomplete text field just like the "add existing entity" button but instead of just adding it, it clones it first and then adds it.

That's unrelated to the current feature request.

njbarrett’s picture

Ok I misunderstood the requirements of the issue. I guess it helps people who want to clone other referenced entities but not clone entities that aren't already referenced.

In that case I still believe there shouldn't be the option to allow users to clone entities on the inline form single value widget, as it's not possible.

eneko1907’s picture

Tested patch at #60, clone functionality is there, working well for my use case.

Thanks!
Inigo

ASGAlex’s picture

#60 works well, but with "Auto generate the product title" setting there is small misleading bug: after cloning an variation the title field is shown like cloned too. Autogeneration itself will be triggered on product save.

When adding product variation with title autogeneration, the message "Will be auto-generated when the form is saved." displays in place of title.

bojanz’s picture

Status: Needs work » Fixed

Committed #60.

Thank you everyone for this amazing multi-year effort.

  • bojanz committed edbfeba on 7.x-1.x
    Issue #1590146 by leanderl, vasike, danreb, zengenuity, tmsimont, bojanz...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

John Pitcairn’s picture

RAWDESK’s picture

Great effort to improve usability ! I am in big favour of #60

BITANUBE’s picture

+1000 for the patch #60 . Can't not be applied with drush or patch manager but manually was a great solution.
Congratulations !

bojanz’s picture

@BITANUBE
There's nothing to apply, it has been in -dev for the last 8 months.

BITANUBE’s picture

Ouch ! Thanxs ! For the next time !

belaustegui’s picture

@bojanz I'm trying to find this functionality in the 8.x version of this module, but it seems that it is only available in the 7.x version.
Is this correct?

bojanz’s picture

@belaustegui
Correct, nobody has ported it yet.

AntiNSA’s picture

Where is it in the version? I am trying to clone variant... but cant find it anywhere...

ilya.no’s picture

Version: 7.x-1.x-dev » 8.x-1.0-beta1
FileSize
9.12 KB
9.12 KB

Hello all!
Thanks for your efforts and the patch! I've created D8 patch, based on it. As stated in this comment, I've used "Duplicate" instead of "Clone". Patches are for 8.x-1.0-beta1 and dev versions.

ilya.no’s picture

Version: 8.x-1.0-beta1 » 8.x-1.x-dev
Lukas von Blarer’s picture

This issue is closed. Can a maintainer re-open it or otherwise we create a new one.

bojanz’s picture

Opened #2962706: Add the ability to duplicate referenced entities and moved the #77 patch here.

In the future please don't respond to closed issues, I completely missed this.