Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Tried to find corresponding issue - but could only find closed 6.x issues.
My question is: What should "admin/content/webform" show?
Right now it shows all nodes, that potentially could have a webform. I instinctively feel that it should show alle nodes with a specific webform, with at least one component. I guess the record_exist value should be TRUE.
But maybe it's just me :-)
Comment | File | Size | Author |
---|---|---|---|
#9 | webform-3.x-only_show_webform_nodes-1562756-9.patch | 876 bytes | weseze |
Comments
Comment #1
quicksketchThis page shows:
- ALL "webform" nodes.
- Any other webform-enabled nodes that have ANY CHANGES to the webform default configuration (components, e-mails, settings, etc.)
The second condition is enforced indirectly. Since Webform will not allow rows in the "webform" database table unless the webform settings differ from the defaults. Note that other modules that extend Webform may break this rule if they don't properly implement hook_webform_node_defaults_alter().
Comment #2
quicksketchUnless there are further questions.
Comment #3
mazaza CreditAttribution: mazaza commentedActually there is - sorry for the delay - but I tried to make certain that I'm not missing something completely.
If I enable webforms (admin/config/content/webform) on a content type called "Page", and I have 1000 pages - but only 5 of them have webforms - the admin/content/webform still shows 1000 pages.
I tried to look at the code, and could find anything that tried to "hide" the 995 pages without a form.
webform.admin.inc
This returns all "1000 Pages". Looking further into the node-object - loading it - I found that the $node->webform['record_exists'] was set to TRUE for my 5 pages with webforms.
So writing myself a new theme function (theme_webform_admin_content), I could make a little test, like this:
...
$node_test = node_load($node->nid);
$has_webform = $node_test->webform['record_exists'];
if ($has_webform) {
.. build row
}
Actually the Record Exist - set by the Hook you mention - seems very, so I wondered why it wasn't used.
I think that would be the "normal" behaviour. - only showing pages with webform! But I'm sorry if I've missed something completely.
Comment #4
quicksketchHm, you're right. That's quite dumb. We go through all this work to make sure that the "webform" database table is nice and clean, but then the SELECT statement is just directly against the "node" table! We should be able to fix this easily enough by adding an INNER JOIN to the webform table.
Comment #5
mazaza CreditAttribution: mazaza commentedThanks for the reply.
INNER JOIN seems right to me too.
Comment #6
alisonHi all -- did you end up implementing what was described in quicksketch's comment? (#4)
It doesn't look like it to me, but I haven't dug too deep yet, wanted to check in with this issue queue first.
Comment #7
quicksketchNo this hasn't been fixed yet. I'm the maintainer too, so I just need to implement my own suggestion. ;)
If you're interested in making the change yourself and filing a patch that could help cut down on testing time for me.
Comment #8
alisonOkay :) I will try!
Comment #9
weseze CreditAttribution: weseze commentedAttached is a patch that changes the query with a join on the webform table to only show nodes that actually have a webform.
Comment #10
weseze CreditAttribution: weseze commentedComment #11
quicksketchThanks @weseze! I'll try to make sure this gets in the next release.
Comment #12
ParisLiakos CreditAttribution: ParisLiakos commentedclosed #1995608: admin/content/webform can crash as duplicate
Comment #13
quicksketchUpon applying this patch, I get this error on the admin/content/webform page:
Looks like
fetchAllAssoc('n.nid')
just needs to befetchAllAssoc('nid')
(like ParisLiakos's version in the other issue). Corrected and committed.Comment #15
caspervoogt CreditAttribution: caspervoogt commentedjust wanted to chime in to say I too have been struggling with this one. I just updated to 7.x-3.19 and this patch apparently is not included there. So I switched to the dev branch and that works great. But I thought you should know it's not in 7.x-3.19, and I think it ought to be.