Skip to content

Commit 8221da9

Browse files
miaulalalamiaulalala
authored
Merge pull request #2532 from nextcloud/fix/2531/comments-not-marked-read
fix(notifications): mark activity notifications as read when viewing …
2 parents 5d2d343 + 53b58bb commit 8221da9

2 files changed

Lines changed: 138 additions & 1 deletion

File tree

‎lib/Controller/APIv2Controller.php‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
use OCP\IURLGenerator;
2626
use OCP\IUser;
2727
use OCP\IUserSession;
28+
use OCP\Notification\IManager as INotificationManager;
2829

2930
class APIv2Controller extends OCSController {
3031
/** @var string */
@@ -63,6 +64,7 @@ public function __construct(
6364
protected IPreview $preview,
6465
protected IMimeTypeDetector $mimeTypeDetector,
6566
protected ViewInfoCache $infoCache,
67+
protected INotificationManager $notificationManager,
6668
) {
6769
parent::__construct($appName, $request);
6870
$this->activityManager = $activityManager;
@@ -264,6 +266,22 @@ protected function get($filter, $since, $limit, $previews, $filterObjectType, $f
264266
$preparedActivities[] = $activity;
265267
}
266268

269+
// When viewing activities for a specific file, mark corresponding activity
270+
// notifications as processed so they clear from the notification bell
271+
if ($this->objectType !== '' && $this->objectId !== 0 && !empty($this->user)) {
272+
$deferred = $this->notificationManager->defer();
273+
foreach ($preparedActivities as $activity) {
274+
$notification = $this->notificationManager->createNotification();
275+
$notification->setApp($activity['app'] ?? 'activity')
276+
->setUser($this->user)
277+
->setObject('activity_notification', (string)$activity['activity_id']);
278+
$this->notificationManager->markProcessed($notification);
279+
}
280+
if ($deferred) {
281+
$this->notificationManager->flush();
282+
}
283+
}
284+
267285
return new DataResponse($preparedActivities, Http::STATUS_OK, $headers);
268286
}
269287

‎tests/Controller/APIv2ControllerTest.php‎

Lines changed: 120 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@
4343
use OCP\IURLGenerator;
4444
use OCP\IUser;
4545
use OCP\IUserSession;
46+
use OCP\Notification\IManager as INotificationManager;
47+
use OCP\Notification\INotification;
4648
use PHPUnit\Framework\Attributes\DataProvider;
4749
use PHPUnit\Framework\Attributes\Group;
4850
use PHPUnit\Framework\MockObject\MockObject;
@@ -63,6 +65,7 @@ class APIv2ControllerTest extends TestCase {
6365
protected IUserSession&MockObject $userSession;
6466
protected IMimeTypeDetector&MockObject $mimeTypeDetector;
6567
protected ViewInfoCache&MockObject $infoCache;
68+
protected INotificationManager&MockObject $notificationManager;
6669
protected IL10N $l10n;
6770
protected APIv2Controller $controller;
6871

@@ -78,6 +81,10 @@ protected function setUp(): void {
7881
$this->userSession = $this->createMock(IUserSession::class);
7982
$this->mimeTypeDetector = $this->createMock(IMimeTypeDetector::class);
8083
$this->infoCache = $this->createMock(ViewInfoCache::class);
84+
$this->notificationManager = $this->createMock(INotificationManager::class);
85+
$notification = $this->createMock(INotification::class);
86+
$notification->method($this->anything())->willReturnSelf();
87+
$this->notificationManager->method('createNotification')->willReturn($notification);
8188
$this->request = $this->createMock(IRequest::class);
8289

8390
$this->controller = $this->getController();
@@ -96,7 +103,8 @@ protected function getController(array $methods = []): APIv2Controller|MockObjec
96103
$this->userSession,
97104
$this->preview,
98105
$this->mimeTypeDetector,
99-
$this->infoCache
106+
$this->infoCache,
107+
$this->notificationManager,
100108
);
101109
}
102110

@@ -113,6 +121,7 @@ protected function getController(array $methods = []): APIv2Controller|MockObjec
113121
$this->preview,
114122
$this->mimeTypeDetector,
115123
$this->infoCache,
124+
$this->notificationManager,
116125
])
117126
->onlyMethods($methods)
118127
->getMock();
@@ -494,6 +503,116 @@ public function testGet(int $time, string $objectType, int $objectId, array $add
494503
], $result->getData());
495504
}
496505

506+
public function testGetMarksActivityNotificationsProcessedForFile(): void {
507+
$controller = $this->getController(['validateParameters', 'generateHeaders']);
508+
$controller->method('generateHeaders')->willReturnArgument(0);
509+
$controller->method('validateParameters');
510+
511+
self::invokePrivate($controller, 'objectType', ['files']);
512+
self::invokePrivate($controller, 'objectId', [42]);
513+
self::invokePrivate($controller, 'user', ['user1']);
514+
515+
$this->data->expects($this->once())
516+
->method('get')
517+
->willReturn([
518+
'data' => [
519+
['activity_id' => 11, 'app' => 'files', 'timestamp' => 1234567890, 'object_type' => 'files', 'object_id' => 42],
520+
['activity_id' => 22, 'app' => 'comments', 'timestamp' => 1234567891, 'object_type' => 'files', 'object_id' => 42],
521+
],
522+
'headers' => ['ETag' => 'abc'],
523+
'has_more' => false,
524+
]);
525+
526+
$notification = $this->createMock(INotification::class);
527+
$notification->method($this->anything())->willReturnSelf();
528+
529+
$this->notificationManager->expects($this->once())
530+
->method('defer')
531+
->willReturn(true);
532+
$this->notificationManager->expects($this->exactly(2))
533+
->method('createNotification')
534+
->willReturn($notification);
535+
$this->notificationManager->expects($this->exactly(2))
536+
->method('markProcessed')
537+
->with($notification);
538+
$this->notificationManager->expects($this->once())
539+
->method('flush');
540+
541+
$result = self::invokePrivate($controller, 'get', ['all', 0, 50, false, 'files', 42, 'desc']);
542+
$this->assertSame(Http::STATUS_OK, $result->getStatus());
543+
}
544+
545+
public function testGetSkipsFlushWhenAlreadyDeferred(): void {
546+
$controller = $this->getController(['validateParameters', 'generateHeaders']);
547+
$controller->method('generateHeaders')->willReturnArgument(0);
548+
$controller->method('validateParameters');
549+
550+
self::invokePrivate($controller, 'objectType', ['files']);
551+
self::invokePrivate($controller, 'objectId', [42]);
552+
self::invokePrivate($controller, 'user', ['user1']);
553+
554+
$this->data->expects($this->once())
555+
->method('get')
556+
->willReturn([
557+
'data' => [
558+
['activity_id' => 11, 'app' => 'files', 'timestamp' => 1234567890, 'object_type' => 'files', 'object_id' => 42],
559+
],
560+
'headers' => ['ETag' => 'abc'],
561+
'has_more' => false,
562+
]);
563+
564+
$notification = $this->createMock(INotification::class);
565+
$notification->method($this->anything())->willReturnSelf();
566+
567+
$this->notificationManager->expects($this->once())
568+
->method('defer')
569+
->willReturn(false);
570+
$this->notificationManager->expects($this->once())
571+
->method('createNotification')
572+
->willReturn($notification);
573+
$this->notificationManager->expects($this->once())
574+
->method('markProcessed')
575+
->with($notification);
576+
$this->notificationManager->expects($this->never())
577+
->method('flush');
578+
579+
$result = self::invokePrivate($controller, 'get', ['all', 0, 50, false, 'files', 42, 'desc']);
580+
$this->assertSame(Http::STATUS_OK, $result->getStatus());
581+
}
582+
583+
public function testGetDoesNotMarkNotificationsForGlobalStream(): void {
584+
$controller = $this->getController(['validateParameters', 'generateHeaders']);
585+
$controller->method('generateHeaders')->willReturnArgument(0);
586+
$controller->method('validateParameters');
587+
588+
// No objectType/objectId set — global stream
589+
self::invokePrivate($controller, 'objectType', ['']);
590+
self::invokePrivate($controller, 'objectId', [0]);
591+
self::invokePrivate($controller, 'user', ['user1']);
592+
593+
$this->data->expects($this->once())
594+
->method('get')
595+
->willReturn([
596+
'data' => [
597+
['activity_id' => 11, 'app' => 'files', 'timestamp' => 1234567890, 'object_type' => 'files', 'object_id' => 42],
598+
],
599+
'headers' => ['ETag' => 'abc'],
600+
'has_more' => false,
601+
]);
602+
603+
$this->notificationManager->expects($this->never())
604+
->method('defer');
605+
$this->notificationManager->expects($this->never())
606+
->method('createNotification');
607+
$this->notificationManager->expects($this->never())
608+
->method('markProcessed');
609+
$this->notificationManager->expects($this->never())
610+
->method('flush');
611+
612+
$result = self::invokePrivate($controller, 'get', ['all', 0, 50, false, '', 0, 'desc']);
613+
$this->assertSame(Http::STATUS_OK, $result->getStatus());
614+
}
615+
497616
public static function dataGetNotModified(): array {
498617
return [
499618
[[], null],

0 commit comments

Comments
 (0)
⚡