Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show notifications menu only to users with permission for the feature #18184

Merged

Conversation

AndyButland
Copy link
Contributor

Prerequisites

  • I have added steps to test this contribution in the description below

Fixes: #18167

Description

Issue seemed to be caused by using a different method to add this action to the menu. When we do it the way we were, we have no Action property set, and hence the menu item is considered as one that's not affected by permissions.

Switching it to be registered in the way the other menu items were resolves this, and I don't see any downsides in doing so.

To Test:

  • Verify that a user in a user group with the "Notifications" permission can manage notifications.
  • Verify that a user in a user group without the "Notifications" permission cannot manage notifications.

Copy link
Contributor

@nikolajlauridsen nikolajlauridsen left a comment

Choose a reason for hiding this comment

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

Looks good, tests good 👍

@nikolajlauridsen nikolajlauridsen merged commit e741124 into v13/dev Feb 3, 2025
20 checks passed
@nikolajlauridsen nikolajlauridsen deleted the v13/bugfix/use-permissions-for-notifications branch February 3, 2025 12:25
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.

2 participants