Skip to content

Commit e875fdb

Browse files
authored
Notifications: show them based on permissions (#11117)
* Notifications: show them based on permissions Expand `.for_user()` to accept `resource=` 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 Related #11113 * Minor docstring change * Keep APIv3 behavior * Fix tests * Make the `resource=` argument required
1 parent de976b2 commit e875fdb

File tree

11 files changed

+106
-52
lines changed

11 files changed

+106
-52
lines changed

readthedocs/api/v3/views.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,7 @@ class NotificationsForUserViewSet(
421421
filterset_class = NotificationFilter
422422

423423
def get_queryset(self):
424-
return Notification.objects.for_user(self.request.user)
424+
return Notification.objects.for_user(self.request.user, resource="all")
425425

426426

427427
class NotificationsProjectViewSet(

readthedocs/builds/views.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,7 @@
1414
from django.views.generic import DetailView, ListView
1515
from requests.utils import quote
1616

17-
from readthedocs.builds.constants import (
18-
BUILD_FINAL_STATES,
19-
)
17+
from readthedocs.builds.constants import BUILD_FINAL_STATES
2018
from readthedocs.builds.filters import BuildListFilter
2119
from readthedocs.builds.models import Build, Version
2220
from readthedocs.core.permissions import AdminPermission
@@ -214,4 +212,5 @@ def get_context_data(self, **kwargs):
214212
issue_url = scheme.format(**scheme_dict)
215213
issue_url = urlparse(issue_url).geturl()
216214
context["issue_url"] = issue_url
215+
context["notifications"] = build.notifications.all()
217216
return context

readthedocs/core/context_processors.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,10 @@ def user_notifications(request):
4444

4545
user_notifications = Notification.objects.none()
4646
if request.user.is_authenticated:
47-
user_notifications = Notification.objects.for_user(request.user)
47+
user_notifications = Notification.objects.for_user(
48+
request.user,
49+
resource=request.user,
50+
)
4851

4952
return {
5053
"user_notifications": user_notifications,

readthedocs/notifications/querysets.py

Lines changed: 70 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,15 @@ def cancel(self, message_id, attached_to):
6666
modified=timezone.now(),
6767
)
6868

69-
def for_user(self, user):
69+
def for_user(self, user, resource):
7070
"""
71-
Retrieve all notifications related to a particular user.
71+
Retrieve notifications related to resource for a particular user.
7272
73-
Given a user, returns all the notifications that:
73+
Given a user, returns all the notifications for the specified ``resource``
74+
considering permissions (e.g. does not return any notification if the ``user``
75+
doesn't have admin permissions on the ``resource``).
76+
77+
If ``resource="all"``, it returns the following notifications:
7478
7579
- are attached to an ``Organization`` where the user is owner
7680
- are attached to a ``Project`` where the user is admin
@@ -82,32 +86,71 @@ def for_user(self, user):
8286
from readthedocs.organizations.models import Organization
8387
from readthedocs.projects.models import Project
8488

85-
# http://chibisov.github.io/drf-extensions/docs/#usage-with-generic-relations
86-
user_notifications = self.filter(
87-
attached_to_content_type=ContentType.objects.get_for_model(User),
88-
attached_to_id=user.pk,
89-
)
89+
if resource == "all":
90+
# http://chibisov.github.io/drf-extensions/docs/#usage-with-generic-relations
91+
user_notifications = self.filter(
92+
attached_to_content_type=ContentType.objects.get_for_model(User),
93+
attached_to_id=user.pk,
94+
)
9095

91-
project_notifications = self.filter(
92-
attached_to_content_type=ContentType.objects.get_for_model(Project),
93-
attached_to_id__in=AdminPermission.projects(
94-
user,
95-
admin=True,
96-
member=False,
97-
).values("id"),
98-
)
96+
project_notifications = self.filter(
97+
attached_to_content_type=ContentType.objects.get_for_model(Project),
98+
attached_to_id__in=AdminPermission.projects(
99+
user,
100+
admin=True,
101+
member=False,
102+
).values("id"),
103+
)
104+
105+
organization_notifications = self.filter(
106+
attached_to_content_type=ContentType.objects.get_for_model(
107+
Organization
108+
),
109+
attached_to_id__in=AdminPermission.organizations(
110+
user,
111+
owner=True,
112+
member=False,
113+
).values("id"),
114+
)
99115

100-
organization_notifications = self.filter(
101-
attached_to_content_type=ContentType.objects.get_for_model(Organization),
102-
attached_to_id__in=AdminPermission.organizations(
116+
# Return all the notifications related to this user attached to:
117+
# User, Project and Organization models where the user is admin.
118+
return (
119+
user_notifications | project_notifications | organization_notifications
120+
).filter(state__in=(UNREAD, READ))
121+
122+
if isinstance(resource, User):
123+
if user == resource:
124+
return self.filter(
125+
attached_to_content_type=ContentType.objects.get_for_model(
126+
resource
127+
),
128+
attached_to_id=resource.pk,
129+
state__in=(UNREAD, READ),
130+
)
131+
132+
if isinstance(resource, Project):
133+
if resource in AdminPermission.projects(user, admin=True, member=False):
134+
return self.filter(
135+
attached_to_content_type=ContentType.objects.get_for_model(
136+
resource
137+
),
138+
attached_to_id=resource.pk,
139+
state__in=(UNREAD, READ),
140+
)
141+
142+
if isinstance(resource, Organization):
143+
if resource in AdminPermission.organizations(
103144
user,
104145
owner=True,
105146
member=False,
106-
).values("id"),
107-
)
108-
109-
# Return all the notifications related to this user attached to:
110-
# User, Project and Organization models where the user is admin.
111-
return (
112-
user_notifications | project_notifications | organization_notifications
113-
).filter(state__in=(UNREAD, READ))
147+
):
148+
return self.filter(
149+
attached_to_content_type=ContentType.objects.get_for_model(
150+
resource
151+
),
152+
attached_to_id=resource.pk,
153+
state__in=(UNREAD, READ),
154+
)
155+
156+
return self.none()

readthedocs/notifications/tests/test_querysets.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,9 @@ def test_for_user(self):
145145
state=CANCELLED,
146146
)
147147

148-
assert [n.message_id for n in Notification.objects.for_user(user)] == [
148+
assert [
149+
n.message_id for n in Notification.objects.for_user(user, resource="all")
150+
] == [
149151
"user:notification:read",
150152
"user:notification:unread",
151153
]

readthedocs/notifications/tests/test_signals.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,9 @@ def test_user_email_verified(self):
6565
verified=False,
6666
)
6767

68-
self.assertEqual(Notification.objects.for_user(user).count(), 1)
68+
self.assertEqual(Notification.objects.for_user(user, resource="all").count(), 1)
6969

70-
notification = Notification.objects.for_user(user).first()
70+
notification = Notification.objects.for_user(user, resource="all").first()
7171
self.assertEqual(notification.message_id, MESSAGE_EMAIL_VALIDATION_PENDING)
7272
self.assertEqual(notification.state, UNREAD)
7373

@@ -77,7 +77,7 @@ def test_user_email_verified(self):
7777
notification.refresh_from_db()
7878

7979
# 0 notifications to show to the user (none UNREAD)
80-
self.assertEqual(Notification.objects.for_user(user).count(), 0)
80+
self.assertEqual(Notification.objects.for_user(user, resource="all").count(), 0)
8181

8282
# 1 notification exists attached to this user, tho
8383
self.assertEqual(

readthedocs/organizations/templates/organizations/organization_detail.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@
99
{% block organization-bar-details %}active{% endblock %}
1010

1111
{% block content %}
12-
{% if organization.notifications.exists %}
12+
{% if notifications %}
1313
<ul class="notifications">
14-
{% for notification in organization.notifications.all %}
14+
{% for notification in notifications %}
1515
<li class="notification">
1616
{{ notification.get_message.get_rendered_body|safe }}
1717
</li>

readthedocs/organizations/views/public.py

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from vanilla import DetailView, GenericView, ListView
99

1010
from readthedocs.core.filters import FilterContextMixin
11+
from readthedocs.notifications.models import Notification
1112
from readthedocs.organizations.filters import (
1213
OrganizationProjectListFilterSet,
1314
OrganizationTeamListFilterSet,
@@ -38,7 +39,7 @@ class DetailOrganization(FilterContextMixin, OrganizationView, DetailView):
3839

3940
"""Display information about an organization."""
4041

41-
template_name = 'organizations/organization_detail.html'
42+
template_name = "organizations/organization_detail.html"
4243
admin_only = False
4344

4445
filterset_class = OrganizationProjectListFilterSet
@@ -48,10 +49,7 @@ def get_context_data(self, **kwargs):
4849
context = super().get_context_data(**kwargs)
4950
org = self.get_object()
5051
projects = (
51-
Project.objects
52-
.for_user(self.request.user)
53-
.filter(organizations=org)
54-
.all()
52+
Project.objects.for_user(self.request.user).filter(organizations=org).all()
5553
)
5654
if settings.RTD_EXT_THEME_ENABLED:
5755
context["filter"] = self.get_filterset(
@@ -69,6 +67,10 @@ def get_context_data(self, **kwargs):
6967
context["owners"] = org.owners.all()
7068

7169
context["projects"] = projects
70+
context["notifications"] = Notification.objects.for_user(
71+
self.request.user,
72+
resource=org,
73+
)
7274
return context
7375

7476

@@ -95,15 +97,15 @@ def get_queryset(self):
9597

9698
def get_success_url(self):
9799
return reverse_lazy(
98-
'organization_members',
100+
"organization_members",
99101
args=[self.get_organization().slug],
100102
)
101103

102104

103105
# Team Views
104106
class ListOrganizationTeams(FilterContextMixin, OrganizationTeamView, ListView):
105-
template_name = 'organizations/team_list.html'
106-
context_object_name = 'teams'
107+
template_name = "organizations/team_list.html"
108+
context_object_name = "teams"
107109
admin_only = False
108110

109111
filterset_class = OrganizationTeamListFilterSet
@@ -127,13 +129,13 @@ def get_context_data(self, **kwargs):
127129

128130

129131
class ListOrganizationTeamMembers(OrganizationTeamMemberView, ListView):
130-
template_name = 'organizations/team_detail.html'
131-
context_object_name = 'team_members'
132+
template_name = "organizations/team_detail.html"
133+
context_object_name = "team_members"
132134
admin_only = False
133135

134136
def get_context_data(self, **kwargs):
135137
context = super().get_context_data(**kwargs)
136-
context['projects'] = self.get_team().projects.all()
138+
context["projects"] = self.get_team().projects.all()
137139
return context
138140

139141

readthedocs/projects/views/public.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
from readthedocs.core.permissions import AdminPermission
3232
from readthedocs.core.resolver import Resolver
3333
from readthedocs.core.utils.extend import SettingsOverrideObject
34+
from readthedocs.notifications.models import Notification
3435
from readthedocs.projects.filters import ProjectVersionListFilterSet
3536
from readthedocs.projects.models import Project
3637
from readthedocs.projects.templatetags.projects_tags import sort_version_aware
@@ -143,6 +144,10 @@ def get_context_data(self, **kwargs):
143144
self.request.user,
144145
project,
145146
)
147+
context["notifications"] = Notification.objects.for_user(
148+
self.request.user,
149+
resource=project,
150+
)
146151

147152
return context
148153

readthedocs/templates/builds/build_detail.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@
170170

171171
This temporal until we render these notifications from the front-end.
172172
{% endcomment %}
173-
{% for notification in build.notifications.all %}
173+
{% for notification in notifications %}
174174
{% if notification.get_message.type != "error" %}
175175
<p class="build-ideas">
176176
{{ notification.get_message.get_rendered_body|safe }}
@@ -216,7 +216,7 @@ <h3>{% trans "Build Errors" %}</h3>
216216

217217
This temporal until we render these notifications from the front-end.
218218
{% endcomment %}
219-
{% for notification in build.notifications.all %}
219+
{% for notification in notifications %}
220220
{% if notification.get_message.type == "error" %}
221221
<div class="build-error">
222222
<h3>{{ notification.get_message.type|title }}</h3>

readthedocs/templates/core/project_bar_base.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ <h1>
3434
This notification will be shown here using the same style we used before.
3535
{% endcomment %}
3636

37-
{% for notification in project.notifications.all %}
37+
{% for notification in notifications %}
3838
<p class="build-failure">
3939
{{ notification.get_message.get_rendered_body|safe }}
4040
</p>

0 commit comments

Comments
 (0)