Another thing we need from the signup limits design spec I wrote (and posted over at #359412-3: Move code for signup limits per status per node into a submodule) are some additions to the signup API. See the spec for details, (pages 2-3 in particular).

In particular, we're going to want the following:

/**
 * Hook to alter signup data before a signup is created or updated.
 *
 * @param $signup
 *   Reference to the fully-loaded signup object representing the signup.
 */
function hook_signup_data_alter(&$signup) {
  // TODO
}

/**
 * Hook invoked after a signup has been created.
 *
 * @param $signup
 *   The fully-loaded signup object representing the new signup.
 */
function hook_signup_create($signup) {
  // TODO
}

/**
 * Hook invoked after a signup has been updated.
 *
 * @param $signup
 *   The fully-loaded signup object representing the updated signup.
 */
function hook_signup_update($signup) {
  // TODO
}

Not only is this going to help the signup limit stuff and #581652: Add a {signup_log}.count_towards_limit column and adjust limit logic accordingly, it's also going to simplify some yucky code in signup_status, and it's going to make the signup API much more useful for programatic signups. Stay tuned for a patch...

Comments

dww’s picture

Status: Active » Needs review
StatusFileSize
new8.15 KB
dww’s picture

For example, here's a patch to signup_status.module to take advantage of the new APIs. Almost all of the lines added by the patch are comments. ;)

dww’s picture

Actually, I think this is cleaner. Instead of having signup.module itself try to populate the $signup object based on the form values, we should just pass the array of form values as a 2nd arg to hook_signup_data_alter(), and then add-ons can insert whatever values they know about themselves. Also, signup_status should be implementing hook_schema_alter() since it really is altering the {signup_log} schema (for better or for worse).

dww’s picture

Moved the missing hook_schema_alter() part over to #581768: signup_status_schema_alter() is missing for the change to {signup_log} since that's a (basically) unrelated bug. Already committed that to HEAD, so if you use latest HEAD of signup_status, that hunk will fail from #3 -- fear not.

dww’s picture

StatusFileSize
new8.49 KB

Slight code cleanup/reorg in signup_save_signup(), otherwise, same patch...

dww’s picture

StatusFileSize
new9.52 KB

A few changes based on an IRC review from Ezra:

A) We don't need the $is_new parameter for signup_save_signup(), since we can just decide that based on if $signup->sid is defined.

B) While we're touching it, we should use $account instead of $user inside signup_sign_up_user() for safety...

dww’s picture

To help avoid a DX WTF here, I proposed (and Ezra agreed) we should rename hook_signup_create() to hook_signup_insert() to be more like core's hook_nodeapi(). It's the hook invoked right after the signup has been inserted into the DB...

dww’s picture

Status: Needs review » Fixed

Committed #7 to HEAD and DRUPAL-6--1 of signup, and HEAD of signup_status. Yay.

#581652: Add a {signup_log}.count_towards_limit column and adjust limit logic accordingly is going to be a *lot* easier now.

Status: Fixed » Closed (fixed)

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