Add inactive user command #28171
Add inactive user command #28171
Conversation
core/Command/User/Inactive.php
Outdated
| protected function execute(InputInterface $input, OutputInterface $output) { | ||
| $days = $input->getArgument('days'); | ||
| if($days < 1) { | ||
| throw new InvalidArgumentException('Days must be above zero'); |
There was a problem hiding this comment.
also restrict to integers ?
core/Command/User/Inactive.php
Outdated
| } | ||
|
|
||
| $inactive = []; | ||
| $this->userManager->callForSeenUsers(function(IUser $user) use (&$inactive, $days) { |
There was a problem hiding this comment.
Sad that we can't just do a SQL query to retrieve those. The good thing at least is that the user manager now only operated on oc_accounts, not on remote systems.
There was a problem hiding this comment.
We can, since we're looking for oc_accounts where last_login != NULL, but I thought it was best to use the usermanager methods for this
There was a problem hiding this comment.
Yes, you're right. I think this approach is fine if we want it backportable.
core/Command/User/Inactive.php
Outdated
| $this->userManager->callForSeenUsers(function(IUser $user) use (&$inactive, $days) { | ||
| $lastLogin = $user->getLastLogin(); | ||
| // Difference between now and last login, into days, and rounded down | ||
| $daysSinceLastLogin = floor((time() - $lastLogin) / (60*60*24)); |
There was a problem hiding this comment.
For technical accuracy, I'd store time() in a variable before the loop. Imagine that if this loop takes several hours due to millions of entries, the value of time() will advance as well.
| ->willReturn(Base::OUTPUT_FORMAT_JSON); | ||
|
|
||
| $this->userManager->expects($this->once()) | ||
| ->method('callForSeenUsers'); |
There was a problem hiding this comment.
Please also add a test that actually runs the function in the loop and checks the response format.
You should be able to use ->will($this->returnCallback(function($func) { ... }) and then call the function from within your callback with active and inactive users.
PVince81
left a comment
PVince81
left a comment
There was a problem hiding this comment.
Looks good in general, a few more minor tweaks would be nice.
Please add unit test for the callback.
|
isnt this something we can handle within the report command? |
|
could do, but i read the description of the report command:
and that didnt sound too similar to:
so I made a new command :) |
|
Explanation makes sense 👍 |
|
@tomneedham please backport to stable10, looks safe enough |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
Allows admins to retrieve a parsable list of users that have been seen at least once, but have been inactive for a certain amount of days (no login since then). Scriptable because json output is supported with
--output-json.Usage:
./occ user:inactive 30for all users who have no logged in for 30 days.Motivation and Context
Admins wanting reports on inactive users in their instances.
How Has This Been Tested?
Types of changes
Checklist: