-
Notifications
You must be signed in to change notification settings - Fork 0
DEV-687 Implement save calendar events #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: preview
Are you sure you want to change the base?
Changes from all commits
57fb17a
b92556c
3b02dc6
cbfc6ad
d7d101b
28ee62c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| from typing import TypedDict, cast | ||
|
|
||
| from rest_framework import serializers, status | ||
| from rest_framework.request import Request | ||
| from rest_framework.response import Response | ||
| from rest_framework.views import APIView | ||
|
|
||
| from hoagiecalendar.api.user_events_view import EventSerializer | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. import from event_views |
||
| from hoagiecalendar.models import Event | ||
|
|
||
|
|
||
| class SaveEventRequestSerializer(serializers.Serializer): | ||
| event_id = serializers.BigIntegerField() | ||
|
|
||
|
|
||
| class SaveEventRequestData(TypedDict): | ||
| event_id: int | ||
|
|
||
|
|
||
| class UserSavedEventsView(APIView): | ||
| def get(self, request: Request) -> Response: | ||
| user = request.user | ||
| saved_events = user.saved_events.all() | ||
| serializer = EventSerializer(saved_events, many=True, context={"request": request}) | ||
|
|
||
| return Response(serializer.data, status=status.HTTP_200_OK) | ||
|
|
||
| def post(self, request: Request) -> Response: | ||
| serializer = SaveEventRequestSerializer(data=request.data) | ||
| if not serializer.is_valid(): | ||
| return Response({"error": serializer.errors}, status=status.HTTP_400_BAD_REQUEST) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| validated_data = cast(SaveEventRequestData, serializer.validated_data) | ||
|
|
||
| try: | ||
| event = Event.objects.get(pk=validated_data["event_id"]) | ||
| event.saved_by.add(request.user) | ||
|
|
||
| return Response({"message": "event saved"}, status=status.HTTP_201_CREATED) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's return the updated event serialized here with status 200 since no new object was created. |
||
| except Event.DoesNotExist: | ||
| return Response({"error": "event not found"}, status=status.HTTP_404_NOT_FOUND) | ||
| except Exception as e: | ||
| return Response({"error": f"an error occured while saving event for user: {e}"}) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this needs a status 500. also, can we leave out the actual error |
||
|
|
||
| def delete(self, request: Request) -> Response: | ||
| serializer = SaveEventRequestSerializer(data=request.data) | ||
| if not serializer.is_valid(): | ||
| return Response({"error": serializer.errors}, status=status.HTTP_400_BAD_REQUEST) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| validated_data = cast(SaveEventRequestData, serializer.validated_data) | ||
|
|
||
| try: | ||
| event = Event.objects.get(pk=validated_data["event_id"]) | ||
| event.saved_by.add(request.user) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be delete |
||
|
|
||
| return Response({"message": "event unsaved"}, status=status.HTTP_200_OK) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be status 204. Also no message needed since there is no data to return. |
||
| except Event.DoesNotExist: | ||
| return Response({"error": "event not found"}, status=status.HTTP_404_NOT_FOUND) | ||
| except Exception as e: | ||
| return Response({"error": f"an error occured while unsaving event for user: {e}"}) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this also needs a status 500. also, can we leave out the actual error |
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you make a tests folder and put this in there? also delete the existing test.py file.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That wasn't meant to be committed. I can remove it for now and then add a more thorough test case later. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| from django.test import TestCase | ||
|
|
||
| from hoagiecalendar.api.event_views import EventSerializer | ||
| from hoagiecalendar.models.event import Event | ||
| from hoagiecalendar.models.user import User | ||
|
|
||
|
|
||
| # Test creating model and getting attendees count. | ||
| class EventModelTestCase(TestCase): | ||
| def setUp(self): | ||
| self.owner = User.objects.create_user(username="testuser", email="test@hoagie.io", password="testpass") | ||
|
|
||
| self.event = Event.objects.create( | ||
| start="2024-01-01T10:00:00Z", | ||
| end="2024-01-01T12:00:00Z", | ||
| name="Test Event", | ||
| location="Test Location", | ||
| description="Test Description", | ||
| host="Test Host", | ||
| owner=self.owner, | ||
| ) | ||
|
|
||
| def test_attending_count(self): | ||
| event = Event.objects.get(pk=self.event.pk) | ||
|
|
||
| self.assertEqual(event.attending_count(), 0) | ||
|
|
||
| event.attendees.add(self.owner) | ||
|
|
||
| self.assertEqual(event.attending_count(), 1) | ||
|
|
||
| def test_serializing(self): | ||
| serializer = EventSerializer(self.event) | ||
| deserializer = EventSerializer(data=serializer.data) | ||
|
|
||
| self.assertTrue(deserializer.is_valid(), str(deserializer.errors)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will trigger a query per event when serializing to get whether the user saved the event. Can we do a query set annotation instead where we take the events we want to serialize, make a query to add the is_saved field into the query set, and then put the annotated query set into the serializer? This way we only make one query for all the events we want to serialize. Make sure all of the calls to the Event Serializer are updated to reflect changes (merge from preview first).