Skip to content

Commit

Permalink
fix(users): use correct active user count
Browse files Browse the repository at this point in the history
Signed-off-by: Benjamin Gaussorgues <[email protected]>
  • Loading branch information
Altahrim committed Oct 10, 2024
1 parent 8330f5b commit 39c3180
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 27 deletions.
2 changes: 1 addition & 1 deletion apps/settings/lib/Controller/UsersController.php
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ public function usersList(): TemplateResponse {
$recentUsersGroup = [
'id' => '__nc_internal_recent',
'name' => $this->l10n->t('Recently active'),
'usercount' => $userCount,
'usercount' => $this->userManager->countSeenUsers(),
];

$disabledUsersGroup = [
Expand Down
48 changes: 26 additions & 22 deletions lib/private/User/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -742,45 +742,49 @@ public function validateUserId(string $uid, bool $checkDataDirectory = false): v
/**
* Gets the list of user ids sorted by lastLogin, from most recent to least recent
*
* @param int|null $limit how many users to fetch
* @param int|null $limit how many users to fetch (default: 25, max: 100)
* @param int $offset from which offset to fetch
* @param string $search search users based on search params
* @return list<string> list of user IDs

Check failure on line 748 in lib/private/User/Manager.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

MoreSpecificReturnType

lib/private/User/Manager.php:748:13: MoreSpecificReturnType: The declared return type 'list<string>' for OC\User\Manager::getLastLoggedInUsers is more specific than the inferred return type 'array<array-key, mixed>' (see https://psalm.dev/070)
*/
public function getLastLoggedInUsers(?int $limit = null, int $offset = 0, string $search = ''): array {
// We can't load all users who already logged in
$limit = min(100, $limit ?: 25);

$connection = \OC::$server->getDatabaseConnection();
$queryBuilder = $connection->getQueryBuilder();
$queryBuilder->select('login.userid')
->from('preferences', 'login')
->where($queryBuilder->expr()->eq('login.appid', $queryBuilder->expr()->literal('login')))
->andWhere($queryBuilder->expr()->eq('login.configkey', $queryBuilder->expr()->literal('lastLogin')))
->orderBy('login.configvalue', 'DESC')
->setFirstResult($offset)
->setMaxResults($limit);
->setMaxResults($limit)
->orderBy('login.configvalue', 'DESC')
->addOrderBy($queryBuilder->func()->lower('login.userid'), 'ASC');

if ($search === '') {
$queryBuilder->addOrderBy($queryBuilder->func()->lower('userid'), 'ASC');
} else {
$queryBuilder->leftJoin('login', 'preferences', 'email', $queryBuilder->expr()->andX(
$queryBuilder->expr()->eq('login.userid', 'email.userid'),
$queryBuilder->expr()->eq('email.appid', $queryBuilder->expr()->literal('settings')),
$queryBuilder->expr()->eq('email.configkey', $queryBuilder->expr()->literal('email')),
));
$queryBuilder->join('login', 'users', 'user', $queryBuilder->expr()->eq('login.userid', 'user.uid'));
$queryBuilder->andWhere($queryBuilder->expr()->orX(
$queryBuilder->expr()->iLike('uid', $queryBuilder->createPositionalParameter('%' . $connection->escapeLikeParameter($search) . '%')),
$queryBuilder->expr()->iLike('displayname', $queryBuilder->createPositionalParameter('%' . $connection->escapeLikeParameter($search) . '%')),
$queryBuilder->expr()->iLike('email.configvalue', $queryBuilder->createPositionalParameter('%' . $connection->escapeLikeParameter($search) . '%')),
));
$queryBuilder->addOrderBy('uid_lower', 'ASC');
return $queryBuilder

Check failure on line 766 in lib/private/User/Manager.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

LessSpecificReturnStatement

lib/private/User/Manager.php:766:11: LessSpecificReturnStatement: The type 'array<array-key, mixed>' is more general than the declared return type 'list<string>' for OC\User\Manager::getLastLoggedInUsers (see https://psalm.dev/129)
->executeQuery()
->fetchAll(\PDO::FETCH_COLUMN);
}

$result = $queryBuilder->executeQuery();
/** @var list<string> $uids */
$uids = $result->fetchAll(\PDO::FETCH_COLUMN);
$result->closeCursor();
$queryBuilder->leftJoin('login', 'preferences', 'email', $queryBuilder->expr()->andX(
$queryBuilder->expr()->eq('login.userid', 'email.userid'),
$queryBuilder->expr()->eq('email.appid', $queryBuilder->expr()->literal('settings')),
$queryBuilder->expr()->eq('email.configkey', $queryBuilder->expr()->literal('email')),
));

return $uids;
$displayNameMatches = $this->searchDisplayName($search);
$matchedUids = array_map(static fn (IUser $u): string => $u->getUID(), $displayNameMatches);

return $queryBuilder

Check failure on line 780 in lib/private/User/Manager.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

LessSpecificReturnStatement

lib/private/User/Manager.php:780:10: LessSpecificReturnStatement: The type 'array<array-key, mixed>' is more general than the declared return type 'list<string>' for OC\User\Manager::getLastLoggedInUsers (see https://psalm.dev/129)
->andWhere($queryBuilder->expr()->orX(
$queryBuilder->expr()->iLike('login.userid', $queryBuilder->createPositionalParameter('%' . $connection->escapeLikeParameter($search) . '%')),
$queryBuilder->expr()->iLike('email.configvalue', $queryBuilder->createPositionalParameter('%' . $connection->escapeLikeParameter($search) . '%')),
$queryBuilder->expr()->in('login.userid', $queryBuilder->createNamedParameter($matchedUids, IQueryBuilder::PARAM_STR_ARRAY)),
))
->executeQuery()
->fetchAll(\PDO::FETCH_COLUMN);
}

private function verifyUid(string $uid, bool $checkDataDirectory = false): bool {
Expand Down
13 changes: 9 additions & 4 deletions tests/lib/User/ManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use OCP\ICacheFactory;
use OCP\IConfig;
use OCP\IUser;
use OCP\IUserManager;
use Psr\Log\LoggerInterface;
use Test\TestCase;

Expand Down Expand Up @@ -579,7 +580,7 @@ public function testCountUsersTwoBackends(): void {
}

public function testCountUsersOnlyDisabled(): void {
$manager = \OC::$server->getUserManager();
$manager = \OCP\Server::get(IUserManager::class);
// count other users in the db before adding our own
$countBefore = $manager->countDisabledUsers();

Expand All @@ -604,7 +605,7 @@ public function testCountUsersOnlyDisabled(): void {
}

public function testCountUsersOnlySeen(): void {
$manager = \OC::$server->getUserManager();
$manager = \OCP\Server::get(IUserManager::class);
// count other users in the db before adding our own
$countBefore = $manager->countSeenUsers();

Expand All @@ -630,7 +631,7 @@ public function testCountUsersOnlySeen(): void {
}

public function testCallForSeenUsers(): void {
$manager = \OC::$server->getUserManager();
$manager = \OCP\Server::get(IUserManager::class);
// count other users in the db before adding our own
$count = 0;
$function = function (IUser $user) use (&$count) {
Expand Down Expand Up @@ -664,8 +665,12 @@ public function testCallForSeenUsers(): void {
}

public function testRecentlyActive(): void {
$manager = \OCP\Server::get(IUserManager::class);
$config = \OCP\Server::get(IConfig::class);
$manager = \OCP\Server::get(IUserManager::class);
// Restore backend, otherwise test fail when launching all tests in tests/lib
$backend = \OCP\Server::get(Database::class);
$manager->clearBackends();
$manager->registerBackend($backend);

// Create some users
$now = (string)time();
Expand Down

0 comments on commit 39c3180

Please sign in to comment.