Use user displayname cache for comment mentions

Signed-off-by: Joas Schilling <coding@schilljs.com>
pull/33614/head
Joas Schilling 3 years ago
parent cab0f1327e
commit ae67eba84b
No known key found for this signature in database
GPG Key ID: C400AAF20C1BB6FC
  1. 10
      apps/comments/lib/Notification/Notifier.php
  2. 80
      apps/comments/tests/Unit/Notification/NotifierTest.php
  3. 12
      lib/private/Server.php

@ -100,9 +100,9 @@ class Notifier implements INotifier {
$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 @@ class Notifier implements INotifier {
$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;
}
}

@ -39,27 +39,28 @@ use OCP\IUser;
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 @@ class NotifierTest extends TestCase {
$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 @@ class NotifierTest extends TestCase {
$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 @@ class NotifierTest extends TestCase {
$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 @@ class NotifierTest extends TestCase {
$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 @@ class NotifierTest extends TestCase {
$this->userManager
->expects($this->never())
->method('get');
->method('getDisplayName');
$this->notifier->prepare($this->notification, $this->lc);
}
@ -411,7 +403,7 @@ class NotifierTest extends TestCase {
$this->userManager
->expects($this->never())
->method('get');
->method('getDisplayName');
$this->notifier->prepare($this->notification, $this->lc);
}
@ -422,12 +414,6 @@ class NotifierTest extends TestCase {
$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 @@ class NotifierTest extends TestCase {
$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 @@ class NotifierTest extends TestCase {
$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 @@ class NotifierTest extends TestCase {
$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 @@ class NotifierTest extends TestCase {
$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 @@ class NotifierTest extends TestCase {
$this->userManager
->expects($this->once())
->method('get')
->method('getDisplayName')
->with('huraga')
->willReturn($user);
->willReturn($displayName);
$this->notifier->prepare($this->notification, $this->lc);
}

@ -1186,14 +1186,12 @@ class Server extends ServerContainer implements IServerContainer {
$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;

Loading…
Cancel
Save