Move code for signup limits per status per node into a submodule

Note: ASU has contracted me to work on this. I wrote a design spec that I'm attaching here for interested parties. I'll be implementing this in the very short term -- basically all this week. If anyone has any thoughts/concerns about the attached spec, please comment here ASAP. Thanks!

First step is #578502: Move signup-related settings from node/N/edit to node/N/signups/settings if anyone is interested/available to review/test that.

#578502: Move signup-related settings from node/N/edit to node/N/signups/settings is now committed to signup. I posted an initial patch and some screenshots for the altered settings UI over at #241869: Improve interface for setting number of signups allowed per status if anyone wants to look...

One note about http://drupal.org/files/issues/359412-3.signup_status_limit.spec_.pdf -- since I wrote that spec, I've been thinking more about all of this and decided that the proposed {signup_log}.count_towards_limit value described on page #3 should really be an integer, not a bool, to handle the often-requested feature of "how do I signup myself and 3 friends?"... In that case, we'd save 4 in {signup_log}.count_towards_limit instead of 1. Make sense?

Also, moving this to the new component for limit-related issues.

I did an initial commit of the new signup_status_limit module which basically just includes the settings UI. So #241869: Improve interface for setting number of signups allowed per status is now fixed, and I'm going to focus on making those settings actually do something. ;)

No, that's not dupe. We're just going to have a sane way to solve that once this is all done...

Here's an initial patch and some screenshots of trying to enforce the per-status limits based on the UI changes already committed and the spec.

The more I actually play with this, the more I have questions about how this aspect of the UI should really work... ;)

Attached patch is pretty much exactly how I envisioned it in the spec: when a signup form is being generated, the only valid status options are ones that haven't yet reached their limit. If all the status options are full, the signup form itself is replaced with a message (see 359412-10.signups_full_error.png). There's a special validation callback for the signup form to ensure that when the user is actually trying to submit the form, if the selected signup status is now full, they get a form validation error (see 359412-10.status_full_error.png).

However, here are some edge cases that seem a bit weird:

A) If a page is rendered with a signup form when all the status options are full, even if someone cancels or the limits change, the signup form is still gone and the user still thinks that signups are totally full.

B) While trying to signup, if they hit a validation error of any kind (either due to a full status or otherwise), if the totals or limits have changed, the status options available in the form will change, even on the validation error screen. In a way, this is a feature, and is the opposite behavior from (A), but it's still slightly wierd.

C) If a status was available when the signup form is rendered, but is full when they try to submit it, I have to keep the full option available in the list of choices, or FAPI throws the dreaded "An illegal choice has been detected" error. So, in this one case, you're presented with a status choice that will keep failing if you don't change it. Unless of course, someone with that status cancels or the limit is increased in the meanwhile, then it'll magically start being a valid choice again (see B). ;)

D) Via VBO, a signup admin can easily overfill a given status by mass editing signups and changing N signups to use a status that only has M available slots (where M < N). Again, maybe this is a feature, not a bug, and signup admins should be able to do whatever they want and disregard status values. However, in contrast, the form for signup admins to add other users is currently just another instance of the 'signup_form' form ID, and that form is therefore enforcing the per-status limits as described here...

Any thoughts and input on these issues, or usability feedback from testing the attached patch (use HEAD of signup_status and the end of DRUPAL-6--1 of signup) would be most appreciated.

Thanks!
-Derek

Ok, based on the feedback, I committed #10 to HEAD (with a minor change -- if there are no visible signup status options at all, we don't try to alter a NULL form element).

So, at this point, limits are *only* enforced during the end-user signup UI. Programatic signups or status changes aren't covered. However, I had a long IRC chat with ezra-g about all this. Basically, there's really no good way for signup_status_limit itself to enforce per-status limits in these cases. For example, having signup_sign_up_user() fail if the status limit is exceeded is the wrong time to fail if something like uc_signup or signup_pay is trying to save the signup after processing payment. :( Really, add-on modules should be asked to check these limits themselves at the appropriate point in their own code paths and workflows.

If/when #503368: Default Status on Signup is done, we can add some code to signup_status_limit so that if a new signup is happening and there's no visible signup_status form element (in which case the default status would be used) we could enforce the limit on that default status as if it was part of the signup form. However, that's obviously blocked on #503368.

So, this is back to active for any other changes that might need to happen to signup_status_limit itself, but the basic shell is now working...

FYI: committed a few follow-up fixes for this:
http://drupal.org/cvs?commit=264608
http://drupal.org/cvs?commit=264620

I opened up #581652: Add a {signup_log}.count_towards_limit column and adjust limit logic accordingly for the signup schema + API changes outlined in the spec I attached at #3... The only other changes to signup_status as a result of that don't really impact signup_status_limit module itself. So, I think this can safely be marked 'fixed' at this point... Folks interested in the outcome of the spec should meet me over at #581652.

Oh right, except I forgot the D5 to D6 data migration part. That should be handled here.

Yay for a trivial upgrade path. ;) It's so rare (but nice) when it works out like that. Tested this with a copy of ASU's {signup_status_node_limit} table and it worked just fine.

Ha, I knew it was too easy to be so simple. ;) I forgot that we also want to populate {signup_status_limit_node_setting} for any nodes that have per-status limits from the D5 table. ;) So, I added the following query:

db_query("INSERT INTO {signup_status_limit_node_setting} (nid, limit_type) SELECT DISTINCT nid, 'status' FROM {signup_status_limit_node_limit}");

Re-tested this, and it's good, so I committed to HEAD.

Phew! ;)