Skip to content

Commit 4142aee

Browse files
miaulalalamiaulalala
committed
fix(notifications): dismiss comment mention notifications when viewed in activity sidebar
Add a `DELETE /notifications/dismiss/{id}` endpoint to the comments NotificationsController that marks the `comments/comment/mention` notification as processed without redirecting. When comments are loaded in the activity sidebar, the frontend calls this endpoint for any comment that mentions the current user, so the notification is cleared from the bell without requiring the user to navigate via the notification link. Fixes: nextcloud/activity#2531 AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Anna Larch <anna@nextcloud.com>
1 parent f55b3fb commit 4142aee

307 files changed

Lines changed: 536 additions & 278 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎apps/comments/appinfo/routes.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,6 @@
1010
return [
1111
'routes' => [
1212
['name' => 'Notifications#view', 'url' => '/notifications/view/{id}', 'verb' => 'GET'],
13+
['name' => 'Notifications#dismiss', 'url' => '/notifications/{id}', 'verb' => 'DELETE'],
1314
]
1415
];

‎apps/comments/lib/Controller/NotificationsController.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@
88

99
use OCP\AppFramework\Controller;
1010
use OCP\AppFramework\Http;
11+
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
1112
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
1213
use OCP\AppFramework\Http\Attribute\OpenAPI;
1314
use OCP\AppFramework\Http\Attribute\PublicPage;
15+
use OCP\AppFramework\Http\DataResponse;
1416
use OCP\AppFramework\Http\NotFoundResponse;
1517
use OCP\AppFramework\Http\RedirectResponse;
1618
use OCP\Comments\IComment;
@@ -92,6 +94,36 @@ public function view(string $id): RedirectResponse|NotFoundResponse {
9294
}
9395
}
9496

97+
/**
98+
* Dismiss the mention notification for a comment
99+
*
100+
* @param string $id ID of the comment
101+
*
102+
* @return DataResponse<Http::STATUS_OK, array{}, array{}>|DataResponse<Http::STATUS_FORBIDDEN, array{}, array{}>|DataResponse<Http::STATUS_NOT_FOUND, array{}, array{}>
103+
*
104+
* 200: Notification dismissed successfully
105+
* 403: Not logged in
106+
* 404: Comment not found
107+
*/
108+
#[NoAdminRequired]
109+
public function dismiss(string $id): DataResponse {
110+
$currentUser = $this->userSession->getUser();
111+
if (!$currentUser instanceof IUser) {
112+
return new DataResponse([], Http::STATUS_FORBIDDEN);
113+
}
114+
115+
try {
116+
$comment = $this->commentsManager->get($id);
117+
if ($comment->getObjectType() !== 'files') {
118+
return new DataResponse([], Http::STATUS_NOT_FOUND);
119+
}
120+
$this->markProcessed($comment, $currentUser);
121+
return new DataResponse([]);
122+
} catch (\Exception $e) {
123+
return new DataResponse([], Http::STATUS_NOT_FOUND);
124+
}
125+
}
126+
95127
/**
96128
* Marks the notification about a comment as processed
97129
*/

‎apps/comments/src/comments-activity-tab.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,14 @@
66
import type { INode } from '@nextcloud/files'
77
import type { App } from 'vue'
88

9+
import { getCurrentUser } from '@nextcloud/auth'
10+
import axios from '@nextcloud/axios'
11+
import { generateUrl } from '@nextcloud/router'
912
import { createPinia } from 'pinia'
1013
import { createApp } from 'vue'
1114
import logger from './logger.ts'
1215
import { getComments } from './services/GetComments.ts'
16+
import { markCommentsAsRead } from './services/ReadComments.ts'
1317

1418
/**
1519
* Register the comments plugins for the Activity sidebar
@@ -50,6 +54,26 @@ export function registerCommentsPlugins() {
5054
},
5155
)
5256
logger.debug('Loaded comments', { node, comments })
57+
58+
// Mark all comments as read and clear the unread badge in the files list
59+
if (node.fileid) {
60+
markCommentsAsRead('files', node.fileid, new Date())
61+
.then(() => node.update({ 'comments-unread': 0 }))
62+
.catch(() => {})
63+
}
64+
65+
// Mark mention notifications as read for comments that mention the current user
66+
const currentUser = getCurrentUser()
67+
if (currentUser) {
68+
for (const comment of comments) {
69+
const mentions = Object.values(comment.props?.mentions ?? {}) as { mentionType: string, mentionId: string }[]
70+
const isMentioned = comment.props?.id && mentions.some((m) => m.mentionType === 'user' && m.mentionId === currentUser.uid)
71+
if (isMentioned) {
72+
axios.delete(generateUrl('/apps/comments/notifications/{id}', { id: comment.props.id }))
73+
.catch(() => {})
74+
}
75+
}
76+
}
5377
const { default: CommentView } = await import('./views/ActivityCommentEntry.vue')
5478

5579
return comments.map((comment) => ({

‎apps/comments/tests/Unit/Controller/NotificationsTest.php‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
namespace OCA\Comments\Tests\Unit\Controller;
1111

1212
use OCA\Comments\Controller\NotificationsController;
13+
use OCP\AppFramework\Http\DataResponse;
1314
use OCP\AppFramework\Http\NotFoundResponse;
1415
use OCP\AppFramework\Http\RedirectResponse;
1516
use OCP\Comments\IComment;
@@ -163,6 +164,104 @@ public function testViewInvalidComment(): void {
163164
$this->assertInstanceOf(NotFoundResponse::class, $response);
164165
}
165166

167+
public function testDismissNotLoggedIn(): void {
168+
$this->session->expects($this->once())
169+
->method('getUser')
170+
->willReturn(null);
171+
172+
$this->commentsManager->expects($this->never())
173+
->method('get');
174+
$this->notificationManager->expects($this->never())
175+
->method('markProcessed');
176+
177+
$response = $this->notificationsController->dismiss('42');
178+
$this->assertInstanceOf(DataResponse::class, $response);
179+
$this->assertSame(403, $response->getStatus());
180+
}
181+
182+
public function testDismissSuccess(): void {
183+
$comment = $this->createMock(IComment::class);
184+
$comment->expects($this->any())
185+
->method('getObjectType')
186+
->willReturn('files');
187+
$comment->expects($this->any())
188+
->method('getId')
189+
->willReturn('1234');
190+
191+
$this->commentsManager->expects($this->once())
192+
->method('get')
193+
->with('42')
194+
->willReturn($comment);
195+
196+
$user = $this->createMock(IUser::class);
197+
$user->expects($this->any())
198+
->method('getUID')
199+
->willReturn('user');
200+
201+
$this->session->expects($this->once())
202+
->method('getUser')
203+
->willReturn($user);
204+
205+
$notification = $this->createMock(INotification::class);
206+
$notification->expects($this->any())
207+
->method($this->anything())
208+
->willReturn($notification);
209+
210+
$this->notificationManager->expects($this->once())
211+
->method('createNotification')
212+
->willReturn($notification);
213+
$this->notificationManager->expects($this->once())
214+
->method('markProcessed')
215+
->with($notification);
216+
217+
$response = $this->notificationsController->dismiss('42');
218+
$this->assertInstanceOf(DataResponse::class, $response);
219+
$this->assertSame(200, $response->getStatus());
220+
}
221+
222+
public function testDismissInvalidComment(): void {
223+
$this->commentsManager->expects($this->once())
224+
->method('get')
225+
->with('42')
226+
->willThrowException(new NotFoundException());
227+
228+
$user = $this->createMock(IUser::class);
229+
$this->session->expects($this->once())
230+
->method('getUser')
231+
->willReturn($user);
232+
233+
$this->notificationManager->expects($this->never())
234+
->method('markProcessed');
235+
236+
$response = $this->notificationsController->dismiss('42');
237+
$this->assertInstanceOf(DataResponse::class, $response);
238+
$this->assertSame(404, $response->getStatus());
239+
}
240+
241+
public function testDismissNonFileComment(): void {
242+
$comment = $this->createMock(IComment::class);
243+
$comment->expects($this->any())
244+
->method('getObjectType')
245+
->willReturn('calendar');
246+
247+
$this->commentsManager->expects($this->once())
248+
->method('get')
249+
->with('42')
250+
->willReturn($comment);
251+
252+
$user = $this->createMock(IUser::class);
253+
$this->session->expects($this->once())
254+
->method('getUser')
255+
->willReturn($user);
256+
257+
$this->notificationManager->expects($this->never())
258+
->method('markProcessed');
259+
260+
$response = $this->notificationsController->dismiss('42');
261+
$this->assertInstanceOf(DataResponse::class, $response);
262+
$this->assertSame(404, $response->getStatus());
263+
}
264+
166265
public function testViewNoFile(): void {
167266
$comment = $this->createMock(IComment::class);
168267
$comment->expects($this->any())

‎apps/files_sharing/src/views/SharingDetailsTab.vue‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,9 +1131,6 @@ export default {
11311131
11321132
// ugly hack to make code work - we need the id to be set but at the same time we need to keep values we want to update
11331133
this.share._share.id = share.id
1134-
// Similarly the token is always set by the backend when the
1135-
// share is created.
1136-
this.share._share.token = share.token
11371134
await this.queueUpdate(...permissionsAndAttributes)
11381135
// Also a ugly hack to update the updated permissions
11391136
for (const prop of permissionsAndAttributes) {

‎build/eslint-baseline-legacy.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
},
1717
"apps/files_sharing/src/views/SharingDetailsTab.vue": {
1818
"vue/no-mutating-props": {
19-
"count": 24
19+
"count": 23
2020
}
2121
},
2222
"apps/files_sharing/src/views/SharingLinkList.vue": {

‎cypress/e2e/comments/comments-unread.cy.ts‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/**
2+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
3+
* SPDX-License-Identifier: AGPL-3.0-or-later
4+
*/
5+
6+
import type { User } from '@nextcloud/e2e-test-server/cypress'
7+
8+
import axios from 'axios'
9+
import { getInlineActionEntryForFile, triggerInlineActionForFile } from '../files/FilesUtils.ts'
10+
11+
/**
12+
* Add a comment to a file via the DAV API.
13+
*
14+
* @param user The user adding the comment
15+
* @param fileId The numeric file ID
16+
* @param message The comment text
17+
*/
18+
function addCommentViaApi(user: User, fileId: number, message: string) {
19+
cy.clearCookies().then(async () => {
20+
await axios({
21+
url: `${Cypress.env('baseUrl')}/remote.php/dav/comments/files/${fileId}`,
22+
method: 'POST',
23+
auth: {
24+
username: user.userId,
25+
password: user.password,
26+
},
27+
headers: { 'Content-Type': 'application/json' },
28+
data: JSON.stringify({ actorType: 'users', verb: 'comment', message }),
29+
})
30+
})
31+
}
32+
33+
describe('Comments: unread badge in the files list', { testIsolation: true }, () => {
34+
let fileOwner: User
35+
let fileId: number
36+
37+
beforeEach(() => {
38+
// The admin user can comment on any file without needing a share
39+
const admin = { userId: 'admin', password: 'admin' } as User
40+
41+
cy.createRandomUser().then(($user) => {
42+
fileOwner = $user
43+
cy.uploadContent(fileOwner, new Blob(['hello']), 'text/plain', '/commented-file.txt')
44+
.then((response) => {
45+
fileId = Number.parseInt(response.headers['oc-fileid'] ?? '0')
46+
// Add a comment as admin so fileOwner has an unread comment
47+
addCommentViaApi(admin, fileId, 'Hey, take a look at this!')
48+
})
49+
})
50+
})
51+
52+
it('shows the unread comments badge when there are unread comments', () => {
53+
cy.login(fileOwner)
54+
cy.visit('/apps/files')
55+
56+
getInlineActionEntryForFile('commented-file.txt', 'comments-unread')
57+
.should('be.visible')
58+
.find('button')
59+
.should('have.attr', 'aria-label')
60+
.and('match', /new comment/)
61+
})
62+
63+
it('removes the unread badge after opening the sidebar comments tab', () => {
64+
cy.login(fileOwner)
65+
cy.visit('/apps/files')
66+
67+
// Verify badge is present first
68+
getInlineActionEntryForFile('commented-file.txt', 'comments-unread')
69+
.should('be.visible')
70+
71+
// Intercept the PROPPATCH that marks comments as read
72+
cy.intercept('PROPPATCH', /\/remote\.php\/dav\/comments\/files\//).as('markRead')
73+
74+
// Click the badge — opens the comments or activity sidebar
75+
triggerInlineActionForFile('commented-file.txt', 'comments-unread')
76+
77+
cy.get('[data-cy-sidebar]').should('be.visible')
78+
79+
// The read-marker PROPPATCH must have been sent
80+
cy.wait('@markRead')
81+
82+
// Badge must be gone without a page reload
83+
getInlineActionEntryForFile('commented-file.txt', 'comments-unread')
84+
.should('not.exist')
85+
})
86+
87+
it('badge stays absent after closing and re-opening the sidebar', () => {
88+
cy.login(fileOwner)
89+
cy.visit('/apps/files')
90+
91+
cy.intercept('PROPPATCH', /\/remote\.php\/dav\/comments\/files\//).as('markRead')
92+
93+
triggerInlineActionForFile('commented-file.txt', 'comments-unread')
94+
cy.get('[data-cy-sidebar]').should('be.visible')
95+
cy.wait('@markRead')
96+
97+
// Close the sidebar
98+
cy.get('[data-cy-sidebar] .app-sidebar__close').click({ force: true })
99+
cy.get('[data-cy-sidebar]').should('not.be.visible')
100+
101+
// Badge must still be gone
102+
getInlineActionEntryForFile('commented-file.txt', 'comments-unread')
103+
.should('not.exist')
104+
})
105+
})

‎dist/3577-3577.js.map‎

Lines changed: 0 additions & 1 deletion
This file was deleted.

‎dist/3577-3577.js.map.license‎

Lines changed: 0 additions & 1 deletion
This file was deleted.

dist/3577-3577.js renamed to dist/5638-5638.js

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)