I am proposing (and currently coding a patch for) a hook in this module which fires when push_notifications_store_token is called. I think we want to offer two possibilities here:

  • Allow the hook to return a value of false which would stop execution before the token is written to the database.
  • Allow the hook to modify the values passed in to it. (e.g. token, type_id, uid)

Code to follow.

(As a related note, we may consider adding one which fires when a push is done. It would be nice to allow programmers to hook into both of these events.)

CommentFileSizeAuthor
#4 1705990-hook-push-notifications-store-token.patch1.61 KBwebkenny

Comments

webkenny’s picture

webkenny commented
Status: Active » Needs work

Original thoughts. Seems modifying only the token is probably the best idea right now. Another implementation may need to add a lot of lines to convert the 3 variables into an array and then back out again. If we think it'll be useful, I'll do so. In the meantime, thinking of rolling this patch. Feedback welcome:

function push_notifications_store_token($token = '', $type_id = '', $uid = '') {
  // Let modules modify the token before it is saved to the database.
  foreach(module_implements('push_notifications_token') as $module){
    $token = module_invoke($module, 'push_notifications_token', $token);
  }

  if (!is_string($token) || !is_numeric($type_id) || !is_numeric($uid)) {
    return FALSE;
  }

If they decide to set $token to false, it will not write anything. Could be useful for doing additional access checks not covered by the module itself. (e.g. is the user part of a group)

haagendazs’s picture

haagendazs commented

Hi webkenny,

Great idea to add a hook that allows other modules to modify the token if they need to. My suggestion for adding the hook would be changing the name to 'push_notifications_store_token' and change it as following:

function push_notifications_store_token($token = '', $type_id = '', $uid = '') {
  // Let modules modify the token before it is saved to the database.
  foreach(module_implements('push_notifications_store_token') as $module) {
    $function = $module . '_push_notifications_store_token';
    $function($token);
  }

  if (!is_string($token) || !is_numeric($type_id) || !is_numeric($uid)) {
    return FALSE;
  }

This way, other modules could implement the hook and pass the token by reference, for example

function other_module_push_notifications_store_token(&$token) {
  if (somethingOrOther) {
    $token = null;
  }
}

What do you think?

haagendazs’s picture

haagendazs commented

Small change: I noticed that you probably need the type_id and uid in the hook. With that in mind, the change should probably be:

function push_notifications_store_token($token = '', $type_id = '', $uid = '') {
  // Let modules modify the token before it is saved to the database.
  foreach(module_implements('push_notifications_store_token') as $module) {
    $function = $module . '_push_notifications_store_token';
    $function($token, $type_id, $uid);
  }

  if (!is_string($token) || !is_numeric($type_id) || !is_numeric($uid)) {
    return FALSE;
  }

Other modules could then use it like this:

function other_module_push_notifications_store_token(&$token, $type_id, $uid) {
  if (somethingOrOther) {
    $token = null;
  }
webkenny’s picture

webkenny commented
Status: Needs work » Needs review
StatusFileSize
new 1705990-hook-push-notifications-store-token.patch1.61 KB

Excellent! I was, in fact, hoping to pass all 3 but only have to modify token if needed. Beautifully done. :) Patch attached. I also added an api.php file to document the hook and any future ones we may consider.

haagendazs’s picture

haagendazs commented
Assigned: Unassigned » haagendazs
Status: Needs review » Closed (fixed)

Thanks for the patch file and the additional api.php file. I applied the patch to the 7.x-1.x branch here: http://drupalcode.org/project/push_notifications.git/commit/f0d7e7b4ce1d....