Problem/Motivation
When a test user is never logged in the account is not blocked.
Steps to reproduce
- Create a new test account.
- Run cron
- Account is not blocked
Proposed resolution
In \Drupal\role_test_accounts\RoleTestAccountsManager::setStatusForRoleTestAccounts replace
foreach ($this->getAllRoleTestAccounts() as $user) {
if ($user->getLastAccessedTime()) {
if ($user->isActive() && ($this->time->getRequestTime() - $block_after) > $user->getLastAccessedTime()) {
$user->block();
$user->save();
}
elseif ($user->isBlocked() && ($this->time->getRequestTime() - $block_after) < $user->getLastAccessedTime()) {
$user->activate();
$user->save();
}
}
}
with
foreach ($this->getAllRoleTestAccounts() as $user) {
if ($user->isActive() && ($this->time->getRequestTime() - $block_after) > $user->getLastAccessedTime()) {
$user->block();
$user->save();
}
elseif ($user->isBlocked() && ($this->time->getRequestTime() - $block_after) < $user->getLastAccessedTime()) {
$user->activate();
$user->save();
}
}
Remaining tasks
Create patch
- Review patch
Comments
Comment #2
perry.franken commentedBlock users who are never logged in.
Comment #3
perry.franken commentedComment #4
idebr commentedComment #5
idebr commentedRole test accounts are now blocked when the module is enabled, which somewhat defeats the purpose of this module.
Comment #6
bartlangelaanMaybe it would be better to use the user creation time when the user has never logged in.
So, something like:
and always use
$last_accessinstead of$user->getLastAccessedTime().This makes sure that the test users are not immediately blocked after they are created, but are still blocked after a certain amount of time.
Comment #7
bartlangelaanComment #8
perry.franken commentednew patch with suggested solution
Comment #9
perry.franken commentedComment #10
bartlangelaanI think the new solution looks good!
However, it looks like you accidentally added the previous patch to this patch.
Can you clean the patch and re-submit it?
Comment #11
perry.franken commentedFixed the patch
Comment #12
perry.franken commentedComment #13
perry.franken commentedComment #14
avpadernoComment #15
bartlangelaanWhile the patch looks good, it looks like it doesn't work. This is because
$user->getLastAccessedTime();always returns a string. When an account was never accessed, it returns "0". This doesn't invoke the null coalescing operator, so it doesn't give the desired effect.Something like this is more likely to work:
Comment #16
perry.franken commentedComment #17
perry.franken commentedComment #18
bartlangelaanLooks good!
Comment #20
idebr commentedCommitted, thanks!