-
Notifications
You must be signed in to change notification settings - Fork 0
Auth upgrades #89
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
Auth upgrades #89
Conversation
|
@EarthenSky this PR needs the alembic migrations to be fixed, would you be able to open a PR for that 🥺 also, I'm gonna add some test HTML files, but am gonna wait for alembic to not be f'ed so i can run this locally |
|
@EarthenSky pls review 😋 if u can otherwise ill merge after we meet next |
|
(i.e. after alembic stuff is fixed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! A few important changes, a few unimportant changes.
Have you tested running login.html yet? If not (due to alembic not working), then please do so now. However, you should be able to test without using alembic using the test database, which gets filled with new data using load_test_db.py (no need for running the migrations)
5089038 to
6883469
Compare
6883469 to
d62b126
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just ensure the alembic migration is checked, then this looks good!
Oh, also POST logout.
src/alembic/versions/e74292feb010_add_profile_picture_url_to_site_user.py
Outdated
Show resolved
Hide resolved
src/alembic/versions/e74292feb010_add_profile_picture_url_to_site_user.py
Outdated
Show resolved
Hide resolved
|
@EarthenSky sorry for late reply, was busy this weekend with a Weezer concert and stormhacks - I did run the migrations locally and everything worked well! ++ Ran with the test HTML page Could you pretty please reply to my comments 🥺 - gracias sir |
|
tested that it seems to be working okay! |
Description
Some upgrades for the auth module, considering new endpoint at /api instead of api.sfucsss.org
Changes