uc_roles_cron improperly handles revoke/delete of roles for recurring fees

Project:Replacement for uc_roles
Component:Code
Category:
Priority:critical
Assigned:Unassigned
Status:active
Description
Project: 
uc_roles2

I haven't looked to see how this is handled in Ubercart 2, but in Ubercart 5.x-1.7, there is occasionally an issue with the uc_roles_cron function and how it handles revoking and deleting roles. This is specifically when using the Recurring Fees module to automatically renew roles. The problem occurs due to the timing of the cron run, and how it matches what needs to happen with uc_roles_cron.

Before I get into a specific example, we are running Drupal 5.19, Ubercart 5.x-1.7, and processing our transactions with Authorize.net. We run cron once an hour, at the top of the hour (x:00). Out of approximately 1,000 orders, this issue has occurred about 10 times. Fairly good, but we want this to be zero. Here is the example:

--------------------

*** SCENARIO ***

1) Tue, 28 Jul 2009 19:01:10 GMT: A user subscribed on our site and purchased a one-month subscription. He also chose to have his billing automatically recur each month. When the subscription was purchased, the next billing date and the expiration date were set to Fri, 28 Aug 2009 19:01:10 GMT.

2) Fri, 28 Aug 2009 19:01:15 GMT (one month later): uc_roles_cron runs and revokes the user's subscriber role because the expiration date had been reached. The recurring billing did not run at this time, I assume because it was not added to the queue until the hour following expiration.

3) Fri, 28 Aug 2009 20:01:04 GMT (one hour later): The recurring fee transaction is processed, and a new subscriber record is added to the uc_roles_expirations table.

4) At this point, the order has successfully renewed, and the user has regained their subscriber role.

5) Fri, 28 Aug 2009 20:01:07 GMT: The uc_roles_cron is running, and it deletes the record in the uc_roles_expirations table which was just added in Step 3. The user now has a recurring fee associated with their account, but no longer has the role. Also when I look at the $user array at this time, the subscriber role is certainly not present, therefore the "delete" function in uc_roles_cron is going to run.

--------------------

On first glance, it seems as though uc_roles_cron runs into a problem because the "revoke" function (in the _role_action function) has already removed the subscriber role from the $user array. See the code below from _role_action:

    case 'revoke':
      $roles_list = $user->roles;
      unset($roles_list[$rid]);
      user_save($user, array('roles' => $roles_list));
      db_query("DELETE FROM {uc_roles_expirations} WHERE uid = %d AND rid = %d", $user->uid, $rid);
      break;

Once the subscriber role has been revoked, there is an issue when uc_roles_cron runs the next hour. See the code below from uc_cron_run:

  $result = db_query("SELECT * FROM {uc_roles_expirations}");
  while ($expiration = db_fetch_object($result)) {
    $user = user_load(array('uid' => $expiration->uid));
    if (is_array($user->roles) && count($user->roles) > 0) {
      if (!in_array($expiration->rid, array_keys($user->roles))) {
         _role_action('delete', $user, $expiration->rid);
      }

The issue here is the line of code which says, "if (!in_array($expiration->rid, array_keys($user->roles)))". In Step 5 of the scenario above, there is a new $expiration->rid created. However because the subscriber role has been deleted from $user->roles, the condition is evaluated as TRUE. The next step is the _role_action to delete the record from the uc_roles_expirations table.

I'm not sure how to solve this just yet, but in the interim I think I'm going to comment out the delete option in the uc_roles_cron function. "Revoke" handles the situation correctly, although it does cause a one-hour period of time where a user may not have their subscriber role while waiting for the next cron run.

Any thoughts or ideas while I think about this more would be appreciated.

Version: 
Ubercart 1.7
flahertypj's picture
Offline
Joined: 01/20/2010
Juice: 2
Still an issue

This is still an issue in the latest release.