From 691d5b293b0a6de472d3d1afb49e3caeaa62de40 Mon Sep 17 00:00:00 2001 From: Leandro de Souza Date: Fri, 17 Nov 2023 10:49:57 -0300 Subject: [PATCH] Fix (notifications): Adds a method for defining/removing a notification/token. Adds tests for the api and some fixes with them --- src/api/v1/push_notifications/schemas.py | 4 + src/api/v1/push_notifications/urls.py | 2 +- src/api/v1/push_notifications/views.py | 42 +++- src/push_notifications/exc.py | 6 + src/push_notifications/models.py | 4 +- src/push_notifications/services.py | 22 +- tests/conftest.py | 4 +- .../test_push_notification_endpoints.py | 231 ++++++++++++++++++ 8 files changed, 305 insertions(+), 10 deletions(-) create mode 100644 tests/test_api/test_v1/test_push_notification_endpoints.py diff --git a/src/api/v1/push_notifications/schemas.py b/src/api/v1/push_notifications/schemas.py index 0714af2..c19a1de 100644 --- a/src/api/v1/push_notifications/schemas.py +++ b/src/api/v1/push_notifications/schemas.py @@ -51,3 +51,7 @@ class PushNotificationReadManyInputSchema(serializers.Serializer): class PushNotificationReadManyOutputSchema(serializers.Serializer): read = serializers.IntegerField() + + +class PushNotificationSetTokenInputSchema(serializers.Serializer): + notification_token = serializers.CharField() diff --git a/src/api/v1/push_notifications/urls.py b/src/api/v1/push_notifications/urls.py index acba305..8c25496 100644 --- a/src/api/v1/push_notifications/urls.py +++ b/src/api/v1/push_notifications/urls.py @@ -3,6 +3,6 @@ from . import views router = SimpleRouter(trailing_slash=False) -router.register("notifications", views.PushNotificationViewSet, basename="push_notifications") +router.register("", views.PushNotificationViewSet, basename="notifications") urlpatterns = router.urls diff --git a/src/api/v1/push_notifications/views.py b/src/api/v1/push_notifications/views.py index 4ca7638..6704f8a 100644 --- a/src/api/v1/push_notifications/views.py +++ b/src/api/v1/push_notifications/views.py @@ -4,9 +4,10 @@ from rest_framework.request import Request from rest_framework.response import Response +from app.consts.http import HttpStatusCode from app.drf.openapi import limit_offset_openapi_schema, openapi_schema from app.drf.viewsets import AppViewSet -from push_notifications import selectors, services +from push_notifications import exc, selectors, services from . import schemas @@ -35,7 +36,7 @@ def list(self, request: Request) -> Response: summary="Read many notifications", description="Reads a list of notifications", request=schemas.PushNotificationReadManyInputSchema, - responses={200: schemas.PushNotificationReadManyOutputSchema}, + responses={HttpStatusCode.HTTP_200_OK: schemas.PushNotificationReadManyOutputSchema}, tags=["push notifications"], operation_id="push-notifications-read", add_bad_request_response=True, @@ -51,7 +52,7 @@ def read_many(self, request: Request) -> Response: summary="Read notification", description="Reads a single notification", request=None, - responses={200: schemas.PushNotificationOutputSchema}, + responses={HttpStatusCode.HTTP_200_OK: schemas.PushNotificationOutputSchema}, tags=["push notifications"], operation_id="push-notification-read", add_not_found_response=True, @@ -59,11 +60,42 @@ def read_many(self, request: Request) -> Response: add_unauthorized_response=True, ) @action(methods=["PATCH"], detail=True) - def read(self, request: Request, id: int) -> Response: + def read(self, request: Request, pk: int) -> Response: notification = get_object_or_404( selectors.push_notification_get_viewable_qs(user=request.user), - pk=id, + pk=pk, ) notification = services.push_notification_read(push_notification=notification) out_srlzr = schemas.PushNotificationOutputSchema(instance=notification) return Response(data=out_srlzr.data) + + @openapi_schema( + summary="Set Notification token", + description="Defines the notification for the current user", + request=schemas.PushNotificationSetTokenInputSchema, + responses={HttpStatusCode.HTTP_200_OK: None}, + tags=["push notifications"], + operation_id="push-notification-set-token", + add_unauthorized_response=True, + add_bad_request_response=True, + raises=[exc.InvalidNotificationToken], + ) + @action(methods=["PUT"], detail=False) + def token(self, request: Request) -> Response: + data = self.get_valid_data(schemas.PushNotificationSetTokenInputSchema) + services.push_notification_set_token(user=request.user, **data) + return Response(status=HttpStatusCode.HTTP_200_OK) + + @openapi_schema( + summary="Delete Notification token", + description="Removes the notification token from the current user (Opt-out)", + request=None, + responses={HttpStatusCode.HTTP_200_OK: None}, + tags=["push notifications"], + operation_id="push-notification-delete-token", + add_unauthorized_response=True, + ) + @token.mapping.delete + def delete_token(self, request: Request) -> Response: + services.push_notification_set_token(user=request.user, notification_token=None) + return Response(status=HttpStatusCode.HTTP_200_OK) diff --git a/src/push_notifications/exc.py b/src/push_notifications/exc.py index 141a7ab..b54cd22 100644 --- a/src/push_notifications/exc.py +++ b/src/push_notifications/exc.py @@ -1,5 +1,11 @@ +from django.utils.translation import gettext_lazy as _ + from app.exceptions import ApplicationError class UnableToSendPushNotification(ApplicationError): pass + + +class InvalidNotificationToken(ApplicationError): + error_message = _("Invalid notification token, {reason}") diff --git a/src/push_notifications/models.py b/src/push_notifications/models.py index ea864b6..389cd9e 100644 --- a/src/push_notifications/models.py +++ b/src/push_notifications/models.py @@ -115,8 +115,8 @@ def __str__(self): def enriched_data(self): return { "id": self.id, - "createdAt": str(self.created_at), - "readAt": str(self.read_at), + "createdAt": self.created_at.isoformat(), + "readAt": self.read_at.isoformat() if self.read_at else None, "timeSinceCreated": timesince(self.created_at), "kind": self.kind, "meta": self.data, diff --git a/src/push_notifications/services.py b/src/push_notifications/services.py index 51733ab..f8f3c49 100644 --- a/src/push_notifications/services.py +++ b/src/push_notifications/services.py @@ -1,7 +1,9 @@ from datetime import datetime, timedelta from typing import Any, Sequence +from django.core.exceptions import ValidationError from django.utils import timezone +from django.utils.translation import gettext as _ from app import consts from app.ext import di @@ -14,7 +16,7 @@ from app.models import BaseModel from users.models import User -from . import models, selectors +from . import exc, models, selectors def push_notification_create( @@ -239,3 +241,21 @@ def push_notification_read_many(*, reader: User, ids: Sequence[int]) -> int: if ids: qs = qs.filter(pk__in=ids) return qs.update(read_at=timezone.now(), status=consts.push_notification.Status.READ) + + +def push_notification_set_token(*, user: User, notification_token: str | None) -> None: + # We aren't calling full_clean here on User so we don't raise any other errors that + # aren't from this specific field + + if notification_token is not None: + if not notification_token: + # Because we have blank=True, and we accept None values, raise if someone attempt + # to send a blank string "" + raise exc.InvalidNotificationToken( + message_format_kwargs={"reason": _("it may not be blank")} + ) + max_length = User._meta.get_field("notification_token").max_length or 64 + if len(notification_token) > max_length: + raise exc.InvalidNotificationToken(message_format_kwargs={"reason": _("value too big")}) + user.notification_token = notification_token + user.save() diff --git a/tests/conftest.py b/tests/conftest.py index 4c29d94..6d371e6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,4 +1,5 @@ import pytest +from django.utils import timezone from tests.fakes import FakePushNotificationExternalService from users.models import User @@ -22,7 +23,8 @@ def visitor_user(): user.set_password("password") user.full_clean() user.save() - return user + timezone.activate(user.time_zone) + yield user @pytest.fixture(autouse=True) diff --git a/tests/test_api/test_v1/test_push_notification_endpoints.py b/tests/test_api/test_v1/test_push_notification_endpoints.py new file mode 100644 index 0000000..ba2936e --- /dev/null +++ b/tests/test_api/test_v1/test_push_notification_endpoints.py @@ -0,0 +1,231 @@ +import pytest +from django.test.client import Client +from django.urls import reverse +from django.utils import timezone +from django.utils.timesince import timesince + +from app.consts.http import HttpStatusCode +from app.consts.push_notification import Status +from push_notifications import services +from push_notifications.models import PushNotification +from users.models import User + + +@pytest.fixture +def visitor_notification(visitor_user: User): + return services.push_notification_create( + user=visitor_user, + kind="int_comm", + title="Hey!", + description="Hello world", + data={"foo": "bar"}, + ) + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "method_name,url_name,reverse_args", + [ + ("GET", "api:v1:push_notifications:notifications-list", ()), + ("PATCH", "api:v1:push_notifications:notifications-read", (1,)), + ("PATCH", "api:v1:push_notifications:notifications-read-many", ()), + ("PUT", "api:v1:push_notifications:notifications-token", ()), + ("DELETE", "api:v1:push_notifications:notifications-token", ()), + ], +) +def test_pn_endpoints_requires_authentication(client: Client, method_name, url_name, reverse_args): + url = reverse(url_name, args=reverse_args) + methods = {"GET": client.get, "PATCH": client.patch, "PUT": client.put, "DELETE": client.delete} + method = methods[method_name] + + response = method(url) + assert response.status_code == HttpStatusCode.HTTP_401_UNAUTHORIZED + + +@pytest.mark.django_db +def test_pn_list_returns_expected_output(visitor_notification: PushNotification, client: Client): + url = reverse("api:v1:push_notifications:notifications-list") + client.force_login(user=visitor_notification.user) + response = client.get(url) + assert response.status_code == HttpStatusCode.HTTP_200_OK + data = response.json() + assert len(data["results"]) == 1 + result = data["results"][0] + assert result == { + "id": visitor_notification.id, + "title": visitor_notification.title, + "description": visitor_notification.description, + "read_at": None, + "kind": { + "value": visitor_notification.kind, + "human": visitor_notification.get_kind_display(), + }, + "status": { + "value": visitor_notification.status, + "human": visitor_notification.get_status_display(), + }, + "data": { + "id": str(visitor_notification.id), + "createdAt": visitor_notification.created_at.isoformat(), + "readAt": None, + "timeSinceCreated": timesince(visitor_notification.created_at), + "kind": visitor_notification.kind, + "meta": visitor_notification.data, + }, + } + + +@pytest.mark.django_db +def test_pn_list_query_params_invalid(visitor_notification: PushNotification, client: Client): + url = reverse("api:v1:push_notifications:notifications-list") + client.force_login(user=visitor_notification.user) + response = client.get(url, data={"only_read": "foo"}) + assert response.status_code == HttpStatusCode.HTTP_400_BAD_REQUEST + + +@pytest.mark.django_db +def test_pn_list_query_params_valid(visitor_notification: PushNotification, client: Client): + assert visitor_notification.read_at is None + + url = reverse("api:v1:push_notifications:notifications-list") + client.force_login(user=visitor_notification.user) + response = client.get(url, data={"only_read": True}) + assert response.status_code == HttpStatusCode.HTTP_200_OK + assert len(response.json()["results"]) == 0 + + visitor_notification.read_at = timezone.now() + visitor_notification.save() + + response = client.get(url, data={"only_read": True}) + assert len(response.json()["results"]) == 1 + + response = client.get(url, data={"only_read": False}) + assert len(response.json()["results"]) == 0 + + +@pytest.mark.django_db +def test_pn_read_many_all_valid_ids(visitor_notification: PushNotification, client: Client): + url = reverse("api:v1:push_notifications:notifications-read-many") + client.force_login(user=visitor_notification.user) + response = client.patch( + url, data={"ids": [visitor_notification.id]}, content_type="application/json" + ) + assert response.status_code == HttpStatusCode.HTTP_200_OK + assert response.json()["read"] == 1 + + +@pytest.mark.django_db +def test_pn_read_many_ids_not_set(visitor_notification: PushNotification, client: Client): + url = reverse("api:v1:push_notifications:notifications-read-many") + client.force_login(user=visitor_notification.user) + # We can send None/null to read all notifications + response = client.patch(url, data={"ids": None}, content_type="application/json") + assert response.status_code == HttpStatusCode.HTTP_200_OK + assert response.json()["read"] == 1 + + # reset + visitor_notification.read_at = None + visitor_notification.save(force_update=True) + + # We also can send a empty/null data and would read all notifications + response = client.patch(url) + assert response.status_code == HttpStatusCode.HTTP_200_OK + assert response.json()["read"] == 1 + + +@pytest.mark.django_db +def test_pn_read_many_ids_from_other_user_no_update_happens( + visitor_notification: PushNotification, admin_client: Client +): + url = reverse("api:v1:push_notifications:notifications-read-many") + # We're requesting from a user that has no visibility to the notification + response = admin_client.patch( + url, data={"ids": [visitor_notification.id]}, content_type="application/json" + ) + assert response.status_code == HttpStatusCode.HTTP_200_OK + assert response.json()["read"] == 0 + + +@pytest.mark.django_db +def test_pn_read_id_from_other_user_404( + visitor_notification: PushNotification, admin_client: Client +): + # We're requesting from a user that has no visibility to the notification + url = reverse("api:v1:push_notifications:notifications-read", args=(visitor_notification.id,)) + response = admin_client.patch(url) + assert response.status_code == HttpStatusCode.HTTP_404_NOT_FOUND + + +@pytest.mark.django_db +def test_pn_read_id_already_read(visitor_notification: PushNotification, client: Client): + # if the notification is already read, it should succeed, but no updates should happen + now = timezone.localtime() + visitor_notification.read_at = now + visitor_notification.save() + + url = reverse("api:v1:push_notifications:notifications-read", args=(visitor_notification.id,)) + client.force_login(visitor_notification.user) + response = client.patch(url) + assert response.status_code == HttpStatusCode.HTTP_200_OK + data = response.json() + assert data["read_at"] == now.isoformat() + + +@pytest.mark.django_db +def test_pn_read_id_expected_output(visitor_notification: PushNotification, client: Client): + # if the notification is not read, it should succeed and updates should happen + assert visitor_notification.read_at is None + assert visitor_notification.status != Status.READ + + url = reverse("api:v1:push_notifications:notifications-read", args=(visitor_notification.id,)) + client.force_login(visitor_notification.user) + response = client.patch(url) + assert response.status_code == HttpStatusCode.HTTP_200_OK + data = response.json() + assert data["read_at"] is not None + assert data["status"]["value"] == Status.READ + + visitor_notification = PushNotification.objects.get(pk=visitor_notification.pk) + assert visitor_notification.read_at is not None + assert visitor_notification.status == Status.READ + + +@pytest.mark.django_db +def test_pn_set_token(client: Client, visitor_user: User): + assert visitor_user.notification_token is None + + url = reverse("api:v1:push_notifications:notifications-token") + client.force_login(visitor_user) + response = client.put( + url, data={"notification_token": "foobar"}, content_type="application/json" + ) + assert response.status_code == HttpStatusCode.HTTP_200_OK + with pytest.raises(TypeError): + # We don't return any data, so trying to get the json should return an error + response.json() + + visitor_user = User.objects.get(pk=visitor_user.pk) + assert visitor_user.notification_token == "foobar" + + +@pytest.mark.django_db +def test_pn_set_token_invalid(client: Client, visitor_user: User): + # Someone could try to opt-out of notifications trying to use this endpoint + # sending a null value, but that's not how we expect it. For that they should + # send a DELETE request instead + url = reverse("api:v1:push_notifications:notifications-token") + client.force_login(visitor_user) + response = client.put(url, data={"notification_token": None}, content_type="application/json") + assert response.status_code == HttpStatusCode.HTTP_400_BAD_REQUEST + assert "notification_token" in response.json()["extra"]["fields"] + + +@pytest.mark.django_db +def test_pn_delete_token(client: Client, visitor_user: User): + url = reverse("api:v1:push_notifications:notifications-token") + client.force_login(visitor_user) + response = client.delete(url) + assert response.status_code == HttpStatusCode.HTTP_200_OK + with pytest.raises(TypeError): + # We don't return any data, so trying to get the json should return an error + response.json()