Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix missing requirements #490

Open
kevinzwang opened this issue Mar 27, 2022 · 9 comments
Open

Fix missing requirements #490

kevinzwang opened this issue Mar 27, 2022 · 9 comments
Assignees
Labels
bug Something isn't working

Comments

@kevinzwang
Copy link
Member

kevinzwang commented Mar 27, 2022

Ex:

  • CS 61C should have the Physical Science breadth tag
  • Cogsci 1 should have the Social and Behavioral Science breadth tag

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

@kevinzwang kevinzwang added backend bug Something isn't working labels Mar 27, 2022
@Boomaa23
Copy link
Contributor

Boomaa23 commented Apr 16, 2022

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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Member Author

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

@Boomaa23
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member

@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
Copy link
Contributor

Boomaa23 commented May 4, 2022

@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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants