Updated: Comment #7

Defining a default webform on the content type settings form that includes a fieldset does build a webform on node creation that includes all the components of the exported form, but components assigned to a fieldset appear outside their fieldset. The fieldset is rendered empty.

Fieldset components appear empty because in commit 18e4a9c the pid of components is explicitly set to zero:

<?php
      foreach ($node->webform['components'] as $index => $component) {
        $node->webform['components'][$index]['nid'] = $node->nid;
        $node->webform['components'][$index]['pid'] = 0;

?>

Commenting out the assignment here allows the default webform for the content type to build properly, with components assigned to the fieldset.

What problem is resetting pid to 0 in webform_share_node_insert solving?

CommentFileSizeAuthor
#1 webform_share-keep_pid-2081745-1.patch877 bytesFatherShawn
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

FatherShawn’s picture

Here's the patch removing this line in the event that doesn't cause problems for other use cases...

roball’s picture

Issue summary: View changes
Status: Active » Reviewed & tested by the community

Thank you FatherShawn for pointing out this problem source and providing the simple but effective patch. When pasting a ws-export code into into a webform-enabled content type's "Web form default components" textarea box
at "Edit (admin/structure/types/manage/webform) -> Publishing options", a newly created node of that type will NOT have the webform components populated properly if the webform contains fields within fieldsets.

After applying the patch of #1 the webform components will get properly populated.

I am not aware of side effects the patch may introduce. I can confirm it provides a very important fix for a critical bug.

Pasting the ws-export code into an existing webform-enabled node via the "Webform -> Import" does not work properly - that would create a horribly wrong form and spit a lot of PHP error messages (see for example #1956622: Imported form components out of order, some missing for this other critical bug).

FatherShawn’s picture

@roball I'm glad the patch works for you too! It is still working fine on the site I'm developing as well.

jamesbisset’s picture

Not working here I'm afraid.

I have a webform with around 80 fields on multiple pages, in fieldsets. I've modified the code in my Webform Share 7.x-1.x-dev to remove the line identified above in #1.

When I import the webform to a new node with no components on a different site, the new component page lists the page breaks and the fieldsets, but no fields inside the fieldsets.

However, inspecting the database reveals that all the components have been inserted, complete with cids and pids. So although in the admin UI or the node View I can see around 10 components, in the database I can see 80.

I thought I'd spotted something significant in the way the 'extra' column data was being written out and imported, but even if there are discrepancies, I'm not sure why that should make a difference.

Webform 7.x-4.0-beta 3

jamesbisset’s picture

Status: Reviewed & tested by the community » Needs work

I took another look in the clear light of day...

Webform Share allocates each component on import a new consecutive id (cid) starting at 1.

The unpatched Webform Share also zeros any parent id (pid) on import, which means that the relationship between fieldsets and fields is broken.

The patch at #1 keeps the original pid intact, but crucially, the pid on import may no longer match the cid of the parent.

Example:

Original fieldset - cid: 67, pid: 0
original field - cid : 68, pid: 67

imported fieldset - cid: 5, pid: 0
imported field - cid: 6, pid: 67

Sometimes, blind luck will mean that imported fields remain in their field sets. But it's just as likely that fields will vanish altogether because the pid references a fieldset which does not exist.

So the solution will involve some way of maintaining the parent child relationship between fields and fieldsets, while rewriting the cids and pids on import.

FatherShawn’s picture

Is the underlying problem then that component id [cid] is not unique between installations?

So it seems to me that we either work with webform along the lines of #298268-15: Clone across forms (D5&D6) to get cid to be unique, or we process the webform on export and/or import.

FatherShawn’s picture

Issue summary: View changes

@jamesbisset I spent more time with this code this morning and re-read your original post. Your use case is webform import, which is being discussed in #1956622: Imported form components out of order, some missing. This issue pertains to a very similar problem when pasting the import into the content type config so that it is generated on every new node. So the code I'm looking at is in webform_share_node_insert.

awolfey’s picture

Status: Needs work » Reviewed & tested by the community

I can confirm that #1 works for node inserts. Thanks FatherShawn.

Alan D.’s picture

Priority: Critical » Major
Status: Reviewed & tested by the community » Fixed

What problem is resetting pid to 0 in webform_share_node_insert solving?

Nasty PDO exceptions from memory's way back in a past life, but maybe overzealous due to the other errors??

But with other changes, this being an import on a clean form, this should be ok...

For better or worse, committed.

Thanks

  • Alan D. committed 3a9dbe1 on 7.x-1.x authored by FatherShawn
    Issue #2081745 by FatherShawn: Default Webform Fieldset Members Loose...

Status: Fixed » Closed (fixed)

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

roball’s picture

Thank you Alan D. for committing this and a view other important patches! Now it would be nice to have a new release (7.x-1.3). The current one is already more than 3 years old, anyway. What do you think?

Thanks!