Skip to content

Commit

Permalink
APIv3: endpoints for notifications (#11009)
Browse files Browse the repository at this point in the history
* Initial work for notifications API endpoints

* Implement missing endpoints

- `/api/v3/organizations/<slug>/notifications/`
- `/api/v3/organizations/<slug>/notifications/<notification_pk>/`
- `/api/v3/users/<username>/notifications/`
- `/api/v3/users/<username>/notifications/<notification_pk>/`

Note that only notifications endpoints are available for `users` and
`organizations` currently. So, the following endpoints return 404 on purpose:

- `/api/v3/organizations/`
- `/api/v3/organizations/<slug>/`
- `/api/v3/users/`
- `/api/v3/users/<username>/`

We can implement them when we are ready, but the pattern and structure of URLs
is already prepared to support them.

* Lint

* Remove outdated comments

* Refactor "notifications for user" as QuerySet

This logic will be re-used when rendering templates.

* Comments

* Use `Subquery` to improve the db query

* APIv3: lot of tests for the new endpoints

* APIv3: update tests to make them pass

* APIv3: update permissions on notification endpoints and add tests

* APIv3: remove old comment code

* APIv3: update `AdminPermission` to match what's expected

* Wrong attribute
  • Loading branch information
humitos authored Jan 22, 2024
1 parent cae5320 commit b8767cf
Show file tree
Hide file tree
Showing 25 changed files with 1,236 additions and 108 deletions.
9 changes: 9 additions & 0 deletions readthedocs/api/v3/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from readthedocs.builds.constants import BUILD_FINAL_STATES
from readthedocs.builds.models import Build, Version
from readthedocs.notifications.models import Notification
from readthedocs.oauth.models import RemoteOrganization, RemoteRepository
from readthedocs.projects.models import Project

Expand Down Expand Up @@ -58,6 +59,14 @@ def get_running(self, queryset, name, value):
return queryset.filter(state__in=BUILD_FINAL_STATES)


class NotificationFilter(filters.FilterSet):
class Meta:
model = Notification
fields = [
"state",
]


class RemoteRepositoryFilter(filters.FilterSet):
name = filters.CharFilter(field_name="name", lookup_expr="icontains")
full_name = filters.CharFilter(field_name="full_name", lookup_expr="icontains")
Expand Down
40 changes: 39 additions & 1 deletion readthedocs/api/v3/mixins.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
from django.contrib.auth.models import User
from django.shortcuts import get_object_or_404
from rest_framework import status
from rest_framework.response import Response

from readthedocs.builds.models import Version
from readthedocs.builds.models import Build, Version
from readthedocs.core.history import safe_update_change_reason, set_change_reason
from readthedocs.organizations.models import Organization
from readthedocs.projects.models import Project
Expand Down Expand Up @@ -67,6 +68,14 @@ class NestedParentObjectMixin:
"organizations__slug",
]

BUILD_LOOKUP_NAMES = [
"build__id",
]

USER_LOOKUP_NAMES = [
"user__username",
]

def _get_parent_object_lookup(self, lookup_names):
query_dict = self.get_parents_query_dict()
for lookup in lookup_names:
Expand All @@ -84,6 +93,10 @@ def _get_parent_project(self):

return get_object_or_404(Project, slug=slug)

def _get_parent_build(self):
pk = self._get_parent_object_lookup(self.BUILD_LOOKUP_NAMES)
return get_object_or_404(Build, pk=pk)

def _get_parent_version(self):
project_slug = self._get_parent_object_lookup(self.PROJECT_LOOKUP_NAMES)
slug = self._get_parent_object_lookup(self.VERSION_LOOKUP_NAMES)
Expand All @@ -106,6 +119,15 @@ def _get_parent_organization(self):
slug=slug,
)

def _get_parent_user(self):
username = self._get_parent_object_lookup(self.USER_LOOKUP_NAMES)
username = username or self.kwargs.get("user_username")

return get_object_or_404(
User,
username=username,
)


class ProjectQuerySetMixin(NestedParentObjectMixin):

Expand Down Expand Up @@ -219,6 +241,22 @@ def get_queryset(self):
return self.listing_objects(queryset, self.request.user)


class UserQuerySetMixin(NestedParentObjectMixin):

"""
Mixin to define queryset permissions for ViewSet only in one place.
All APIv3 user' ViewSet should inherit this mixin, unless specific permissions
required. In that case, a specific mixin for that case should be defined.
"""

def has_admin_permission(self, requesting_user, accessing_user):
if requesting_user == accessing_user:
return True

return False


class UpdateMixin:

"""Make PUT to return 204 on success like PATCH does."""
Expand Down
30 changes: 25 additions & 5 deletions readthedocs/api/v3/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ class PublicDetailPrivateListing(BasePermission):
* Always give permission for a ``detail`` request
* Only give permission for ``listing`` request if user is admin of the project
However, for notification endpoints we only allow users with access to the project.
"""

def has_permission(self, request, view):
Expand All @@ -61,11 +63,29 @@ def has_permission(self, request, view):
if view.detail and view.action in ("list", "retrieve", "superproject"):
# detail view is only allowed on list/retrieve actions (not
# ``update`` or ``partial_update``).
return True

project = view._get_parent_project()
if view.has_admin_permission(request.user, project):
return True
if view.basename not in (
"projects-notifications",
"projects-builds-notifications",
):
# We don't want to give detail access to resources'
# notifications to users that don't have access to those
# resources.
return True

if view.basename.startswith("projects"):
project = view._get_parent_project()
if view.has_admin_permission(request.user, project):
return True

if view.basename.startswith("organizations"):
organization = view._get_parent_organization()
if view.has_admin_permission(request.user, organization):
return True

if view.basename.startswith("users"):
user = view._get_parent_user()
if view.has_admin_permission(request.user, user):
return True

return False

Expand Down
81 changes: 58 additions & 23 deletions readthedocs/api/v3/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from readthedocs.core.resolver import Resolver
from readthedocs.core.utils import slugify
from readthedocs.core.utils.extend import SettingsOverrideObject
from readthedocs.notifications.messages import registry
from readthedocs.notifications.models import Notification
from readthedocs.oauth.models import RemoteOrganization, RemoteRepository
from readthedocs.organizations.models import Organization, Team
Expand Down Expand Up @@ -61,26 +62,55 @@ class Meta:
fields = []


# TODO: decide whether or not include a `_links` field on the object
#
# This also includes adding `/api/v3/notifications/<pk>` endpoint,
# which I'm not sure it's useful at this point.
#
# class NotificationLinksSerializer(BaseLinksSerializer):
# _self = serializers.SerializerMethodField()
# attached_to = serializers.SerializerMethodField()
class NotificationLinksSerializer(BaseLinksSerializer):
_self = serializers.SerializerMethodField()

# def get__self(self, obj):
# path = reverse(
# "notifications-detail",
# kwargs={
# "pk": obj.pk,
# },
# )
# return self._absolute_url(path)
def get__self(self, obj):
content_type_name = obj.attached_to_content_type.name
if content_type_name == "user":
url = "users-notifications-detail"
path = reverse(
url,
kwargs={
"notification_pk": obj.pk,
"parent_lookup_user__username": obj.attached_to.username,
},
)

# def get_attached_to(self, obj):
# return None
elif content_type_name == "build":
url = "projects-builds-notifications-detail"
project_slug = obj.attached_to.project.slug
path = reverse(
url,
kwargs={
"notification_pk": obj.pk,
"parent_lookup_project__slug": project_slug,
"parent_lookup_build__id": obj.attached_to_id,
},
)

elif content_type_name == "project":
url = "projects-notifications-detail"
project_slug = obj.attached_to.slug
path = reverse(
url,
kwargs={
"notification_pk": obj.pk,
"parent_lookup_project__slug": project_slug,
},
)

elif content_type_name == "organization":
url = "organizations-notifications-detail"
path = reverse(
url,
kwargs={
"notification_pk": obj.pk,
"parent_lookup_organization__slug": obj.attached_to.slug,
},
)

return self._absolute_url(path)


class BuildLinksSerializer(BaseLinksSerializer):
Expand Down Expand Up @@ -122,7 +152,7 @@ def get_project(self, obj):

def get_notifications(self, obj):
path = reverse(
"project-builds-notifications-list",
"projects-builds-notifications-list",
kwargs={
"parent_lookup_project__slug": obj.project.slug,
"parent_lookup_build__id": obj.pk,
Expand Down Expand Up @@ -242,6 +272,10 @@ class Meta:


class NotificationCreateSerializer(serializers.ModelSerializer):
message_id = serializers.ChoiceField(
choices=sorted([(key, key) for key in registry.messages])
)

class Meta:
model = Notification
fields = [
Expand All @@ -253,11 +287,10 @@ class Meta:


class NotificationSerializer(serializers.ModelSerializer):
message = NotificationMessageSerializer(source="get_message")
message = NotificationMessageSerializer(source="get_message", read_only=True)
attached_to_content_type = serializers.SerializerMethodField()
# TODO: review these fields
# _links = BuildLinksSerializer(source="*")
# urls = BuildURLsSerializer(source="*")
_links = NotificationLinksSerializer(source="*", read_only=True)
attached_to_id = serializers.IntegerField(read_only=True)

class Meta:
model = Notification
Expand All @@ -269,7 +302,9 @@ class Meta:
"attached_to_content_type",
"attached_to_id",
"message",
"_links",
]
read_only_fields = ["dismissable", "news"]

def get_attached_to_content_type(self, obj):
return obj.attached_to_content_type.name
Expand Down
46 changes: 46 additions & 0 deletions readthedocs/api/v3/tests/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import django_dynamic_fixture as fixture
from django.contrib.auth.models import User
from django.contrib.contenttypes.models import ContentType
from django.core.cache import cache
from django.test import TestCase
from django.test.utils import override_settings
Expand All @@ -13,9 +14,15 @@

from readthedocs.builds.constants import TAG
from readthedocs.builds.models import Build, Version
from readthedocs.core.notifications import MESSAGE_EMAIL_VALIDATION_PENDING
from readthedocs.doc_builder.exceptions import BuildCancelled
from readthedocs.notifications.models import Notification
from readthedocs.organizations.models import Organization
from readthedocs.projects.constants import PUBLIC
from readthedocs.projects.models import Project
from readthedocs.projects.notifications import MESSAGE_PROJECT_SKIP_BUILDS
from readthedocs.redirects.models import Redirect
from readthedocs.subscriptions.notifications import MESSAGE_ORGANIZATION_DISABLED


@override_settings(
Expand Down Expand Up @@ -109,6 +116,7 @@ def setUp(self):
Project,
id=2,
slug="others-project",
name="others-project",
related_projects=[],
main_language_project=None,
users=[self.other],
Expand All @@ -124,6 +132,44 @@ def setUp(self):
has_htmlzip=True,
)

self.organization = fixture.get(
Organization,
id=1,
pub_date=self.created,
modified_date=self.modified,
name="organization",
slug="organization",
owners=[self.me],
)
self.organization.projects.add(self.project)

self.notification_organization = fixture.get(
Notification,
attached_to_content_type=ContentType.objects.get_for_model(
self.organization
),
attached_to_id=self.organization.pk,
message_id=MESSAGE_ORGANIZATION_DISABLED,
)
self.notification_project = fixture.get(
Notification,
attached_to_content_type=ContentType.objects.get_for_model(self.project),
attached_to_id=self.project.pk,
message_id=MESSAGE_PROJECT_SKIP_BUILDS,
)
self.notification_build = fixture.get(
Notification,
attached_to_content_type=ContentType.objects.get_for_model(self.build),
attached_to_id=self.build.pk,
message_id=BuildCancelled.CANCELLED_BY_USER,
)
self.notification_user = fixture.get(
Notification,
attached_to_content_type=ContentType.objects.get_for_model(self.me),
attached_to_id=self.me.pk,
message_id=MESSAGE_EMAIL_VALIDATION_PENDING,
)

self.client = APIClient()

def tearDown(self):
Expand Down
Loading

0 comments on commit b8767cf

Please sign in to comment.