Skip to content

Commit dd800d2

Browse files
miaulalalamiaulalala
authored
Merge pull request #2607 from nextcloud/backport/2532/stable31
[stable31] fix(notifications): mark activity notifications as read when viewing file activities
2 parents e98127e + ac069ee commit dd800d2

2 files changed

Lines changed: 140 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;
@@ -241,6 +243,22 @@ protected function get($filter, $since, $limit, $previews, $filterObjectType, $f
241243
$preparedActivities[] = $activity;
242244
}
243245

246+
// When viewing activities for a specific file, mark corresponding activity
247+
// notifications as processed so they clear from the notification bell
248+
if ($this->objectType !== '' && $this->objectId !== 0 && !empty($this->user)) {
249+
$deferred = $this->notificationManager->defer();
250+
foreach ($preparedActivities as $activity) {
251+
$notification = $this->notificationManager->createNotification();
252+
$notification->setApp($activity['app'] ?? 'activity')
253+
->setUser($this->user)
254+
->setObject('activity_notification', (string)$activity['activity_id']);
255+
$this->notificationManager->markProcessed($notification);
256+
}
257+
if ($deferred) {
258+
$this->notificationManager->flush();
259+
}
260+
}
261+
244262
return new DataResponse($preparedActivities, Http::STATUS_OK, $headers);
245263
}
246264

‎tests/Controller/APIv2ControllerTest.php‎

Lines changed: 122 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@
4242
use OCP\IURLGenerator;
4343
use OCP\IUser;
4444
use OCP\IUserSession;
45+
use OCP\Notification\IManager as INotificationManager;
46+
use OCP\Notification\INotification;
4547
use PHPUnit\Framework\MockObject\MockObject;
4648

4749
/**
@@ -81,6 +83,9 @@ class APIv2ControllerTest extends TestCase {
8183
/** @var ViewInfoCache|MockObject */
8284
protected $infoCache;
8385

86+
/** @var INotificationManager|MockObject */
87+
protected $notificationManager;
88+
8489
/** @var IL10N */
8590
protected $l10n;
8691

@@ -100,6 +105,10 @@ protected function setUp(): void {
100105
$this->mimeTypeDetector = $this->createMock(IMimeTypeDetector::class);
101106
$this->infoCache = $this->createMock(ViewInfoCache::class);
102107
$this->request = $this->createMock(IRequest::class);
108+
$this->notificationManager = $this->createMock(INotificationManager::class);
109+
$notification = $this->createMock(INotification::class);
110+
$notification->method($this->anything())->willReturnSelf();
111+
$this->notificationManager->method('createNotification')->willReturn($notification);
103112

104113
$this->controller = $this->getController();
105114
}
@@ -121,7 +130,8 @@ protected function getController(array $methods = []): APIv2Controller {
121130
$this->userSession,
122131
$this->preview,
123132
$this->mimeTypeDetector,
124-
$this->infoCache
133+
$this->infoCache,
134+
$this->notificationManager,
125135
);
126136
}
127137

@@ -138,6 +148,7 @@ protected function getController(array $methods = []): APIv2Controller {
138148
$this->preview,
139149
$this->mimeTypeDetector,
140150
$this->infoCache,
151+
$this->notificationManager,
141152
])
142153
->onlyMethods($methods)
143154
->getMock();
@@ -523,6 +534,116 @@ public function testGet(int $time, string $objectType, int $objectId, array $add
523534
], $result->getData());
524535
}
525536

537+
public function testGetMarksActivityNotificationsProcessedForFile(): void {
538+
$controller = $this->getController(['validateParameters', 'generateHeaders']);
539+
$controller->method('generateHeaders')->willReturnArgument(0);
540+
$controller->method('validateParameters');
541+
542+
self::invokePrivate($controller, 'objectType', ['files']);
543+
self::invokePrivate($controller, 'objectId', [42]);
544+
self::invokePrivate($controller, 'user', ['user1']);
545+
546+
$this->data->expects($this->once())
547+
->method('get')
548+
->willReturn([
549+
'data' => [
550+
['activity_id' => 11, 'app' => 'files', 'timestamp' => 1234567890, 'object_type' => 'files', 'object_id' => 42],
551+
['activity_id' => 22, 'app' => 'comments', 'timestamp' => 1234567891, 'object_type' => 'files', 'object_id' => 42],
552+
],
553+
'headers' => ['ETag' => 'abc'],
554+
'has_more' => false,
555+
]);
556+
557+
$notification = $this->createMock(INotification::class);
558+
$notification->method($this->anything())->willReturnSelf();
559+
560+
$this->notificationManager->expects($this->once())
561+
->method('defer')
562+
->willReturn(true);
563+
$this->notificationManager->expects($this->exactly(2))
564+
->method('createNotification')
565+
->willReturn($notification);
566+
$this->notificationManager->expects($this->exactly(2))
567+
->method('markProcessed')
568+
->with($notification);
569+
$this->notificationManager->expects($this->once())
570+
->method('flush');
571+
572+
$result = self::invokePrivate($controller, 'get', ['all', 0, 50, false, 'files', 42, 'desc']);
573+
$this->assertSame(Http::STATUS_OK, $result->getStatus());
574+
}
575+
576+
public function testGetSkipsFlushWhenAlreadyDeferred(): void {
577+
$controller = $this->getController(['validateParameters', 'generateHeaders']);
578+
$controller->method('generateHeaders')->willReturnArgument(0);
579+
$controller->method('validateParameters');
580+
581+
self::invokePrivate($controller, 'objectType', ['files']);
582+
self::invokePrivate($controller, 'objectId', [42]);
583+
self::invokePrivate($controller, 'user', ['user1']);
584+
585+
$this->data->expects($this->once())
586+
->method('get')
587+
->willReturn([
588+
'data' => [
589+
['activity_id' => 11, 'app' => 'files', 'timestamp' => 1234567890, 'object_type' => 'files', 'object_id' => 42],
590+
],
591+
'headers' => ['ETag' => 'abc'],
592+
'has_more' => false,
593+
]);
594+
595+
$notification = $this->createMock(INotification::class);
596+
$notification->method($this->anything())->willReturnSelf();
597+
598+
$this->notificationManager->expects($this->once())
599+
->method('defer')
600+
->willReturn(false);
601+
$this->notificationManager->expects($this->once())
602+
->method('createNotification')
603+
->willReturn($notification);
604+
$this->notificationManager->expects($this->once())
605+
->method('markProcessed')
606+
->with($notification);
607+
$this->notificationManager->expects($this->never())
608+
->method('flush');
609+
610+
$result = self::invokePrivate($controller, 'get', ['all', 0, 50, false, 'files', 42, 'desc']);
611+
$this->assertSame(Http::STATUS_OK, $result->getStatus());
612+
}
613+
614+
public function testGetDoesNotMarkNotificationsForGlobalStream(): void {
615+
$controller = $this->getController(['validateParameters', 'generateHeaders']);
616+
$controller->method('generateHeaders')->willReturnArgument(0);
617+
$controller->method('validateParameters');
618+
619+
// No objectType/objectId set — global stream
620+
self::invokePrivate($controller, 'objectType', ['']);
621+
self::invokePrivate($controller, 'objectId', [0]);
622+
self::invokePrivate($controller, 'user', ['user1']);
623+
624+
$this->data->expects($this->once())
625+
->method('get')
626+
->willReturn([
627+
'data' => [
628+
['activity_id' => 11, 'app' => 'files', 'timestamp' => 1234567890, 'object_type' => 'files', 'object_id' => 42],
629+
],
630+
'headers' => ['ETag' => 'abc'],
631+
'has_more' => false,
632+
]);
633+
634+
$this->notificationManager->expects($this->never())
635+
->method('defer');
636+
$this->notificationManager->expects($this->never())
637+
->method('createNotification');
638+
$this->notificationManager->expects($this->never())
639+
->method('markProcessed');
640+
$this->notificationManager->expects($this->never())
641+
->method('flush');
642+
643+
$result = self::invokePrivate($controller, 'get', ['all', 0, 50, false, '', 0, 'desc']);
644+
$this->assertSame(Http::STATUS_OK, $result->getStatus());
645+
}
646+
526647
public function dataGetNotModified(): array {
527648
return [
528649
[[], null],

0 commit comments

Comments
 (0)