Imported form components out of order, some missing

This patch fixes the component order by using the component id (cid) from the import file. This ensures that components with a parent id (pid) are linked to the correct parent.

It works for an import into an empty webform but not if there is an existing component with the same cid. In that case we need the work proposed here: https://drupal.org/node/1289958#comment-6332384

It doesn't fix a problem with component relations. So i just removed both checkboxes from import form and rewrite import code to

function webform_share_components_update_form_submit($form, &$form_state) {
  $node = $form_state['node'];
  if ($webform = _webform_share_eval($form_state['values']['import'])) {
	  //YES I UNDERSTAND THIS IS BAD. BUT THERE'S NO OTHER WAY TO SAFELY CLEAN ExISTING WEBFORM
	webform_node_delete($node);
	unset($node->webform);	  
	webform_node_insert($node);
        $node->webform = $webform;
	foreach($node->webform['components'] as &$component)
	{
           $component['nid']=$node->nid;
	}
	 
      $webform['nid'] = $node->nid;
     node_save($node);
  }
  $form_state['redirect'] = 'node/' . $node->nid . '/webform';
}

for those who wants to use Only components... and Keep existing... settings to work should fix this to smth like

function webform_share_components_update_form_submit($form, &$form_state) {
  $node = $form_state['node'];
  if ($webform = _webform_share_eval($form_state['values']['import'])) {
    // Load the original node and use this to map any fields based off
    // the form_key parameter. We need to preserve the existing cid if
    // possible to preserve the submission data.
    $original = node_load($node->nid, NULL, TRUE);
	// Overwrite the entire form if the user is updating everything.s
	if (empty($form_state['values']['components_only']) && empty($form_state['values']['keep_existing_components'])) {
	  //YES I UNDERSTAND THIS IS BAD. BUT THERE'S NO OTHER WAY TO SAFELY CLEAN ExISTING WEBFORM
	  webform_node_delete($node);
	  unset($node->webform);	  
	  webform_node_insert($node);
      $node->webform = $webform;
	  foreach($node->webform['components'] as &$component)
	  {
		$component['nid']=$node->nid;
	  }
	 
      $webform['nid'] = $node->nid;
    }
	if (!empty($form_state['values']['keep_existing_components']) || !empty($form_state['values']['components_only'])) {
		$existing_components = array();
		foreach ($original->webform['components'] as $cid => $component) {
		  $existing_components[$component['form_key']] = $cid;
		}
		$next_id = empty($existing_components) ? 1 : max($existing_components) + 1;	
		ksort($webform['components']);
		// Map the imported components to the existing webform components.
		$node->webform['components'] = array();
		//store old keys to restore child -> parent associations
		$oldKeys=array_keys($webform['components']);
		foreach ($webform['components'] as $index => $component) {
		  if (isset($existing_components[$component['form_key']])) {
			$cid = $existing_components[$component['form_key']];
			unset($existing_components[$component['form_key']]);
		  }
		  else {
			$cid = $next_id++;
		  }
		  //store new key against old values
		  $oldKeys[$component['cid']]=$next_id;
		  $component['cid'] = $cid;
		  $component['nid'] = $node->nid;
		  $node->webform['components'][$cid] = $component;
		}
		//replace old parent ids with new ones
		foreach ($webform['components'] as $index => $component) {
			if(isset($component['pid']) && $component['pid']>0 && isset($oldKeys[$component['pid']]))
				$component['pid']=$oldKeys[$component['pid']];
				
		}
		// If requested, re-add the existing components rather than allowing these
		// to be deleted. Existing cid values are safe to reuse.
	   if(!empty($form_state['values']['keep_existing_components']))
		  foreach ($existing_components as $form_key => $cid) {
			$node->webform['components'][$cid] = $original->webform['components'][$cid];
		  }
    }
    node_save($node);
  }
  $form_state['redirect'] = 'node/' . $node->nid . '/webform';
}

This should keep your existing components intact and will process your relations (pid->cid) to set new values. Though i do not test this as i don not need this. THIS IS JUST MY SUGGESTION SO BE CAREFULL

That's great, it works for me. I've created a new patch from your code. Thanks.

changed status

I don't think we can delete and recreate sadly.

+      webform_node_delete($node);
+      unset($node->webform); 

First line will delete all webform components from db
second will remove webform instance from node and this will allow us to create new webform instance. so this is just an emulation of node_delete and node_insert

If I check to NOT leave the current components, I get an error re: a key_restraint

My code wasn't supposed to work. i do not need these options so i do not fix 'em and just suggest for other coders
Import with all options disabled works like a charm with any component and relations

I also have a fieldset that fails to import properly.

If I leave the default webform field empty in the content type and simply try to import into another node, nothing happens if both option boxes are unchecked. If either or both option boxes are checked I get:
PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '10-1' for key 'PRIMARY': INSERT INTO {webform_component} (nid, cid, pid, form_key, name, type, value, extra, required, weight) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7, :db_insert_placeholder_8, :db_insert_placeholder_9); Array ( [:db_insert_placeholder_0] => 10 [:db_insert_placeholder_1] => 1 [:db_insert_placeholder_2] => 0 [:db_insert_placeholder_3] => staff_contact [:db_insert_placeholder_4] => Staff Contact [:db_insert_placeholder_5] => hidden [:db_insert_placeholder_6] => [node:author:mail] [:db_insert_placeholder_7] => a:2:{s:7:"private";i:0;s:11:"hidden_type";s:5:"value";} [:db_insert_placeholder_8] => 0 [:db_insert_placeholder_9] => 0 ) in webform_component_insert() (line 760 of /Users/shawn/Sites/edli/sites/all/modules/contrib/webform/includes/webform.components.inc).

If I paste the import code into the webform default components field in the content type settings, then on content creation I get a webform that contains all the components but the fields are not assigned to the fieldset - the fieldset is empty and the fields are siblings to the fieldset.

The patch in #3 doesn't fix the problem with setting a default webform in the content type. The form is created but the fieldset element appears at the end, empty. Since my issue is caused specifically in the code for inserting default webforms in new nodes: webform_share_node_insert(), I've split that off into it's own issue: #2081745: Default Webform Fieldset Members Loose Their Assignment on Node Insert

Can confirm #11 as a workaround. With the simple patch attached to that other issue, webform components will get properly populated when the WS-export code was pasted into the content type's "Web form default components" textarea box.

#1 worked for me and solved the missing fields in imported fieldsets - many thx!

@kevster: Did you mean #1 or #11? For me, the solution pointed out in #11 worked.

Hi @roball - #1 worked for me, I manually patched as was a simple change - I scrapped the curent form and started a new one and all went in ok.

It appears the main issue is caused by not tracking original parent component IDs to the new generated component ID. This leaves out nested components because the hierarchy can't be properly built by Webform.

Also because of potential nested components, the original method of getting the "next_cid" may fail causing "PDOException: SQLSTATE[23000]: Integrity constraint violation" errors.

Patches #1 and #3 did didn't fix the problem for me. I'm attaching a patch the tracks old "pid" to "cid" values and correctly sets them during the import. The patch also calls to the database to get the "next_cid".

I hope the OP doesn't mind but I updated the original issue summary with these additional details. I also linked other issues I believe to be related to this.

IMHO, this bug makes this module unusable in most situations. I hope my patch helps others.

Posting a new patch after finding a bug when tested on our production environments. Don't use #16, use #19 instead.

@cravecode - this sounds like it could also address the related issue #2081745: Default Webform Fieldset Members Loose Their Assignment on Node Insert I'm looking forward to testing it!

This patch adds rules export and import capability to the webform_share module.

@mmkc thanks for the contribution! I'm a bit confused though, how is patch #21 related to this specific issue?

Please ignore comment #21, a new issue was created and a new patch for that is in the queue waiting for review.

Hi,

The patch in #19 worked perfectly for me, so I've cleaned it up a little to meet Drupal coding standards and removed the unnecessary comments.

See attached!

Can we get this committed? This module could do with some TLC!

I think that there is a small logical flaw here:

$cid = $next_id++;

Which would make $cid === max cid. So I think it should be this??

$cid = ++$next_id;

But pushed to dev.

Please report any issues.

Cheers everybody (and sorry for taking so long to get into the queues)

Or maybe not? Was the $cid = ++$next_id; change inc things by one to many?