Users are NEVER removed from inactivity list [#504038]

Due to erroneous handling of database access in lines 234-245 function inactive_user_cron(), users are never removed from table inactive_users.

These lines should be changed to:

// reset notifications if recent user activity
    $users = db_query('SELECT uid FROM {inactive_users} WHERE uid <> 1'));
    if ($users) {
      while($uid = db_fetch_object($users)) {
        $u = db_fetch_object(db_query('SELECT access, name FROM {users} WHERE uid = %d', $uid->uid));
        if ($u->access > time() - 604800) {
          // user activity in last week, remove from inactivity table
          db_query('DELETE FROM {inactive_users} WHERE uid = %d', $uid->uid);
          watchdog('user', t('recent user activity: %user removed from inactivity list', array('%user' => $u->name)), WATCHDOG_NOTICE, l(t('edit user'), "admin/user/edit/$uid->uid"));
        }
      }
    }

The bugs are in all versions of this module.

Plus, in general, some of the database queries are substantially not efficient.

Comments

Comment #1

ekrispin CreditAttribution: ekrispin commented 27 June 2009 at 17:22

Sorry, there is one extra ")" in second line.

It should be:

// reset notifications if recent user activity
    $users = db_query('SELECT uid FROM {inactive_users} WHERE uid <> 1');
    if ($users) {
      while($uid = db_fetch_object($users)) {
        $u = db_fetch_object(db_query('SELECT access, name FROM {users} WHERE uid = %d', $uid->uid));
        if ($u->access > time() - 604800) {
          // user activity in last week, remove from inactivity table
          db_query('DELETE FROM {inactive_users} WHERE uid = %d', $uid->uid);
          watchdog('user', t('recent user activity: %user removed from inactivity list', array('%user' => $u->name)), WATCHDOG_NOTICE, l(t('edit user'), "admin/user/edit/$uid->uid"));
        }
      }
    }

Comment #2

deekayen CreditAttribution: deekayen commented 29 June 2009 at 14:52

Status:

Active

» Needs work

Don't you think it can be cut down a lot more than that? I haven't tried it, but I'm picturing something like:

db_query('DELETE FROM {inactive_users} WHERE uid IN (SELECT uid FROM {inactive_users} AS iu INNER JOIN {users} AS u ON u.uid = iu.uid WHERE u.access > %d)', array(time() - 6048800));