Simplenews Triggers and the $user object = FAIL

I believe that this has to do with trying to execute an Action to add/remove a user from a Role when the Simplenews (un)subscribe Trigger fires.

A brief history: We have two sites on the same Drupal codebase. The tables that the sites share are "profile_fields", "profile_values", "role", "users", and "users_roles". We also use Simplenews with the Simplenews Roles module, so that we can modify who is subscribed to what newsletter via Role assignment between the two sites that we are running (simplenews isn't share, so we use the Roles to sync the subscriptions). The problem with Simplenews Roles is that when a user unsubscribes, they are added back into the subscription on a cron run unless I create a Rule or Action/Trigger to fire when simplenews receives an unsubscribe.

This is likely an issue with the Trigger that Simplenews creates whereas they may not populate the $user object with its Roles.

Looking into this today. If it's a Simplenews issue, I'll move this over to that project. I put it here, though, because that's where I first encountered the issue.

Switched this issue over to Simplenews, although the Action in the VBO module *should* still check the data before parsing it.

I think the main concern that I have is what information (data/variables) the trigger makes available to whatever action you assign to it. In my case, trying to change a user's Role based on whether they are subscribing or unsubscribing, which isn't working because it appears the trigger doesn't provide any arguments for the action to use. I could be mistaken and of course will be looking into this more.

There are some interesting points made in the following post:
http://drupal.org/node/308922

The better move, as many have pointed out, would be to change this code over to Rules. I may end up having to doing that if I cannot fix this with community suggestions/fixes using Actions/Triggers.

So, the "fix" I found can be done in VBO, and is rather easy. Here is a test that one can use in their own copy of the VBO "user_roles.action.inc" (at the end of the file):

<?php
function views_bulk_operations_user_roles_action(&$user, $context) {
  $user = user_load($user->uid); //Ensures that the action has the appropriate values for $user and avoids checking $roles to see if it is an array.
  $roles = $user->roles;
  foreach($roles as $rid=>$role) {
    drupal_set_message("DEBUG: views_bulk_operations_user_roles_action: Role found ".$role);  //Proof that the roles loaded (which they weren't when Simplenews used this action w/o the user_load() line above).
  }
  $selected = (is_array($context['add_roles']) ? $context['add_roles'] : array()) +
    (is_array($context['remove_roles']) ? $context['remove_roles'] : array());
  $result = db_query("SELECT rid, name FROM {role} WHERE rid IN (%s)", implode(',', array_keys($selected)));
  while ($role = db_fetch_object($result)) {
    if (isset($context['add_roles'][$role->rid])) {
      $add_roles[$role->rid] = $role->name;
      drupal_set_message("DEBUG: views_bulk_operations_user_roles_action: Added to Role ".$role->name);  //Proof that the roles loaded as defined in the action
    }
    if (isset($context['remove_roles'][$role->rid])) {
      $remove_roles[$role->rid] = $role->name;
      drupal_set_message("DEBUG: views_bulk_operations_user_roles_action: Removed from Role ".$role->name);  //Proof that the roles loaded as defined in the action
    }
  }
  if (!empty($add_roles)) {
    $roles += $add_roles;
  }
  if (!empty($remove_roles)) {
    $roles = array_diff($roles, $remove_roles);
  }
  user_save($user, array('roles' => $roles));
}

?>

I'm torn... I really think that the views_bulk_operations_user_roles_action() could benefit from the "user_load()" line I added. Switching back to VBO for completeness. Someone can correct this if I'm in the wrong.

I tested this and confirmed that it works for Simplenews, AND maintains the previous roles. It wasn't maintaining the previous user's roles before I added the user_load() line (again, for triggers that may not completely load the $user var when calling an action). Granted, this doesn't need to be a "fix" for VBO, but I'm not sure how to make the Simplenews module's Trigger pass the full $user object, so please feel free to comment accordingly.

:)

After looking into how Simplenews passes (or actually, doesn't) the $user object, I found that they recreate it as a $subscription object that is slimmed down. This seems to be the opposite of what should happen. Instead, I propose that this be rewritten to add the $subscription specific values to a fully loaded $user object. Otherwise, the actions that the trigger fires will be missing a TON of valuable information.

If I get a chance, I'll take a stab at this. Until then, I'm leaving the user_load() in my VBO.

Files of concern in Simplenews are:
simplenews.module
simplenews_action.module

Functions of concern in Simplenews are:
_simplenews_user_load() - Minor, but consider using for properly building the Simplenews-version of the $user object.
simplenews_subscribe_user() - Fires simplenews_call_action(), so is the "hook_trigger_name" of "simplenews_simplenews" not required as mentioned in http://drupal.org/node/375833
simplenews_unsubscribe_user() - Same as with simplenews_subscribe_user().
simplenews_get_subscription() - Might be more taxing on the system, but consider returning a properly built Simplenews-version of the $user object.
simplenews_call_actions() - This is a biggie. I don't understand why, if "user" Actions are permitted through this Trigger, that this would send an incomplete $subscription object instead of a filled $user object. I also noticed that this does not use the function call module_invoke_all('simplenews', 'subscribe', $account) as mentioned in the http://drupal.org/node/375833
simplenews_simplenews() - Does not exist (if needed) as mentioned in the http://drupal.org/node/375833

Needs some more review.

Switching back to Simplenews since this seems to mostly impact that module's functionality, but tagging with VBO to make this easier to find.