Conversation
david-venhoff
left a comment
There was a problem hiding this comment.
Thanks, I have a few comments
|
@david-venhoff I think this PR is ready for review again :) Do you think you could have another look at it? :) Feel free to unresolve a conversation, if you're running into an issue at one point :) |
There was a problem hiding this comment.
Thanks, now the url works me 🎉
I have added some more comments, but if you prefer to be unblocked early you can dismiss them since most of them are stylistic or have only a small functional impact. For this case I will already give the approve.
I got a bit side tracked and changed the view and serializer to use a model serializer, similar to how FeedbackSerializer and FeedbackViewset work right now. One advantage is that the debug api view at http://localhost:8080/api/v2/analytics/events is now more helpful. The changes are below as a diff:
Details
diff --git a/lunes_cms/analytics/api/serializers.py b/lunes_cms/analytics/api/serializers.py
index e153a63..eedcfa7 100644
--- a/lunes_cms/analytics/api/serializers.py
+++ b/lunes_cms/analytics/api/serializers.py
@@ -2,18 +2,20 @@ from rest_framework import serializers
from ..models import AnalyticsEvent
-class AnalyticsEventSerializer(serializers.Serializer):
+class AnalyticsEventSerializer(serializers.ModelSerializer):
"""
Serializer class
"""
- installation_id = serializers.CharField(max_length=255)
- event_type = serializers.CharField(max_length=100)
- timestamp = serializers.DateTimeField()
- payload = serializers.JSONField()
+ class Meta:
+ """
+ Define model and the corresponding fields
+ """
- def create(self, validated_data):
- return AnalyticsEvent.objects.create(**validated_data)
-
- def update(self, instance, validated_data):
- raise NotImplementedError("AnalyticsEventSerializer does not support updates")
+ model = AnalyticsEvent
+ fields = (
+ "installation_id",
+ "event_type",
+ "timestamp",
+ "payload",
+ )
diff --git a/lunes_cms/analytics/api/urls.py b/lunes_cms/analytics/api/urls.py
index 49deefe..325ecfb 100644
--- a/lunes_cms/analytics/api/urls.py
+++ b/lunes_cms/analytics/api/urls.py
@@ -1,11 +1,15 @@
from typing import List, Union
-from django.urls import path, URLPattern, URLResolver
+from django.urls import path, URLPattern, URLResolver, include
-from .views import AnalyticsEventView
+from .views import AnalyticsEventViewSet
+from ...api.utils import OptionalSlashRouter
app_name = "analytics"
+router = OptionalSlashRouter()
+router.register(r"events", AnalyticsEventViewSet, basename="analytics_event")
+
urlpatterns: List[Union[URLPattern, URLResolver]] = [
- path("events/", AnalyticsEventView.as_view(), name="analytics_event"),
+ path("", include(router.urls)),
]
diff --git a/lunes_cms/analytics/api/views.py b/lunes_cms/analytics/api/views.py
index f53a212..ee4f25b 100644
--- a/lunes_cms/analytics/api/views.py
+++ b/lunes_cms/analytics/api/views.py
@@ -1,9 +1,8 @@
-from rest_framework.views import APIView
-from rest_framework.response import Response
+from rest_framework import mixins, viewsets
from rest_framework.request import Request
-from rest_framework import status
from rest_framework.throttling import SimpleRateThrottle
+
from .serializers import AnalyticsEventSerializer
@@ -24,21 +23,10 @@ class InstallationRateThrottle(SimpleRateThrottle):
return f"rl:installation:{installation_id}"
-class AnalyticsEventView(APIView):
+class AnalyticsEventViewSet(mixins.CreateModelMixin, viewsets.GenericViewSet):
"""
View class for analytics events
"""
throttle_classes = [InstallationRateThrottle]
-
- def post(self, request: Request) -> Response:
- """
- POST method for analytics events
- :param request: Current request
- :return: Either OK response or error response
- """
- serializer = AnalyticsEventSerializer(data=request.data)
- if serializer.is_valid():
- serializer.save()
- return Response({"status": "ok"}, status=status.HTTP_201_CREATED)
- return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
+ serializer_class = AnalyticsEventSerializer
e69ca48 to
032558a
Compare
032558a to
03ddf3a
Compare
Short description
This PR sets up the foundation for the analytics model and API. Additionally it also adds the AnalyticsEvent model and an endpoint for creating an event.
Proposed changes
Resolved issues
Fixes: #661