In patch #1811210 the logic around blocking users was corrected. However - the fix was incomplete as it prevented simpletests from running correctly. I believe the problems all stem from the variable named: $notexpired_user.
$notexpired_user = db_select('inactive_users', 'ia')->fields('ia', array('uid'))->condition('ia.warned_user_block_timestamp', REQUEST_TIME, '<')->condition('ia.uid', $user->uid)->execute()->rowCount();
In the above mentioned patch the logical selector that follows was fixed:
if ($user->uid && $notexpired_user == 0 && ($user->created < (REQUEST_TIME - $block_time)))
The problem with the simpletests and why they are failing is that the code as written will never respect the warning time for the test cases. The Request time will always be less than the warning time and users are thus blocked regardless of when the warning time variable was generated in the inactive user table for that uid.
The other issue and perhaps the overriding cause of the weird behavior in user block warnings is that user block notifications function differently now then they did when the module was created. User block warnings as worded in the admin screens and as coded in the original module were intended to be sent out to a user at a time interval that is relative to the time interval selected for the user blocking. However, the current behavior for these messages is that they are sent out after the block warning time has elapsed after their last login. This essentially duplicates the functionality of the notify inactive user and changes the useful functionality of warning a user that their account is about to blocked at some meaningful interval before blocking so that the user might log in and prevent it from being blocked.
As far as i can tell the sql that selects these users in this way was introduced in the port of this module from Drupal 5 to Drupal 6 in this commit commit. First mention of this change and its impact was in this comment.
Let's fix this sort of meta issue around user warning blocks and the simple tests that are failing around this functionality. Users will still be able to notify users when they haven't logged in within a given period and we will reintroduce the intended functionality around block user warnings.
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | inactive_user-fix-user-block-warn-and-tests-1904904.patch | 3.44 KB | thebruce |
Comments
Comment #1
thebruce commentedHere is a patch that fixes the simpletests for the module and changes the formula for user warning blocks to use the following:
Send a warning if the date of the last login or user created date if they have never logged in is less than the REQUEST_TIME - the time interval that must pass for a user to be blocked + the amount of lead time we want to warn somebody or
Last Access < REQUEST_TIME - $block_time + $warn_timeThis is a correction over the initial formula before the change mentioned in the original post which was:
Last Access < REQUEST_TIME - $block_time - $warn_timeConsider the following:
A user last logged in at 60 units of time past the Unix Epoch. We set a $block_time of 60 units of time at which point we want a users account to blocked. So at 121 units of time past the Unix epoch that user should be blocked. Because we are considerate we would like them to be warned. We choose a time period of 2 units of time that they should be warned.
Let's take a look at the above formulas for whether a message should be sent based on a cron job that runs at 118 units of time and one that runs at 119 units of time.
For the cron job at 118 units of time:
60 < 118 - 60 + 2
60 < 60 This is not true - don't send a message yet
For the cron job at 119 units of time:
60 < 119 - 60 + 2
60 < 61 This is true - warn they user that in 2 weeks they are going to be blocked.
Using the old formula at 118 units of time:
60 < 118 - 60 - 2
60 < 56 This is not true don't send a message yet
Using the old formula at 119 units of time:
60 < 119 - 60 - 2
60 < 57 This is still not true don't send a message yet - even though in 2 weeks they will be blocked.
Given they way the code was written however - what would have happened is that the user would have been messaged before they were blocked and the blocked warn message in the inactive table would have still given them two weeks to login, however the time indication to administrators is a little misleading. With the block time really at 120 units + 2 with the old way.
Comment #2
deekayen commentedCommitted.