From cfafbc84157a70557deca66e969b57912b66dc8f Mon Sep 17 00:00:00 2001 From: yemkareems Date: Thu, 4 Jul 2024 12:38:40 +0530 Subject: [PATCH] fix: removed references to old disabled users code. refactored query as per getDisplayNames function. limit and offset added to query. default limit set to 25. Signed-off-by: yemkareems --- .../lib/Controller/UsersController.php | 31 +++---------------- lib/private/AllConfig.php | 26 ++++++---------- lib/private/User/Manager.php | 21 ++----------- lib/public/IConfig.php | 4 ++- 4 files changed, 20 insertions(+), 62 deletions(-) diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 18c24f47cae..beb240681d0 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -278,7 +278,7 @@ class UsersController extends AUserData { * 200: Users details returned based on last logged in information */ public function getLastLoggedInUsers(string $search = '', - ?int $limit = null, + ?int $limit = 25, int $offset = 0, ): DataResponse { $currentUser = $this->userSession->getUser(); @@ -294,34 +294,11 @@ class UsersController extends AUserData { $users = []; - // Admin? Or SubAdmin? + // For Admin alone user sorting based on lastLogin. For sub admin and groups this is not supported $uid = $currentUser->getUID(); - $subAdminManager = $this->groupManager->getSubAdmin(); if ($this->groupManager->isAdmin($uid)) { $users = $this->userManager->getUsersSortedByLastLogin($limit, $offset, $search); $users = array_map(fn (IUser $user): string => $user->getUID(), $users); - } elseif ($subAdminManager->isSubAdmin($currentUser)) { - $subAdminOfGroups = $subAdminManager->getSubAdminsGroups($currentUser); - - $users = []; - /* We have to handle offset ourselve for correctness */ - $tempLimit = ($limit === null ? null : $limit + $offset); - foreach ($subAdminOfGroups as $group) { - $users = array_merge( - $users, - array_map( - fn (IUser $user): string => $user->getUID(), - array_filter( - $group->searchUsers($search, ($tempLimit === null ? null : $tempLimit - count($users))), - fn (IUser $user): bool => !$user->isEnabled() - ) - ) - ); - if (($tempLimit !== null) && (count($users) >= $tempLimit)) { - break; - } - } - $users = array_slice($users, $offset); } $usersDetails = []; @@ -332,13 +309,13 @@ class UsersController extends AUserData { // We still want to return all other accounts, but this one was removed from the backends // yet they are still in our database. Might be a LDAP remnant. $userData = null; - $this->logger->warning('Found one disabled account that was removed from its backend, but still exists in Nextcloud database', ['accountId' => $userId]); + $this->logger->warning('Found one account that was removed from its backend, but still exists in Nextcloud database', ['accountId' => $userId]); } // Do not insert empty entry if ($userData !== null) { $usersDetails[$userId] = $userData; } else { - // Currently logged in user does not have permissions to see this user + // Currently logged-in user does not have permissions to see this user // only showing its id $usersDetails[$userId] = ['id' => $userId]; } diff --git a/lib/private/AllConfig.php b/lib/private/AllConfig.php index 756ceabdbea..87b9c795f49 100644 --- a/lib/private/AllConfig.php +++ b/lib/private/AllConfig.php @@ -494,38 +494,32 @@ class AllConfig implements IConfig { /** * Gets the list of users based on their lastLogin info asc or desc * + * @param int $limit how many users to fetch + * @param int $offset from which offset to fetch * @param string $search search users based on search params * @return array of user IDs */ - public function getLastLoggedInUsers($search): array { + public function getLastLoggedInUsers(int $limit = 25, int $offset = 0, string $search = ''): array { // TODO - FIXME $this->fixDIInit(); $query = $this->connection->getQueryBuilder(); - $lastLoginSubSelect = $this->connection->getQueryBuilder(); - $lastLoginSubSelect->select('configvalue') - ->from('preferences', 'p2') - ->where($lastLoginSubSelect->expr()->andX( - $lastLoginSubSelect->expr()->eq('p2.userid', 'uid'), - $lastLoginSubSelect->expr()->eq('p2.appid', $lastLoginSubSelect->expr()->literal('login')), - $lastLoginSubSelect->expr()->eq('p2.configkey', $lastLoginSubSelect->expr()->literal('lastLogin')), - )); - $orderByExpression = $query->createFunction('(' . $lastLoginSubSelect->getSQL() .')'); - - $query->select('uid', 'displayname', $orderByExpression) + $query->select('uid', 'displayname') ->from('users', 'u') ->leftJoin('u', 'preferences', 'p', $query->expr()->andX( $query->expr()->eq('userid', 'uid'), - $query->expr()->eq('appid', $query->expr()->literal('settings')), - $query->expr()->eq('configkey', $query->expr()->literal('email'))) + $query->expr()->eq('appid', $query->expr()->literal('login')), + $query->expr()->eq('configkey', $query->expr()->literal('lastLogin'))) ) // sqlite doesn't like re-using a single named parameter here ->where($query->expr()->iLike('uid', $query->createPositionalParameter('%' . $this->connection->escapeLikeParameter($search) . '%'))) ->orWhere($query->expr()->iLike('displayname', $query->createPositionalParameter('%' . $this->connection->escapeLikeParameter($search) . '%'))) ->orWhere($query->expr()->iLike('configvalue', $query->createPositionalParameter('%' . $this->connection->escapeLikeParameter($search) . '%'))) - ->orderBy($orderByExpression, 'DESC') - ->addOrderBy('uid_lower', 'ASC'); + ->orderBy($query->func()->lower('configvalue'), 'DESC') + ->addOrderBy('uid_lower', 'ASC') + ->setFirstResult($offset) + ->setMaxResults($limit); $result = $query->executeQuery(); $displayNames = []; diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index 0ae24397f5f..93ba7f0eb6e 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -345,30 +345,15 @@ class Manager extends PublicEmitter implements IUserManager { /** * @return IUser[] */ - public function getUsersSortedByLastLogin(?int $limit = null, int $offset = 0, $search = ''): array { - $users = $this->config->getLastLoggedInUsers($search); - $users = array_combine( + public function getUsersSortedByLastLogin(?int $limit = 25, int $offset = 0, $search = ''): array { + $users = $this->config->getLastLoggedInUsers($limit, $offset, $search); + return array_combine( $users, array_map( fn (string $uid): IUser => new LazyUser($uid, $this), $users ) ); - - $tempLimit = ($limit === null ? null : $limit + $offset); - foreach ($this->backends as $backend) { - if (($tempLimit !== null) && (count($users) >= $tempLimit)) { - break; - } - if ($backend instanceof IProvideEnabledStateBackend) { - $backendUsers = $backend->getDisabledUserList(($tempLimit === null ? null : $tempLimit - count($users))); - foreach ($backendUsers as $uid) { - $users[$uid] = new LazyUser($uid, $this, null, $backend); - } - } - } - - return array_slice($users, $offset, $limit); } diff --git a/lib/public/IConfig.php b/lib/public/IConfig.php index a91fe1e62b0..b74292855a8 100644 --- a/lib/public/IConfig.php +++ b/lib/public/IConfig.php @@ -253,9 +253,11 @@ interface IConfig { /** * Gets the list of users based on their lastLogin info asc or desc * + * @param int $limit how many records to fetch + * @param int $offset from which offset to fetch * @param string $search search users based on search params * @return array of user IDs * @since 30.0.0 */ - public function getLastLoggedInUsers($search); + public function getLastLoggedInUsers(int $limit, int $offset, string $search): array; } -- 2.39.5