search

asuc-octo / berkeleytime

UC Berkeley enrollment info
https://berkeleytime.com
MIT License
53 stars 9 forks source link

Fix missing requirements #490

Open kevinzwang opened 2 years ago

kevinzwang commented 2 years ago

Ex:

We should find the root cause of the issue (SIS API not giving us all the data? Us dropping the data during preprocessing or serving?) and then develop a fix if possible

Boomaa23 commented 2 years ago

I've narrowed this down to some playlist issue. We take in the data correctly from SIS (this is backed up by logs) and that data is written to the "temporary storage" CSV files correctly. The playlist supposedly updates, however none of the L&S requirements work (for any semester). Other filters work, just not the L&S ones. Will continue investigating.

Boomaa23 commented 2 years ago

Found it: see here

The playlist queryset excludes all ls category playlists except for the current semester. Since the SIS API endpoint for the current semester (actually both Spring and Fall 2022) is returning a status of 401, the length of this playlist is zero and thus none of the L&S filters work.

Regardless of this endpoint problem, it seems like excluding all but the most recent semester means none of the previous semester' L&S filters would work. I'm not as sure of that one though.

zacharyzollman commented 2 years ago

That's interesting, it seems like filtering was introduced in this commit, so maybe L&S requirements for past semesters have been missing since November 2020?

kevinzwang commented 2 years ago

@Boomaa23 interesting. Could you find out why the endpoint is returning 401?

Boomaa23 commented 2 years ago

I believe this is because it uses the wrong API URL (this may have changed last year at some point). We haven't really seen the effects of this since it uses cached values for previous years (but as noted above these are not used anyways).

See here: The endpoint is: https://apis.berkeley.edu/sis/v1/classes/sections

When it should be (per here): https://apis.berkeley.edu/uat/sis/v1/classes/sections

Boomaa23 commented 2 years ago

Update: this comment:

I believe this is because it uses the wrong API URL (this may have changed last year at some point). We haven't really seen the effects of this since it uses cached values for previous years (but as noted above these are not used anyways).

See here: The endpoint is: https://apis.berkeley.edu/sis/v1/classes/sections

When it should be (per here): https://apis.berkeley.edu/uat/sis/v1/classes/sections

Is not why the endpoint was returning 401. See #497 for fix.

The current-semester L&S parser thing is still a valid issue.

zacharyzollman commented 2 years ago

@Boomaa23 I'm a bit confused about the status of this issue, could you clarify what the problem is with the requirements at this point? I know at one point searches for courses that satisfy breadth requirements were not showing any classes but they do seem to be showing up now, even for past semesters.

Boomaa23 commented 2 years ago

@zacharyzollman the filter/search is based on the current semesters' classes. So if we took for example fall 2016, the classes filtered would be from the fall 2022 list. In essence we just don't use the whole caching system that is already built for filtering based on previous semesters.

Why is this bad? Say in Fall 2016 we have classes CLASS 1A . In Fall 2022 we have CLASS 1A and CLASS 1B. CLASS 1B is a L&S breadth for Fall 2022 but was not in Fall 2016 (say they added it or something). If we went and searching in Fall 2016, we'd find CLASS 1B which is incorrect for that time.

zacharyzollman commented 2 years ago

@Boomaa23 oh ok, that makes sense. Thank you for clarifying!