diff --git a/apps/comments/lib/Notification/Notifier.php b/apps/comments/lib/Notification/Notifier.php index 7c6a40133ee1c..4ddb7295bfec8 100644 --- a/apps/comments/lib/Notification/Notifier.php +++ b/apps/comments/lib/Notification/Notifier.php @@ -100,9 +100,9 @@ public function prepare(INotification $notification, string $languageCode): INot $displayName = $comment->getActorId(); $isDeletedActor = $comment->getActorType() === ICommentsManager::DELETED_USER; if ($comment->getActorType() === 'users') { - $commenter = $this->userManager->get($comment->getActorId()); - if ($commenter instanceof IUser) { - $displayName = $commenter->getDisplayName(); + $commenter = $this->userManager->getDisplayName($comment->getActorId()); + if ($commenter !== null) { + $displayName = $commenter; } } @@ -171,8 +171,8 @@ public function commentToRichMessage(IComment $comment): array { $mentions = $comment->getMentions(); foreach ($mentions as $mention) { if ($mention['type'] === 'user') { - $user = $this->userManager->get($mention['id']); - if (!$user instanceof IUser) { + $userDisplayName = $this->userManager->getDisplayName($mention['id']); + if ($userDisplayName === null) { continue; } } diff --git a/apps/comments/tests/Unit/Notification/NotifierTest.php b/apps/comments/tests/Unit/Notification/NotifierTest.php index 330530f0000da..ecd22ffd9e360 100644 --- a/apps/comments/tests/Unit/Notification/NotifierTest.php +++ b/apps/comments/tests/Unit/Notification/NotifierTest.php @@ -39,27 +39,28 @@ use OCP\IUserManager; use OCP\L10N\IFactory; use OCP\Notification\INotification; +use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; class NotifierTest extends TestCase { /** @var Notifier */ protected $notifier; - /** @var IFactory|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IFactory|MockObject */ protected $l10nFactory; - /** @var IL10N|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IL10N|MockObject */ protected $l; - /** @var IRootFolder|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IRootFolder|MockObject */ protected $folder; - /** @var ICommentsManager|\PHPUnit\Framework\MockObject\MockObject */ + /** @var ICommentsManager|MockObject */ protected $commentsManager; - /** @var IURLGenerator|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IURLGenerator|MockObject */ protected $url; - /** @var IUserManager|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IUserManager|MockObject */ protected $userManager; - /** @var INotification|\PHPUnit\Framework\MockObject\MockObject */ + /** @var INotification|MockObject */ protected $notification; - /** @var IComment|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IComment|MockObject */ protected $comment; /** @var string */ protected $lc = 'tlh_KX'; @@ -97,15 +98,7 @@ public function testPrepareSuccess() { $displayName = 'Huraga'; $message = '@Huraga mentioned you in a comment on "Gre\'thor.odp"'; - /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */ - $user = $this->createMock(IUser::class); - $user->expects($this->once()) - ->method('getDisplayName') - ->willReturn($displayName); - /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $you */ - $you = $this->createMock(IUser::class); - - /** @var Node|\PHPUnit\Framework\MockObject\MockObject $node */ + /** @var Node|MockObject $node */ $node = $this->createMock(Node::class); $node ->expects($this->atLeastOnce()) @@ -213,10 +206,10 @@ public function testPrepareSuccess() { $this->userManager ->expects($this->exactly(2)) - ->method('get') + ->method('getDisplayName') ->willReturnMap([ - ['huraga', $user], - ['you', $you], + ['huraga', $displayName], + ['you', 'You'], ]); $this->notifier->prepare($this->notification, $this->lc); @@ -226,10 +219,7 @@ public function testPrepareSuccessDeletedUser() { $fileName = 'Gre\'thor.odp'; $message = 'You were mentioned on "Gre\'thor.odp", in a comment by a user that has since been deleted'; - /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $you */ - $you = $this->createMock(IUser::class); - - /** @var Node|\PHPUnit\Framework\MockObject\MockObject $node */ + /** @var Node|MockObject $node */ $node = $this->createMock(Node::class); $node ->expects($this->atLeastOnce()) @@ -334,9 +324,11 @@ public function testPrepareSuccessDeletedUser() { $this->userManager ->expects($this->once()) - ->method('get') - ->with('you') - ->willReturn($you); + ->method('getDisplayName') + ->willReturnMap([ + ['huraga', null], + ['you', 'You'], + ]); $this->notifier->prepare($this->notification, $this->lc); } @@ -373,7 +365,7 @@ public function testPrepareDifferentApp() { $this->userManager ->expects($this->never()) - ->method('get'); + ->method('getDisplayName'); $this->notifier->prepare($this->notification, $this->lc); } @@ -411,7 +403,7 @@ public function testPrepareNotFound() { $this->userManager ->expects($this->never()) - ->method('get'); + ->method('getDisplayName'); $this->notifier->prepare($this->notification, $this->lc); } @@ -422,12 +414,6 @@ public function testPrepareDifferentSubject() { $displayName = 'Huraga'; - /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */ - $user = $this->createMock(IUser::class); - $user->expects($this->once()) - ->method('getDisplayName') - ->willReturn($displayName); - $this->folder ->expects($this->never()) ->method('getById'); @@ -472,9 +458,9 @@ public function testPrepareDifferentSubject() { $this->userManager ->expects($this->once()) - ->method('get') + ->method('getDisplayName') ->with('huraga') - ->willReturn($user); + ->willReturn($displayName); $this->notifier->prepare($this->notification, $this->lc); } @@ -485,12 +471,6 @@ public function testPrepareNotFiles() { $displayName = 'Huraga'; - /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */ - $user = $this->createMock(IUser::class); - $user->expects($this->once()) - ->method('getDisplayName') - ->willReturn($displayName); - $this->folder ->expects($this->never()) ->method('getById'); @@ -536,9 +516,9 @@ public function testPrepareNotFiles() { $this->userManager ->expects($this->once()) - ->method('get') + ->method('getDisplayName') ->with('huraga') - ->willReturn($user); + ->willReturn($displayName); $this->notifier->prepare($this->notification, $this->lc); } @@ -549,12 +529,6 @@ public function testPrepareUnresolvableFileID() { $displayName = 'Huraga'; - /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */ - $user = $this->createMock(IUser::class); - $user->expects($this->once()) - ->method('getDisplayName') - ->willReturn($displayName); - $userFolder = $this->createMock(Folder::class); $this->folder->expects($this->once()) ->method('getUserFolder') @@ -609,9 +583,9 @@ public function testPrepareUnresolvableFileID() { $this->userManager ->expects($this->once()) - ->method('get') + ->method('getDisplayName') ->with('huraga') - ->willReturn($user); + ->willReturn($displayName); $this->notifier->prepare($this->notification, $this->lc); } diff --git a/lib/private/Server.php b/lib/private/Server.php index 7223c3b8ae374..8bbc966854255 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -1186,14 +1186,12 @@ public function __construct($webRoot, \OC\Config $config) { $manager->registerDisplayNameResolver('user', function ($id) use ($c) { $manager = $c->get(IUserManager::class); - $user = $manager->get($id); - if (is_null($user)) { - $l = $c->getL10N('core'); - $displayName = $l->t('Unknown user'); - } else { - $displayName = $user->getDisplayName(); + $userDisplayName = $manager->getDisplayName($id); + if ($userDisplayName === null) { + $l = $c->get(IFactory::class)->get('core'); + return $l->t('Unknown user'); } - return $displayName; + return $userDisplayName; }); return $manager;