Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Status support - No Push and browser notifications when in DND #724

Merged
merged 3 commits into from
Aug 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion docs/ocs-endpoint-v2.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ In order to find out if notifications is installed/enabled on the server you can
"delete-all",
"icons",
"rich-strings",
"action-web"
"action-web",
"user-status"
]
}
}
Expand Down Expand Up @@ -103,6 +104,13 @@ Status | Explanation
`204 No Content` | please slow down the polling to once per hour, since there are no apps that can generate notifications
`304 Not Modified` | The provided `If-None-Match` matches the ETag, response body is empty

### Headers

Status | Explanation
---|---
`ETag` | See https://tools.ietf.org/html/rfc7232#section-2.3
`X-Nextcloud-User-Status` | Only available with the `user-status` capability. The user status (`away`, `dnd`, `offline`, `online`) should be taken into account and in case of `dnd` no notifications should be directly shown.

### Specification

Optional elements are still set in the array, the value is just empty:
Expand Down
2 changes: 1 addition & 1 deletion js/notifications.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/notifications.js.map

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions lib/Capabilities.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public function getCapabilities(): array {
'icons',
'rich-strings',
'action-web',
'user-status',
],
'push' => [
'devices',
Expand Down
24 changes: 21 additions & 3 deletions lib/Controller/EndpointController.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
use OCP\Notification\IAction;
use OCP\Notification\IManager;
use OCP\Notification\INotification;
use OCP\UserStatus\IManager as IUserStatusManager;
use OCP\UserStatus\IUserStatus;

class EndpointController extends OCSController {
/** @var Handler */
Expand All @@ -44,6 +46,8 @@ class EndpointController extends OCSController {
private $l10nFactory;
/** @var IUserSession */
private $session;
/** @var IUserStatusManager */
private $userStatusManager;
/** @var Push */
private $push;

Expand All @@ -53,13 +57,15 @@ public function __construct(string $appName,
IManager $manager,
IFactory $l10nFactory,
IUserSession $session,
IUserStatusManager $userStatusManager,
Push $push) {
parent::__construct($appName, $request);

$this->handler = $handler;
$this->manager = $manager;
$this->l10nFactory = $l10nFactory;
$this->session = $session;
$this->userStatusManager = $userStatusManager;
$this->push = $push;
}

Expand All @@ -71,10 +77,20 @@ public function __construct(string $appName,
* @return DataResponse
*/
public function listNotifications(string $apiVersion): DataResponse {
$userStatus = $this->userStatusManager->getUserStatuses([
$this->getCurrentUser(),
]);

$headers = ['X-Nextcloud-User-Status' => IUserStatus::ONLINE];
if (isset($userStatus[$this->getCurrentUser()])) {
$userStatus = $userStatus[$this->getCurrentUser()];
$headers['X-Nextcloud-User-Status'] = $userStatus->getStatus();
}

// When there are no apps registered that use the notifications
// We stop polling for them.
if (!$this->manager->hasNotifiers()) {
return new DataResponse(null, Http::STATUS_NO_CONTENT);
return new DataResponse(null, Http::STATUS_NO_CONTENT, $headers);
}

$filter = $this->manager->createNotification();
Expand All @@ -98,11 +114,13 @@ public function listNotifications(string $apiVersion): DataResponse {
}

$eTag = $this->generateETag($notificationIds);

$headers['ETag'] = $eTag;
if ($apiVersion !== 'v1' && $this->request->getHeader('If-None-Match') === $eTag) {
return new DataResponse([], Http::STATUS_NOT_MODIFIED);
return new DataResponse([], Http::STATUS_NOT_MODIFIED, $headers);
}

return new DataResponse($data, Http::STATUS_OK, ['ETag' => $eTag]);
return new DataResponse($data, Http::STATUS_OK, $headers);
}

/**
Expand Down
18 changes: 18 additions & 0 deletions lib/Push.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
use OCP\L10N\IFactory;
use OCP\Notification\IManager as INotificationManager;
use OCP\Notification\INotification;
use OCP\UserStatus\IManager as IUserStatusManager;
use OCP\UserStatus\IUserStatus;
use Symfony\Component\Console\Output\OutputInterface;

class Push {
Expand All @@ -58,6 +60,8 @@ class Push {
protected $clientService;
/** @var ICache */
protected $cache;
/** @var IUserStatusManager */
protected $userStatusManager;
/** @var IFactory */
protected $l10nFactory;
/** @var ILogger */
Expand All @@ -77,6 +81,7 @@ public function __construct(IDBConnection $connection,
IUserManager $userManager,
IClientService $clientService,
ICacheFactory $cacheFactory,
IUserStatusManager $userStatusManager,
IFactory $l10nFactory,
ILogger $log) {
$this->db = $connection;
Expand All @@ -87,6 +92,7 @@ public function __construct(IDBConnection $connection,
$this->userManager = $userManager;
$this->clientService = $clientService;
$this->cache = $cacheFactory->createDistributed('pushtokens');
$this->userStatusManager = $userStatusManager;
$this->l10nFactory = $l10nFactory;
$this->log = $log;
}
Expand Down Expand Up @@ -124,6 +130,18 @@ public function pushToDevice(int $id, INotification $notification, ?OutputInterf
return;
}

$userStatus = $this->userStatusManager->getUserStatuses([
$notification->getUser(),
]);

if (isset($userStatus[$notification->getUser()])) {
$userStatus = $userStatus[$notification->getUser()];
if ($userStatus->getStatus() === IUserStatus::DND) {
$this->printInfo('User status is set to DND');
return;
}
}

$devices = $this->getDevicesForUser($notification->getUser());
if (empty($devices)) {
$this->printInfo('No devices found for user');
Expand Down
8 changes: 8 additions & 0 deletions src/App.vue
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export default {
shutdown: false,
notifications: [],
lastETag: null,
userStatus: null,

/** @type {number} */
pollInterval: 30000, // milliseconds
Expand Down Expand Up @@ -99,6 +100,12 @@ export default {

return imagePath('notifications', iconPath)
},

showBrowserNotifications() {
return this.backgroundFetching
&& this.webNotificationsGranted
&& this.userStatus !== 'dnd'
},
},

mounted: function() {
Expand Down Expand Up @@ -216,6 +223,7 @@ export default {
this._setPollingInterval(300000)
return
} else if (response.data !== undefined && response.data.ocs !== undefined && response.data.ocs.data !== undefined && Array.isArray(response.data.ocs.data)) {
this.userStatus = response.headers['x-nextcloud-user-status']
this.lastETag = response.headers.etag
this.notifications = response.data.ocs.data
} else {
Expand Down
3 changes: 1 addition & 2 deletions src/Components/Notification.vue
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,7 @@ export default {
})

// Parents: TransitionGroup > NotificationsList
if (this.$parent.$parent.backgroundFetching
&& this.$parent.$parent.webNotificationsGranted) {
if (this.$parent.$parent.showBrowserNotifications) {
this._createWebNotification()
}
},
Expand Down
1 change: 1 addition & 0 deletions tests/Unit/CapabilitiesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public function testGetCapabilities() {
'icons',
'rich-strings',
'action-web',
'user-status',
],
'push' => [
'devices',
Expand Down
7 changes: 6 additions & 1 deletion tests/Unit/Controller/EndpointControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
use OCP\Notification\IAction;
use OCP\Notification\IManager;
use OCP\Notification\INotification;
use OCP\UserStatus\IManager as IUserStatusManager;
use PHPUnit\Framework\MockObject\MockObject;
use function Sabre\Event\Loop\instance;

Expand All @@ -55,7 +56,8 @@ class EndpointControllerTest extends TestCase {

/** @var IUserSession|MockObject */
protected $session;

/** @var IUserStatusManager|MockObject */
protected $userStatusManager;
/** @var EndpointController */
protected $controller;

Expand All @@ -72,6 +74,7 @@ protected function setUp(): void {
$this->manager = $this->createMock(IManager::class);
$this->l10nFactory = $this->createMock(IFactory::class);
$this->session = $this->createMock(IUserSession::class);
$this->userStatusManager = $this->createMock(IUserStatusManager::class);
$this->user = $this->createMock(IUser::class);
$this->push = $this->createMock(Push::class);

Expand All @@ -93,6 +96,7 @@ protected function getController(array $methods = [], $username = 'username') {
$this->manager,
$this->l10nFactory,
$this->session,
$this->userStatusManager,
$this->push
);
}
Expand All @@ -105,6 +109,7 @@ protected function getController(array $methods = [], $username = 'username') {
$this->manager,
$this->l10nFactory,
$this->session,
$this->userStatusManager,
$this->push,
])
->setMethods($methods)
Expand Down
6 changes: 6 additions & 0 deletions tests/Unit/PushTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
use OCP\L10N\IFactory;
use OCP\Notification\IManager as INotificationManager;
use OCP\Notification\INotification;
use OCP\UserStatus\IManager as IUserStatusManager;
use PHPUnit\Framework\MockObject\MockObject;

/**
Expand Down Expand Up @@ -69,6 +70,8 @@ class PushTest extends TestCase {
protected $cacheFactory;
/** @var ICache|MockObject */
protected $cache;
/** @var IUserStatusManager|MockObject */
protected $userStatusManager;
/** @var IFactory|MockObject */
protected $l10nFactory;
/** @var ILogger|MockObject */
Expand All @@ -86,6 +89,7 @@ protected function setUp(): void {
$this->clientService = $this->createMock(IClientService::class);
$this->cacheFactory = $this->createMock(ICacheFactory::class);
$this->cache = $this->createMock(ICache::class);
$this->userStatusManager = $this->createMock(IUserStatusManager::class);
$this->l10nFactory = $this->createMock(IFactory::class);
$this->logger = $this->createMock(ILogger::class);

Expand All @@ -110,6 +114,7 @@ protected function getPush(array $methods = []) {
$this->userManager,
$this->clientService,
$this->cacheFactory,
$this->userStatusManager,
$this->l10nFactory,
$this->logger,
])
Expand All @@ -126,6 +131,7 @@ protected function getPush(array $methods = []) {
$this->userManager,
$this->clientService,
$this->cacheFactory,
$this->userStatusManager,
$this->l10nFactory,
$this->logger
);
Expand Down