[OP#42323] mirror OP notifications to NC notifications

.github/workflows/ci.yml

@@ -71,6 +71,7 @@ jobs:
           git clone --depth 1 https://github.com/nextcloud/server.git -b ${{ matrix.nextcloudVersion }}
           cd server && git submodule update --init
           ./occ maintenance:install --admin-pass=admin
+          git clone --depth 1 https://github.com/nextcloud/notifications.git -b ${{ matrix.nextcloudVersion }} apps/notifications
 
       - name: PHP stan
         run: make phpstan

bootstrap.php

@@ -17,6 +17,7 @@
 $classLoader->addPsr4("OCA\\OpenProject\\Settings\\", __DIR__ . '/lib/Settings', true);
 $classLoader->addPsr4("OCP\\", $serverPath . '/lib/public', true);
 $classLoader->addPsr4("OC\\", $serverPath . '/lib/private', true);
+$classLoader->addPsr4("OCA\\Notifications\\", $serverPath . '/apps/notifications/lib/', true);
 $classLoader->addPsr4("OCA\\Files\\Event\\", $serverPath . '/apps/files/lib/Event', true);
 $classLoader->addPsr4("OCA\\OpenProject\\AppInfo\\", __DIR__ . '/lib/AppInfo', true);
 $classLoader->addPsr4("OCA\\OpenProject\\Controller\\", __DIR__ . '/lib/Controller', true);

lib/Controller/ConfigController.php

@@ -98,7 +98,6 @@ public function clearUserInfo(string $userId = null) {
 		$this->config->deleteUserValue($userId, Application::APP_ID, 'user_id');
 		$this->config->deleteUserValue($userId, Application::APP_ID, 'user_name');
 		$this->config->deleteUserValue($userId, Application::APP_ID, 'refresh_token');
-		$this->config->deleteUserValue($userId, Application::APP_ID, 'last_notification_check');
 	}
 
 	/**

lib/Notification/Notifier.php

@@ -12,15 +12,18 @@
 namespace OCA\OpenProject\Notification;
 
 use InvalidArgumentException;
+use OCA\OpenProject\Service\OpenProjectAPIService;
+use OCP\IConfig;
 use OCP\IURLGenerator;
 use OCP\IUserManager;
 use OCP\L10N\IFactory;
+use OCP\Notification\IDismissableNotifier;
 use OCP\Notification\IManager as INotificationManager;
 use OCP\Notification\INotification;
 use OCP\Notification\INotifier;
 use OCA\OpenProject\AppInfo\Application;
 
-class Notifier implements INotifier {
+class Notifier implements INotifier, IDismissableNotifier {
 
 	/** @var IFactory */
 	protected $factory;
@@ -34,6 +37,15 @@ class Notifier implements INotifier {
 	/** @var IURLGenerator */
 	protected $url;
 
+	/**
+	 * @var OpenProjectAPIService
+	 */
+	private $openprojectAPIService;
+
+	/**
+	 * @var IConfig
+	 */
+	private $config;
 	/**
 	 * @param IFactory $factory
 	 * @param IUserManager $userManager
@@ -43,11 +55,15 @@ class Notifier implements INotifier {
 	public function __construct(IFactory $factory,
 								IUserManager $userManager,
 								INotificationManager $notificationManager,
-								IURLGenerator $urlGenerator) {
+								IURLGenerator $urlGenerator,
+								OpenProjectAPIService $openprojectAPIService,
+								IConfig $config) {
 		$this->factory = $factory;
 		$this->userManager = $userManager;
 		$this->notificationManager = $notificationManager;
 		$this->url = $urlGenerator;
+		$this->openprojectAPIService = $openprojectAPIService;
+		$this->config = $config;
 	}
 
 	/**
@@ -85,23 +101,39 @@ public function prepare(INotification $notification, string $languageCode): INot
 		$l = $this->factory->get('integration_openproject', $languageCode);
 
 		switch ($notification->getSubject()) {
-		case 'new_open_tickets':
+		case 'op_notification':
 			$p = $notification->getSubjectParameters();
-			$nbNotifications = (int) ($p['nbNotifications'] ?? 0);
-			$content = $l->t('OpenProject activity');
+			$openprojectUrl = $this->config->getAppValue(
+				Application::APP_ID, 'oauth_instance_url'
+			);
+			$link = OpenProjectAPIService::sanitizeUrl(
+				$openprojectUrl . '/notifications/details/' . $p['wpId'] . '/activity/'
+			);
+			// see https://github.com/nextcloud/server/issues/1706 for docs
 			$richSubjectInstance = [
 				'type' => 'file',
 				'id' => 0,
-				'name' => $p['link'],
+				'name' => $link,
 				'path' => '',
-				'link' => $p['link'],
+				'link' => $link,
 			];
+			$message = $p['projectTitle'] . ' - ';
+			foreach ($p['reasons'] as $reason) {
+				$message .= $reason . ',';
+			}
+			$message = rtrim($message, ',');
+			$message .= ' ' . $l->t('by') . ' ';
+
+			foreach ($p['actors'] as $actor) {
+				$message .= $actor . ',';
+			}
+			$message = rtrim($message, ',');
 
-			$notification->setParsedSubject($content)
+			$notification->setParsedSubject('(' . $p['count']. ') ' . $p['resourceTitle'])
 				->setParsedMessage('--')
-				->setLink($p['link'] ?? '')
+				->setLink($link)
 				->setRichMessage(
-					$l->n('You have %s new notification in {instance}', 'You have %s new notifications in {instance}', $nbNotifications, [$nbNotifications]),
+					$message,
 					[
 						'instance' => $richSubjectInstance,
 					]
@@ -114,4 +146,27 @@ public function prepare(INotification $notification, string $languageCode): INot
 			throw new InvalidArgumentException();
 		}
 	}
+
+
+	/**
+	 * @inheritDoc
+	 */
+	public function dismissNotification(INotification $notification): void {
+		if ($notification->getApp() !== Application::APP_ID) {
+			throw new \InvalidArgumentException('Unhandled app');
+		}
+		$refreshNotificationsInProgress = $this->config->getUserValue(
+			$notification->getUser(),
+			Application::APP_ID,
+			'refresh-notifications-in-progress',
+			'false'
+		);
+		if ($refreshNotificationsInProgress === 'false') {
+			$parameters = $notification->getSubjectParameters();
+			$this->openprojectAPIService->markAllNotificationsOfWorkPackageAsRead(
+				$parameters['wpId'],
+				$notification->getUser()
+			);
+		}
+	}
 }

lib/Service/OpenProjectAPIService.php

@@ -14,6 +14,7 @@
 use DateTime;
 use DateTimeZone;
 use Exception;
+use OCA\Notifications\Handler;
 use OCP\Files\Node;
 use OCA\OpenProject\Exception\OpenprojectErrorException;
 use OCA\OpenProject\Exception\OpenprojectResponseException;
@@ -89,6 +90,12 @@ class OpenProjectAPIService {
 	 * @var ICache
 	 */
 	private $cache = null;
+
+	/**
+	 * @var Handler
+	 */
+	private $handler;
+
 	/**
 	 * Service to make requests to OpenProject v3 (JSON) API
 	 */
@@ -103,7 +110,8 @@ public function __construct(
 								IClientService $clientService,
 								IRootFolder $storage,
 								IURLGenerator $urlGenerator,
-								ICacheFactory $cacheFactory) {
+								ICacheFactory $cacheFactory,
+								Handler $handler = null) {
 		$this->appName = $appName;
 		$this->userManager = $userManager;
 		$this->avatarManager = $avatarManager;
@@ -114,7 +122,7 @@ public function __construct(
 		$this->client = $clientService->newClient();
 		$this->storage = $storage;
 		$this->urlGenerator = $urlGenerator;
-
+		$this->handler = $handler;
 		$this->cache = $cacheFactory->createDistributed();
 	}
 
@@ -135,38 +143,84 @@ public function checkNotifications(): void {
 	 * @return void
 	 */
 	private function checkNotificationsForUser(string $userId): void {
+		if ($this->handler === null) {
+			// notifications app seems not to exist
+			return;
+		}
 		$accessToken = $this->config->getUserValue($userId, Application::APP_ID, 'token');
 		$notificationEnabled = ($this->config->getUserValue(
 			$userId,
 			Application::APP_ID,
 			'notification_enabled',
 			$this->config->getAppValue(Application::APP_ID, 'default_enable_notifications', '0')) === '1');
 		if ($accessToken && $notificationEnabled) {
-			$lastNotificationCheck = $this->config->getUserValue($userId, Application::APP_ID, 'last_notification_check');
-			$lastNotificationCheck = $lastNotificationCheck === '' ? 0 : (int)$lastNotificationCheck;
-			$newLastNotificationCheck = time();
-			$openprojectUrl = $this->config->getAppValue(Application::APP_ID, 'oauth_instance_url');
 			$notifications = $this->getNotifications($userId);
-			if (!isset($notifications['error']) && count($notifications) > 0) {
+			$aggregatedNotifications = [];
+			if (!isset($notifications['error'])) {
+				foreach ($notifications as $n) {
+					$wpId = preg_replace('/.*\//', '', $n['_links']['resource']['href']);
+					if (!array_key_exists($wpId, $aggregatedNotifications)) {
+						$aggregatedNotifications[$wpId] = [
+							'wpId' => $wpId,
+							'resourceTitle' => $n['_links']['resource']['title'],
+							'projectTitle' => $n['_links']['project']['title'],
+							'count' => 1,
+							'updatedAt' => $n['updatedAt']
+						];
+					} else {
+						$storedUpdatedAt = \Safe\strtotime($aggregatedNotifications[$wpId]['updatedAt']);
+						if (\Safe\strtotime($n['updatedAt']) > $storedUpdatedAt) {
+							// currently the code never comes here because the notifications are ordered
+							// by 'updatedAt' but as backup I would keep it
+							$aggregatedNotifications[$wpId]['updatedAt'] = $n['updatedAt'];
+						}
+						$aggregatedNotifications[$wpId]['count']++;
+					}
+					$aggregatedNotifications[$wpId]['reasons'][] = $n['reason'];
+					$aggregatedNotifications[$wpId]['actors'][] = $n['_links']['actor']['title'];
+				}
+				$manager = $this->notificationManager;
+				$notificationsFilter = $manager->createNotification();
+				$notificationsFilter->setApp(Application::APP_ID)
+					->setUser($userId);
 				$this->config->setUserValue(
 					$userId,
 					Application::APP_ID,
-					'last_notification_check',
-					"$newLastNotificationCheck"
+					'refresh-notifications-in-progress',
+					'true'
 				);
-				$nbRelevantNotifications = 0;
-				foreach ($notifications as $n) {
-					$createdAt = new DateTime($n['createdAt']);
-					if ($createdAt->getTimestamp() > $lastNotificationCheck) {
-						$nbRelevantNotifications++;
+				$currentNotifications = $this->handler->get($notificationsFilter);
+				foreach ($currentNotifications as $notificationId => $currentNotification) {
+					$parametersCurrentNotifications = $currentNotification->getSubjectParameters();
+					$wpId = $parametersCurrentNotifications['wpId'];
+					if (isset($aggregatedNotifications[$wpId])) {
+						$currentNotificationUpdateTime = \Safe\strtotime($parametersCurrentNotifications['updatedAt']);
+						$newNotificationUpdateTime = \Safe\strtotime($aggregatedNotifications[$wpId]['updatedAt']);
+
+						if ($newNotificationUpdateTime <= $currentNotificationUpdateTime) {
+							// nothing changed with any notification associated with that WP
+							// so get rid of it
+							unset($aggregatedNotifications[$wpId]);
+						} else {
+							$manager->markProcessed($currentNotification);
+						}
+					} else { // there are no notifications in OP associated with that WP
+						$manager->markProcessed($currentNotification);
 					}
 				}
-				if ($nbRelevantNotifications > 0) {
-					$this->sendNCNotification($userId, 'new_open_tickets', [
-						'nbNotifications' => $nbRelevantNotifications,
-						'link' => self::sanitizeUrl($openprojectUrl . '/notifications')
-					]);
+
+				foreach ($aggregatedNotifications as $n) {
+					$n['reasons'] = array_unique($n['reasons']);
+					$n['actors'] = array_unique($n['actors']);
+					// TODO can we use https://github.com/nextcloud/notifications/blob/master/docs/notification-workflow.md#defer-and-flush ?
+					$this->sendNCNotification($userId, 'op_notification', $n);
 				}
+				$this->config->setUserValue(
+					$userId,
+					Application::APP_ID,
+					'refresh-notifications-in-progress',
+					'false'
+				);
 			}
 		}
 	}
@@ -232,7 +286,13 @@ public function getNotifications(string $userId): array {
 		$result = $this->request($userId, 'notifications', $params);
 		if (isset($result['error'])) {
 			return $result;
-		} elseif (!isset($result['_embedded']['elements'])) {
+		} elseif (
+			!isset($result['_embedded']['elements']) ||
+			(   // if there is an element, it also has to contain '_links'
+				isset($result['_embedded']['elements'][0]) &&
+				!isset($result['_embedded']['elements'][0]['_links'])
+			)
+		) {
 			return ['error' => 'Malformed response'];
 		}
 

tests/lib/Controller/ConfigControllerTest.php

@@ -702,21 +702,19 @@ public function testSetAdminConfigClearUserDataChangeNCOauthClient(
 
 		if ($deleteUserValues === true) {
 			$configMock
-				->expects($this->exactly(12)) // 6 times for each user
+				->expects($this->exactly(10)) // 5 times for each user
 				->method('deleteUserValue')
 				->withConsecutive(
 					['admin', 'integration_openproject', 'token'],
 					['admin', 'integration_openproject', 'login'],
 					['admin', 'integration_openproject', 'user_id'],
 					['admin', 'integration_openproject', 'user_name'],
 					['admin', 'integration_openproject', 'refresh_token'],
-					['admin', 'integration_openproject', 'last_notification_check'],
 					[$this->user1->getUID(), 'integration_openproject', 'token'],
 					[$this->user1->getUID(), 'integration_openproject', 'login'],
 					[$this->user1->getUID(), 'integration_openproject', 'user_id'],
 					[$this->user1->getUID(), 'integration_openproject', 'user_name'],
 					[$this->user1->getUID(), 'integration_openproject', 'refresh_token'],
-					[$this->user1->getUID(), 'integration_openproject', 'last_notification_check'],
 				);
 		} else {
 			$configMock