DEV-177: Add Support for Time Filtering in Backend#385
DEV-177: Add Support for Time Filtering in Backend#385
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Changes: simplified the search to adhere to Spencer's suggestion: simplify the time filtering to only do it based on the lectures for the class and not even worry about whether the precepts/labs/etc fit in the query time., take into account lectures, seminars, and studios |
hannahchoi05
left a comment
hannahchoi05
left a comment
There was a problem hiding this comment.
Left a few comments!
| sections = Section.objects.filter(course=course, | ||
| term__term_code=term).prefetch_related('classmeeting_set') | ||
| else: | ||
| sections = course.section_set.all() |
There was a problem hiding this comment.
use prefetch_related; all() can cause inefficient queries.
| if not class_meetings: | ||
| continue | ||
|
|
||
| meeting = class_meetings[0] |
There was a problem hiding this comment.
For courses like SML201, there are 2 lectures. Instead of just using class_meetings[0] we want to iterate over all instances of that class_meeting to check times.
| if not sections: | ||
| return False | ||
|
|
||
| primary_types = ["Lecture", "Seminar", "Studio"] |
There was a problem hiding this comment.
See ECS389, CWR343, CWR202, etc. (almost all CWR courses). I think we should include 'Class' in this as well.
| # Parse start_time and end_time | ||
| try: | ||
| start_time = dtime.fromisoformat(start_time_str) | ||
| except Exception: |
There was a problem hiding this comment.
For this line (and line 109), we should use ValueError instead of Exception for clarity.
2 participants
References
Proposed Changes
search_coursesto take start and end times for filtering in the form of a string :HH:MM:SScourse_fits_time_constraint(course, start_time, end_time, term)which takes in a course, the current term, and the restrictions on time and returns whether or not a student can take this course during the restricted timesSome Notes