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] Implement BANNER API course database population #515

Closed

Conversation

MNThomson
Copy link
Member

@MNThomson MNThomson commented Jun 6, 2024

Description

Courseup currently does not have any data for 202405 and beyond due to some UVic api changes. Luckily there is a banner api that just gets all that data for you!

curl -v --cookie "JSESSIONID=blahbahblahblah" \
https://banner.uvic.ca/StudentRegistrationSsb/ssb/searchResults/searchResults?txt_term=202405&pageMaxSize=10000

(NOTE: there is some scuffed session stuff going on, so before you can run the above command, you have to go to https://banner.uvic.ca/StudentRegistrationSsb/ssb/classSearch/classSearch in your browser and click Search with all of the filter boxes empty)

Now, the above API does require a JSESSIONID cookie.....oh no! Obviously no one wants to have their cookie stored somewhere that other people can retrieve it. So the CI workflow called populate-database now takes an additional input parameter: cookie. After use, the logoff endpoint is called to invalid the session cookie. Woot woot, no cookies leaked!

Next Steps:

  • Wrangle the API response into the expected Sections[] datatype that was already in use
  • Unhackify the implementation to something that's merge-able

fixes: #513

Checklist

  • The code follows all style guidelines.
  • The code passes all required tests.
  • The code is documented.
  • The code includes tests.
  • I have self-reviewed my changes and have done QA.

General Comments

I wanted to procrastinate an assignment yesterday so I just did this.

Now, I have absolutely no clue if just populating the db is enough to get CU working for future semesters. I don't know if the course scraper is being used in different contexts and not just hitting the firebase db. This is where someone else will have to provide guidance.

Is this a PR that the @VikeLabs/courseup team will support? Should I continue development on this?

Copy link

vercel bot commented Jun 6, 2024

@MNThomson is attempting to deploy a commit to the VikeLabs Team on Vercel.

A member of the Team first needs to authorize it.

@szeckirjr
Copy link
Contributor

szeckirjr commented Jun 6, 2024

this is super sweet!! 👀 heads up there's a banner client with some types already under functions/src/banner too that might be worth checking out, I think it does some of the more basic stuff for querying banner data.

It's been such a transition time with no people knowledgeable with the processes (especially related to backend/data flow/scraper stuff), so we didn't really get anywhere trying to solve the errors we were getting when populating the DB for past couple of terms. I believe having the data in the DB is enough, but I don't understand a lot of that end of the process to say for sure tbh. IIRC there was either missing data from banner or some other error that didn't let us run the populate script for previous courses

EDIT: Looked more into this and it's sick, I like the cookie circumvention idea. Definitely something worth putting some work on to replace course scraper completely 🤞

@MNThomson
Copy link
Member Author

dont think it will be enough. the database isnt used that much.
when it pulls only courses in session it will hit the db otherwise it hits the scraper for that term

from @aomi on Discord

Sounds like there would be significant work to the backend and I (with my limited knowledge) see two paths forward:

  1. Every request hits the database. This would probably be expensive but is also probably the "proper answer"
  2. The JSON blob with course data is hardcoded into courseup and then periodically updated by volunteers using their own cookie

Not sure this is something I would be able to spearhead with my limited technical knowledge of how the backend of CU works. I'm happy to help out in the future if someone else wants to take a lead on this.

@MNThomson MNThomson closed this Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Summer 2024 Missing
2 participants