From e1ddfdbb177defb2077dbab0adfb5c60ae9e0521 Mon Sep 17 00:00:00 2001 From: aadalal <57609353+AaDalal@users.noreply.github.com> Date: Mon, 5 Feb 2024 19:26:27 -0500 Subject: [PATCH] Misc cleanups --- backend/PennCourses/settings/base.py | 1 + .../management/commands/load_degrees.py | 5 ++- backend/degree/serializers.py | 8 +--- backend/degree/urls.py | 10 +---- backend/degree/utils/parse_degreeworks.py | 38 +++++++++++-------- backend/degree/views.py | 35 +++++------------ 6 files changed, 41 insertions(+), 56 deletions(-) diff --git a/backend/PennCourses/settings/base.py b/backend/PennCourses/settings/base.py index 7e3bbe4ec..e499c9bda 100644 --- a/backend/PennCourses/settings/base.py +++ b/backend/PennCourses/settings/base.py @@ -49,6 +49,7 @@ "options.apps.OptionsConfig", "django.contrib.admindocs", "django_extensions", + "django_filters", "alert", "courses", "plan", diff --git a/backend/degree/management/commands/load_degrees.py b/backend/degree/management/commands/load_degrees.py index 82ebce209..481207a5d 100644 --- a/backend/degree/management/commands/load_degrees.py +++ b/backend/degree/management/commands/load_degrees.py @@ -2,6 +2,7 @@ import re from os import listdir, path from textwrap import dedent +import logging from django.core.management.base import BaseCommand from django.db import transaction @@ -14,7 +15,8 @@ class Command(BaseCommand): help = dedent( """ Load degrees from degreeworks JSONs named like year-program-degree-major-concentration - (without the .json extension). + (without the .json extension). Note that this command deletes any existing degrees that + overlap with the new degrees to be loaded. """ ) @@ -41,6 +43,7 @@ def handle(self, *args, **kwargs): if program not in program_code_to_name: print(f"Skipping {degree_file} because {program} is not an applicable program code") continue + print("Loading", degree_file, "...") with transaction.atomic(): Degree.objects.filter( diff --git a/backend/degree/serializers.py b/backend/degree/serializers.py index a785616bd..06a81f7b3 100644 --- a/backend/degree/serializers.py +++ b/backend/degree/serializers.py @@ -7,8 +7,8 @@ from courses.serializers import CourseListSerializer from degree.models import Degree, DegreePlan, DoubleCountRestriction, Fulfillment, Rule - class DegreeListSerializer(serializers.ModelSerializer): + class Meta: model = Degree fields = "__all__" @@ -18,8 +18,6 @@ class RuleSerializer(serializers.ModelSerializer): class Meta: model = Rule fields = "__all__" - - # Allow recursive serialization of rules RuleSerializer._declared_fields["rules"] = RuleSerializer( many=True, read_only=True, source="children" @@ -30,8 +28,6 @@ class DoubleCountRestrictionSerializer(serializers.ModelSerializer): class Meta: model = DoubleCountRestriction fields = "__all__" - read_only_fields = "__all__" - class DegreeDetailSerializer(serializers.ModelSerializer): rules = RuleSerializer(many=True, read_only=True) @@ -40,8 +36,6 @@ class DegreeDetailSerializer(serializers.ModelSerializer): class Meta: model = Degree fields = "__all__" - read_only_fields = "__all__" - class FulfillmentSerializer(serializers.ModelSerializer): course = CourseListSerializer( diff --git a/backend/degree/urls.py b/backend/degree/urls.py index 16c3825f7..24f00838d 100644 --- a/backend/degree/urls.py +++ b/backend/degree/urls.py @@ -3,8 +3,7 @@ from rest_framework_nested.routers import NestedDefaultRouter from degree.views import ( - DegreeDetail, - DegreeList, + DegreeViewset, DegreePlanViewset, FulfillmentViewSet, courses_for_rule, @@ -13,16 +12,11 @@ router = DefaultRouter() router.register(r"degreeplans", DegreePlanViewset, basename="degreeplan") +router.register(r"degrees", DegreeViewset, basename="degree") fulfillments_router = NestedDefaultRouter(router, r"degreeplans", lookup="degreeplan") fulfillments_router.register(r"fulfillments", FulfillmentViewSet, basename="degreeplan-fulfillment") urlpatterns = [ - path("degrees/", DegreeList.as_view(), name="degree-list"), - path( - "degree_detail/", - DegreeDetail.as_view(), - name="degree-detail", - ), path("courses/", courses_for_rule, name="courses-for-rule"), path("", include(router.urls)), path("", include(fulfillments_router.urls)), diff --git a/backend/degree/utils/parse_degreeworks.py b/backend/degree/utils/parse_degreeworks.py index ffde00c62..b2f378f8b 100644 --- a/backend/degree/utils/parse_degreeworks.py +++ b/backend/degree/utils/parse_degreeworks.py @@ -2,7 +2,7 @@ from degree.models import Degree, Rule from degree.utils.departments import ENG_DEPTS, SAS_DEPTS, WH_DEPTS - +import logging def parse_coursearray(courseArray) -> Q: """ @@ -25,7 +25,7 @@ def parse_coursearray(courseArray) -> Q: elif number[:-1].isdigit() and number[-1] == "@": course_q &= Q(full_code__startswith=f"{discipline}-{number[:-1]}") else: - print(f"WARNING: non-integer course number: {number}") + logging.warn(f"Non-integer course number: {number}") case discipline, number, end: if number.isdigit() and end.isdigit(): course_q &= Q( @@ -34,7 +34,7 @@ def parse_coursearray(courseArray) -> Q: code__lte=int(end), ) else: - print("WARNING: non-integer course number or numberEnd") + logging.warn(f"Non-integer course number or numberEnd: (number) {number} (numberEnd) {end}") connector = "AND" # the connector to the next element; and by default if "withArray" in course: @@ -69,13 +69,13 @@ def parse_coursearray(courseArray) -> Q: f"Unsupported college in withArray: {filter['valueList'][0]}" ) case "DWRESIDENT": - print("WARNING: ignoring DWRESIDENT") + logging.info("ignoring DWRESIDENT") sub_q = Q() case "DWGRADE": - print("WARNING: ignoring DWGRADE") + logging.info("ignoring DWGRADE") sub_q = Q() case "DWCOURSENUMBER": - print("WARNING: ignoring DWCOURSENUMBER") + logging.info("ignoring DWCOURSENUMBER") sub_q = Q() case _: raise LookupError(f"Unknown filter type in withArray: {filter['code']}") @@ -92,7 +92,7 @@ def parse_coursearray(courseArray) -> Q: connector = filter["connector"] if len(course_q) == 0: - print("Warning: empty course query") + logging.warn("Empty course query") continue match course.get("connector"): @@ -104,7 +104,7 @@ def parse_coursearray(courseArray) -> Q: raise LookupError(f"Unknown connector type in courseArray: {course['connector']}") if len(q) == 0: - print("Warning: empty query") + logging.warn("empty query") return q @@ -132,8 +132,16 @@ def evaluate_condition(condition, degree) -> bool: attribute = degree.concentration case "PROGRAM": attribute = degree.program - case _: - raise ValueError(f"Unknowable left type in ifStmt: {comparator['left']}") + case "BANNERGPA": + logging.info("ignoring ifStmt with BANNERGPA") + return True + case "ATTRIBUTE": # TODO: what is this? + logging.info("ignoring ifStmt with ATTRIBUTE") + return False # Assume they don't have this ATTRIBUTE + case "COLLEGE": + attribute = degree.program.split("_")[0] # e.g., WU from WU_BSE or EU from EU_BSE + case "ALLDEGREES" | "WUEXPTGRDTRM" | "-COURSE-" | "NUMMAJORS" | "NUMCONCS" | "MINOR" | _: + raise ValueError(f"Unknowable left type in ifStmt: {comparator}") match comparator["operator"]: case "=": return attribute == comparator["right"] @@ -157,7 +165,7 @@ def parse_rulearray( A ruleArray consists of a list of rule objects that contain a requirement object. """ for rule_json in ruleArray: - this_rule = Rule(parent=parent, degree=None) + this_rule = Rule(parent=parent, degree=None, title=rule_json["label"]) rules.append(this_rule) rule_req = rule_json["requirement"] @@ -199,7 +207,7 @@ def parse_rulearray( evaluation = evaluate_condition(rule_req["leftCondition"], degree) except ValueError as e: assert e.args[0].startswith("Unknowable left type in ifStmt") - print("Warning: " + e.args[0]) + logging.warn(e.args[0]) continue # do nothing if we can't evaluate b/c of insufficient info match rule_json["booleanEvaluation"]: @@ -214,7 +222,6 @@ def parse_rulearray( f"Unknown boolean evaluation in ifStmt: \ {rule_json['booleanEvaluation']}" ) - assert degreeworks_eval is None or evaluation == bool(degreeworks_eval) # add if part or else part, depending on evaluation of the condition @@ -226,7 +233,8 @@ def parse_rulearray( if "ruleArray" in rule_json: parse_rulearray(rule_json["ruleArray"], degree, rules, parent=parent) else: - print("WARNING: subset has no ruleArray") + this_rule.q = repr(Q()) # General elective + logging.info("subset has no ruleArray") case "Group": # this is nested parse_rulearray(rule_json["ruleArray"], degree, rules, parent=this_rule) this_rule.num = int(rule_req["numberOfGroups"]) @@ -261,4 +269,4 @@ def parse_degreeworks(json: dict, degree: Degree) -> list[Rule]: rules.append(degree_req) parse_rulearray(requirement["ruleArray"], degree, rules, parent=degree_req) - return rules + return rules \ No newline at end of file diff --git a/backend/degree/views.py b/backend/degree/views.py index ade665c94..8c59a0553 100644 --- a/backend/degree/views.py +++ b/backend/degree/views.py @@ -5,6 +5,8 @@ from rest_framework.exceptions import ValidationError from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response +from django_filters.rest_framework import DjangoFilterBackend +from rest_framework.filters import SearchFilter from courses.models import Course from courses.serializers import CourseListSerializer @@ -19,37 +21,20 @@ from PennCourses.docs_settings import PcxAutoSchema -class DegreeList(generics.ListAPIView): +class DegreeViewset(viewsets.ReadOnlyModelViewSet): """ Retrieve a list of all Degree objects. """ - schema = PcxAutoSchema( - response_codes={ - "degree-list": {"GET": {200: "[DESCRIBE_RESPONSE_SCHEMA]Degrees listed successfully."}} - }, - ) - - serializer_class = DegreeListSerializer queryset = Degree.objects.all() + filter_backends = [DjangoFilterBackend, SearchFilter] + search_fields = ["program", "degree", "concentration", "year"] + filterset_fields = search_fields - -class DegreeDetail(generics.RetrieveAPIView): - """ - Retrieve a detailed look at a specific Degree. Includes all details necessary to display degree - info, including degree requirements that this degree needs. - """ - - schema = PcxAutoSchema( - response_codes={ - "degree-detail": { - "GET": {200: "[DESCRIBE_RESPONSE_SCHEMA]Degree detail retrieved successfully."} - } - }, - ) - - serializer_class = DegreeDetailSerializer - queryset = Degree.objects.all() + def get_serializer_class(self): + if self.action == "list": + return DegreeListSerializer + return DegreeDetailSerializer class DegreePlanViewset(AutoPrefetchViewSetMixin, viewsets.ModelViewSet):