From 026dc92c114fc4a3fa117d325f34fe75b85b0b64 Mon Sep 17 00:00:00 2001 From: lukmzig Date: Thu, 28 May 2026 10:52:40 +0200 Subject: [PATCH] fix user listing and add more test cases --- src/User/Repository/UserRepository.php | 7 +- .../User/Repository/UserRepositoryTest.php | 58 +++++++++ tests/Unit/User/Service/UserServiceTest.php | 120 +++++++++++++++++- 3 files changed, 177 insertions(+), 8 deletions(-) diff --git a/src/User/Repository/UserRepository.php b/src/User/Repository/UserRepository.php index ac5d875d0..fbbf470f9 100644 --- a/src/User/Repository/UserRepository.php +++ b/src/User/Repository/UserRepository.php @@ -99,6 +99,7 @@ public function updateUser(UserInterface $user): void public function getUserListingByRoleId(int $roleId, ?int $excludeUserId = null): UserListing { $listing = new UserListing(); + $listing->setCondition('`type` = :type', ['type' => 'user']); if ($excludeUserId !== null) { $listing->addConditionParam('id != :excludeUser', ['excludeUser' => $excludeUserId]); } @@ -174,8 +175,8 @@ public function searchUser(string $searchQuery): array try { $userListing = new UserListing(); $userListing->setCondition( - 'name LIKE ? OR firstname LIKE ? OR lastname LIKE ? OR email LIKE ? OR id = ?', - [$q, $q, $q, $q, (int)$searchQuery] + 'type = ? AND (name LIKE ? OR firstname LIKE ? OR lastname LIKE ? OR email LIKE ? OR id = ?)', + ['user', $q, $q, $q, $q, (int)$searchQuery] ); $userListing->setOrder('ASC'); $userListing->setOrderKey('name'); @@ -204,7 +205,7 @@ public function getUsersByNames(array $names): array $listing = new UserListing(); $placeholders = implode(',', array_fill(0, count($names), '?')); - $listing->setCondition('name IN (' . $placeholders . ')', $names); + $listing->setCondition('name IN (' . $placeholders . ') AND type = ?', [...$names, 'user']); $listing->load(); return $listing->getUsers(); diff --git a/tests/Unit/User/Repository/UserRepositoryTest.php b/tests/Unit/User/Repository/UserRepositoryTest.php index f62bcb209..20b4d03ea 100644 --- a/tests/Unit/User/Repository/UserRepositoryTest.php +++ b/tests/Unit/User/Repository/UserRepositoryTest.php @@ -16,6 +16,7 @@ use Codeception\Stub\Expected; use Codeception\Test\Unit; use Pimcore\Bundle\StaticResolverBundle\Models\User\UserResolverInterface; +use Pimcore\Bundle\StudioBackendBundle\Exception\Api\DatabaseException; use Pimcore\Bundle\StudioBackendBundle\Exception\Api\NotFoundException; use Pimcore\Bundle\StudioBackendBundle\Security\Service\SecurityServiceInterface; use Pimcore\Bundle\StudioBackendBundle\User\Repository\UserRepository; @@ -69,4 +70,61 @@ public function testDeleteUser() $userRepository = new UserRepository($securityServiceMock, $userResolverMock); $userRepository->deleteUser($userMock); } + + public function testCreateUser(): void + { + $expectedUser = new User(); + $expectedUser->setId(42); + + $securityServiceMock = $this->makeEmpty(SecurityServiceInterface::class); + $userResolverMock = $this->makeEmpty(UserResolverInterface::class, [ + 'create' => Expected::once(function (array $params) use ($expectedUser) { + $this->assertSame(5, $params['parentId']); + $this->assertSame('testuser', $params['name']); + $this->assertSame('', $params['password']); + $this->assertTrue($params['active']); + + return $expectedUser; + }), + ]); + + $userRepository = new UserRepository($securityServiceMock, $userResolverMock); + $result = $userRepository->createUser('testuser', 5); + + $this->assertSame($expectedUser, $result); + } + + public function testUpdateUserSuccess(): void + { + $securityServiceMock = $this->makeEmpty(SecurityServiceInterface::class); + $userResolverMock = $this->makeEmpty(UserResolverInterface::class); + + $userMock = $this->makeEmpty(UserInterface::class, [ + 'save' => Expected::once(function () use (&$userMock) { + return $userMock; + }), + ]); + + $userRepository = new UserRepository($securityServiceMock, $userResolverMock); + $userRepository->updateUser($userMock); + } + + public function testUpdateUserThrowsDatabaseException(): void + { + $securityServiceMock = $this->makeEmpty(SecurityServiceInterface::class); + $userResolverMock = $this->makeEmpty(UserResolverInterface::class); + + $userMock = $this->makeEmpty(UserInterface::class, [ + 'save' => function () { + throw new \Exception('Connection lost'); + }, + 'getId' => 7, + ]); + + $userRepository = new UserRepository($securityServiceMock, $userResolverMock); + + $this->expectException(DatabaseException::class); + $this->expectExceptionMessage('Error updating user with id 7: Connection lost'); + $userRepository->updateUser($userMock); + } } diff --git a/tests/Unit/User/Service/UserServiceTest.php b/tests/Unit/User/Service/UserServiceTest.php index 45e8f6605..b37cb335c 100644 --- a/tests/Unit/User/Service/UserServiceTest.php +++ b/tests/Unit/User/Service/UserServiceTest.php @@ -18,15 +18,18 @@ use Exception; use Pimcore\Bundle\StudioBackendBundle\Exception\Api\DatabaseException; use Pimcore\Bundle\StudioBackendBundle\Exception\Api\ForbiddenException; +use Pimcore\Bundle\StudioBackendBundle\Exception\Api\NotFoundException; use Pimcore\Bundle\StudioBackendBundle\Security\Service\SecurityServiceInterface; use Pimcore\Bundle\StudioBackendBundle\User\Hydrator\SimpleUserHydratorInterface; use Pimcore\Bundle\StudioBackendBundle\User\Hydrator\UserHydratorInterface; use Pimcore\Bundle\StudioBackendBundle\User\Hydrator\UserTreeNodeHydratorInterface; use Pimcore\Bundle\StudioBackendBundle\User\Repository\UserFolderRepositoryInterface; use Pimcore\Bundle\StudioBackendBundle\User\Repository\UserRepositoryInterface; +use Pimcore\Bundle\StudioBackendBundle\User\Schema\User as UserSchema; use Pimcore\Bundle\StudioBackendBundle\User\Service\UserService; use Pimcore\Model\User; use Pimcore\Model\UserInterface; +use ReflectionClass; use Symfony\Component\EventDispatcher\EventDispatcherInterface; /** @@ -95,23 +98,130 @@ public function testDeleteUser() $userService->deleteUser(1); } + public function testDeleteUserWhenBothAreAdmins(): void + { + $userToDelete = new User(); + $userToDelete->setAdmin(true); + + $securityServiceMock = $this->makeEmpty(SecurityServiceInterface::class, [ + 'getCurrentUser' => $this->makeEmpty(UserInterface::class, ['isAdmin' => true]), + ]); + + $userRepositoryMock = $this->makeEmpty(UserRepositoryInterface::class, [ + 'getUserById' => $userToDelete, + 'deleteUser' => Expected::once(), + ]); + + $userService = $this->getUserService($securityServiceMock, $userRepositoryMock); + $userService->deleteUser(1); + } + + public function testGetUserByIdThrowsForbiddenWhenNonAdminViewsAdmin(): void + { + $adminUser = new User(); + $adminUser->setAdmin(true); + + $securityServiceMock = $this->makeEmpty(SecurityServiceInterface::class, [ + 'getCurrentUser' => $this->makeEmpty(UserInterface::class, ['isAdmin' => false]), + ]); + + $userRepositoryMock = $this->makeEmpty(UserRepositoryInterface::class, [ + 'getUserById' => $adminUser, + ]); + + $userService = $this->getUserService($securityServiceMock, $userRepositoryMock); + + $this->expectException(ForbiddenException::class); + $this->expectExceptionMessage('Only admins can view other admins'); + $userService->getUserById(1); + } + + public function testGetUserByIdSuccess(): void + { + $user = new User(); + $user->setAdmin(false); + + $userSchema = (new ReflectionClass(UserSchema::class))->newInstanceWithoutConstructor(); + + $securityServiceMock = $this->makeEmpty(SecurityServiceInterface::class, [ + 'getCurrentUser' => $this->makeEmpty(UserInterface::class, ['isAdmin' => true]), + ]); + + $userRepositoryMock = $this->makeEmpty(UserRepositoryInterface::class, [ + 'getUserById' => $user, + ]); + + $userHydratorMock = $this->makeEmpty(UserHydratorInterface::class, [ + 'hydrate' => $userSchema, + ]); + + $eventDispatcherMock = $this->makeEmpty(EventDispatcherInterface::class, [ + 'dispatch' => Expected::once(function (object $event) { + return $event; + }), + ]); + + $userService = $this->getUserService( + $securityServiceMock, + $userRepositoryMock, + $eventDispatcherMock, + $userHydratorMock + ); + + $result = $userService->getUserById(1); + $this->assertSame($userSchema, $result); + } + + public function testGetUserNameByIdReturnsName(): void + { + $user = new User(); + $user->setName('johndoe'); + + $userRepositoryMock = $this->makeEmpty(UserRepositoryInterface::class, [ + 'getUserById' => $user, + ]); + + $userService = $this->getUserService( + $this->makeEmpty(SecurityServiceInterface::class), + $userRepositoryMock + ); + + $this->assertSame('johndoe', $userService->getUserNameById(1)); + } + + public function testGetUserNameByIdReturnsNullWhenNotFound(): void + { + $userRepositoryMock = $this->makeEmpty(UserRepositoryInterface::class, [ + 'getUserById' => function () { + throw new NotFoundException('User', 999); + }, + ]); + + $userService = $this->getUserService( + $this->makeEmpty(SecurityServiceInterface::class), + $userRepositoryMock + ); + + $this->assertNull($userService->getUserNameById(999)); + } + private function getUserService( SecurityServiceInterface $securityServiceMock, - UserRepositoryInterface $userRepositoryMock + UserRepositoryInterface $userRepositoryMock, + ?EventDispatcherInterface $eventDispatcherMock = null, + ?UserHydratorInterface $userHydratorMock = null, ): UserService { $userTreeNodeHydratorMock = $this->makeEmpty(UserTreeNodeHydratorInterface::class); - $eventDispatcherMock = $this->makeEmpty(EventDispatcherInterface::class); $userFolderRepositoryMock = $this->makeEmpty(UserFolderRepositoryInterface::class); - $userHydratorMock = $this->makeEmpty(UserHydratorInterface::class); $simpleUserHydratorMock = $this->makeEmpty(SimpleUserHydratorInterface::class); return new UserService( $userRepositoryMock, $userTreeNodeHydratorMock, - $eventDispatcherMock, + $eventDispatcherMock ?? $this->makeEmpty(EventDispatcherInterface::class), $securityServiceMock, $userFolderRepositoryMock, - $userHydratorMock, + $userHydratorMock ?? $this->makeEmpty(UserHydratorInterface::class), $simpleUserHydratorMock ); }