-
Notifications
You must be signed in to change notification settings - Fork 7
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
root v2 at this point #38
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
this commit should make the server unsuable since the logic for the queries/mutations is for the old schema. DO NOT RUN THIS COMMIT.
This commit unfortunately removes the leaderboard updation via leetcode and codeforces stats to simplify my work for now. Will have to add it back later.
ivinjabraham
force-pushed
the
decouple-shuttle
branch
2 times, most recently
from
January 15, 2025 13:46
774383c
to
7a9a6ee
Compare
This commit changes a lot more things than just the graphql interface. Also likely introduces a few extra regressions. ¯\_(ツ)_/¯
ivinjabraham
force-pushed
the
decouple-shuttle
branch
from
January 17, 2025 07:12
03324a4
to
3518e18
Compare
ivinjabraham
force-pushed
the
decouple-shuttle
branch
from
January 17, 2025 08:23
44de20c
to
8326b0a
Compare
ivinjabraham
force-pushed
the
decouple-shuttle
branch
from
January 17, 2025 08:28
8326b0a
to
e2207f7
Compare
Wreck-X
previously approved these changes
Jan 21, 2025
Wreck-X
reviewed
Jan 21, 2025
Wreck-X
approved these changes
Jan 25, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #24. And I'll be rewriting some of the current system in hopes to clean up some of the unintuitive stuff that crept in. I could make these into two separate PRs, but I'd rather not let the latter be delayed after decoupling.
Please take code reviews more seriously so that we don't have to keep rewriting stuff.
Changelog
Sqlx::migrate!
embeds the migrations into the binary, so unnecessary migrations will bloat the binary....and more
These are some pretty chonky and important commits, worth reviewing twice:
TODO
Future Work
I'll be removing some of the stuff that isn't actively used now to simplify (lol) this pull request. This will be a list of stuff to do later as well as the list of stuff that I removed.
Anything to do with
ActiveProjects
.The leaderboard features in main are a huge mess starting from the relations and it seems whoever reviewed it did not pay attention. Hence, I think it's just better if @swayam-agrahari rewrote it at a later date. Some of the problems include:
problems_solved
, confusing columns likecontests_participated
andtotal_contests
, fields likebest_rank
being non-nullable.fetch_leetcode_stats
.update_leaderboard
doing way too many things for one function.add_or_update
mutation for username updation.This is ignoring pedantic, but still important, problems like
fetch_{x}_stats
being functions that insert data into the DB unlike what the name implies. Additionally, since tests are mainly for the leaderboard feature, that was removed too.HMAC or any other form of "authentication" in the request itself. The solution to this is obviously to prevent users from accessing the API unless authorized, not to send auth information in requests. This means we'll need to integrate basic CRUD operations on Home for members (and other models in the future). Note: markAttendance still has HMAC verification.