Fix deleting notifications with numeric user ID

Signed-off-by: Joas Schilling <[email protected]>

lib/App.php

@@ -83,7 +83,7 @@ public function markProcessed(INotification $notification): void {
 		}
 		foreach ($deleted as $user => $notifications) {
 			foreach ($notifications as $data) {
-				$this->push->pushDeleteToDevice($user, $data['id'], $data['app']);
+				$this->push->pushDeleteToDevice((string) $user, $data['id'], $data['app']);
 			}
 		}
 		if (!$isAlreadyDeferring) {

lib/Controller/EndpointController.php

@@ -182,7 +182,7 @@ public function deleteNotification(int $id): DataResponse {
 
 		try {
 			$notification = $this->handler->getById($id, $this->getCurrentUser());
-			$deleted = $this->handler->deleteById($id, $this->getCurrentUser());
+			$deleted = $this->handler->deleteById($id, $this->getCurrentUser(), $notification);
 
 			if ($deleted) {
 				$this->push->pushDeleteToDevice($this->getCurrentUser(), $id, $notification->getApp());

lib/Handler.php

@@ -110,7 +110,7 @@ public function delete(INotification $notification): array {
 
 		foreach ($deleted as $user => $entries) {
 			foreach ($entries as $entry) {
-				$this->deleteById($entry['id'], $user, $notifications[$entry['id']]);
+				$this->deleteById($entry['id'], (string) $user, $notifications[$entry['id']]);
 			}
 		}
 

tests/Integration/app/lib/Controller/EndpointController.php

@@ -44,13 +44,14 @@ public function __construct($appName, IRequest $request, IManager $manager) {
 	/**
 	 * @NoCSRFRequired
 	 *
+	 * @param string $userId
 	 * @return DataResponse
 	 */
-	public function addNotification() {
+	public function addNotification(string $userId = 'test1') {
 		$notification = $this->manager->createNotification();
 		$notification->setApp($this->request->getParam('app', 'notificationsintegrationtesting'))
 			->setDateTime(\DateTime::createFromFormat('U', $this->request->getParam('timestamp', 1449585176))) // 2015-12-08T14:32:56+00:00
-			->setUser($this->request->getParam('user', 'test1'))
+			->setUser($this->request->getParam('user', $userId))
 			->setSubject($this->request->getParam('subject', 'testing'))
 			->setLink($this->request->getParam('link', 'https://example.tld/'))
 			->setMessage($this->request->getParam('message', 'message'))

tests/Integration/features/bootstrap/FeatureContext.php

@@ -70,10 +70,8 @@ public function __construct() {
 	 * @param string $user
 	 */
 	public function hasNotifications(string $user) {
-		if ($user === 'test1') {
-			$response = $this->setTestingValue('POST', 'apps/notificationsintegrationtesting/notifications', null);
-			$this->assertStatusCode($response, 200);
-		}
+		$response = $this->setTestingValue('POST', 'apps/notificationsintegrationtesting/notifications?userId=' . $user, null);
+		$this->assertStatusCode($response, 200);
 	}
 
 	/**
@@ -83,10 +81,8 @@ public function hasNotifications(string $user) {
 	 * @param TableNode|null $formData
 	 */
 	public function receiveNotification(string $user, TableNode $formData) {
-		if ($user === 'test1') {
-			$response = $this->setTestingValue('POST', 'apps/notificationsintegrationtesting/notifications', $formData);
-			$this->assertStatusCode($response, 200);
-		}
+		$response = $this->setTestingValue('POST', 'apps/notificationsintegrationtesting/notifications?userId=' . $user, $formData);
+		$this->assertStatusCode($response, 200);
 	}
 
 	/**
@@ -165,28 +161,26 @@ protected function getArrayOfNotificationsResponded(ResponseInterface $response)
 	 * @param string $missingLast
 	 */
 	public function userNumNotifications(string $user, int $numNotifications, string $api, string $missingLast) {
-		if ($user === 'test1') {
-			$this->sendingTo('GET', '/apps/notifications/api/' . $api . '/notifications?format=json');
-			$this->assertStatusCode($this->response, 200);
-
-			$previousNotificationIds = [];
-			if ($missingLast) {
-				Assert::assertNotEmpty($this->notificationIds);
-				$previousNotificationIds = end($this->notificationIds);
-			}
+		$this->sendingTo('GET', '/apps/notifications/api/' . $api . '/notifications?format=json');
+		$this->assertStatusCode($this->response, 200);
 
-			$this->checkNumNotifications((int) $numNotifications);
+		$previousNotificationIds = [];
+		if ($missingLast) {
+			Assert::assertNotEmpty($this->notificationIds);
+			$previousNotificationIds = end($this->notificationIds);
+		}
 
-			if ($missingLast) {
-				$now = end($this->notificationIds);
-				if ($missingLast === ' missing the last one') {
-					array_unshift($now, $this->deletedNotification);
-				} else {
-					$now[] = $this->deletedNotification;
-				}
+		$this->checkNumNotifications((int) $numNotifications);
 
-				Assert::assertEquals($previousNotificationIds, $now);
+		if ($missingLast) {
+			$now = end($this->notificationIds);
+			if ($missingLast === ' missing the last one') {
+				array_unshift($now, $this->deletedNotification);
+			} else {
+				$now[] = $this->deletedNotification;
 			}
+
+			Assert::assertEquals($previousNotificationIds, $now);
 		}
 	}
 

tests/Integration/features/delete-notifications-v1.feature

@@ -45,7 +45,7 @@ Feature: delete-notifications
     Given user "test1" has notifications
     Given user "test1" has notifications
     Given user "test1" has notifications
-    Then user "test1" has 3 notifications on v2
-    And delete all notifications on v2
+    Then user "test1" has 3 notifications on v1
+    And delete all notifications on v1
     And status code is 200
-    And user "test1" has 0 notifications on v2
+    And user "test1" has 0 notifications on v1

tests/Integration/features/delete-notifications-v2.feature

@@ -1,6 +1,7 @@
 Feature: delete-notifications
   Background:
     Given user "test1" exists
+    Given user "123456" exists
     Given as user "test1"
 
   Scenario: Delete first notification
@@ -12,6 +13,16 @@ Feature: delete-notifications
     And status code is 200
     And user "test1" has 2 notifications on v2 missing the first one
 
+  Scenario: Delete first notification as numeric user
+    Given as user "123456"
+    Given user "123456" has notifications
+    Given user "123456" has notifications
+    Given user "123456" has notifications
+    Then user "123456" has 3 notifications on v2
+    And delete first notification on v2
+    And status code is 200
+    And user "123456" has 2 notifications on v2 missing the first one
+
   Scenario: Delete same notification twice
     Given user "test1" has notifications
     Given user "test1" has notifications
@@ -45,7 +56,17 @@ Feature: delete-notifications
     Given user "test1" has notifications
     Given user "test1" has notifications
     Given user "test1" has notifications
-    Then user "test1" has 3 notifications on v1
-    And delete all notifications on v1
+    Then user "test1" has 3 notifications on v2
+    And delete all notifications on v2
+    And status code is 200
+    And user "test1" has 0 notifications on v2
+
+  Scenario: Delete all notifications as numeric user
+    Given as user "123456"
+    Given user "123456" has notifications
+    Given user "123456" has notifications
+    Given user "123456" has notifications
+    Then user "123456" has 3 notifications on v2
+    And delete all notifications on v2
     And status code is 200
-    And user "test1" has 0 notifications on v1
+    And user "123456" has 0 notifications on v2