Fix status "modify signup count" code to actually do something
dww - September 19, 2009 - 01:14
| Project: | Signup Status |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | dww |
| Status: | closed |
Description
Since #359411: Add a hook to allow modules to alter the total signup count was never implemented, the UI in the signup_status module to indicate which status values should count towards the signup limit never really worked.
Now that #581734: Improve the signup API: add hook_signup_data_alter(), hook_signup_create(), and hook_signup_update() is done and #581652: Add a {signup_log}.count_towards_limit column and adjust limit logic accordingly is on the way, we can finally fix this in signup_status. Stay tuned for a patch...
#1
#2
Committed #1 to HEAD.
However, I just thought of an edge case we need to fix. If an admin changes the "Modifies signup count" value on status values that have existing signups in them, we need to do a potentially heavy operation of updating all the {signup_log} records accordingly. Plus, we need to check the effective signup total vs. the limit on any nodes we touched, so this definitely needs batch API...
So, this would be back to active, but it should be blocked on #581846: Cleanup the signup_status admin UI and provide status weights landing so that we don't touch the admin UI with separate patches...
#3
#581846: Cleanup the signup_status admin UI and provide status weights is in, this is active again.
#4
#5
Rerolled to keep up with HEAD.
#6
Now that we've got this function, inside the schema update where signup is adding the {signup_log}.count_towards_limit column, we should invoke this batch-aware function to properly initialize {signup_log}.count_towards_limit on sites that have signup_status and have some status options configured to not modify the signup limit...
I tested this, and setting more batches inside an update.php batch works just fine. Phew. ;)
#7
#582250: When you delete a status, signups are left with bogus data made me realize that we also need to deal with some of this batch stuff over there, since it's possible the new status we're reassigning signups to has a different value for mod_signup_count. ;) Fun... I'll see how quick/easy I can reroll this.
#8
That could have been a lot worse. ;)
#9
The patch applies cleanly.
Any suggestions for how best to test the changes?
#10
Suggestions for testing:
a) create some signup-enabled test nodes, and give them some relatively low signup limits (e.g. only 3 slots).
b) define a few different status values that should/shouldn't impact the total, e.g. "yes", "maybe" and "no".
c) signup some dummy users with the different status values, using a few yes/no, and a lot of maybe.
d) visit admin/settings/signup_status, and tweak if "maybe" counts or not. you should see signups open and close on your nodes, assuming the maybes are enough to either push the effective total over/under the limits...
#11
I setup a test as described. Works slick. I like it.
#12
Committed to HEAD. Thanks for the tests!
#13
Automatically closed -- issue fixed for 2 weeks with no activity.