I was just doing some testing on 5.x-2.6 and noticed an interesting issue. I enabled signups on a node and it worked as expected. I then disabled signups for that node but the signup form was still being displayed because there was still a row in the signup table for the node. There are no rows in signup_log for this signup.

I traced signup_save_node() and determined that the code at the bottom of the function that is supposed to delete the db entry is not working correctly because has_signup_record is never set in the case.

I just removed the 'if' test and delete the signup row unconditionally.

CommentFileSizeAuthor
#3 335190_disable_signup_per_node_bug.D5-2.patch806 bytesdww
#3 335190_disable_signup_per_node_bug.HEAD_.patch823 bytesdww
signup_disable.patch501 bytesbkat

Comments

bkat’s picture

bkat commented
Title: Disable signup on load leaves signup enabled » Disable signup on node leaves signup enabled

correct typo in title

dww’s picture

dww
we/he/they
commented
Assigned: Unassigned » dww
Status: Active » Needs work

Thanks for the report. Sorry about the bug. :( I'll take a closer look at this tomorrow, but I'm not surprised I broke this... ;)

dww’s picture

dww
we/he/they
commented
Status: Needs work » Needs review
StatusFileSize
new 335190_disable_signup_per_node_bug.HEAD_.patch823 bytes
new 335190_disable_signup_per_node_bug.D5-2.patch806 bytes

Yeah, upon closer inspection of the code, your patch is basically the right solution now that signup_node_save() exists (see #328822: Remove global $form_values from hook_nodeapi() and otherwise clean up the logic). So, I just rerolled the patch not to leave the dead code commented out, and ported to HEAD. Tests of both of these patches welcome.

dww’s picture

dww
we/he/they
commented
Status: Needs review » Fixed

Did some more testing myself, and committed to HEAD and DRUPAL-5--2. This will be out in 6.x-1.0-rc2 (or 6.x-1.0 if I jump straight there) and 5.x-2.7.

bkat’s picture

bkat commented

Works for me obviously. Thanks!

Status: Fixed » Closed (fixed)

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