Merge pull request #2982 from nextcloud/better-notification-links
Set the link of the notification on render instead of creation
This commit is contained in:
commit
97a63ad689
|
@ -23,7 +23,6 @@ namespace OCA\Comments\Notification;
|
|||
|
||||
use OCP\Comments\CommentsEvent;
|
||||
use OCP\Comments\IComment;
|
||||
use OCP\IURLGenerator;
|
||||
use OCP\IUserManager;
|
||||
use OCP\Notification\IManager;
|
||||
|
||||
|
@ -34,25 +33,19 @@ class Listener {
|
|||
/** @var IUserManager */
|
||||
protected $userManager;
|
||||
|
||||
/** @var IURLGenerator */
|
||||
protected $urlGenerator;
|
||||
|
||||
/**
|
||||
* Listener constructor.
|
||||
*
|
||||
* @param IManager $notificationManager
|
||||
* @param IUserManager $userManager
|
||||
* @param IURLGenerator $urlGenerator
|
||||
*/
|
||||
public function __construct(
|
||||
IManager $notificationManager,
|
||||
IUserManager $userManager,
|
||||
IURLGenerator $urlGenerator
|
||||
IUserManager $userManager
|
||||
) {
|
||||
|
||||
$this->notificationManager = $notificationManager;
|
||||
$this->userManager = $userManager;
|
||||
$this->urlGenerator = $urlGenerator;
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -100,11 +93,7 @@ class Listener {
|
|||
->setApp('comments')
|
||||
->setObject('comment', $comment->getId())
|
||||
->setSubject('mention', [ $comment->getObjectType(), $comment->getObjectId() ])
|
||||
->setDateTime($comment->getCreationDateTime())
|
||||
->setLink($this->urlGenerator->linkToRouteAbsolute(
|
||||
'comments.Notifications.view',
|
||||
['id' => $comment->getId()])
|
||||
);
|
||||
->setDateTime($comment->getCreationDateTime());
|
||||
|
||||
return $notification;
|
||||
}
|
||||
|
|
|
@ -139,7 +139,11 @@ class Notifier implements INotifier {
|
|||
]
|
||||
);
|
||||
}
|
||||
$notification->setIcon($this->url->getAbsoluteURL($this->url->imagePath('core', 'actions/comment.svg')));
|
||||
$notification->setIcon($this->url->getAbsoluteURL($this->url->imagePath('core', 'actions/comment.svg')))
|
||||
->setLink($this->url->linkToRouteAbsolute(
|
||||
'comments.Notifications.view',
|
||||
['id' => $comment->getId()])
|
||||
);
|
||||
|
||||
return $notification;
|
||||
break;
|
||||
|
|
|
@ -46,14 +46,12 @@ class ListenerTest extends TestCase {
|
|||
protected function setUp() {
|
||||
parent::setUp();
|
||||
|
||||
$this->notificationManager = $this->getMockBuilder('\OCP\Notification\IManager')->getMock();
|
||||
$this->userManager = $this->getMockBuilder('\OCP\IUserManager')->getMock();
|
||||
$this->urlGenerator = $this->getMockBuilder('OCP\IURLGenerator')->getMock();
|
||||
$this->notificationManager = $this->createMock(\OCP\Notification\IManager::class);
|
||||
$this->userManager = $this->createMock(\OCP\IUserManager::class);
|
||||
|
||||
$this->listener = new Listener(
|
||||
$this->notificationManager,
|
||||
$this->userManager,
|
||||
$this->urlGenerator
|
||||
$this->userManager
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
@ -30,8 +30,6 @@ use OCP\Http\Client\IClientService;
|
|||
use OCP\IConfig;
|
||||
use OCP\IGroup;
|
||||
use OCP\IGroupManager;
|
||||
use OCP\IURLGenerator;
|
||||
use OCP\IUser;
|
||||
use OCP\Notification\IManager;
|
||||
|
||||
class BackgroundJob extends TimedJob {
|
||||
|
@ -51,10 +49,7 @@ class BackgroundJob extends TimedJob {
|
|||
/** @var IClientService */
|
||||
protected $client;
|
||||
|
||||
/** @var IURLGenerator */
|
||||
protected $urlGenerator;
|
||||
|
||||
/** @var IUser[] */
|
||||
/** @var string[] */
|
||||
protected $users;
|
||||
|
||||
/**
|
||||
|
@ -65,9 +60,8 @@ class BackgroundJob extends TimedJob {
|
|||
* @param IGroupManager $groupManager
|
||||
* @param IAppManager $appManager
|
||||
* @param IClientService $client
|
||||
* @param IURLGenerator $urlGenerator
|
||||
*/
|
||||
public function __construct(IConfig $config, IManager $notificationManager, IGroupManager $groupManager, IAppManager $appManager, IClientService $client, IURLGenerator $urlGenerator) {
|
||||
public function __construct(IConfig $config, IManager $notificationManager, IGroupManager $groupManager, IAppManager $appManager, IClientService $client) {
|
||||
// Run once a day
|
||||
$this->setInterval(60 * 60 * 24);
|
||||
|
||||
|
@ -76,7 +70,6 @@ class BackgroundJob extends TimedJob {
|
|||
$this->groupManager = $groupManager;
|
||||
$this->appManager = $appManager;
|
||||
$this->client = $client;
|
||||
$this->urlGenerator = $urlGenerator;
|
||||
}
|
||||
|
||||
protected function run($argument) {
|
||||
|
@ -97,8 +90,7 @@ class BackgroundJob extends TimedJob {
|
|||
|
||||
$status = $updater->check();
|
||||
if (isset($status['version'])) {
|
||||
$url = $this->urlGenerator->linkToRouteAbsolute('settings.AdminSettings.index') . '#updater';
|
||||
$this->createNotifications('core', $status['version'], $url, $status['versionstring']);
|
||||
$this->createNotifications('core', $status['version'], $status['versionstring']);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -110,8 +102,7 @@ class BackgroundJob extends TimedJob {
|
|||
foreach ($apps as $app) {
|
||||
$update = $this->isUpdateAvailable($app);
|
||||
if ($update !== false) {
|
||||
$url = $this->urlGenerator->linkToRouteAbsolute('settings.AppSettings.viewApps') . '#app-' . $app;
|
||||
$this->createNotifications($app, $update, $url);
|
||||
$this->createNotifications($app, $update);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -121,10 +112,9 @@ class BackgroundJob extends TimedJob {
|
|||
*
|
||||
* @param string $app
|
||||
* @param string $version
|
||||
* @param string $url
|
||||
* @param string $visibleVersion
|
||||
*/
|
||||
protected function createNotifications($app, $version, $url, $visibleVersion = '') {
|
||||
protected function createNotifications($app, $version, $visibleVersion = '') {
|
||||
$lastNotification = $this->config->getAppValue('updatenotification', $app, false);
|
||||
if ($lastNotification === $version) {
|
||||
// We already notified about this update
|
||||
|
@ -138,8 +128,7 @@ class BackgroundJob extends TimedJob {
|
|||
$notification = $this->notificationManager->createNotification();
|
||||
$notification->setApp('updatenotification')
|
||||
->setDateTime(new \DateTime())
|
||||
->setObject($app, $version)
|
||||
->setLink($url);
|
||||
->setObject($app, $version);
|
||||
|
||||
if ($visibleVersion !== '') {
|
||||
$notification->setSubject('update_available', ['version' => $visibleVersion]);
|
||||
|
|
|
@ -24,7 +24,10 @@
|
|||
namespace OCA\UpdateNotification\Notification;
|
||||
|
||||
|
||||
use OCP\IGroupManager;
|
||||
use OCP\IURLGenerator;
|
||||
use OCP\IUser;
|
||||
use OCP\IUserSession;
|
||||
use OCP\L10N\IFactory;
|
||||
use OCP\Notification\IManager;
|
||||
use OCP\Notification\INotification;
|
||||
|
@ -41,6 +44,12 @@ class Notifier implements INotifier {
|
|||
/** @var IFactory */
|
||||
protected $l10NFactory;
|
||||
|
||||
/** @var IUserSession */
|
||||
protected $userSession;
|
||||
|
||||
/** @var IGroupManager */
|
||||
protected $groupManager;
|
||||
|
||||
/** @var string[] */
|
||||
protected $appVersions;
|
||||
|
||||
|
@ -50,11 +59,15 @@ class Notifier implements INotifier {
|
|||
* @param IURLGenerator $url
|
||||
* @param IManager $notificationManager
|
||||
* @param IFactory $l10NFactory
|
||||
* @param IUserSession $userSession
|
||||
* @param IGroupManager $groupManager
|
||||
*/
|
||||
public function __construct(IURLGenerator $url, IManager $notificationManager, IFactory $l10NFactory) {
|
||||
public function __construct(IURLGenerator $url, IManager $notificationManager, IFactory $l10NFactory, IUserSession $userSession, IGroupManager $groupManager) {
|
||||
$this->url = $url;
|
||||
$this->notificationManager = $notificationManager;
|
||||
$this->l10NFactory = $l10NFactory;
|
||||
$this->userSession = $userSession;
|
||||
$this->groupManager = $groupManager;
|
||||
$this->appVersions = $this->getAppVersions();
|
||||
}
|
||||
|
||||
|
@ -76,6 +89,10 @@ class Notifier implements INotifier {
|
|||
|
||||
$parameters = $notification->getSubjectParameters();
|
||||
$notification->setParsedSubject($l->t('Update to %1$s is available.', [$parameters['version']]));
|
||||
|
||||
if ($this->isAdmin()) {
|
||||
$notification->setLink($this->url->linkToRouteAbsolute('settings.AdminSettings.index') . '#updater');
|
||||
}
|
||||
} else {
|
||||
$appInfo = $this->getAppInfo($notification->getObjectType());
|
||||
$appName = ($appInfo === null) ? $notification->getObjectType() : $appInfo['name'];
|
||||
|
@ -92,6 +109,10 @@ class Notifier implements INotifier {
|
|||
'name' => $appName,
|
||||
]
|
||||
]);
|
||||
|
||||
if ($this->isAdmin()) {
|
||||
$notification->setLink($this->url->linkToRouteAbsolute('settings.AppSettings.viewApps') . '#app-' . $notification->getObjectType());
|
||||
}
|
||||
}
|
||||
|
||||
$notification->setIcon($this->url->getAbsoluteURL($this->url->imagePath('updatenotification', 'notification.svg')));
|
||||
|
@ -113,6 +134,19 @@ class Notifier implements INotifier {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @return bool
|
||||
*/
|
||||
protected function isAdmin() {
|
||||
$user = $this->userSession->getUser();
|
||||
|
||||
if ($user instanceof IUser) {
|
||||
return $this->groupManager->isAdmin($user->getUID());
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
protected function getCoreVersions() {
|
||||
return implode('.', \OCP\Util::getVersion());
|
||||
}
|
||||
|
|
|
@ -45,18 +45,15 @@ class BackgroundJobTest extends TestCase {
|
|||
protected $appManager;
|
||||
/** @var IClientService|\PHPUnit_Framework_MockObject_MockObject */
|
||||
protected $client;
|
||||
/** @var IURLGenerator|\PHPUnit_Framework_MockObject_MockObject */
|
||||
protected $urlGenerator;
|
||||
|
||||
public function setUp() {
|
||||
parent::setUp();
|
||||
|
||||
$this->config = $this->getMockBuilder('OCP\IConfig')->getMock();
|
||||
$this->notificationManager = $this->getMockBuilder('OCP\Notification\IManager')->getMock();
|
||||
$this->groupManager = $this->getMockBuilder('OCP\IGroupManager')->getMock();
|
||||
$this->appManager = $this->getMockBuilder('OCP\App\IAppManager')->getMock();
|
||||
$this->client = $this->getMockBuilder('OCP\Http\Client\IClientService')->getMock();
|
||||
$this->urlGenerator = $this->getMockBuilder('OCP\IURLGenerator')->getMock();
|
||||
$this->config = $this->createMock(\OCP\IConfig::class);
|
||||
$this->notificationManager = $this->createMock(\OCP\Notification\IManager::class);
|
||||
$this->groupManager = $this->createMock(\OCP\IGroupManager::class);
|
||||
$this->appManager = $this->createMock(\OCP\App\IAppManager::class);
|
||||
$this->client = $this->createMock(\OCP\Http\Client\IClientService::class);
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -70,8 +67,7 @@ class BackgroundJobTest extends TestCase {
|
|||
$this->notificationManager,
|
||||
$this->groupManager,
|
||||
$this->appManager,
|
||||
$this->client,
|
||||
$this->urlGenerator
|
||||
$this->client
|
||||
);
|
||||
} {
|
||||
return $this->getMockBuilder('OCA\UpdateNotification\Notification\BackgroundJob')
|
||||
|
@ -81,7 +77,6 @@ class BackgroundJobTest extends TestCase {
|
|||
$this->groupManager,
|
||||
$this->appManager,
|
||||
$this->client,
|
||||
$this->urlGenerator,
|
||||
])
|
||||
->setMethods($methods)
|
||||
->getMock();
|
||||
|
@ -160,20 +155,12 @@ class BackgroundJobTest extends TestCase {
|
|||
}
|
||||
|
||||
if ($notification === null) {
|
||||
$this->urlGenerator->expects($this->never())
|
||||
->method('linkToRouteAbsolute');
|
||||
|
||||
$job->expects($this->never())
|
||||
->method('createNotifications');
|
||||
} else {
|
||||
$this->urlGenerator->expects($this->once())
|
||||
->method('linkToRouteAbsolute')
|
||||
->with('settings.AdminSettings.index')
|
||||
->willReturn('admin-url');
|
||||
|
||||
$job->expects($this->once())
|
||||
->method('createNotifications')
|
||||
->willReturn('core', $notification, 'admin-url#updater', $readableVersion);
|
||||
->willReturn('core', $notification, $readableVersion);
|
||||
}
|
||||
|
||||
$this->invokePrivate($job, 'checkCoreUpdate');
|
||||
|
@ -188,7 +175,7 @@ class BackgroundJobTest extends TestCase {
|
|||
['app2', '1.9.2'],
|
||||
],
|
||||
[
|
||||
['app2', '1.9.2', 'apps-url#app-app2'],
|
||||
['app2', '1.9.2'],
|
||||
],
|
||||
],
|
||||
];
|
||||
|
@ -215,11 +202,6 @@ class BackgroundJobTest extends TestCase {
|
|||
->method('isUpdateAvailable')
|
||||
->willReturnMap($isUpdateAvailable);
|
||||
|
||||
$this->urlGenerator->expects($this->exactly(sizeof($notifications)))
|
||||
->method('linkToRouteAbsolute')
|
||||
->with('settings.AppSettings.viewApps')
|
||||
->willReturn('apps-url');
|
||||
|
||||
$mockedMethod = $job->expects($this->exactly(sizeof($notifications)))
|
||||
->method('createNotifications');
|
||||
call_user_func_array([$mockedMethod, 'withConsecutive'], $notifications);
|
||||
|
@ -229,9 +211,9 @@ class BackgroundJobTest extends TestCase {
|
|||
|
||||
public function dataCreateNotifications() {
|
||||
return [
|
||||
['app1', '1.0.0', 'link1', '1.0.0', false, false, null, null],
|
||||
['app2', '1.0.1', 'link2', '1.0.0', '1.0.0', true, ['user1'], [['user1']]],
|
||||
['app3', '1.0.1', 'link3', false, false, true, ['user2', 'user3'], [['user2'], ['user3']]],
|
||||
['app1', '1.0.0', '1.0.0', false, false, null, null],
|
||||
['app2', '1.0.1', '1.0.0', '1.0.0', true, ['user1'], [['user1']]],
|
||||
['app3', '1.0.1', false, false, true, ['user2', 'user3'], [['user2'], ['user3']]],
|
||||
];
|
||||
}
|
||||
|
||||
|
@ -240,14 +222,13 @@ class BackgroundJobTest extends TestCase {
|
|||
*
|
||||
* @param string $app
|
||||
* @param string $version
|
||||
* @param string $url
|
||||
* @param string|false $lastNotification
|
||||
* @param string|false $callDelete
|
||||
* @param bool $createNotification
|
||||
* @param string[]|null $users
|
||||
* @param array|null $userNotifications
|
||||
*/
|
||||
public function testCreateNotifications($app, $version, $url, $lastNotification, $callDelete, $createNotification, $users, $userNotifications) {
|
||||
public function testCreateNotifications($app, $version, $lastNotification, $callDelete, $createNotification, $users, $userNotifications) {
|
||||
$job = $this->getJob([
|
||||
'deleteOutdatedNotifications',
|
||||
'getUsersToNotify',
|
||||
|
@ -299,10 +280,6 @@ class BackgroundJobTest extends TestCase {
|
|||
->method('setSubject')
|
||||
->with('update_available')
|
||||
->willReturnSelf();
|
||||
$notification->expects($this->once())
|
||||
->method('setLink')
|
||||
->with($url)
|
||||
->willReturnSelf();
|
||||
|
||||
if ($userNotifications !== null) {
|
||||
$mockedMethod = $notification->expects($this->exactly(sizeof($userNotifications)))
|
||||
|
@ -323,7 +300,7 @@ class BackgroundJobTest extends TestCase {
|
|||
->method('createNotification');
|
||||
}
|
||||
|
||||
$this->invokePrivate($job, 'createNotifications', [$app, $version, $url]);
|
||||
$this->invokePrivate($job, 'createNotifications', [$app, $version]);
|
||||
}
|
||||
|
||||
public function dataGetUsersToNotify() {
|
||||
|
|
|
@ -24,7 +24,9 @@ namespace OCA\UpdateNotification\Tests\Notification;
|
|||
|
||||
|
||||
use OCA\UpdateNotification\Notification\Notifier;
|
||||
use OCP\IGroupManager;
|
||||
use OCP\IURLGenerator;
|
||||
use OCP\IUserSession;
|
||||
use OCP\L10N\IFactory;
|
||||
use OCP\Notification\IManager;
|
||||
use OCP\Notification\INotification;
|
||||
|
@ -38,6 +40,10 @@ class NotifierTest extends TestCase {
|
|||
protected $notificationManager;
|
||||
/** @var IFactory|\PHPUnit_Framework_MockObject_MockObject */
|
||||
protected $l10nFactory;
|
||||
/** @var IUserSession|\PHPUnit_Framework_MockObject_MockObject */
|
||||
protected $userSession;
|
||||
/** @var IGroupManager|\PHPUnit_Framework_MockObject_MockObject */
|
||||
protected $groupManager;
|
||||
|
||||
public function setUp() {
|
||||
parent::setUp();
|
||||
|
@ -45,6 +51,8 @@ class NotifierTest extends TestCase {
|
|||
$this->urlGenerator = $this->createMock(IURLGenerator::class);
|
||||
$this->notificationManager = $this->createMock(IManager::class);
|
||||
$this->l10nFactory = $this->createMock(IFactory::class);
|
||||
$this->userSession = $this->createMock(IUserSession::class);
|
||||
$this->groupManager = $this->createMock(IGroupManager::class);
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -56,7 +64,9 @@ class NotifierTest extends TestCase {
|
|||
return new Notifier(
|
||||
$this->urlGenerator,
|
||||
$this->notificationManager,
|
||||
$this->l10nFactory
|
||||
$this->l10nFactory,
|
||||
$this->userSession,
|
||||
$this->groupManager
|
||||
);
|
||||
} {
|
||||
return $this->getMockBuilder(Notifier::class)
|
||||
|
@ -64,6 +74,8 @@ class NotifierTest extends TestCase {
|
|||
$this->urlGenerator,
|
||||
$this->notificationManager,
|
||||
$this->l10nFactory,
|
||||
$this->userSession,
|
||||
$this->groupManager,
|
||||
])
|
||||
->setMethods($methods)
|
||||
->getMock();
|
||||
|
|
Loading…
Reference in New Issue