Skip to content

fix(comments): dismiss mention notifications and clear unread badge when viewed in activity sidebar - #60617

Draft
miaulalala wants to merge 2 commits into
masterfrom
fix/2531/comments-not-marked-read
Draft

miaulalala wants to merge 2 commits into
masterfrom
fix/2531/comments-not-marked-read

Conversation

@miaulalala

Copy link
Copy Markdown
Contributor

Summary

  • When comments are loaded via the Activity sidebar integration, call markCommentsAsRead() (DAV PROPPATCH) so the file-row unread comment bubble clears after viewing
  • Update the frontend file node immediately via node.update({ 'comments-unread': 0 }) so the bubble disappears without waiting for a PROPFIND refresh
  • Add a new DELETE /apps/comments/notifications/{id} endpoint that marks a comments/comment/mention notification as processed without redirecting
  • Call that endpoint for each comment that mentions the current user when the Activity sidebar loads, so the notification bell clears without the user having to navigate via the notification link

Without this, opening the Activity sidebar for a file never triggered a DAV read-marker update, so the unread comment bubble persisted in the Files list even after viewing all comments. The notification bell also kept showing comment-mention notifications after the user had already read them in the sidebar.

Fixes: nextcloud/activity#2531

Test plan

  • With the Activity app enabled, open a file's Activity sidebar that has unread comments — the chat bubble on the file row should disappear after the sidebar loads
  • If you are @mentioned in a comment and open the Activity sidebar for that file, the notification bell entry for the mention should be cleared
  • Confirm the new DELETE /apps/comments/notifications/{id} endpoint returns 200 for a valid comment id, 403 if not logged in, 404 for unknown ids
  • Run existing PHP unit tests: NOCOVERAGE=1 ./autotest.sh sqlite apps/comments/tests/Unit/Controller/NotificationsTest.php
  • Cypress tests: cypress/e2e/comments/comments-unread.cy.ts

🤖 Generated with Claude Code

@miaulalala

Copy link
Copy Markdown
Contributor Author

/backport to stable34

@miaulalala

Copy link
Copy Markdown
Contributor Author

/backport to stable33

miaulalala added a commit that referenced this pull request May 20, 2026
…ivity sidebar

Backport of #60617 to stable32.

When the Activity app integration is active, comments are loaded in the
Activity sidebar instead of the Comments tab. The read marker and mention
notification dismissal were never triggered in this path, leaving the
unread bubble in the file list and keeping mention notifications active.

Also adds a new DELETE /notifications/{id} endpoint to dismiss individual
mention notifications by comment ID.

AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Anna Larch <anna@nextcloud.com>
miaulalala added a commit that referenced this pull request May 20, 2026
…hen viewed in activity sidebar

Backport of #60617 for stable31.

When comments are loaded via the Activity sidebar integration, call
markCommentsAsRead() so the file-row unread comment bubble clears after
viewing. Also add a DELETE /notifications/{id} endpoint and call it for
each comment that mentions the current user so the notification bell
clears without navigating 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>
miaulalala added a commit that referenced this pull request May 20, 2026
…ivity sidebar

Backport of #60617 to stable32.

When the Activity app integration is active, comments are loaded in the
Activity sidebar instead of the Comments tab. The read marker and mention
notification dismissal were never triggered in this path, leaving the
unread bubble in the file list and keeping mention notifications active.

Also adds a new DELETE /notifications/{id} endpoint to dismiss individual
mention notifications by comment ID.

AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Anna Larch <anna@nextcloud.com>
janepie pushed a commit that referenced this pull request May 21, 2026
…hen viewed in activity sidebar

Backport of #60617 for stable31.

When comments are loaded via the Activity sidebar integration, call
markCommentsAsRead() so the file-row unread comment bubble clears after
viewing. Also add a DELETE /notifications/{id} endpoint and call it for
each comment that mentions the current user so the notification bell
clears without navigating 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>
miaulalala added a commit that referenced this pull request May 25, 2026
…hen viewed in activity sidebar

Backport of #60617 for stable31.

When comments are loaded via the Activity sidebar integration, call
markCommentsAsRead() so the file-row unread comment bubble clears after
viewing. Also add a DELETE /notifications/{id} endpoint and call it for
each comment that mentions the current user so the notification bell
clears without navigating 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>

@joshtrichards joshtrichards left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Production code changes themselves seem fine, other than the share bit already commented on.

The only other note I have is that the Activity-tab code path that detects a current-user mention and sends the dismissal DELETE has no Cypress coverage.

Comment on lines -1134 to -1136
// Similarly the token is always set by the backend when the
// share is created.
this.share._share.token = share.token

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what this is for - is this change unrelated to the PR?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I have a suspicion this may be contributing the significant generated dist/ updates in this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated, indeed - removed it!

const isMentioned = comment.props?.id && mentions.some((m) => m.mentionType === 'user' && m.mentionId === currentUser.uid)
if (isMentioned) {
axios.delete(generateUrl('/apps/comments/notifications/{id}', { id: comment.props.id }))
.catch(() => {})

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than 100% silently discarding every DAV and notification failure maybe log a debug-level message with the file/comment ID and error so there's still some observability.

$this->markProcessed($comment, $currentUser);
return new DataResponse([]);
} catch (\Exception $e) {
return new DataResponse([], Http::STATUS_NOT_FOUND);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessarily a blocker for this narrow endpoint, but narrowing the caught exception to comment lookup related failures would avoid masking unrelated errors as missing comments and improve diagnosability.

cy.visit('/apps/files')

// Verify badge is present first
getInlineActionEntryForFile('commented-file.txt', 'comments-unread')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this guaranteed to exercise the Activity path (the comments-activity-tab.ts implementation updated in this PR) rather than the old comments sidebar?

/**
* Check if the comments app is using the Activity app integration for the sidebar.
*/
export function isUsingActivityIntegration() {
return loadState('comments', 'activityEnabled', false) && window.OCA?.Activity?.registerSidebarAction !== undefined
}

.should('not.exist')
})

it('badge stays absent after closing and re-opening the sidebar', () => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Says "after closing and re-opening the sidebar" but only closes it, no?

This was referenced Aug 12, 2026
@nextcloud-bot nextcloud-bot mentioned this pull request Aug 20, 2026
@skjnldsv skjnldsv mentioned this pull request Aug 25, 2026
@provokateurin

Copy link
Copy Markdown
Member

Your PR description is AI-written, so I'm closing it as per our AI policy: https://github.com/nextcloud/.github/blob/master/AI_POLICY.md#human-written-communication
If you disagree and my judgement was wrong, feel free to re-open this PR.

@miaulalala miaulalala reopened this Sep 24, 2026
@miaulalala
miaulalala force-pushed the fix/2531/comments-not-marked-read branch from 964ba1c to 84dec6d Compare September 24, 2026 12:25
@miaulalala

Copy link
Copy Markdown
Contributor Author

Your PR description is AI-written, so I'm closing it as per our AI policy: nextcloud/.github@master/AI_POLICY.md#human-written-communication If you disagree and my judgement was wrong, feel free to re-open this PR.

This PR was opened before any such policy existed. The commit and PR messages were AI written but human- reviewed and approved.

… in activity sidebar

Add a `DELETE /notifications/{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

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Anna Larch <anna@nextcloud.com>
Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Anna Larch <anna@nextcloud.com>
@miaulalala
miaulalala force-pushed the fix/2531/comments-not-marked-read branch from 84dec6d to a2b804b Compare September 24, 2026 12:59
@miaulalala
miaulalala marked this pull request as draft September 24, 2026 12:59
@miaulalala

Copy link
Copy Markdown
Contributor Author

Moving this to draft, I can't continue working on it. Open items for whoever picks it up:

  • Playwright tests in tests/playwright/e2e/comments/comments-unread.spec.ts fail: without the Activity app the badge opens the old comments tab, which marks comments as read but doesn't clear the badge without a reload
  • Decide with the server team whether the Playwright setup enables Activity, so the tests cover the Activity sidebar path this PR changes
  • Add a test for dismissing the mention notification
  • First test's comment seeding request failed once locally, check for flakiness

@miaulalala miaulalala removed their assignment Sep 24, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Comments don't get marked as read

4 participants