From 11e0d60ed4a966846d5786208afb86ed9cee948a Mon Sep 17 00:00:00 2001 From: Nikhil-Penn Date: Sun, 16 Feb 2025 03:38:39 +0000 Subject: [PATCH 01/10] I think ready for prod now --- backend/courses/models.py | 22 +++++ backend/courses/util.py | 13 +-- backend/plan/models.py | 58 ++++++++++++ backend/plan/serializers.py | 37 +++++++- backend/plan/urls.py | 2 + backend/plan/views.py | 115 +++++++++++++++++++++++- backend/tests/plan/test_schedule.py | 132 +++++++++++++++++++++++++++- 7 files changed, 366 insertions(+), 13 deletions(-) diff --git a/backend/courses/models.py b/backend/courses/models.py index 32f79f1c6..63537324b 100644 --- a/backend/courses/models.py +++ b/backend/courses/models.py @@ -16,6 +16,8 @@ from review.annotations import review_averages + + User = get_user_model() @@ -1126,10 +1128,20 @@ class Meeting(models.Model): section = models.ForeignKey( Section, + null=True, on_delete=models.CASCADE, related_name="meetings", help_text="The Section object to which this class meeting belongs.", ) + + associated_break = models.ForeignKey( + "plan.Break", + null=True, + on_delete=models.CASCADE, + related_name="meetings", + help_text="The Section object to which this class meeting belongs.", + ) + day = models.CharField( max_length=1, help_text="The single day on which the meeting takes place (one of M, T, W, R, or F).", @@ -1180,6 +1192,16 @@ class Meeting(models.Model): ), ) + def clean(self): + super().clean() + if (self.section is None and self.associated_break is None) or (self.section is not None and self.associated_break is not None): + raise ValidationError("Either the section field of associated_break field must be populated, but not both") + + def save(self, *args, **kwargs): + self.clean() + super().save(*args, **kwargs) + + class Meta: unique_together = (("section", "day", "start", "end", "room"),) diff --git a/backend/courses/util.py b/backend/courses/util.py index 77f06f612..9cbdc9fac 100644 --- a/backend/courses/util.py +++ b/backend/courses/util.py @@ -29,6 +29,7 @@ StatusUpdate, User, ) +from plan.models import Break from review.management.commands.mergeinstructors import resolve_duplicates @@ -465,7 +466,7 @@ def clean_meetings(meetings): }.values() -def set_meetings(section, meetings): +def set_meetings(obj, meetings): meetings = clean_meetings(meetings) for meeting in meetings: @@ -473,9 +474,9 @@ def set_meetings(section, meetings): meeting_times = [ f"{meeting['days']} {meeting['begin_time']} - {meeting['end_time']}" for meeting in meetings ] - section.meeting_times = json.dumps(meeting_times) + obj.meeting_times = json.dumps(meeting_times) - section.meetings.all().delete() + obj.meetings.all().delete() for meeting in meetings: online = ( not meeting["building_code"] @@ -492,8 +493,10 @@ def set_meetings(section, meetings): start_date = extract_date(meeting.get("start_date")) end_date = extract_date(meeting.get("end_date")) for day in list(meeting["days"]): + meeting = Meeting.objects.update_or_create( - section=section, + section=obj if isinstance(obj, Section) else None, + associated_break=obj if isinstance(obj, Break) else None, day=day, start=start_time, end=end_time, @@ -504,7 +507,6 @@ def set_meetings(section, meetings): }, ) - def add_associated_sections(section, linked_sections): semester = section.course.semester section.associated_sections.clear() @@ -533,7 +535,6 @@ def set_crosslistings(course, crosslistings): course.primary_listing = primary_course return - def upsert_course_from_opendata(info, semester, missing_sections=None): dept_code = info.get("subject") or info.get("course_department") assert dept_code, json.dumps(info, indent=2) diff --git a/backend/plan/models.py b/backend/plan/models.py index 72345460d..73ee3015f 100644 --- a/backend/plan/models.py +++ b/backend/plan/models.py @@ -90,3 +90,61 @@ class Meta: def __str__(self): return f"PrimarySchedule(User: {self.user}, Schedule ID: {self.schedule_id})" + + + +class Break(models.Model): + """ + Holds break objects created by users on PCP. + """ + + person = models.ForeignKey( + get_user_model(), + on_delete=models.CASCADE, + help_text="The person (user) who created this break.", + ) + + location_string = models.CharField( + max_length=80, + null=True, + help_text=dedent( + """ + This represents the location that the user can input themselves. + Will use a building object from a drop down or have it validated or something so it can interact with map. + Didn't want to run into issue of users creating arbitrary Room objects, so just using a char field + """ + ) #TODO: Don't know how I want to do buildings yet. + ) + + name = models.CharField( + max_length=255, + help_text=dedent( + """ + The user's name for the break. No two breaks can match in all of the fields + `[name, person]` + """ + ), + ) + + + meeting_times = models.TextField( + blank=True, + help_text=dedent( + """ + A JSON-stringified list of meeting times of the form + `{days code} {start time} - {end time}`, e.g. + `["MWF 09:00 AM - 10:00 AM","F 11:00 AM - 12:00 PM","T 05:00 PM - 06:00 PM"]` for + PHYS-151-001 (2020A). Each letter of the days code is of the form M, T, W, R, F for each + day of the work week, respectively (and multiple days are combined with concatenation). + To access the Meeting objects for this section, the related field `meetings` can be used. + """ + ), + ) + + class Meta: + unique_together = (("person"),) + + def __str__(self): + return "User: %s, Break ID: %s" % (self.person, self.id) + + diff --git a/backend/plan/serializers.py b/backend/plan/serializers.py index 3ac31a7e2..e1a53e156 100644 --- a/backend/plan/serializers.py +++ b/backend/plan/serializers.py @@ -1,7 +1,8 @@ +from textwrap import dedent from rest_framework import serializers -from courses.serializers import PublicUserSerializer, SectionDetailSerializer -from plan.models import PrimarySchedule, Schedule +from courses.serializers import PublicUserSerializer, SectionDetailSerializer, MeetingSerializer, MeetingWithBuildingSerializer +from plan.models import PrimarySchedule, Schedule, Break class ScheduleSerializer(serializers.ModelSerializer): @@ -34,3 +35,35 @@ class PrimaryScheduleSerializer(serializers.ModelSerializer): class Meta: model = PrimarySchedule fields = ["user", "schedule"] + + +class BreakSerializer(serializers.ModelSerializer): + + meetings = serializers.SerializerMethodField( + read_only=True, + help_text=dedent( + """ + A list of the meetings of this section (each meeting is a continuous span of time + during which a section would meet). + """ + ), + ) + id = serializers.IntegerField( + read_only=False, required=False, help_text="The id of the schedule." + ) + + + class Meta: + model = Break + exclude = ["person"] + + + def get_meetings(self, obj): + include_location = self.context.get("include_location", False) + if include_location: + meetings_serializer = MeetingWithBuildingSerializer(obj.meetings, many=True) + else: + meetings_serializer = MeetingSerializer(obj.meetings, many=True) + + return meetings_serializer.data + diff --git a/backend/plan/urls.py b/backend/plan/urls.py index 0876e5a8e..ec8e878ad 100644 --- a/backend/plan/urls.py +++ b/backend/plan/urls.py @@ -6,6 +6,7 @@ CalendarAPIView, PrimaryScheduleViewSet, ScheduleViewSet, + BreakViewSet, recommend_courses_view, ) @@ -13,6 +14,7 @@ router = routers.DefaultRouter() router.register(r"schedules", ScheduleViewSet, basename="schedules") router.register(r"primary-schedules", PrimaryScheduleViewSet, basename="primary-schedules") +router.register(r"breaks", BreakViewSet, basename="breaks") urlpatterns = [ path("/calendar/", CalendarAPIView.as_view(), name="calendar-view"), diff --git a/backend/plan/views.py b/backend/plan/views.py index 846bebef2..1338c5ece 100644 --- a/backend/plan/views.py +++ b/backend/plan/views.py @@ -14,13 +14,13 @@ from rest_framework import status, viewsets from rest_framework.decorators import api_view, permission_classes, schema from rest_framework.exceptions import PermissionDenied -from rest_framework.permissions import IsAuthenticated +from rest_framework.permissions import IsAuthenticated, AllowAny from rest_framework.response import Response from rest_framework.views import APIView from courses.models import Course, Meeting, Section from courses.serializers import CourseListSerializer -from courses.util import get_course_and_section, get_current_semester, normalize_semester +from courses.util import get_course_and_section, get_current_semester, normalize_semester, set_meetings from courses.views import get_accepted_friends from PennCourses.docs_settings import PcxAutoSchema from PennCourses.settings.base import PATH_REGISTRATION_SCHEDULE_NAME @@ -31,8 +31,8 @@ vectorize_user, vectorize_user_by_courses, ) -from plan.models import PrimarySchedule, Schedule -from plan.serializers import PrimaryScheduleSerializer, ScheduleSerializer +from plan.models import PrimarySchedule, Schedule, Break +from plan.serializers import PrimaryScheduleSerializer, ScheduleSerializer, BreakSerializer @api_view(["POST"]) @@ -617,6 +617,113 @@ def get_queryset(self, semester=None): ) return queryset +class BreakViewSet(AutoPrefetchViewSetMixin, viewsets.ModelViewSet): + + serializer_class = BreakSerializer + permission_classes = [IsAuthenticated] + + def get_serializer_context(self): + context = super().get_serializer_context() + include_location_str = "False" + # TODO: figure out how we want to do locations. + context.update({"include_location": eval(include_location_str)}) + return context + + def update(self, request): + break_id = request.data.get("id") + if not break_id: + return Response({"detail": "Break id is required for update."}, + status=status.HTTP_400_BAD_REQUEST) + + try: + current_break = self.get_queryset().get(id=break_id) + except Break.DoesNotExist: + return Response({"detail": "Break not found."}, + status=status.HTTP_404_NOT_FOUND) + except Exception as e: + return Response({"detail": "Error retrieving break: " + str(e)}, + status=status.HTTP_400_BAD_REQUEST) + + name = request.data.get("name") + if not name: + return Response({"detail": "Break name is required."}, + status=status.HTTP_400_BAD_REQUEST) + location_string = request.data.get("location_string") + + current_break.name = name + current_break.location_string = location_string + + meetings = request.data.get("meetings") + try: + set_meetings(current_break, meetings) + except Exception as e: + return Response({"detail": "Error setting meetings: " + str(e)}, + status=status.HTTP_400_BAD_REQUEST) + + try: + current_break.save() + except Exception as e: + return Response({"detail": "Error saving break: " + str(e)}, + status=status.HTTP_400_BAD_REQUEST) + + return Response({"message": "success", "id": current_break.id}, + status=status.HTTP_200_OK) + + def create(self, request, *args, **kwargs): + break_id = request.data.get("id") + if break_id and Break.objects.filter(id=break_id).exists(): + return self.update(request) + + name = request.data.get("name") + if not name: + return Response({"detail": "Break name is required."}, + status=status.HTTP_400_BAD_REQUEST) + location_string = request.data.get("location_string") + + try: + if break_id: + new_break = self.get_queryset().create( + person=request.user, + name=name, + location_string=location_string, + id=break_id, + ) + else: + new_break = self.get_queryset().create( + person=request.user, + name=name, + location_string=location_string, + ) + except IntegrityError as e: + return Response( + {"detail": "IntegrityError encountered while trying to create: " + str(e.__cause__)}, + status=status.HTTP_400_BAD_REQUEST, + ) + except Exception as e: + return Response( + {"detail": "Error creating break: " + str(e)}, + status=status.HTTP_400_BAD_REQUEST, + ) + + meetings = request.data.get("meetings") + try: + set_meetings(new_break, meetings) + except Exception as e: + return Response({"detail": "Error setting meetings: " + str(e)}, + status=status.HTTP_400_BAD_REQUEST) + + return Response({"message": "success", "id": new_break.id}, + status=status.HTTP_201_CREATED) + + def get_queryset(self): + return Break.objects.filter(person=self.request.user).prefetch_related( + "meetings", # Prefetch the related meetings + "meetings__room", # Prefetch the related rooms for each meeting + ) + + + + class CalendarAPIView(APIView): schema = PcxAutoSchema( diff --git a/backend/tests/plan/test_schedule.py b/backend/tests/plan/test_schedule.py index 2d3bd1a03..6854a5a6d 100644 --- a/backend/tests/plan/test_schedule.py +++ b/backend/tests/plan/test_schedule.py @@ -10,9 +10,16 @@ from alert.models import AddDropPeriod from courses.util import get_average_reviews, invalidate_current_semester_cache from PennCourses.settings.base import PATH_REGISTRATION_SCHEDULE_NAME -from plan.models import Schedule +from plan.models import Schedule, Break + +from courses.models import Meeting + from tests.courses.util import create_mock_data_with_reviews +from django.urls import reverse +from django.db import IntegrityError +from rest_framework import status + User = get_user_model() @@ -1224,3 +1231,126 @@ def test_update_from_path_nonexistent_sections(self, mock_request): path_schedule = schedule self.assertEqual(len(path_schedule["sections"]), 1) self.assertEqual(path_schedule["sections"][0]["id"], "CIS-120-001") + + +class BreakViewSetTests(TestCase): + def setUp(self): + # Create and authenticate a test user + self.user = User.objects.create_user(username="testuser", password="testpassword") + self.client = APIClient() + self.client.login(username="testuser", password="testpassword") + + # Create an initial Break instance to test update operations + self.break_obj = Break.objects.create( + person=self.user, + name="Morning Break", + location_string="Cafeteria" + ) + self.break_obj.save() + + # Use Django's reverse to dynamically get the correct endpoint + self.break_list_url = reverse("breaks-list") # /api/plan/breaks/ + self.break_detail_url = reverse("breaks-detail", kwargs={"pk": self.break_obj.id}) # /api/plan/breaks/{id}/ + + @patch("accounts.authentication.requests.post") + def test_create_break(self, mock_set_meetings): + """ + Ensure that posting a new break (without an "id") creates the break + and returns a 201 response. + """ + data = { + "name": "Afternoon Break", + "location_string": "Lobby", + "meetings": [] # No meetings provided + } + response = self.client.post(self.break_list_url, data, format="json") + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + # Check that a new Break was created with the proper fields. + new_break_id = response.data.get("id") + self.assertIsNotNone(new_break_id) + new_break = Break.objects.get(id=new_break_id) + self.assertEqual(new_break.name, "Afternoon Break") + self.assertEqual(new_break.location_string, "Lobby") + + # Verify that set_meetings was called with the new break and meetings list. + mock_set_meetings.assert_called_once_with(new_break, []) + self.assertEqual(response.data.get("message"), "success") + + @patch("plan.views.set_meetings") + def test_update_break_existing(self, mock_set_meetings): + """ + If a break with the provided "id" exists, sending a POST request with that id should update it. + """ + data = { + "id": self.break_obj.id, + "name": "Updated Break", + "location_string": "New Location", + "meetings": [ + { + "days": "MT", + "begin_time_24": 900, + "begin_time": "9:00 AM", + "end_time_24": 1000, + "end_time": "10:00 AM", + }, + ] + } + response = self.client.post(self.break_list_url, data, format="json") + self.assertEqual(response.status_code, status.HTTP_200_OK) + + # Refresh the object from the database to confirm changes. + self.break_obj.refresh_from_db() + self.assertEqual(self.break_obj.name, "Updated Break") + self.assertEqual(self.break_obj.location_string, "New Location") + + self.assertEqual(response.data.get("message"), "success") + self.assertEqual(response.data.get("id"), self.break_obj.id) + + def test_update_break_nonexistent(self): + """ + Posting a break update with an id that does not exist should create a break with that ID. + """ + data = { + "id": 9999, # An id that does not exist for this user + "name": "Nonexistent Break", + "location_string": "Nowhere", + "meetings": [] + } + response = self.client.post(self.break_list_url, data, format="json") + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertEqual(response.data.get("message"), "success") + + new_break = Break.objects.get(id = response.data.get("id")) + + self.assertEqual(new_break.name, "Nonexistent Break") + self.assertEqual(new_break.location_string, "Nowhere") + + def test_list_breaks(self): + """ + Ensure that a GET request to the break-list endpoint returns the list + of breaks for the authenticated user. + """ + response = self.client.get(self.break_list_url, format="json") + self.assertEqual(response.status_code, status.HTTP_200_OK) + # Check that our initial break ("Morning Break") is in the returned list. + print(response.data) + break_names = [item.get("name") for item in response.data] + self.assertIn("Morning Break", break_names) + + def test_get_break_by_id(self): + """ + Ensure that retrieving a specific break by its id works. + """ + response = self.client.get(self.break_detail_url, format="json") + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["name"], self.break_obj.name) + self.assertEqual(response.data["location_string"], self.break_obj.location_string) + + def test_delete_break(self): + """ + Ensure that deleting a break works. + """ + response = self.client.delete(self.break_detail_url, format="json") + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + self.assertFalse(Break.objects.filter(id=self.break_obj.id).exists()) \ No newline at end of file From 32b0538555a6dfa09fa2ee5aac50ae366499a18b Mon Sep 17 00:00:00 2001 From: Nikhil-Penn Date: Sun, 16 Feb 2025 14:59:34 +0000 Subject: [PATCH 02/10] linting issues fixed :| --- backend/courses/models.py | 13 ++-- backend/courses/util.py | 4 +- backend/plan/models.py | 16 +++-- backend/plan/serializers.py | 13 ++-- backend/plan/urls.py | 2 +- backend/plan/views.py | 92 ++++++++++++++++------------- backend/tests/plan/test_schedule.py | 38 ++++++------ 7 files changed, 94 insertions(+), 84 deletions(-) diff --git a/backend/courses/models.py b/backend/courses/models.py index 63537324b..e1536d452 100644 --- a/backend/courses/models.py +++ b/backend/courses/models.py @@ -16,8 +16,6 @@ from review.annotations import review_averages - - User = get_user_model() @@ -1194,13 +1192,16 @@ class Meeting(models.Model): def clean(self): super().clean() - if (self.section is None and self.associated_break is None) or (self.section is not None and self.associated_break is not None): - raise ValidationError("Either the section field of associated_break field must be populated, but not both") - + if (self.section is None and self.associated_break is None) or ( + self.section is not None and self.associated_break is not None + ): + raise ValidationError( + "Either the section field of associated_break field must be populated, but not both" + ) + def save(self, *args, **kwargs): self.clean() super().save(*args, **kwargs) - class Meta: unique_together = (("section", "day", "start", "end", "room"),) diff --git a/backend/courses/util.py b/backend/courses/util.py index 9cbdc9fac..c413d5a52 100644 --- a/backend/courses/util.py +++ b/backend/courses/util.py @@ -493,7 +493,7 @@ def set_meetings(obj, meetings): start_date = extract_date(meeting.get("start_date")) end_date = extract_date(meeting.get("end_date")) for day in list(meeting["days"]): - + meeting = Meeting.objects.update_or_create( section=obj if isinstance(obj, Section) else None, associated_break=obj if isinstance(obj, Break) else None, @@ -507,6 +507,7 @@ def set_meetings(obj, meetings): }, ) + def add_associated_sections(section, linked_sections): semester = section.course.semester section.associated_sections.clear() @@ -535,6 +536,7 @@ def set_crosslistings(course, crosslistings): course.primary_listing = primary_course return + def upsert_course_from_opendata(info, semester, missing_sections=None): dept_code = info.get("subject") or info.get("course_department") assert dept_code, json.dumps(info, indent=2) diff --git a/backend/plan/models.py b/backend/plan/models.py index 73ee3015f..595543993 100644 --- a/backend/plan/models.py +++ b/backend/plan/models.py @@ -92,7 +92,6 @@ def __str__(self): return f"PrimarySchedule(User: {self.user}, Schedule ID: {self.schedule_id})" - class Break(models.Model): """ Holds break objects created by users on PCP. @@ -109,11 +108,13 @@ class Break(models.Model): null=True, help_text=dedent( """ - This represents the location that the user can input themselves. - Will use a building object from a drop down or have it validated or something so it can interact with map. - Didn't want to run into issue of users creating arbitrary Room objects, so just using a char field + This represents the location that the user can input themselves. + Will use a building object from a drop down or have it validated + or something so it can interact with map. + Didn't want to run into issue of users creating arbitrary + Room objects, so just using a char field """ - ) #TODO: Don't know how I want to do buildings yet. + ), # TODO: Don't know how I want to do buildings yet. ) name = models.CharField( @@ -126,7 +127,6 @@ class Break(models.Model): ), ) - meeting_times = models.TextField( blank=True, help_text=dedent( @@ -140,11 +140,9 @@ class Break(models.Model): """ ), ) - + class Meta: unique_together = (("person"),) def __str__(self): return "User: %s, Break ID: %s" % (self.person, self.id) - - diff --git a/backend/plan/serializers.py b/backend/plan/serializers.py index e1a53e156..3c5d23e2c 100644 --- a/backend/plan/serializers.py +++ b/backend/plan/serializers.py @@ -1,8 +1,14 @@ from textwrap import dedent + from rest_framework import serializers -from courses.serializers import PublicUserSerializer, SectionDetailSerializer, MeetingSerializer, MeetingWithBuildingSerializer -from plan.models import PrimarySchedule, Schedule, Break +from courses.serializers import ( + MeetingSerializer, + MeetingWithBuildingSerializer, + PublicUserSerializer, + SectionDetailSerializer, +) +from plan.models import Break, PrimarySchedule, Schedule class ScheduleSerializer(serializers.ModelSerializer): @@ -52,11 +58,9 @@ class BreakSerializer(serializers.ModelSerializer): read_only=False, required=False, help_text="The id of the schedule." ) - class Meta: model = Break exclude = ["person"] - def get_meetings(self, obj): include_location = self.context.get("include_location", False) @@ -66,4 +70,3 @@ def get_meetings(self, obj): meetings_serializer = MeetingSerializer(obj.meetings, many=True) return meetings_serializer.data - diff --git a/backend/plan/urls.py b/backend/plan/urls.py index ec8e878ad..b0e6dbdc8 100644 --- a/backend/plan/urls.py +++ b/backend/plan/urls.py @@ -3,10 +3,10 @@ from rest_framework_nested import routers from plan.views import ( + BreakViewSet, CalendarAPIView, PrimaryScheduleViewSet, ScheduleViewSet, - BreakViewSet, recommend_courses_view, ) diff --git a/backend/plan/views.py b/backend/plan/views.py index 1338c5ece..e3d0b362f 100644 --- a/backend/plan/views.py +++ b/backend/plan/views.py @@ -14,13 +14,18 @@ from rest_framework import status, viewsets from rest_framework.decorators import api_view, permission_classes, schema from rest_framework.exceptions import PermissionDenied -from rest_framework.permissions import IsAuthenticated, AllowAny +from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.views import APIView from courses.models import Course, Meeting, Section from courses.serializers import CourseListSerializer -from courses.util import get_course_and_section, get_current_semester, normalize_semester, set_meetings +from courses.util import ( + get_course_and_section, + get_current_semester, + normalize_semester, + set_meetings, +) from courses.views import get_accepted_friends from PennCourses.docs_settings import PcxAutoSchema from PennCourses.settings.base import PATH_REGISTRATION_SCHEDULE_NAME @@ -31,8 +36,8 @@ vectorize_user, vectorize_user_by_courses, ) -from plan.models import PrimarySchedule, Schedule, Break -from plan.serializers import PrimaryScheduleSerializer, ScheduleSerializer, BreakSerializer +from plan.models import Break, PrimarySchedule, Schedule +from plan.serializers import BreakSerializer, PrimaryScheduleSerializer, ScheduleSerializer @api_view(["POST"]) @@ -617,8 +622,9 @@ def get_queryset(self, semester=None): ) return queryset + class BreakViewSet(AutoPrefetchViewSetMixin, viewsets.ModelViewSet): - + serializer_class = BreakSerializer permission_classes = [IsAuthenticated] @@ -632,24 +638,26 @@ def get_serializer_context(self): def update(self, request): break_id = request.data.get("id") if not break_id: - return Response({"detail": "Break id is required for update."}, - status=status.HTTP_400_BAD_REQUEST) - + return Response( + {"detail": "Break id is required for update."}, status=status.HTTP_400_BAD_REQUEST + ) + try: current_break = self.get_queryset().get(id=break_id) except Break.DoesNotExist: - return Response({"detail": "Break not found."}, - status=status.HTTP_404_NOT_FOUND) + return Response({"detail": "Break not found."}, status=status.HTTP_404_NOT_FOUND) except Exception as e: - return Response({"detail": "Error retrieving break: " + str(e)}, - status=status.HTTP_400_BAD_REQUEST) - + return Response( + {"detail": "Error retrieving break: " + str(e)}, status=status.HTTP_400_BAD_REQUEST + ) + name = request.data.get("name") if not name: - return Response({"detail": "Break name is required."}, - status=status.HTTP_400_BAD_REQUEST) + return Response( + {"detail": "Break name is required."}, status=status.HTTP_400_BAD_REQUEST + ) location_string = request.data.get("location_string") - + current_break.name = name current_break.location_string = location_string @@ -657,29 +665,31 @@ def update(self, request): try: set_meetings(current_break, meetings) except Exception as e: - return Response({"detail": "Error setting meetings: " + str(e)}, - status=status.HTTP_400_BAD_REQUEST) - + return Response( + {"detail": "Error setting meetings: " + str(e)}, status=status.HTTP_400_BAD_REQUEST + ) + try: current_break.save() except Exception as e: - return Response({"detail": "Error saving break: " + str(e)}, - status=status.HTTP_400_BAD_REQUEST) - - return Response({"message": "success", "id": current_break.id}, - status=status.HTTP_200_OK) - + return Response( + {"detail": "Error saving break: " + str(e)}, status=status.HTTP_400_BAD_REQUEST + ) + + return Response({"message": "success", "id": current_break.id}, status=status.HTTP_200_OK) + def create(self, request, *args, **kwargs): break_id = request.data.get("id") if break_id and Break.objects.filter(id=break_id).exists(): return self.update(request) - + name = request.data.get("name") if not name: - return Response({"detail": "Break name is required."}, - status=status.HTTP_400_BAD_REQUEST) + return Response( + {"detail": "Break name is required."}, status=status.HTTP_400_BAD_REQUEST + ) location_string = request.data.get("location_string") - + try: if break_id: new_break = self.get_queryset().create( @@ -696,7 +706,10 @@ def create(self, request, *args, **kwargs): ) except IntegrityError as e: return Response( - {"detail": "IntegrityError encountered while trying to create: " + str(e.__cause__)}, + { + "detail": "IntegrityError encountered while trying to create: " + + str(e.__cause__) + }, status=status.HTTP_400_BAD_REQUEST, ) except Exception as e: @@ -704,27 +717,24 @@ def create(self, request, *args, **kwargs): {"detail": "Error creating break: " + str(e)}, status=status.HTTP_400_BAD_REQUEST, ) - + meetings = request.data.get("meetings") try: set_meetings(new_break, meetings) except Exception as e: - return Response({"detail": "Error setting meetings: " + str(e)}, - status=status.HTTP_400_BAD_REQUEST) - - return Response({"message": "success", "id": new_break.id}, - status=status.HTTP_201_CREATED) + return Response( + {"detail": "Error setting meetings: " + str(e)}, status=status.HTTP_400_BAD_REQUEST + ) + + return Response({"message": "success", "id": new_break.id}, status=status.HTTP_201_CREATED) def get_queryset(self): return Break.objects.filter(person=self.request.user).prefetch_related( - "meetings", # Prefetch the related meetings - "meetings__room", # Prefetch the related rooms for each meeting + "meetings", # Prefetch the related meetings + "meetings__room", # Prefetch the related rooms for each meeting ) - - - class CalendarAPIView(APIView): schema = PcxAutoSchema( custom_path_parameter_desc={ diff --git a/backend/tests/plan/test_schedule.py b/backend/tests/plan/test_schedule.py index 6854a5a6d..145808019 100644 --- a/backend/tests/plan/test_schedule.py +++ b/backend/tests/plan/test_schedule.py @@ -4,22 +4,17 @@ from django.contrib.auth import get_user_model from django.db.models.signals import post_save from django.test import TestCase +from django.urls import reverse from options.models import Option +from rest_framework import status from rest_framework.test import APIClient from alert.models import AddDropPeriod from courses.util import get_average_reviews, invalidate_current_semester_cache from PennCourses.settings.base import PATH_REGISTRATION_SCHEDULE_NAME -from plan.models import Schedule, Break - -from courses.models import Meeting - +from plan.models import Break, Schedule from tests.courses.util import create_mock_data_with_reviews -from django.urls import reverse -from django.db import IntegrityError -from rest_framework import status - User = get_user_model() @@ -1242,15 +1237,15 @@ def setUp(self): # Create an initial Break instance to test update operations self.break_obj = Break.objects.create( - person=self.user, - name="Morning Break", - location_string="Cafeteria" + person=self.user, name="Morning Break", location_string="Cafeteria" ) self.break_obj.save() # Use Django's reverse to dynamically get the correct endpoint self.break_list_url = reverse("breaks-list") # /api/plan/breaks/ - self.break_detail_url = reverse("breaks-detail", kwargs={"pk": self.break_obj.id}) # /api/plan/breaks/{id}/ + self.break_detail_url = reverse( + "breaks-detail", kwargs={"pk": self.break_obj.id} + ) # /api/plan/breaks/{id}/ @patch("accounts.authentication.requests.post") def test_create_break(self, mock_set_meetings): @@ -1261,7 +1256,7 @@ def test_create_break(self, mock_set_meetings): data = { "name": "Afternoon Break", "location_string": "Lobby", - "meetings": [] # No meetings provided + "meetings": [], # No meetings provided } response = self.client.post(self.break_list_url, data, format="json") self.assertEqual(response.status_code, status.HTTP_201_CREATED) @@ -1280,21 +1275,22 @@ def test_create_break(self, mock_set_meetings): @patch("plan.views.set_meetings") def test_update_break_existing(self, mock_set_meetings): """ - If a break with the provided "id" exists, sending a POST request with that id should update it. + If a break with the provided "id" exists, sending a POST + request with that id should update it. """ data = { "id": self.break_obj.id, "name": "Updated Break", "location_string": "New Location", "meetings": [ - { + { "days": "MT", "begin_time_24": 900, "begin_time": "9:00 AM", "end_time_24": 1000, "end_time": "10:00 AM", }, - ] + ], } response = self.client.post(self.break_list_url, data, format="json") self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -1315,14 +1311,14 @@ def test_update_break_nonexistent(self): "id": 9999, # An id that does not exist for this user "name": "Nonexistent Break", "location_string": "Nowhere", - "meetings": [] + "meetings": [], } response = self.client.post(self.break_list_url, data, format="json") self.assertEqual(response.status_code, status.HTTP_201_CREATED) self.assertEqual(response.data.get("message"), "success") - - new_break = Break.objects.get(id = response.data.get("id")) - + + new_break = Break.objects.get(id=response.data.get("id")) + self.assertEqual(new_break.name, "Nonexistent Break") self.assertEqual(new_break.location_string, "Nowhere") @@ -1353,4 +1349,4 @@ def test_delete_break(self): """ response = self.client.delete(self.break_detail_url, format="json") self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) - self.assertFalse(Break.objects.filter(id=self.break_obj.id).exists()) \ No newline at end of file + self.assertFalse(Break.objects.filter(id=self.break_obj.id).exists()) From cb2cb9ce816918b797f58d9e140916ce74bd8b0a Mon Sep 17 00:00:00 2001 From: Nikhil-Penn Date: Sun, 16 Feb 2025 17:39:34 +0000 Subject: [PATCH 03/10] db migrations --- ..._associated_break_alter_meeting_section.py | 37 ++++++++++++ backend/plan/migrations/0010_break.py | 60 +++++++++++++++++++ ...11_alter_break_unique_together_and_more.py | 28 +++++++++ 3 files changed, 125 insertions(+) create mode 100644 backend/courses/migrations/0066_meeting_associated_break_alter_meeting_section.py create mode 100644 backend/plan/migrations/0010_break.py create mode 100644 backend/plan/migrations/0011_alter_break_unique_together_and_more.py diff --git a/backend/courses/migrations/0066_meeting_associated_break_alter_meeting_section.py b/backend/courses/migrations/0066_meeting_associated_break_alter_meeting_section.py new file mode 100644 index 000000000..b9f030514 --- /dev/null +++ b/backend/courses/migrations/0066_meeting_associated_break_alter_meeting_section.py @@ -0,0 +1,37 @@ +# Generated by Django 5.0.2 on 2025-01-26 19:34 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("courses", "0065_topic_historical_probabilities_fall_and_more"), + ("plan", "0010_break"), + ] + + operations = [ + migrations.AddField( + model_name="meeting", + name="associated_break", + field=models.ForeignKey( + help_text="The Section object to which this class meeting belongs.", + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="meetings", + to="plan.break", + ), + ), + migrations.AlterField( + model_name="meeting", + name="section", + field=models.ForeignKey( + help_text="The Section object to which this class meeting belongs.", + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="meetings", + to="courses.section", + ), + ), + ] diff --git a/backend/plan/migrations/0010_break.py b/backend/plan/migrations/0010_break.py new file mode 100644 index 000000000..03659ca99 --- /dev/null +++ b/backend/plan/migrations/0010_break.py @@ -0,0 +1,60 @@ +# Generated by Django 5.0.2 on 2025-01-26 19:34 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("plan", "0009_alter_schedule_sections"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name="Break", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, primary_key=True, serialize=False, verbose_name="ID" + ), + ), + ( + "location_string", + models.CharField( + help_text="\nThis represents the location that the user can input themselves. \nWill use a building object from a drop down or have it validated or something so it can interact with map.\nDidn't want to run into issue of users creating arbitrary Room objects, so just using a char field\n", + max_length=80, + null=True, + ), + ), + ( + "name", + models.CharField( + help_text="\nThe user's name for the break. No two breaks can match in all of the fields\n`[name, person]`\n", + max_length=255, + ), + ), + ( + "meeting_times", + models.TextField( + blank=True, + help_text='\nA JSON-stringified list of meeting times of the form\n`{days code} {start time} - {end time}`, e.g.\n`["MWF 09:00 AM - 10:00 AM","F 11:00 AM - 12:00 PM","T 05:00 PM - 06:00 PM"]` for\nPHYS-151-001 (2020A). Each letter of the days code is of the form M, T, W, R, F for each\nday of the work week, respectively (and multiple days are combined with concatenation).\nTo access the Meeting objects for this section, the related field `meetings` can be used.\n', + ), + ), + ( + "person", + models.ForeignKey( + help_text="The person (user) who created this break.", + on_delete=django.db.models.deletion.CASCADE, + to=settings.AUTH_USER_MODEL, + ), + ), + ], + options={ + "unique_together": {("name", "person")}, + }, + ), + ] diff --git a/backend/plan/migrations/0011_alter_break_unique_together_and_more.py b/backend/plan/migrations/0011_alter_break_unique_together_and_more.py new file mode 100644 index 000000000..d953287e0 --- /dev/null +++ b/backend/plan/migrations/0011_alter_break_unique_together_and_more.py @@ -0,0 +1,28 @@ +# Generated by Django 5.0.2 on 2025-02-16 17:38 + +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("plan", "0010_break"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.AlterUniqueTogether( + name="break", + unique_together={("person",)}, + ), + migrations.AlterField( + model_name="break", + name="location_string", + field=models.CharField( + help_text="\nThis represents the location that the user can input themselves.\nWill use a building object from a drop down or have it validated\nor something so it can interact with map.\nDidn't want to run into issue of users creating arbitrary\nRoom objects, so just using a char field\n", + max_length=80, + null=True, + ), + ), + ] From e7061ac2cb05a7a9ae624feacd55113938a3e6d6 Mon Sep 17 00:00:00 2001 From: Nikhil-Penn Date: Sun, 16 Feb 2025 18:23:45 +0000 Subject: [PATCH 04/10] Ok, now breaks should work. --- .../0012_alter_break_unique_together.py | 19 ++++++++++++++ .../0013_alter_break_unique_together.py | 17 +++++++++++++ ...k_unique_break_meeting_times_per_person.py | 25 +++++++++++++++++++ backend/plan/models.py | 9 ++++++- backend/tests/plan/test_schedule.py | 4 +-- 5 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 backend/plan/migrations/0012_alter_break_unique_together.py create mode 100644 backend/plan/migrations/0013_alter_break_unique_together.py create mode 100644 backend/plan/migrations/0014_break_unique_break_meeting_times_per_person.py diff --git a/backend/plan/migrations/0012_alter_break_unique_together.py b/backend/plan/migrations/0012_alter_break_unique_together.py new file mode 100644 index 000000000..5b36721a7 --- /dev/null +++ b/backend/plan/migrations/0012_alter_break_unique_together.py @@ -0,0 +1,19 @@ +# Generated by Django 5.0.2 on 2025-02-16 18:01 + +from django.conf import settings +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("plan", "0011_alter_break_unique_together_and_more"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.AlterUniqueTogether( + name="break", + unique_together={("person", "meeting_times")}, + ), + ] diff --git a/backend/plan/migrations/0013_alter_break_unique_together.py b/backend/plan/migrations/0013_alter_break_unique_together.py new file mode 100644 index 000000000..4c98a8c6c --- /dev/null +++ b/backend/plan/migrations/0013_alter_break_unique_together.py @@ -0,0 +1,17 @@ +# Generated by Django 5.0.2 on 2025-02-16 18:05 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("plan", "0012_alter_break_unique_together"), + ] + + operations = [ + migrations.AlterUniqueTogether( + name="break", + unique_together=set(), + ), + ] diff --git a/backend/plan/migrations/0014_break_unique_break_meeting_times_per_person.py b/backend/plan/migrations/0014_break_unique_break_meeting_times_per_person.py new file mode 100644 index 000000000..1bd74eb9a --- /dev/null +++ b/backend/plan/migrations/0014_break_unique_break_meeting_times_per_person.py @@ -0,0 +1,25 @@ +# Generated by Django 5.0.2 on 2025-02-16 18:10 + +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("plan", "0013_alter_break_unique_together"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.AddConstraint( + model_name="break", + constraint=models.UniqueConstraint( + condition=models.Q( + ("meeting_times__isnull", False), models.Q(("meeting_times", ""), _negated=True) + ), + fields=("person", "meeting_times"), + name="unique_break_meeting_times_per_person", + ), + ), + ] diff --git a/backend/plan/models.py b/backend/plan/models.py index 595543993..6f5fdc162 100644 --- a/backend/plan/models.py +++ b/backend/plan/models.py @@ -2,6 +2,7 @@ from django.contrib.auth import get_user_model from django.db import models +from django.db.models import Q, UniqueConstraint from courses.models import Section @@ -142,7 +143,13 @@ class Break(models.Model): ) class Meta: - unique_together = (("person"),) + constraints = [ + UniqueConstraint( + fields=["person", "meeting_times"], + condition=Q(meeting_times__isnull=False) & ~Q(meeting_times=""), + name="unique_break_meeting_times_per_person", + ) + ] def __str__(self): return "User: %s, Break ID: %s" % (self.person, self.id) diff --git a/backend/tests/plan/test_schedule.py b/backend/tests/plan/test_schedule.py index 145808019..6338d8154 100644 --- a/backend/tests/plan/test_schedule.py +++ b/backend/tests/plan/test_schedule.py @@ -1247,13 +1247,14 @@ def setUp(self): "breaks-detail", kwargs={"pk": self.break_obj.id} ) # /api/plan/breaks/{id}/ - @patch("accounts.authentication.requests.post") + @patch("plan.views.set_meetings") def test_create_break(self, mock_set_meetings): """ Ensure that posting a new break (without an "id") creates the break and returns a 201 response. """ data = { + "id": 2, "name": "Afternoon Break", "location_string": "Lobby", "meetings": [], # No meetings provided @@ -1330,7 +1331,6 @@ def test_list_breaks(self): response = self.client.get(self.break_list_url, format="json") self.assertEqual(response.status_code, status.HTTP_200_OK) # Check that our initial break ("Morning Break") is in the returned list. - print(response.data) break_names = [item.get("name") for item in response.data] self.assertIn("Morning Break", break_names) From cb5c06b87d8ec00fbf82f7591c5678d1e2d84261 Mon Sep 17 00:00:00 2001 From: Nikhil-Penn Date: Sun, 16 Feb 2025 23:35:39 +0000 Subject: [PATCH 05/10] Constraints and listening to daniels comments --- ...alter_meeting_associated_break_and_more.py | 46 +++++++++++++++++++ backend/courses/models.py | 26 +++++++---- 2 files changed, 62 insertions(+), 10 deletions(-) create mode 100644 backend/courses/migrations/0067_alter_meeting_associated_break_and_more.py diff --git a/backend/courses/migrations/0067_alter_meeting_associated_break_and_more.py b/backend/courses/migrations/0067_alter_meeting_associated_break_and_more.py new file mode 100644 index 000000000..cd8f70e67 --- /dev/null +++ b/backend/courses/migrations/0067_alter_meeting_associated_break_and_more.py @@ -0,0 +1,46 @@ +# Generated by Django 5.0.2 on 2025-02-16 23:31 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("courses", "0066_meeting_associated_break_alter_meeting_section"), + ("plan", "0014_break_unique_break_meeting_times_per_person"), + ] + + operations = [ + migrations.AlterField( + model_name="meeting", + name="associated_break", + field=models.ForeignKey( + help_text="The Break object to which this meeting object belongs.", + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="meetings", + to="plan.break", + ), + ), + migrations.AddConstraint( + model_name="meeting", + constraint=models.UniqueConstraint( + condition=models.Q( + ("section__isnull", True), ("associated_break__isnull", True), _connector="OR" + ), + fields=("section", "associated_break"), + name="unique_meeting_either_section_or_break", + ), + ), + migrations.AddConstraint( + model_name="meeting", + constraint=models.UniqueConstraint( + condition=models.Q( + ("section__isnull", True), ("associated_break__isnull", True), _negated=True + ), + fields=("section", "associated_break"), + name="meeting_must_have_section_or_break", + ), + ), + ] diff --git a/backend/courses/models.py b/backend/courses/models.py index e1536d452..8361e4ea5 100644 --- a/backend/courses/models.py +++ b/backend/courses/models.py @@ -7,7 +7,7 @@ from django.core.exceptions import ValidationError from django.core.validators import MaxValueValidator, MinValueValidator from django.db import models, transaction -from django.db.models import OuterRef, Q, Subquery +from django.db.models import OuterRef, Q, Subquery, UniqueConstraint from django.db.models.signals import post_save from django.dispatch import receiver from django.utils import timezone @@ -1137,7 +1137,7 @@ class Meeting(models.Model): null=True, on_delete=models.CASCADE, related_name="meetings", - help_text="The Section object to which this class meeting belongs.", + help_text="The Break object to which this meeting object belongs.", ) day = models.CharField( @@ -1190,14 +1190,6 @@ class Meeting(models.Model): ), ) - def clean(self): - super().clean() - if (self.section is None and self.associated_break is None) or ( - self.section is not None and self.associated_break is not None - ): - raise ValidationError( - "Either the section field of associated_break field must be populated, but not both" - ) def save(self, *args, **kwargs): self.clean() @@ -1205,6 +1197,20 @@ def save(self, *args, **kwargs): class Meta: unique_together = (("section", "day", "start", "end", "room"),) + constraints = [ + # Ensure that a meeting has either a section or an associated_break, but not both + UniqueConstraint( + fields=["section", "associated_break"], + condition=Q(section__isnull=True) | Q(associated_break__isnull=True), + name="unique_meeting_either_section_or_break", + ), + # Ensure that a meeting must have at least one (not both null) + UniqueConstraint( + fields=["section", "associated_break"], + condition=~(Q(section__isnull=True) & Q(associated_break__isnull=True)), + name="meeting_must_have_section_or_break", + ), + ] @staticmethod def int_to_time(time): From bd4ecddbfd0ce842a67ba08fee176213d068a660 Mon Sep 17 00:00:00 2001 From: Nikhil-Penn Date: Sun, 16 Feb 2025 23:45:25 +0000 Subject: [PATCH 06/10] lint --- backend/courses/models.py | 1 - 1 file changed, 1 deletion(-) diff --git a/backend/courses/models.py b/backend/courses/models.py index 8361e4ea5..2e4245ab1 100644 --- a/backend/courses/models.py +++ b/backend/courses/models.py @@ -1190,7 +1190,6 @@ class Meeting(models.Model): ), ) - def save(self, *args, **kwargs): self.clean() super().save(*args, **kwargs) From 25d54aaf907bba6064b6f1cae66dfb4f80c2f487 Mon Sep 17 00:00:00 2001 From: Nikhil-Penn Date: Tue, 25 Feb 2025 23:42:46 +0000 Subject: [PATCH 07/10] removed eval --- backend/plan/views.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/backend/plan/views.py b/backend/plan/views.py index e3d0b362f..8a62fd84d 100644 --- a/backend/plan/views.py +++ b/backend/plan/views.py @@ -630,9 +630,8 @@ class BreakViewSet(AutoPrefetchViewSetMixin, viewsets.ModelViewSet): def get_serializer_context(self): context = super().get_serializer_context() - include_location_str = "False" # TODO: figure out how we want to do locations. - context.update({"include_location": eval(include_location_str)}) + context.update({"include_location": False}) return context def update(self, request): From 03d0122968ef01115e684766dfebbd121869946f Mon Sep 17 00:00:00 2001 From: Nikhil-Penn Date: Tue, 25 Feb 2025 23:43:34 +0000 Subject: [PATCH 08/10] commented out eval --- backend/plan/views.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/plan/views.py b/backend/plan/views.py index 8a62fd84d..f18ed4c3c 100644 --- a/backend/plan/views.py +++ b/backend/plan/views.py @@ -630,8 +630,9 @@ class BreakViewSet(AutoPrefetchViewSetMixin, viewsets.ModelViewSet): def get_serializer_context(self): context = super().get_serializer_context() + #include_location_str = "False" # TODO: figure out how we want to do locations. - context.update({"include_location": False}) + #context.update({"include_location": eval(include_location_str)}) return context def update(self, request): From e55fb6ab60ce4966cc963c7a25689d97f87a61c5 Mon Sep 17 00:00:00 2001 From: Nikhil-Penn Date: Tue, 25 Feb 2025 23:57:09 +0000 Subject: [PATCH 09/10] lint --- backend/plan/views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/plan/views.py b/backend/plan/views.py index f18ed4c3c..b9a28aa00 100644 --- a/backend/plan/views.py +++ b/backend/plan/views.py @@ -630,9 +630,9 @@ class BreakViewSet(AutoPrefetchViewSetMixin, viewsets.ModelViewSet): def get_serializer_context(self): context = super().get_serializer_context() - #include_location_str = "False" + # include_location_str = "False" # TODO: figure out how we want to do locations. - #context.update({"include_location": eval(include_location_str)}) + # context.update({"include_location": eval(include_location_str)}) return context def update(self, request): From ea39663500fd9067fb9bf7e4dce64b6ba17cb0e5 Mon Sep 17 00:00:00 2001 From: Daniel Zhao Date: Mon, 10 Mar 2025 22:55:24 -0500 Subject: [PATCH 10/10] Added frontend support for breaks --- .../plan/migrations/0015_schedule_breaks.py | 22 +++ backend/plan/models.py | 135 ++++++++++-------- backend/plan/serializers.py | 61 ++++---- backend/plan/views.py | 41 +++++- frontend/plan/actions/index.js | 3 +- frontend/plan/components/schedule/Block.tsx | 1 - .../plan/components/schedule/Schedule.tsx | 2 +- .../components/schedule/ScheduleDisplay.tsx | 35 ++++- frontend/plan/reducers/schedule.js | 47 ++++-- 9 files changed, 235 insertions(+), 112 deletions(-) create mode 100644 backend/plan/migrations/0015_schedule_breaks.py diff --git a/backend/plan/migrations/0015_schedule_breaks.py b/backend/plan/migrations/0015_schedule_breaks.py new file mode 100644 index 000000000..1f0632b7d --- /dev/null +++ b/backend/plan/migrations/0015_schedule_breaks.py @@ -0,0 +1,22 @@ +# Generated by Django 5.0.2 on 2025-03-10 18:54 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("plan", "0014_break_unique_break_meeting_times_per_person"), + ] + + operations = [ + migrations.AddField( + model_name="schedule", + name="breaks", + field=models.ManyToManyField( + blank=True, + help_text="\nThe breaks which comprise the schedule. The semester of each of these breaks is assumed to\nmatch the semester defined by the semester field below.\n", + to="plan.break", + ), + ), + ] diff --git a/backend/plan/models.py b/backend/plan/models.py index 6f5fdc162..7ff3e274c 100644 --- a/backend/plan/models.py +++ b/backend/plan/models.py @@ -7,6 +7,68 @@ from courses.models import Section +class Break(models.Model): + """ + Holds break objects created by users on PCP. + """ + + person = models.ForeignKey( + get_user_model(), + on_delete=models.CASCADE, + help_text="The person (user) who created this break.", + ) + + location_string = models.CharField( + max_length=80, + null=True, + help_text=dedent( + """ + This represents the location that the user can input themselves. + Will use a building object from a drop down or have it validated + or something so it can interact with map. + Didn't want to run into issue of users creating arbitrary + Room objects, so just using a char field + """ + ), # TODO: Don't know how I want to do buildings yet. + ) + + name = models.CharField( + max_length=255, + help_text=dedent( + """ + The user's name for the break. No two breaks can match in all of the fields + `[name, person]` + """ + ), + ) + + meeting_times = models.TextField( + blank=True, + help_text=dedent( + """ + A JSON-stringified list of meeting times of the form + `{days code} {start time} - {end time}`, e.g. + `["MWF 09:00 AM - 10:00 AM","F 11:00 AM - 12:00 PM","T 05:00 PM - 06:00 PM"]` for + PHYS-151-001 (2020A). Each letter of the days code is of the form M, T, W, R, F for each + day of the work week, respectively (and multiple days are combined with concatenation). + To access the Meeting objects for this section, the related field `meetings` can be used. + """ + ), + ) + + class Meta: + constraints = [ + UniqueConstraint( + fields=["person", "meeting_times"], + condition=Q(meeting_times__isnull=False) & ~Q(meeting_times=""), + name="unique_break_meeting_times_per_person", + ) + ] + + def __str__(self): + return "User: %s, Break ID: %s" % (self.person, self.id) + + class Schedule(models.Model): """ Used to save schedules created by users on PCP @@ -29,6 +91,17 @@ class Schedule(models.Model): ), ) + breaks = models.ManyToManyField( + Break, + blank=True, + help_text=dedent( + """ + The breaks which comprise the schedule. The semester of each of these breaks is assumed to + match the semester defined by the semester field below. + """ + ), + ) + semester = models.CharField( max_length=5, help_text=dedent( @@ -91,65 +164,3 @@ class Meta: def __str__(self): return f"PrimarySchedule(User: {self.user}, Schedule ID: {self.schedule_id})" - - -class Break(models.Model): - """ - Holds break objects created by users on PCP. - """ - - person = models.ForeignKey( - get_user_model(), - on_delete=models.CASCADE, - help_text="The person (user) who created this break.", - ) - - location_string = models.CharField( - max_length=80, - null=True, - help_text=dedent( - """ - This represents the location that the user can input themselves. - Will use a building object from a drop down or have it validated - or something so it can interact with map. - Didn't want to run into issue of users creating arbitrary - Room objects, so just using a char field - """ - ), # TODO: Don't know how I want to do buildings yet. - ) - - name = models.CharField( - max_length=255, - help_text=dedent( - """ - The user's name for the break. No two breaks can match in all of the fields - `[name, person]` - """ - ), - ) - - meeting_times = models.TextField( - blank=True, - help_text=dedent( - """ - A JSON-stringified list of meeting times of the form - `{days code} {start time} - {end time}`, e.g. - `["MWF 09:00 AM - 10:00 AM","F 11:00 AM - 12:00 PM","T 05:00 PM - 06:00 PM"]` for - PHYS-151-001 (2020A). Each letter of the days code is of the form M, T, W, R, F for each - day of the work week, respectively (and multiple days are combined with concatenation). - To access the Meeting objects for this section, the related field `meetings` can be used. - """ - ), - ) - - class Meta: - constraints = [ - UniqueConstraint( - fields=["person", "meeting_times"], - condition=Q(meeting_times__isnull=False) & ~Q(meeting_times=""), - name="unique_break_meeting_times_per_person", - ) - ] - - def __str__(self): - return "User: %s, Break ID: %s" % (self.person, self.id) diff --git a/backend/plan/serializers.py b/backend/plan/serializers.py index 3c5d23e2c..ff696a7c9 100644 --- a/backend/plan/serializers.py +++ b/backend/plan/serializers.py @@ -11,10 +11,42 @@ from plan.models import Break, PrimarySchedule, Schedule +class BreakSerializer(serializers.ModelSerializer): + + meetings = serializers.SerializerMethodField( + read_only=True, + help_text=dedent( + """ + A list of the meetings of this section (each meeting is a continuous span of time + during which a section would meet). + """ + ), + ) + id = serializers.IntegerField( + read_only=False, required=False, help_text="The id of the schedule." + ) + + class Meta: + model = Break + exclude = ["person"] + + def get_meetings(self, obj): + include_location = self.context.get("include_location", False) + if include_location: + meetings_serializer = MeetingWithBuildingSerializer(obj.meetings, many=True) + else: + meetings_serializer = MeetingSerializer(obj.meetings, many=True) + + return meetings_serializer.data + + class ScheduleSerializer(serializers.ModelSerializer): sections = SectionDetailSerializer( many=True, read_only=False, help_text="The sections in the schedule.", required=True ) + breaks = BreakSerializer( + many=True, read_only=False, help_text="The breaks in the schedule.", required=False + ) id = serializers.IntegerField( read_only=False, required=False, help_text="The id of the schedule." ) @@ -41,32 +73,3 @@ class PrimaryScheduleSerializer(serializers.ModelSerializer): class Meta: model = PrimarySchedule fields = ["user", "schedule"] - - -class BreakSerializer(serializers.ModelSerializer): - - meetings = serializers.SerializerMethodField( - read_only=True, - help_text=dedent( - """ - A list of the meetings of this section (each meeting is a continuous span of time - during which a section would meet). - """ - ), - ) - id = serializers.IntegerField( - read_only=False, required=False, help_text="The id of the schedule." - ) - - class Meta: - model = Break - exclude = ["person"] - - def get_meetings(self, obj): - include_location = self.context.get("include_location", False) - if include_location: - meetings_serializer = MeetingWithBuildingSerializer(obj.meetings, many=True) - else: - meetings_serializer = MeetingSerializer(obj.meetings, many=True) - - return meetings_serializer.data diff --git a/backend/plan/views.py b/backend/plan/views.py index b9a28aa00..94b7df21f 100644 --- a/backend/plan/views.py +++ b/backend/plan/views.py @@ -492,6 +492,18 @@ def get_sections(data, semester, skip_missing=False): raise e return sections + @staticmethod + def get_breaks(data): + raw_breaks = data.get("breaks", []) + breaks = [] + for b in raw_breaks: + break_id = b.get("id") + if break_id: + break_candidate = Break.objects.filter(id=break_id).first() + if break_candidate: + breaks.append(break_candidate) + return breaks + def validate_name(self, request, existing_schedule=None, allow_path=False): name = request.data.get("name") if PATH_REGISTRATION_SCHEDULE_NAME in [ @@ -544,12 +556,21 @@ def update(self, request, pk=None): status=status.HTTP_400_BAD_REQUEST, ) + try: + breaks = self.get_breaks(request.data) + except ObjectDoesNotExist: + return Response( + {"detail": "One or more breaks not found in database."}, + status=status.HTTP_400_BAD_REQUEST, + ) + try: schedule.person = request.user schedule.semester = semester schedule.name = PATH_REGISTRATION_SCHEDULE_NAME if from_path else name schedule.save() schedule.sections.set(sections) + schedule.breaks.set(breaks) return Response({"message": "success", "id": schedule.id}, status=status.HTTP_200_OK) except IntegrityError as e: return Response( @@ -663,7 +684,15 @@ def update(self, request): meetings = request.data.get("meetings") try: - set_meetings(current_break, meetings) + meetings_with_codes = [ + { + **m, + "building_code": None, + "room_code": None, + } + for m in meetings + ] + set_meetings(current_break, meetings_with_codes) except Exception as e: return Response( {"detail": "Error setting meetings: " + str(e)}, status=status.HTTP_400_BAD_REQUEST @@ -719,8 +748,16 @@ def create(self, request, *args, **kwargs): ) meetings = request.data.get("meetings") + meetings_with_codes = [ + { + **m, + "building_code": None, + "room_code": None, + } + for m in meetings + ] try: - set_meetings(new_break, meetings) + set_meetings(new_break, meetings_with_codes) except Exception as e: return Response( {"detail": "Error setting meetings: " + str(e)}, status=status.HTTP_400_BAD_REQUEST diff --git a/frontend/plan/actions/index.js b/frontend/plan/actions/index.js index b3b7ecf3f..69a9b3368 100644 --- a/frontend/plan/actions/index.js +++ b/frontend/plan/actions/index.js @@ -110,8 +110,9 @@ export const addCartItem = (section) => ({ section, }); -export const removeSchedItem = (id) => ({ +export const removeSchedItem = (id, type) => ({ type: REMOVE_SCHED_ITEM, + itemType: type, id, }); diff --git a/frontend/plan/components/schedule/Block.tsx b/frontend/plan/components/schedule/Block.tsx index 8f423c3d4..be8e7eb55 100644 --- a/frontend/plan/components/schedule/Block.tsx +++ b/frontend/plan/components/schedule/Block.tsx @@ -137,7 +137,6 @@ export default function Block(props: BlockProps) { /> )} {false && !coreqFulfilled && } - {id.replace(/-/g, " ")} diff --git a/frontend/plan/components/schedule/Schedule.tsx b/frontend/plan/components/schedule/Schedule.tsx index 1e5334366..a1e7114f9 100644 --- a/frontend/plan/components/schedule/Schedule.tsx +++ b/frontend/plan/components/schedule/Schedule.tsx @@ -140,7 +140,7 @@ const mapStateToProps = (state: any) => { }; const mapDispatchToProps = (dispatch: ThunkDispatch) => ({ - removeSection: (idDashed: string) => dispatch(removeSchedItem(idDashed)), + removeSection: (idDashed: string, type: string) => dispatch(removeSchedItem(idDashed, type)), focusSection: (id: string) => dispatch(fetchCourseDetails(id)), changeMySchedule: (scheduleName: string) => dispatch(changeMySchedule(scheduleName)), diff --git a/frontend/plan/components/schedule/ScheduleDisplay.tsx b/frontend/plan/components/schedule/ScheduleDisplay.tsx index baed5fdf5..fd71f28fa 100644 --- a/frontend/plan/components/schedule/ScheduleDisplay.tsx +++ b/frontend/plan/components/schedule/ScheduleDisplay.tsx @@ -137,6 +137,10 @@ const ScheduleDisplay = ({ schedData.sections || []; + let breaks; + + breaks = friendshipState.activeFriendSchedule?.breaks || schedData.breaks || []; + const notEmpty = sections.length > 0; let startHour = 10.5; @@ -168,6 +172,7 @@ const ScheduleDisplay = ({ day: m.day as Day, start: transformTime(m.start), end: transformTime(m.end), + type: "section", course: { color: s.color, id: s.id, @@ -185,6 +190,30 @@ const ScheduleDisplay = ({ ); } }); + + breaks.forEach((b) => { + if (b.meetings) { + meetings.push( + ...b.meetings.map((m) => ({ + day: m.day as Day, + start: transformTime(m.start), + end: transformTime(m.end), + type: "break", + course: { + color: b.color, + id: b.name, + coreqFulfilled: true + }, + style: { + width: "100%", + left: "0", + }, + })) + ); + } + }); + + startHour = Math.floor( Math.min(startHour, ...meetings.map((m) => m.start)) @@ -239,15 +268,15 @@ const ScheduleDisplay = ({ col: colOffset, }} readOnly={readOnly} - remove={() => removeSection(meeting.course.id)} + remove={() => removeSection(meeting.course.id, meeting.type)} key={`${meeting.course.id}-${meeting.day}`} - focusSection={() => { + focusSection={meeting.type == "section" ? () => { if (isMobileOnly && setTab) { setTab(0); } const split = meeting.course.id.split("-"); focusSection(`${split[0]}-${split[1]}`); - }} + } : () => {}} /> ))} {!notEmpty && !readOnly && } diff --git a/frontend/plan/reducers/schedule.js b/frontend/plan/reducers/schedule.js index 1b80d6390..df8cbb760 100644 --- a/frontend/plan/reducers/schedule.js +++ b/frontend/plan/reducers/schedule.js @@ -185,6 +185,10 @@ const handleUpdateSchedulesOnFrontend = (state, schedulesFromBackend) => { color: getColor(section.id), }) ), + breaks: scheduleFromBackend.breaks.map((br) => ({ + ...br, + color: getColor(br.id), + })), id: scheduleFromBackend.id, pushedToBackend: true, updated_at: Date.now(), @@ -420,20 +424,37 @@ export const schedule = (state = initialState, action) => { case REMOVE_SCHED_ITEM: if (!state.readOnly) { - return { - ...state, - schedules: { - ...state.schedules, - [state.scheduleSelected]: { - ...state.schedules[state.scheduleSelected], - updated_at: Date.now(), - pushedToBackend: false, - sections: state.schedules[ - state.scheduleSelected - ].sections.filter((m) => m.id !== action.id), + if (action.itemType === "section") { + return { + ...state, + schedules: { + ...state.schedules, + [state.scheduleSelected]: { + ...state.schedules[state.scheduleSelected], + updated_at: Date.now(), + pushedToBackend: false, + sections: state.schedules[ + state.scheduleSelected + ].sections.filter((m) => m.id !== action.id), + }, }, - }, - }; + }; + } else { + return { + ...state, + schedules: { + ...state.schedules, + [state.scheduleSelected]: { + ...state.schedules[state.scheduleSelected], + updated_at: Date.now(), + pushedToBackend: false, + breaks: state.schedules[ + state.scheduleSelected + ].breaks.filter((br) => br.name !== action.id), + }, + }, + }; + } } showToast("Cannot remove courses from a friend's schedule!", true);