unc-csxl / csxl.unc.edu

CS Experience Labs' web application.
https://csxl.unc.edu
MIT License
9 stars 7 forks source link

DRAFT: My Courses API Endpoint #499

Closed KrisJordan closed 5 days ago

KrisJordan commented 3 weeks ago

Taking a stab at implementing a fast (single query with eagerly loaded joined data) API endpoint for pulling the logged in user's course membership data organized by terms.

API model exposed should translate pretty directly into the UI exposed; I think this is the right path forward (with an opinionated decision going Term > Course > Section) as opposed to Term > Section* > Course.

An open question remains as to how to pull in the office hours linkage per course (as well as an instructor's link to staffing...) so leaving this marked as draft for now but throwing up as a draft to be on @ajaygandecha's radar if starting to take a crack at My Courses frontend.

ajaygandecha commented 3 weeks ago

@KrisJordan this is a great start on getting our API endpoints to return just the data we need, rather than entire models! I am also liking the ..Overview convention for these model names.

Currently, the My Courses page design shows specific sections rather than just at the course level (for example, COMP 110-001 vs 110-003). Wanted to confirm, based on the way it is written, the TermOverview contains a list of courses for which a user is a member, and the sections listed for each course is only the section that the user is registered for?

KrisJordan commented 3 weeks ago

Yes, we're grouping sections automatically under course for a given term (and elevating a users first role to the course level with an assumption they're always the same). Every course I can think of where one instructor has multiple sections treats them as one big office hours and hiring pool, ultimately.

An alternative would be to go down a more customizable route (like office hours section did) where the pooling of sections to an instructor is a manual process with UI and its own entity. I feel like this adds more complexity than currently worth if we just logically group sections from the perspective of someone belonging to both.

ajaygandecha commented 3 weeks ago

Looking great! @KrisJordan would it be worth separating the current term from the previous terms in the API call, or can we keep it the way it is for now?

KrisJordan commented 3 weeks ago

I think we can punt on a separate API for previous terms given how fast this should be; and how few previous terms will exist in our system. Feels like a 4-5 year runway before it's worth the complexity of an extra API endpoint.

KrisJordan commented 3 weeks ago

Let's keep trucking on this branch since there's no UI yet (and the API may yet need some refinement once there is UI).

ajaygandecha commented 3 weeks ago

@KrisJordan I am going to treat my-courses as an entirely new module separate from the existing OH feature, then move features over as I build up. We can treat office hours as a submodule of my-courses once we get to that functionality!

ajaygandecha commented 3 weeks ago

@KrisJordan perhaps SectionOverview can have two lists: something like current_events and next_event? This grabs the office hours events (in the form of OfficeHourEventOverview) directly and I think this supplies just enough data for the 'My Courses' page.

KrisJordan commented 3 weeks ago

Hmmm maybe the optimization we apply in the MyCoursesService is to do a second query for this data on only the courses in the active term and then populate. Trying to join this event data in for all courses, including historical, would quickly be a pretty expensive query, I think.