Address PR comments

backend/PennCourses/settings/base.py

@@ -210,7 +210,8 @@
 XWALK_S3_BUCKET = "penn.courses"
 XWALK_SRC = "xwalk_csre_number.txt"
 
-# The first semester that used Banner/NGSS for course data
+# The first semester that used Banner/NGSS for course data.
+# This was when course codes changed from 3-digit to 4-digit.
 # Note that the above crosswalk connects pre and post NGSS courses
 FIRST_BANNER_SEM = "2022B"
 

backend/courses/management/commands/recompute_topics.py

@@ -10,7 +10,7 @@
 def recompute_topics(min_semester: str = None, verbose=False):
     """
     Course topics are directly derived from the `Course.parent_course` graph.
-        - Any orphan child gets its own topic.
+        - Any course without a parent gets its own topic.
         - Any single child of a parent inherits the parent's topic.
         - Any child with siblings that exactly matches their parent in full_code
           and title inherits the parent's topic.
@@ -46,14 +46,14 @@ def recompute_topics(min_semester: str = None, verbose=False):
                 if (
                     parent.full_code == course.full_code
                     and parent.title == course.title
-                    or len(parent.children) == 1
+                    or parent.children.count() == 1
                 ):
                     # If a parent has multiple children and none are an exact match, we consider
                     # all the child courses as "branched from" the old. But if one child is an exact
                     # match, we inherit the parent's topic and the rest of the children
                     # are considered "branched from".
                     if course.topic_id != parent.topic_id:
-                        course.topic = course.parent_course.topic
+                        course.topic = parent.topic
                         course.topic.most_recent = course
                         courses_to_update.append(course)
                         topics_to_update.append(course.topic)

backend/courses/views.py

@@ -230,10 +230,10 @@ class CourseDetail(generics.RetrieveAPIView, BaseCourseMixin):
             "courses-detail": {
                 "GET": [
                     {
-                        "name": "check_semester",
+                        "name": "check_offered_in",
                         "in": "query",
                         "description": (
-                            "Specify a `check_semester` to check that the desired course was "
+                            "Specify a `check_offered_in` to check that the desired course was "
                             "offered in a certain semester. 404 will be returned if the course "
                             "does not exist, or was not offered in that semester."
                         ),
@@ -263,13 +263,13 @@ def get_queryset(self):
                 ),
             )
         )
-        check_semester = self.request.GET.get("check_semester")
+        check_offered_in = self.request.query_params.get("check_offered_in")
         queryset = (
             queryset.filter(
                 topic__courses__full_code=self.kwargs["full_code"],
-                topic__courses__semester=check_semester,
+                topic__courses__semester=check_offered_in,
             )
-            if check_semester
+            if check_offered_in
             else queryset
         )
         queryset = self.filter_by_semester(queryset)

backend/plan/views.py

@@ -299,7 +299,7 @@ def get_sections(data):
         return sections
 
     @staticmethod
-    def check_semester(data, sections):
+    def check_offered_in(data, sections):
         for i, s in enumerate(sections):
             if i == 0 and "semester" not in data:
                 data["semester"] = s.course.semester
@@ -328,7 +328,7 @@ def update(self, request, pk=None):
                 status=status.HTTP_400_BAD_REQUEST,
             )
 
-        semester_check_response = self.check_semester(request.data, sections)
+        semester_check_response = self.check_offered_in(request.data, sections)
         if semester_check_response is not None:
             return semester_check_response
 
@@ -360,7 +360,7 @@ def create(self, request, *args, **kwargs):
                 status=status.HTTP_400_BAD_REQUEST,
             )
 
-        semester_check_response = self.check_semester(request.data, sections)
+        semester_check_response = self.check_offered_in(request.data, sections)
         if semester_check_response is not None:
             return semester_check_response
 

backend/review/documentation.py

@@ -132,7 +132,7 @@
                         "type": "string",
                         "description": "The dash-joined department and most-recent code of this course, e.g. `CIS-1200`.",  # noqa E501
                     },
-                    "superseded_last_offered": {
+                    "last_offered_sem_if_superceded": {
                         "type": "string",
                         "description": "The last semester in which this course was offered, if it has been superseded by a more recent course (with the same full code).",  # noqa E501
                     },

backend/review/views.py

@@ -191,7 +191,7 @@ def course_reviews(request, course_code, semester=None):
         if semester
         else False
     )
-    superseded_last_offered = course.semester if superseded else None
+    last_offered_sem_if_superceded = course.semester if superseded else None
 
     course_code = course.full_code
     topic_id = course.topic_id
@@ -265,7 +265,7 @@ def course_reviews(request, course_code, semester=None):
     return Response(
         {
             "code": course["full_code"],
-            "superseded_last_offered": superseded_last_offered,
+            "last_offered_sem_if_superceded": last_offered_sem_if_superceded,
             "name": course["title"],
             "description": course["description"],
             "aliases": aliases,

frontend/review/src/components/DetailsBox.js

@@ -82,7 +82,7 @@ const formsCol = {
  * The box below the course ratings table that contains student comments and semester information.
  */
 export const DetailsBox = forwardRef(
-  ({ course, instructor, type, isCourseEval }, ref) => {
+  ({ course, instructor, semester, type, isCourseEval }, ref) => {
     const [data, setData] = useState({});
     const [viewingRatings, setViewingRatings] = useState(true);
     const [selectedSemester, setSelectedSemester] = useState(null);
@@ -127,7 +127,7 @@ export const DetailsBox = forwardRef(
     useEffect(() => {
       setIsLoading(true);
       if (instructor !== null && course !== null) {
-        apiHistory(course, instructor)
+        apiHistory(course, instructor, semester)
           .then(res => {
             const sections = Object.values(res.sections);
             const fields = [

frontend/review/src/components/GraphBox.js

@@ -325,7 +325,7 @@ const percentSectionChartOptions = {
   }
 };
 
-const GraphBox = ({ courseCode, isAverage, setIsAverage }) => {
+const GraphBox = ({ courseCode, semester, isAverage, setIsAverage }) => {
   const averageOrRecent = isAverage ? "average_plots" : "recent_plots";
 
   const [chartData, setChartData] = useState(null);
@@ -368,7 +368,7 @@ const GraphBox = ({ courseCode, isAverage, setIsAverage }) => {
       handlePCAChartDataResponse(cachedPCAChartDataResponse);
     } else {
       setLoaded(false);
-      apiFetchPCADemandChartData(courseCode)
+      apiFetchPCADemandChartData(courseCode, semester)
         .then(handlePCAChartDataResponse)
         .finally(() => {
           setLoaded(true);