mirror OP notifications to NC notifications

Signed-off-by: Artur Neumann <[email protected]>

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

@@ -85,27 +85,48 @@ 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');
+
+			// see https://github.com/nextcloud/server/issues/1706 for docs
 			$richSubjectInstance = [
 				'type' => 'file',
 				'id' => 0,
 				'name' => $p['link'],
 				'path' => '',
 				'link' => $p['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, ',');
+			$markAsReadAction = $notification->createAction();
+			$markAsReadAction->setLabel('mark_as_read')
+			->setParsedLabel($l->t('Mark as read'))
+			->setPrimary(true)
+			->setLink($this->url->linkToRouteAbsolute(
+				'integration_openproject.openProjectAPI.markNotificationAsRead',
+				['workpackageId' => $p['wpId']]),
+				'DELETE'
+			);
 
-			$notification->setParsedSubject($content)
+			$notification->setParsedSubject('(' . $p['count']. ') ' . $p['resourceTitle'])
 				->setParsedMessage('--')
 				->setLink($p['link'] ?? '')
 				->setRichMessage(
-					$l->n('You have %s new notification in {instance}', 'You have %s new notifications in {instance}', $nbNotifications, [$nbNotifications]),
+					$message,
 					[
 						'instance' => $richSubjectInstance,
 					]
 				)
+				->addParsedAction($markAsReadAction)
 				->setIcon($this->url->getAbsoluteURL($this->url->imagePath(Application::APP_ID, 'app-dark.svg')));
 			return $notification;
 

lib/Service/OpenProjectAPIService.php

@@ -142,30 +142,41 @@ private function checkNotificationsForUser(string $userId): void {
 			'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);
+			$aggregatedNotifications = [];
 			if (!isset($notifications['error']) && count($notifications) > 0) {
-				$this->config->setUserValue(
-					$userId,
-					Application::APP_ID,
-					'last_notification_check',
-					"$newLastNotificationCheck"
-				);
-				$nbRelevantNotifications = 0;
 				foreach ($notifications as $n) {
-					$createdAt = new DateTime($n['createdAt']);
-					if ($createdAt->getTimestamp() > $lastNotificationCheck) {
-						$nbRelevantNotifications++;
+					$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,
+							// TODO according to the docs https://github.com/nextcloud/notifications/blob/master/docs/notification-workflow.md#creating-a-new-notification
+							// links should not be set here
+							'link' => self::sanitizeUrl(
+								$openprojectUrl . '/notifications/details/' . $wpId . '/activity/'
+							)
+						];
+					} else {
+						$aggregatedNotifications[$wpId]['count']++;
 					}
+					$aggregatedNotifications[$wpId]['reasons'][] = $n['reason'];
+					$aggregatedNotifications[$wpId]['actors'][] = $n['_links']['actor']['title'];
 				}
-				if ($nbRelevantNotifications > 0) {
-					$this->sendNCNotification($userId, 'new_open_tickets', [
-						'nbNotifications' => $nbRelevantNotifications,
-						'link' => self::sanitizeUrl($openprojectUrl . '/notifications')
-					]);
+				$manager = $this->notificationManager;
+				$notification = $manager->createNotification();
+				$notification->setApp(Application::APP_ID)
+					->setUser($userId);
+				$manager->markProcessed($notification);
+
+				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);
 				}
 			}
 		}
@@ -232,7 +243,10 @@ 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']) ||
+			!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

tests/lib/Service/OpenProjectAPIServiceCheckNotificationsTest.php

@@ -17,87 +17,22 @@
 use PHPUnit\Framework\TestCase;
 
 class OpenProjectAPIServiceCheckNotificationsTest extends TestCase {
-	/**
-	 * @var string
-	 */
-	private $oPNotificationAPIResponse = '
-					{
-					  "_type": "Collection",
-					  "_embedded": {
-						"elements": [
-						  {
-							"_type": "Notification",
-							"id": 21,
-							"reason": "commented",
-							"createdAt": "2022-05-11T10:10:10Z"
-						  },
-						  {
-							"_type": "Notification",
-							"id": 22,
-							"reason": "commented",
-							"createdAt": "2022-05-12T10:10:10Z"
-						  },
-						  {
-							"_type": "Notification",
-							"id": 23,
-							"reason": "commented",
-							"createdAt": "2022-05-13T10:10:10Z"
-						  },
-						  {
-							"_type": "Notification",
-							"id": 25,
-							"reason": "commented",
-							"createdAt": "2022-05-14T10:10:10Z"
-						  }
-						]
-					  }
-					}';
-
-	/**
-	 * @return array<mixed>
-	 */
-	public function checkNotificationDataProvider(): array {
-		return [
-			[ '', 4, true ], // last_notification_check was not set yet
-			[ '1652132430', 4, true ], // all notifications are were created after the last_notification_check
-			[ '1652350210', 2, true ], // some notifications were created after and some befor the last_notification_check
-			[ '1652605230', 0, false] // all notifications are older that last_notification_check
-		];
-	}
-	/**
-	 * @dataProvider checkNotificationDataProvider
-	 */
-	public function testCheckNotifications(
-		string $lastNotificationCheck,
-		int    $countOfReportedOPNotifications,
-		bool   $nextCloudNotificationFired
-	): void {
+	public function testCheckNotifications(): void {
 		$configMock = $this->getMockBuilder(IConfig::class)->getMock();
 		$configMock
 			->method('getUserValue')
 			->withConsecutive(
 				[$this->anything(), 'integration_openproject', 'token'],
 				[$this->anything(), 'integration_openproject', 'notification_enabled'],
-				[$this->anything(), 'integration_openproject', 'last_notification_check'],
 				[$this->anything(), 'integration_openproject', 'token'],
 				[$this->anything(), 'integration_openproject', 'refresh_token'],
 			)
 			->willReturnOnConsecutiveCalls(
 				'123456',
 				'1',
-				$lastNotificationCheck,
 				'123456',
 				'refresh-token',
 			);
-		$configMock
-			->expects($this->once())
-			->method('setUserValue')
-			->with(
-				$this->anything(),
-				'integration_openproject',
-				'last_notification_check',
-				$this->anything()
-			);
 
 		$configMock
 			->method('getAppValue')
@@ -120,42 +55,82 @@ public function testCheckNotifications(
 			);
 
 		$response = $this->getMockBuilder(IResponse::class)->getMock();
-		$response->method('getBody')->willReturn($this->oPNotificationAPIResponse);
+		$oPNotificationAPIResponse = '{
+			"_type": "Collection",
+			"_embedded": {
+			"elements":
+		';
+		$oPNotificationAPIResponse .= file_get_contents(
+			__DIR__ . '/../../jest/fixtures/notificationsResponse.json'
+		);
+		$oPNotificationAPIResponse .= '}}';
+		$response->method('getBody')->willReturn($oPNotificationAPIResponse);
 		$ocClient = $this->getMockBuilder('\OCP\Http\Client\IClient')->getMock();
 		$ocClient->method('get')->willReturn($response);
 		$clientService = $this->getMockBuilder('\OCP\Http\Client\IClientService')->getMock();
 		$clientService->method('newClient')->willReturn($ocClient);
 
 		$notificationManagerMock = $this->getMockBuilder(IManager::class)->getMock();
 
-		if ($nextCloudNotificationFired) {
-			$notificationMock = $this->getMockBuilder(INotification::class)
-				->getMock();
 
-			$notificationMock
-				->expects($this->once())
-				->method('setSubject')
-				->with(
-					'new_open_tickets',
+		$notificationMock = $this->getMockBuilder(INotification::class)
+			->getMock();
+
+		$notificationMock
+			->expects($this->exactly(3))
+			->method('setSubject')
+			->withConsecutive(
+				[
+					'op_notification',
 					[
-						'nbNotifications' => $countOfReportedOPNotifications,
-						'link' => 'https://openproject/notifications'
+						'wpId' => '36',
+						'resourceTitle' => 'write a software',
+						'projectTitle' => 'Dev-large',
+						'count' => 2,
+						'link' => 'https://openproject/notifications/details/36/activity',
+						'reasons' => ['assigned'],
+						'actors' => ['Admin de DEV user']
 					]
-				);
+				],
+				[
+					'op_notification',
+					[
+						'wpId' => '17',
+						'resourceTitle' => 'Create wireframes for new landing page',
+						'projectTitle' => 'Scrum project',
+						'count' => 5,
+						'link' => 'https://openproject/notifications/details/17/activity',
+						'reasons' => [0 => 'assigned', 3 => 'mentioned'],
+						'actors' => [0 => 'Admin de DEV user', 2 => 'Artur Neumann']
+					]
+				],
+				[
+					'op_notification',
+					[
+						'wpId' => '18',
+						'resourceTitle' => 'Contact form',
+						'projectTitle' => 'Scrum project',
+						'count' => 1,
+						'link' => 'https://openproject/notifications/details/18/activity',
+						'reasons' => ['mentioned'],
+						'actors' => ['Artur Neumann']
+					]
+				]
+			);
+
+		$notificationManagerMock
+			->expects($this->exactly(4)) //once for marking as read and once for every notification
+			->method('createNotification')
+			->willReturn($notificationMock);
 
-			$notificationManagerMock
-				->expects($this->once())
-				->method('createNotification')
-				->willReturn($notificationMock);
+		$notificationManagerMock
+			->expects($this->exactly(3))
+			->method('notify');
+
+		$notificationManagerMock
+			->expects($this->once())
+			->method('markProcessed');
 
-			$notificationManagerMock
-				->expects($this->once())
-				->method('notify');
-		} else {
-			$notificationManagerMock
-				->expects($this->never())
-				->method('notify');
-		}
 		$service = new OpenProjectAPIService(
 			'integration_openproject',
 			\OC::$server->get(IUserManager::class),

tests/lib/Service/OpenProjectAPIServiceTest.php

@@ -508,7 +508,7 @@ public function testGetNotificationsRequest() {
 		$providerResponse
 			->setStatus(Http::STATUS_OK)
 			->addHeader('Content-Type', 'application/json')
-			->setBody(["_embedded" => ["elements" => [['some' => 'data']]]]);
+			->setBody(["_embedded" => ["elements" => [['_links' => 'data']]]]);
 
 		$this->builder
 			->uponReceiving('a GET request to /notifications')
@@ -518,7 +518,7 @@ public function testGetNotificationsRequest() {
 		$result = $this->service->getNotifications(
 			'testUser'
 		);
-		$this->assertSame([['some' => 'data']], $result);
+		$this->assertSame([['_links' => 'data']], $result);
 	}
 
 	/**