-
Notifications
You must be signed in to change notification settings - Fork 3
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
Implement user login #861
base: main
Are you sure you want to change the base?
Implement user login #861
Conversation
@otobot1 you can take a look at the workflow and styling. I'm left with server logging (and maybe converting the foreign key changes to a single transaction). /claim-user for claiming, /verify-claim for verifying the user. |
I should be able to take a look this weekend! |
combine user-claim related pages into single folder
make explicit that we don't want people duplicating their ratings
Tweak UserClaim column and relation names
required by next.js pagesRouter
backend only. frontend tweaks forthcoming. -split userClaim endpoints into a subrouter -place the user and userClaim router files into a new folder -tweak userClaim api to make userClaims non-public, as they don't need to be public -tweak userClaim router for formatting and readability, and make it match existing routers more closely -wrap the userClaim verification db calls into a transaction
-add check for claimedUser to verification endpoint
Hey @ShouvikGhosh2048 . I didn't quite manage to get through the entire thing this weekend, but I think it's in a state where you can keep working on it. I'm happy with the tRPC api now (although we still need server logging), but I didn't have a chance to really look at or adjust the frontend to match the backend changes, so I've left the frontend in a broken state (sorry!). I'll get to it when I can, and will mark this as a draft again when I do. In the meantime, please feel free to work on the remaining pages or any other backend stuff. If you'd like to work on the pages you already created, please mark this PR as a draft and also make both pages work with the backend changes. |
Also make minor tweaks to wording
d628306
to
c1b21e9
Compare
-update comment
-i read somewhere that it's better and found it convincing at the time, but i can't recall why. i'll add a comment to this PR (celestemods-com#861) if i ever remember
change all `onUpdate: Restrict` to `onUpdate: Cascade`
no longer throwing React errors
I made some progress on this tonight. I reviewed the new pages and I think I'm happy with how they work. I fixed all of the React errors and adjusted everything for the new API, but I'm still having permission errors that I haven't had a chance to figure out yet. I haven't been able to get either page to actually render on my end yet, so I may end up making some tweaks in the end. I also haven't had a chance to review the nextAuth changes, but I think any changes to that code will be minor. @ShouvikGhosh2048 I'm done with my first-pass review, so I'm marking this as a draft again. To get this ready for merging, please:
I think that should be it, but I'm pretty tired at the moment. I may add another item or two in the next couple of days if I forgot something. |
only appears for admins
This PR is basically ready to go - there are just a couple of minor tweaks and a handful of follow-up issues to create. Whichever one of us has time first can work on the remaining changes - just communicate so that we aren't duplicating work. Required Changes:
Follow-Up Issues:
|
Closes #766.