Notifications: show them based on permissions #11117
Conversation
| context["notifications"] = Notification.objects.for_user( | ||
| self.request.user, | ||
| resource=org, | ||
| ) |
There was a problem hiding this comment.
Does something like org.notifications.for_user(...) not work?
There was a problem hiding this comment.
I think it works, but we still need to pass resource=org anyways, right? Otherwise, we would need to add more complexity in the underlying .for_user() queryset method I guess.
There was a problem hiding this comment.
when using the manager that way, you don't need to pass resource, since the queryset will be filtered already.
| considering permissions (e.g. does not return any notification if the ``user`` | ||
| doesn't have admin permissions on the ``resource``). | ||
|
|
||
| If ``resource="all"``, it returns the following notifications: |
There was a problem hiding this comment.
What happens if resource is None? If it just returns none(), it should probably be a required field, or default to all.
There was a problem hiding this comment.
Yeah, it returns an empty queryset. I don't remember why I made this decision 🤔 (maybe because I was already sick 🙃 ), but I think it makes more sense to make it a required argument, yeah.
humitos
deleted the
humitos/show-notifications-based-on-user-permissions
branch
Expand
.for_user()to acceptresource=parameter and return only the notifications attached to that resource only if the user has permissions over it. Otherwise, return an empty queryset.Closes #11082
Closes #11113