Merge branch 'master' into pcp-schedule-sharing-frontend

.gitignore

@@ -20,3 +20,7 @@ postgres
 *-test.json
 celerybeat-schedule
 pcr-backup*
+./Pipfile
+./Pipfile.lock
+./package.json
+./yarn.lock

backend/Dockerfile.dev

@@ -5,7 +5,7 @@ LABEL maintainer="Penn Labs"
 WORKDIR /backend
 
 # Copy project dependencies
-COPY Pipfile* .
+COPY Pipfile* ./
 
 # Install backend dependencies
 RUN pipenv install --dev

backend/PennCourses/settings/base.py

@@ -182,7 +182,7 @@
 # Twilio Credentials
 TWILIO_SID = os.environ.get("TWILIO_SID", "")
 TWILIO_AUTH_TOKEN = os.environ.get("TWILIO_TOKEN", "")
-TWILIO_NUMBER = os.environ.get("TWILIO_NUMBER", "+12153984277")
+TWILIO_NUMBER = os.environ.get("TWILIO_NUMBER", "+12157826689")
 
 # Redis
 REDIS_URL = os.environ.get("REDIS_URL", "redis://localhost:6379/1")

backend/alert/views.py

@@ -29,7 +29,7 @@
     translate_semester_inv,
     update_course_from_record,
 )
-from PennCourses.docs_settings import PcxAutoSchema, reverse_func
+from PennCourses.docs_settings import PcxAutoSchema
 
 
 logger = logging.getLogger(__name__)
@@ -249,7 +249,7 @@ class RegistrationViewSet(AutoPrefetchViewSetMixin, viewsets.ModelViewSet):
 
     schema = PcxAutoSchema(
         response_codes={
-            reverse_func("registrations-list"): {
+            "registrations-list": {
                 "POST": {
                     201: "[DESCRIBE_RESPONSE_SCHEMA]Registration successfully created.",
                     400: "Bad request (e.g. given null section).",
@@ -260,7 +260,7 @@ class RegistrationViewSet(AutoPrefetchViewSetMixin, viewsets.ModelViewSet):
                 },
                 "GET": {200: "[DESCRIBE_RESPONSE_SCHEMA]Registrations successfully listed."},
             },
-            reverse_func("registrations-detail", args=["id"]): {
+            "registrations-detail": {
                 "PUT": {
                     200: "Registration successfully updated (or no changes necessary).",
                     400: "Bad request (see route description).",
@@ -274,7 +274,7 @@ class RegistrationViewSet(AutoPrefetchViewSetMixin, viewsets.ModelViewSet):
             },
         },
         override_response_schema={
-            reverse_func("registrations-list"): {
+            "registrations-list": {
                 "POST": {
                     201: {"properties": {"message": {"type": "string"}, "id": {"type": "integer"}}},
                 }
@@ -580,10 +580,10 @@ class RegistrationHistoryViewSet(AutoPrefetchViewSetMixin, viewsets.ReadOnlyMode
 
     schema = PcxAutoSchema(
         response_codes={
-            reverse_func("registrationhistory-list"): {
+            "registrationhistory-list": {
                 "GET": {200: "[DESCRIBE_RESPONSE_SCHEMA]Registration history successfully listed."}
             },
-            reverse_func("registrationhistory-detail", args=["id"]): {
+            "registrationhistory-detail": {
                 "GET": {
                     200: "[DESCRIBE_RESPONSE_SCHEMA]Historic registration detail "
                     "successfully retrieved.",

backend/courses/admin.py

@@ -14,6 +14,7 @@
     Department,
     Instructor,
     Meeting,
+    NGSSRestriction,
     PreNGSSRequirement,
     PreNGSSRestriction,
     Room,
@@ -50,17 +51,31 @@ class InstructorAdmin(admin.ModelAdmin):
 
 
 class AttributeAdmin(admin.ModelAdmin):
-    search_fields = ("code",)
-    list_display = ("code", "school")
+    search_fields = (
+        "code",
+        "description",
+    )
+    list_display = ("code", "school", "description")
+    exclude = ("courses",)
+
+
+class NGSSRestrictionAdmin(admin.ModelAdmin):
+    search_fields = (
+        "code",
+        "restriction_type",
+        "description",
+    )
+    list_display = ("code", "restriction_type", "inclusive", "description")
     exclude = ("courses",)
 
 
 class CourseAdmin(admin.ModelAdmin):
-    search_fields = ("full_code", "department__code", "code", "semester")
+    search_fields = ("full_code", "department__code", "code", "semester", "title")
     autocomplete_fields = ("department", "primary_listing")
     readonly_fields = ("topic", "crosslistings", "course_attributes")
     exclude = ("attributes",)
     list_filter = ("semester",)
+    list_display = ("full_code", "semester", "title")
 
     list_select_related = ("department", "topic")
 
@@ -202,6 +217,7 @@ class StatusUpdateAdmin(admin.ModelAdmin):
 admin.site.register(Meeting, MeetingAdmin)
 admin.site.register(StatusUpdate, StatusUpdateAdmin)
 admin.site.register(Attribute, AttributeAdmin)
+admin.site.register(NGSSRestriction, NGSSRestrictionAdmin)
 
 # https://github.com/sibtc/django-admin-user-profile
 admin.site.unregister(User)

backend/courses/management/commands/form_simple_topics.py

@@ -0,0 +1,81 @@
+from django.core.management.base import BaseCommand
+from django.db import transaction
+from django.db.models import Exists, F, OuterRef
+
+from alert.management.commands.recomputestats import garbage_collect_topics
+from courses.management.commands.load_crosswalk import get_crosswalk_s3
+from courses.models import Course, Topic
+
+
+def form_simple_topics():
+    Course.objects.all().update(topic=None)
+    garbage_collect_topics()
+
+    print("Cleared topics.")
+
+    # create topics for each primary listing
+    primary_listings = Course.objects.filter(primary_listing_id=F("id"))
+    for primary_listing in primary_listings:
+        topic = Topic.objects.create(most_recent=primary_listing)
+        primary_listing.listing_set.all().update(topic=topic)
+
+    print("Created new topics (one per course).")
+
+    # merge topics based on full_code of primary_listing
+    for full_code in primary_listings.values_list("full_code", flat=True):
+        Topic.merge_all(
+            list(
+                set(
+                    course.topic
+                    for course in (
+                        primary_listings.filter(full_code=full_code).select_related("topic")
+                    )
+                )
+            )
+        )
+
+    print("Merged topics based on full_code")
+
+    # use crosswalk
+    crosswalk = get_crosswalk_s3(verbose=True)
+    for old_code, new_codes in crosswalk.items():
+        old_topic = Topic.objects.filter(most_recent__listing_set__full_code=old_code).first()
+        new_course = (
+            Course.objects.filter(
+                full_code__in=new_codes,
+            )
+            .annotate(
+                title_matches=Exists(  # Prefer Course.objects.all().with matching title
+                    Course.objects.all().filter(full_code=old_code, title=OuterRef("title"))
+                ),
+            )
+            .order_by("-title_matches")
+            .select_related("topic")
+            .first()
+        )
+        if old_topic and new_course:
+            new_course.topic.merge_with(old_topic)
+
+    print("Merged topics based on crosswalk")
+
+    garbage_collect_topics()
+
+    print("Done!")
+
+
+class Command(BaseCommand):
+    help = (
+        "This script deletes all existing topics and re-creates them "
+        "so that courses with the same full_code are in the same topic. "
+        "this script also uses the crosswalk. "
+    )
+
+    def handle(self, *args, **kwargs):
+        print(
+            "This script is atomic, meaning either all Topic changes will be comitted to the "
+            "database, or otherwise if an error is encountered, all changes will be rolled back "
+            "and the database will remain as it was before the script was run."
+        )
+
+        with transaction.atomic():
+            form_simple_topics()

backend/courses/management/commands/reset_topics.py

@@ -1,3 +1,4 @@
+import gc
 from textwrap import dedent
 
 from django.core.management.base import BaseCommand
@@ -19,6 +20,7 @@ def fill_topics(verbose=False):
         if not course.topic:
             filled += 1
             course.save()
+            gc.collect()
     if verbose:
         print(f"Filled the topic field of {filled} courses.")
 

backend/courses/models.py

@@ -392,7 +392,7 @@ class Topic(models.Model):
     )
 
     @staticmethod
-    def merge_all(topics):
+    def merge_all(topics: list["Topic"]):
         if not topics:
             raise ValueError("Cannot merge an empty list of topics.")
         with transaction.atomic():
@@ -1359,14 +1359,20 @@ def validate_phone(value):
         """
         Validator to check that a phone number is in the proper form. The number must be in a
         form which is parseable by the
-        [phonenumbers library](https://pypi.org/project/phonenumbers/).
+        [phonenumbers library](https://pypi.org/project/phonenumbers/) and also valid.
+
+        Note: validators are NOT called automatically on model object save. They are called
+        on `object.full_clean()`, and also on serializer validation (returning a 400 if violated).
+        https://docs.djangoproject.com/en/4.2/ref/validators/
         """
-        if value.strip() == "":
+        if not value or not value.strip():
             return
         try:
-            phonenumbers.parse(value, "US")
-        except phonenumbers.phonenumberutil.NumberParseException:
-            raise ValidationError("Enter a valid phone number.")
+            parsed_number = phonenumbers.parse(value, "US")
+            if not phonenumbers.is_valid_number(parsed_number):
+                raise ValueError(f"Invalid phone number '{value}'.")
+        except (phonenumbers.phonenumberutil.NumberParseException, ValueError) as e:
+            raise ValidationError(str(e))
 
     phone = models.CharField(
         blank=True,
@@ -1396,16 +1402,14 @@ def save(self, *args, **kwargs):
         does not throw an error.
         It then calls the normal save method.
         """
-        if self.phone is not None and self.phone.strip() == "":
+        if not self.phone or not self.phone.strip():
             self.phone = None
-        if self.phone is not None:
-            try:
-                phone_number = phonenumbers.parse(self.phone, "US")
-                self.phone = phonenumbers.format_number(
-                    phone_number, phonenumbers.PhoneNumberFormat.E164
-                )
-            except phonenumbers.phonenumberutil.NumberParseException:
-                raise ValidationError("Invalid phone number (this should have been caught already)")
+        if self.phone:
+            # self.phone should be validated by `validate_phone`
+            phone_number = phonenumbers.parse(self.phone, "US")
+            self.phone = phonenumbers.format_number(
+                phone_number, phonenumbers.PhoneNumberFormat.E164
+            )
         super().save(*args, **kwargs)
 
 

backend/courses/util.py

@@ -431,7 +431,7 @@ 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.objects.get_or_create(
+            meeting = Meeting.objects.update_or_create(
                 section=section,
                 day=day,
                 start=start_time,
@@ -516,9 +516,10 @@ def add_attributes(course, attributes):
     course.attributes.clear()
     for attribute in attributes:
         school = identify_school(attribute.get("attribute_code"))
-        attr, _ = Attribute.objects.get_or_create(
+        desc = attribute.get("attribute_desc")
+        attr, _ = Attribute.objects.update_or_create(
             code=attribute.get("attribute_code"),
-            defaults={"description": attribute.get("attribute_desc"), "school": school},
+            defaults={"description": desc, "school": school},
         )
         attr.courses.add(course)
 
@@ -556,12 +557,16 @@ def add_restrictions(course, restrictions):
     """
     course.ngss_restrictions.clear()
     for restriction in restrictions:
-        res, _ = NGSSRestriction.objects.get_or_create(
-            code=restriction.get("restriction_code"),
+        code = restriction.get("restriction_code")
+        description = restriction.get("restriction_desc")
+        restriction_type = restriction.get("restriction_type")
+        inclusive = restriction.get("incl_excl_ind") == "I"
+        res, _ = NGSSRestriction.objects.update_or_create(
+            code=code,
             defaults={
-                "description": restriction.get("restriction_desc"),
-                "restriction_type": restriction.get("restriction_type"),
-                "inclusive": restriction.get("incl_excl_ind") == "I",
+                "description": description,
+                "restriction_type": restriction_type,
+                "inclusive": inclusive,
             },
         )
         res.courses.add(course)