Clean D7 install
Add a Basic Page
Enable webform
Configure webform to apply to Basic Page
Try to add webform components to node added above. Can't. (Upon submission of the component form, there's a message saying the component was added, but it's not in the component list for the node.)
Create a new Basic Page node
Try to add webform components to it. Works ok.
The problem is webform_node_update() issues a drupal_write_record() that generates an UPDATE query only, even if a webform entry for that node has never been made.
Here's a fix.
| Comment | File | Size | Author |
|---|---|---|---|
| #24 | webform-content-type-followup-1008514-24.patch | 491 bytes | David_Rothstein |
| #16 | webform_record_exists-d6.patch | 15.58 KB | quicksketch |
| #16 | webform_record_exists-d7.patch | 14.83 KB | quicksketch |
| webform-node_update.patch | 2.52 KB | effulgentsia |
Comments
Comment #1
quicksketchI thought we already fixed this problem in the Drupal 6 version of Webform, it'd be unusual that we didn't fix it in Drupal 7 also. We'll need to check if this problem still exists in Drupal 6, and if not, what solution we used there.
Comment #2
effulgentsia commentedNope. The exact same bug exists on D6.20, Webform 6.x-3.x-dev. Can you link to the issue where you think it was fixed?
Comment #3
quicksketchOkay here we are. There is another exact issue at #951452: Component not saved/created, but we didn't solve the problem there. I've marked it duplicate since this issue is more clearly stated and includes a patch.
Comment #4
quicksketchI'm not sure why this patch works. We don't call node_save() anywhere when adding a new component; the record is inserted directly into the webform_component table and not through hook_node_insert/update(). So I'm not sure why this patch would help anything (other than the update function which fixes all the existing, but not new, types that have the problem). Perhaps what we should do is manually check for a $node->webform['nid'] in both webform_component_insert() and webform_email_insert() and call webform_node_insert() if necessary?
Comment #5
quicksketchI figured out why this patch works for Acquia but not for me. In Drupal Gardens, you guys combined the component form and the node form together. So when you add a component you call node_save(), thus fixing the problem in changing hook_node_update() works for you. For other Webform users, node_save() is not called when adding a new Webform component so this patch does not add a new row into the "webform" table when a new component is added.
Perhaps we should consider using node_save() in webform_components_form_submit() in addition to this change? I'd also like to get rid of the weird watchdog() call and make it so that the $node->nid always overrides $node->webform['nid'].
Comment #6
effulgentsia commentedHey quicksketch, thanks for digging into that. Sorry for not responding yet. I still plan on looking more into this, but first I have some more work to do in the Media queue. I hope to get back to this soon though.
Comment #7
Rob_Feature commentedThis doesn't seem to be affecting everyone or this issue would be much longer :) Is there a particular config that's causing this or should it be universal? I mostly ask because I see this behavior on some content types (ubercart product classes) but not others (manually created content types)
Comment #8
aanjaneyam commentedI had this issue on the webform content type itself. I never enabled webform for an existing content type. I just installed webform and was created a node for the default webform content type created by the module install. No setting changes almost all the things default. Well I haven't tested it again and it may well have disappeared due to recent changes in Drupal (as quicksketch stated in the other issue which I created and was marked as duplicate)
Comment #9
Teppo Tulppu commentedOn one of my sites, content type webform was created automatically and I was able to add form fields. On another site content type webform was not created automatically and I was not able to add form fields - rendering the module useless.
Comment #10
claudio_c commentedI have problems with the patch as well:
"Fatal error: Cannot redeclare webform_update_7306() (previously declared in /Applications/XAMPP/xamppfiles/htdocs/bbs/sites/all/modules/webform/webform.install:473) in /Applications/XAMPP/xamppfiles/htdocs/bbs/sites/all/modules/webform/webform.install on line 490".
I am using the 'bartik' theme.
Comment #11
miggidy commentedI'm cross-posting this comment from: #951452: Component not saved/created to help future researchers since it worked for me:
*Temp*
To fix this for now, just edit and save the node.
This causes Webform to run it's {webform} check and it will insert your node entry for you.
What we'll need to do is run the check code found in the webform_node_update function (webform.module line 904-911) when components are added / updated... or something to that extent.
We could add this when the Webform tab is loaded (webform_components_page) or when a component is inserted / updated.
Comment #12
dgdgdg commentedMore info ...
I had a content type defined.
I added Webform
I can create new forms under new added content type: Webform
I can NOT create new forms under the previously existing content type and node (see FYI below)
I created new node using original existing content type -> CAN create webform on new node. It DOES show.
FYI
components on original existing content type and existing node are being created in webform_component table ... just not showing (even after clear cache)
Comment #13
Anonymous (not verified) commentedI'm experiencing this same problem. Added 'basic page' to node types associated w/ webform; tried to add fields; message confirming field was added, but it really wasn't. Also, no 'save' button appears, since there are no changes to save.
Comment #14
quicksketchThis is intentional behavior, so I'm not sure why you're mentioning it here. The "Save" button only appears after there are components that you can order. Previously it was rather confusing with both an "Add" and "Save" button shown when there were no components at all. Because there was nothing to save, the "Save" button didn't do anything at all, which was very confusing.
Comment #15
Anonymous (not verified) commentedJust trying to be complete. Of course, it's the intended behavior. Sorry if I muddied the waters.
Comment #16
quicksketchI've taken this patch a different route which I think will be sustainable and at the same time also addresses a few other design issues with the 3.x version. Over in #902608: Delete the admin/content/webform administration page(?) we've discussed that admin/content/webform needs to be "fixed" so that it only includes content that actually has a Webform attached to it. The problem we have is that right now every node that is webform-enabled is expected to have a row in the "webform" table. But as noted here, that causes a problem when you webform-enable a content type that already has a lot of existing rows.
This patch takes the approach where an entry in the "webform" table is only created if there is webform data at all. So that means if you change any of the default webform settings, add a component, or add an e-mail, the Webform module will make an entry into the webform table. If everything is in its default state (no components, no e-mails, and the default settings), then this entry will be removed again.
So this approach is somewhat opposite our current patches. In the original patch attached to this issue, an update function created a row for every node that was webform-enabled. This patch on the other hand actually goes the other way and removes more of the existing webform rows, removing any webform row that doesn't have any matching components (essentially deleting rows that don't have a form attached to them). A nice benefit is that it fixes the issue with admin/content/webform as a side-effect.
I've reviewed both patches several times and added a new SimpleTest to ensure that these rows are added/removed as appropriate. Committed to both 3.x versions.
Comment #17
Anonymous (not verified) commentedd7 patch failed (7 out of 7 hunks not applied.)
Comment #18
quicksketch@seezee: I've already committed the patch to CVS HEAD, so if you're applying the patch again all the hunks will fail. I'd suggest just using a clean check-out.
Comment #19
Anonymous (not verified) commentedOK, thx.
Comment #20
gbernier commentedWill there be a new release soon with this fix quicksketch? I was going to use this on D7 site but if it's going to be awhile I'll go another route.
Comment #21
quicksketch@gbernier: Yes I've been trying to get out a new release for the last week or so. It'll definitely be out in the next couple of days.
Comment #22
gbernier commented@quicksketch Awesome really waiting for this fix for a our first D7 site
Comment #24
David_Rothstein commentedThe tests added here aren't passing, and it seems to be an actual bug with the functionality, due to this:
The attached patch uncomments the return statement, which makes the tests pass :)
Comment #25
quicksketchWeird. Thanks David.
Comment #26
quicksketchCommitted David's fix in #24.