Signup limits per node

alrighty. i'm gonna dig into this patch now and see what we've got here... :) hopefully i'll have it or a variation of it committed by next week.

ok, first lesson in patches 101. you can't generate a patch against a file you've edited in a win32 editor... :)

see here for details and suggestions on this rather odd problem:

did you notice your patch is twice as big as the entire module file?? makes it a bit of a bitch to review... :P a normal patch will only include the lines you've changed, plus a few lines around your change. this is called a unified diff, and you'll want to make sure that you're generating your patch in this format, preferrably with the argument to also include the function where each hunk is from (which is -F ^function in a *nix environment).

i'll be eagerly awaiting your next effort... ;)

may i recommend zend studio for your editing needs. it's not free, but they do give a free trial period--and believe me, it's worth the money if you buy it!

gravyface: oh, dude--you're gonna kick yourself...

zend studio has a setting that lets you choose between line ending style--you've probably got yours set on win32 style... :/

also, could you re-roll w/ this syntax?

diff -u -F ^function signup.module.old signup.module.new > signup.patch

the -F ^function part adds the name of the function to each hunk, which makes reviewing the patch much easier.

oh, and it's also quite a nice timesaver, if you've got cvs working, to just diff against the repo. check the handbook for how to do that...

gravyface: looking at the latest patch, i'm pretty sure it wasn't generated against a current version of signup... :) outside of a few changes to the time functions (which i'm not so sure are needed) and the removal of some form groups, nothing much is added by this patch. thanks for playing, please try again... :P

gravyface: i get an 'access denied' when i try to open that last attachment... :/

how about this for a plan:

email me the patched signup.module, and i'll run the diff against cvs from here. if you don't have my email addy, contact me via my drupal contact page and i'll send it to you.

we'll get this sucker eventually... :)

hm. never got any of your messages. can you catch me on irc? nick is hunmonk...

Great work gravyface - the seat limits are brilliant!

My users complained that after they had signed up they could no longer see how many seats were left, so they would 'cancel signup' and then 'signup' again - a real pain especially if you're getting a notification email each time!

Disclaimer: I've never done any patches and am not a PHP programmer by any means :P
Anyway, I hacked around with the code, and pretty much duplicated some code so that remaining seats show for users who have signed up already. I also included a little bar graph for fun.

I run Windoze so the patching system seems like a drama - can anyone help?

I inserted this at line 297.

$form_data = $hidden;
$total_signups = db_num_rows(db_query('SELECT nid FROM {signup_log} WHERE nid = %d', $node->nid));

if ($node->signup_close_signup_limit > 0) {
$form_data .= "<br />Seat Limit: <strong>" . $node->signup_close_signup_limit . "</strong>";
$form_data .= "<br />Seats Registered: <strong>" . $total_signups . "</strong>";
$form_data .= "<br />Remaining Seats: <strong>" . ($node->signup_close_signup_limit - $total_signups) . "</strong><br /><br />";

// ridiculous bar graph and % readout
$form_data .= "<div style=\"border: 1px solid #666; width: 100px;\"><div style=\"background-color: #ccc; width: " . round(100*($total_signups/$node->signup_close_signup_limit)) . "px\">&nbsp;</div></div>&nbsp;" . round(100*($total_signups/$node->signup_close_signup_limit)) . "% Registered <br /><br />";
}

$form = form($hidden . $form_data . form_submit(t('Cancel Signup')), 'post', url('signup', drupal_get_destination()));
$output = form_group(t('Event Status', array('%title'=>$node->title)), $form);

Yes, I realise the implementation of the bar graph is really bad but my excuse is that it was a bit of an afterthought - that line can be removed completely if necessary.

gravyface: any progress on getting your work into an applicable patch?? the module has undergone several upgrades recently, so you might need to rework things a bit...

kiemce: you'll get along much better in drupal development if you can get CVS working. :) i would personally recommend cygwin--it's been a real lifesaver for me.

hunmonk and gravyface:

I'm not sure what the status of the patch is. So i figured i'd try to sort out the problem creating the patch.

The CVS Id parameter from the top of the file that gravyface included in comment #10 says the version is 1.1.2.5. I checked out a copy of 1.1.2.5 and then did a diff with the module from comment #10.

Lemme just say that it wasn't pretty...gravyface, if you want to resubmit your patches to the community, you need to have your editor not reformat the code. Your editor screwed with the case of the constants ("true" vs. "TRUE", etc), reformatted the braces for functions, reformatted the whitespace at the beginnings of lines and also in comments. AAAGGHHH!!

In any case, i think i got it done...i'm including a patch from version 1.1.2.5 to the module that gravyface included in comment #10.

I also tried applying the patch to the latest 4.6 version in cvs (1.1.2.22), but i failed miserably at that. Sorry.

I figured someone more familiar with the code would be able to perform the patch to the latest 4.6 version.

I'm particularly interested in this because i need it in 4.7...so if you get it working in the latest 4.6 version, i'd really like to see it (or help port it) in 4.7.

Thanks.

...alex...

ARGH. what is it about this file that doesn't want to be patched properly?

I had forgotten that i did my own munging of the files to make my hand-patch working. in particular, i stripped off all extra whitespace at the end of lines in both gravyface's file and in version 1.1.2.5 of the signup.module.

So...the patch won't actually work unless you feed the "-l" flag to the patch. I'm including a second patch that should apply directly to version 1.1.2.5. Please do not use the previous version of my patch.

...alex...

I'm including a second patch that should apply directly to version 1.1.2.5.

heh. latest 4.6 version of signup module is 1.1.2.23 -- that's a long way from 1.1.2.5... :)

we really need to get the patch to apply to the latest version of the module in order to be useful. this is pretty far down my priority list, so if anybody else wants to take a crack at it, that would be great!

I have this working for version 1.1.2.23 now. The patch is included. There are two things that may not be quite right.

The first is that i'm working with mysql-3.23.58, and as per my comment on http://drupal.org/node/44914#comment-70872, the new mysql query doesn't work for mysql3. This patch has the old query put back.

The second thing is that there is one section where i marked that i didn't understand a patch and didn't apply it (the code changd in a way that i don't quite get). It's marked as "__Tango: missing a patch here".

i'm really interested in this for 4.7, if this gets hunmonk's blessing for 4.6, i'll move on to 4.7.

Thanks.

...alex...

i won't accept this patch with the old query, as it breaks mysql 5.x compatibility. see http://drupal.org/node/44914#comment-70990 for more info

Excellent! Now that the SQL query has been sorted out (http://drupal.org/node/44914#comment-71047), i have updated the patch (and upgraded it to the latest 4.6 version (1.1.2.26).

I do have one part of the patch that I was not able to pull in (from the original 1.1.2.5 version), on line 501 of the newly patched file, you'll see:

      #
      # __Tango: missing a patch here.  This is the patch...
      #
#-        . t('View Signups') . '</a> <a href="closesignup/' . $event->nid . '"><br>' . t('Close Event') . '</a>');   
#+        . t('View Signups') . '</a>' . $completed_url . $event->nid . $state_arg . '"><br>' . t($completed_label) . '</a>');

I'm not sure how this translates to the new statement:

    $row[] = l(t('View Signups'), "node/$signup_event->nid/signups") .'<br>'. l(t('Close Event'),
      "closesignup/$signup_event->nid");

If anyone knows how to fix this, let me know. Otherwise, the patch seems to work (on all versions of mysql!) swimmingly.

...alex...

I have updated this patch to the latest CVS/4.7 version (signup.module cvs version 1.33).

In addition, i have also made the following changes:

• separated the signup information data that is shown in the node view to a themeable function theme_signup_view_signup".
• the user can see signup information even if they are not registered (although this can be changed in the themeable function)

i'm postponing this until we get the roadmap for signup figured out, which should hopefully be very soon :)

Any word on this? Really looking forward to this being incorporated (so is my boss).

Yes - the ability to assign a certain number of places/tickets for an event is a must for this module. I'm putting together a site now where this feature would have been a life saver!

status is as follows:

1. i now maintain signup
2. i don't want to add any new functionality to the older releases until i've finished my current top-prio for drupal -- real releases (with version numbers!) for all drupal contribs. see http://drupal.org/node/58066 and http://drupal.org/node/75053 for the gory details.
3. once #2 is done, we'll have a stable and devel branch for each core API version, so i'll finally be able to add lots of new functionality to the 4.6 and 4.7 versions of signup without being afraid of breaking people's sites that like the functionality they have now and just want bug fixes.
4. i have a ton of customizations to signup only running on my private site (4.6) that i'm going to fold back in. this particular issue isn't one of them, so if someone really wants this, they should review/test __Tango's latest patches in #22 and #23, make sure they still apply, work, etc. if not, update them and get them working with the current ends of the various branches of the source.

once there are new branches for real signup releases, i'll mark this issue back to active or patch needs review as appropriate, and y'all can go nuts. ;) i'll happily add this, given the code is reasonable, there are enough reviews/tests, and i can personally verify it works and doesn't introduce any problems (at least whatever i happen to notice during my final review).

thanks for your interest!
-derek

Can we get a patch for latest signup version?

signup.module,v 1.45.2.26

I am getting: 13 out of 13 hunks FAILED -- saving rejects to file signup.module.rej

chad

Yes, chad is right, that would be nice, but only if it's not too much work. Unfortunately, I'm not fit enough with php to do it myself.

Anyway, this module is great, thank you.

Felix.

Subscribing as it would be great to be able to allocate seats/tickets

Derek, now that you are getting close to the 5.0 version of signup, are these features going to be added.

It would be great to limit the number of signups, etc, as mentioned in this lengthy post.

I am going to need this soon myself so I will update the patch and get it working again once dww does a stable release of 5 if no one else gets on it any sooner.

Derek, now that you are getting close to the 5.0 version of signup, are these features going to be added.

certainly not for the 5.x-1.* release. here's my plan:

1) try to fix as many bugs as possible in the near future.

2) make a 5.x-1.0 official release, and leave the DRUPAL-5 branch as a bug-fix only stable branch

3) (at long last) open up new feature development for older signup versions as the ...-2.* series. honestly, i've got a bunch of 4.6.x API code that makes signup much nicer. ;) so, i might go all the way back to that and start with a DRUPAL-4-6--2 branch to commit my changes with the least porting, then forward port them to the newer core APIs. certainly HEAD will become the 5.x-2.* series (effectively like a DRUPAL-5--2 branch, until i decide to start porting signup to the 6.x API). chances are very high i'll make a DRUPAL-4-7--2 branch for new features that work with the 4.7.x core API, if not both DRUPAL-4-7--2 and DRUPAL-4-6--2.

so, this doesn't need to stay "postponed", someone can start working on a new patch for this whenever they want (against any of the branches of signup module that currently exist, depending on what site(s) you have that want this functionality). when it's getting close to RTBC, chances are there will be a DRUPAL-X--2 branch to commit this to.

thanks,
-derek

Hey Derek, #3 above, your point about going back to 4.x and then porting the changes forward to 5.x-2* almost seems like that will break the upgrade path from 5.x-1.x to 5.x-2.x. Will that be the case or will -2.x be a clear upgrade from the 1.0 final release?

If there is a clear upgrade path then I have no problems fixing the patch and applying as well, but I wouldn't want to do it and then find that I'd have to do it again on the 2.x branch only to find out that the 2.x branch looks nothing like 1.0.

the issue of the 5.x-1.* to 5.x-2.* upgrade path is unrelated to the question of if i add a 4.7.x-2.* series (i've changed my mind on 4.6.x-2.* -- that simply ain't gonna happen). whenever the schema changes between 5.x-1.* and 5.x-2.*, we'll have to make sure there's a clean upgrade path (namely, a fully functional signup_update_N() function for the migration). since we'll be using branch-specific numbers for N (e.g. everything for 5.x-2.* will be 5200, 5201, etc) it won't be a problem, even if we add some updates on the DRUPAL-4-7--2 branch specific to 4.7.x-2.*. bottom line, we just have to consider the upgrade path(s) involved, but it's not impossible to get it right, or really, even that much harder than usual.

that's the question of DB updates and schema migration.

another topic is if a given patch for 1 branch will apply cleanly on another. that's an entirely different question. the core API changes between 4.7.x and 5.x are already enough to cause merge conflicts on most patches. certainly, as the code changes between 5.x-1.* and 5.x-2.*, bug fix patches against 5.x-1.* will need porting to apply cleanly to 5.x-2.*. however, that's just the price you have to pay to make progress. ;)

hope that clarifies...

many of the other ideas discussed in the original post and earlier comments are already implemented. so, let's keep this issue focused on the 1 major new feature that's not yet done: signup limits. if there are any other goodies referenced in previous comments or patches that someone would still like to see, please just submit separate new issues about them, since it's much easier to keep track of things if each issue is focused on a single thing. thanks.

so I am not understanding. im using 4.7 still, is it possible to get this limit to work on that version? i am not great at changing drupal's code, and i have tried filemerge on the 1.33 patch above, it obviosuly is not doing what i need it to do. is there no patched module file that works? sorry for being such a newbie herer, but i really need this feature, or there is like no point in using drupal in my current project.

No, there is no currently working patch for this feature. I need it myself, but it is not at the top of my priority list. If you want to expedite the feature you can offer to pay dww or someone else to code up a patch that works with current versions of signup. Otherwise you'll have to wait until someone gets around to doing it.

update: someone's sponsoring me to make this work, so it's going to get done, and done right, in the near future. stay tuned.

Hi.
Yesterday I had a little time and implemented a basic limit feature.
Additionally I implemented a feature that allows users to have multiple signups at one time (needed this to order tickets).
And with this patch signup information shows the title of the signup fields (not the names).

Perhaps you can use it for your work.

greetings,

Stefan

To try multiple signups you can insert a new field in signup.theme (function theme_signup_user_form()):

  $form['signup_form_data']['multiple_signups'] = array(
    '#type' => 'select',
    '#title' => t('Number of tickets'),
    '#size' => 1,
    '#default_value' => 2,
    '#options' => array(1 => 1, 2 => 2, 3 => 3, 4 => 4, 5 => 5, 6 => 6)
  );

Forget to note: this patch applies to signup.module,v 1.74.2.25 2007/03/31 07:15:07.

@stBorchert i can see why you wanted to add both features simultaneously, since they are related. however, i'd prefer to handle these 1 step at a time. also, sadly, i didn't see your patch in this issue when i was working on this off line... alas.

anyway, here's an initial patch (which applies cleanly to HEAD currently) that adds this feature:

• adds a DB update to make sure the schema is right. folks upgrading from older installations won't have signup_close_limit in their {signup} table.
• adds a limit to the per-node signup settings
• if a user signups up and the limit is reached, signups are automatically closed
• views support for this new field is included

the only thing that doesn't yet work is if the limit is reached, and the event hasn't happened yet, and someone cancels, signups should re-open. doesn't look like stBorchert's patch handles that case, either.

anyway, initial testing and feedback would be appreciated.

i'm trying to work limit into the UI for the "signups" admin tab on each node. to make it clean, i'm doing a fairly major overhaul of this tab. attached is a screenshot of my current version. patch is still in progress (i need to isolate more of the code in helpers that can be reused and shared), but i wanted to at least post the screenshot of the UI so folks can comment on that.

here's the patch that makes the above screenshot work. a few of the edge cases aren't handled right, and some of the code should be re-factored for cleaner re-use, but it's mostly working nicely...

No time to test the patch today, but the tab UI changes are looking good.

Looking at the screenshot the only comment I have is that the 'total' seems confusing, Total what? Possibly it should say 'Total signups' or 'Current total'. But then again, it's one of those things that seems obvious once you look at it a second time.

can anyone suggest a good mac program that will put this patch in? i have tried filemerge but it isn't making much sense of it.

nevermind, guess i should rtfa next time. this patch is only for 5.x huh?

"this patch is only for 5.x huh?"

yes. new features are only going into the 5.x-2.* series. if there was sufficient interest (in particular, someone sponsoring me to do it), i'd create a DRUPAL-4-7--2 branch and backport some new features to a 4.7.x-2.* series. but at this point, i'm not planning on doing that.

Patch applies cleanly to HEAD, even with the broadcast patch applied, and seems to be working nicely on the whole.

I can confirm that when limit is reached and someone cancels the event doesn't re-open, ditto if an admin cancels the signup.

I also noticed it's possible for the admin to open signup's again after the limit has been reached, this allows a further individual to sign up, giving you a total of limit+1. Signups are automatically closed again when that extra person signs up. I'm not sure if that's a bug or a feature, I'd lean toward it being the latter.

Apart from the problem with cancelled signups not re-opening I think this is good to go.

re: admin overriding the limit by re-opening, i considered that a feature, and part of the design. i'd rather not force admins to raise the limit just to let someone else signup, but if people feel this is too weird, i could change it.

and yeah, there's simply no code handle re-opening on cancel, so it's no surprise that's not working, yet. ;) i guess this needs work, then, to finish that off.

thanks for the initial feedback/testing.

I have not tested but have a question

Can the limit be configured per node or is it by content type?

If it is per node, then I would think a better way to reopen the signup would be to increase the limit.
Or if you leave it as it is, the reopen signup might be labeled something like reopen for one more signup.

Anything to make it intuitive

@Lowell: "Can the limit be configured per node or is it by content type?"

definitely by node. this feature would suck if it was per content type. ;) in fact, almost no signup-related settings are per content type (except if signups are enabled or disabled by default, and even then, that is ultimately controlled per node, the per content type setting is just the default).

@Lowell and @adrinux: re: the admin re-opening signups when the limit is already reached...

yeah, i guess it's just kinda weird. so, i changed it. ;) once the limit is reached, the admin can't toggle signups open again, they have to change (or remove) the limit. i think this should be more clear and obvious for everyone involved.

new patch with a lot of code reorganization to handle all the cases more cleanly (i hope). it still doesn't handle re-opening on cancel, but now that this much is done, that part should be pretty trivial. the one snag is that currently, once signups are closed, users can't cancel their own signup, so i'll have to tweak that logic, too. :(

so, i'm still leaving this issue as "needs work", but another patch should be coming soon so i can set this back to "needs review". i just wanted to post a checkpoint of what i've got working now in case anyone wants to play with it and provide additional feedback.

hehe, told you it'd be trivial... this patch is 2 lines different from the previous one. ;) if the admin cancels a signup and the total drops below the limit, signups now re-open. however, there are still 2 TODO items:

1. allow user to cancel their own signup if signups are closed (this is really a new feature, but pretty much required due to this limit stuff being added).
2. do *not* re-open signups due to total or limit changes if it's an event-based node and the event has already started (really, passed its "close in advance" time -- basically, if signups were closed by cron, don't reopen them).

almost there... ;)

this patch adds a _signup_event_completed() helper function. the logic to deal with if signups should be re-opened if someone cancels or if the limit is changed now uses this to help decide. if the node is event-enabled and has already passed the "Close X hours in advance" time, signups won't re-open, even if the limit would now allow it.

also, previous couple of patches have left out changes to signup.install and signup_views.inc, which are now included again in this patch.

so, the only remaining problem is that once signups are closed, the user doesn't see their own signup info and doesn't have a "Cancel signup" button to use. stay tuned.

new patch:

• prints the signup info and cancel button if the current user has signed up, even if signups are closed.
• if you edit an event-enabled node and change the start time so that signups should be closed based on the close-in-advance setting, signups are automatically closed.

i'm not sure what i think of this 2nd point. maybe we shouldn't worry about these cases and just let cron handle it. it's a trivial code block to remove if folks think it should go...

there's still some watchdog() cleanup that could be done, and i've noticed some other code cleanup that should happen, but IMHO none of that is worth holding up this patch.

so, please review, test, etc. thanks!

removes a tiny bit of cruft that was still around from earler iterations of this patch.

Mostly working ok.

Possible problem:
In the admin signup overview if you click on 'open event' for a closed event it opens, but that's not reflected in the event signup overview, where it still reports that signups are closed because the limit is reached - presumably this breakage happens because we now need to increase the limit if we want to open an event again. It seems the admin overview open/close signups isn't connected with the limit code.

It seems the admin overview open/close signups isn't connected with the limit code.

yeah, that's true. :( i *hate* the admin overview UI. see http://drupal.org/node/79331 and http://drupal.org/node/79332 for example. ;)

anyway, i was working on a patch to clean all of that up, and make the admin overview table look more like the current per-node administration "table" (the 1-row "Signup summary") on node/N/signups thanks to this patch, with the drop-down for status, the textfield for limit, etc.). then, i'd (finally) be able to rip out those crazy "Open Signups" links and related cruft. it's not *quite* working yet, but it's close... stay tuned.

I think that the closing time is being incorrectly calculated according to the instructions in admin/settings/signup. In the patch, $closing_time is being set by:
$closing_time = time() + (variable_get('signup_close_early', 1) * 3600);

There are two issues with this:

1) variable_get('signup_close_early', 1) should be variable_get('signup_close_early', 0) so that a null offset does not alter the closing time (otherwise it will be off by one hour)

2) I believe that it should be time() - not time() + because: If a negative value is passed in, we want to make the $closing_time greater, not smaller. If a positive value is passed in, we should make $closing time smaller.

This of course depends on the text found at admin/settings/signup: The number of hours before the event which signups will no longer be allowed. Use negative numbers to close signups after the event start (example: -12). If we reversed either the logic in the sentence, or the logic in this function, things would be fine, but as it stands, I believe that the function operates opposite the instructions

+/**
+ * Returns true if the given node is event-enabled, and the start time
+ * has already passed the "Close x hours before" setting.
+ */
+function _signup_event_completed($node) {
+  if (module_exists('event')) {
+    if (is_numeric($node)) {
+      $node = node_load($node);
+    }
+    if (isset($node->event_start)) {
+      $closing_time = time() + (variable_get('signup_close_early', 1) * 3600);
+      if ($node->event_start < $closing_time) {
+        return true;
+      }
+    }
+  }
+  return false;
+}

Woops, I miss understood the logic here. I take back the math error. Sorry!

yeah, sorry that _signup_event_completed() logic is a little confusing. ;)

anyway, i figured out what i was doing wrong with the FAPI hackery to get all my nice little form elements in that sortable table on the admin/content/signup page. yay! so, here's the patch that gives the admin full control to tweak signup status and signup limits on that page, and it's all perfectly integrated with the limit code, and the evil "Close Event" links are gone.

this will cause conflicts for http://drupal.org/node/79332, but those are easy to resolve, and i've got a workspace with both patches applied and they're working beautifully together. ;) in fact, maybe i'll post a patch of both together, just so folks can easily play with the final result...

here's the patch for both http://drupal.org/files/issues/signup_limit_5x-2.patch_7.txt and http://drupal.org/files/issues/signup_admin_status_filter-5x-2.patch_2.txt working nicely together. ;)

Wow, this is cool stuff. I'm really liking how the UI has turned out and the limits are working nicely. I did manage to get and error with this last patch though on the Signup Admin page. When I only have one state of signup (all open or all closed) and try to filter by the non-existent state (that is, if all signups are open and I filter by closed) I get this: warning: Invalid argument supplied for foreach() in /Users/addi/Sites/drupal5/sites/default/modules/signup/signup.module on line 863.. I added a quick if (isset($form['nids'])) around the foreach and it was fine.

Only other comment is it would be nice if the signup limit setting was shown above the email settings in the edit screen. Since it is so small, it is easy to get lost under all of the big email boxes. ;)

Ah were looking good now. Can't see any breakage. Unlike add1sun I see no problem with the limit being beneath the e-mail (though I also have no problem with it being above).

Just one minor detail now I see my CSS working - the filter tweak should probably be just margin-bottom: 0, that looks more consistent and less cramped than margin: 0, revised patch attached.

Marking this one RTBC.

actually, i'm *not* going to commit both of these features at the same time. the filtering of the admin page belongs in http://drupal.org/node/79332 ...

i'm with adrinux on the placement of the limit box at the bottom of the settings and node edit forms. this is a new feature, so people are used to the other settings being first. if they care about limits, they'll find it (especially since you can toggle it on the node/N/signups page and admin overview pages, too). ;)

so, here's a new patch that fixes the bug add1sun mentioned in #65, where the overview form was giving an error if there are no signups that match the current filter (remember, you can still filter by the tabs, even without http://drupal.org/node/79332), or, on a new site, before you create any signup-enabled content just on the overview page itself.

any final complaints? ;)

p.s. @adrinux: the "5x-2" part of these patch filenames is supposed to indicate what version they're targetted for (5.x-2.*). the "patch_N" part is the counter for which iteration of the patch we're on. ;)

i had accidentally left in the menu definitions and callbacks for those evil "Open Signups" and "Close Signups" links, so this patch just rips out cruft that's now dead code. ;) this patch is adding enough code as it is, nice to be able to balance that a little...

doesn't apply now that i committed http://drupal.org/node/79332 -- re-roll coming soon, stay tuned.

screenshot of the latest patch, just for fun. ;)

The work on the UI is great. Everything still seems to be functioning, no issues yet...if you're happy with the code this is RTBC.

even better signup_update_5201() based on my efforts in http://drupal.org/node/137479. obviously, that patch is for 6.x core, so i'm just making a little duplicate _signup_db_column_exists() in here for now, but when porting to 6.x we can rip that out if #137479 gets committed. ;)

either way, with this patch, when users run update 5201, nothing will happen (and no errors will be generated) if the column we need already exists. much nicer than a drupal_set_message() about "you can ignore errors about...".

Looking good to me. No problems encountered, so RTBC baby!

committed to HEAD: http://drupal.org/cvs?commit=64864 yay!

(apologies to anyone who saw that commit on the cvs email list before i had a chance to repair it... a foreign workstation screwed up my usual workflow, and i ended up committing with an empty message at first... luckily, i'm crafty and have the power to correct my own mistakes in such situations). ;)

This looks awesome guys - is there any hope for those of us who can't upgrade to 5 yet and are still on 4.7?

your only hopes for 4.7 are:

1) someone competent for the job volunteers to maintain a DRUPAL-4-7--2 branch of signup for new features like this that are backported to the 4.7.x core API.

2) someone pays me to do that job. ;)

i'm not interested in this for myself, so i'm not going to volunteer, but i'd be willing to do the work if someone wants to pay for it. http://drupal.org/user/46549/contact if you'd like to discuss option #2. ;)