-
Notifications
You must be signed in to change notification settings - Fork 35
leetcode leaderboard page #1981
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
base: dev
Are you sure you want to change the base?
Conversation
…ign2 merge da dev
n8thantran
left a comment
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.
lgtm
api/config/config.example.json
Outdated
| "LED_SIGN": { | ||
| "ENABLED": false | ||
| }, | ||
| "LEETCODE_URL": "NOT_SET", |
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.
would this change in development vs prod
if it wouldnt, can we just do an ENABLED true/false
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.
👍
| const AuditLog = require('../models/AuditLog.js'); | ||
| const membershipState = require('../../util/constants').MEMBERSHIP_STATE; | ||
|
|
||
| router.get('/getAllUsers', async (req, res) => { |
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.
make life simple
| router.get('/getAllUsers', async (req, res) => { | |
| router.get('/', async (req, res) => { |
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.
👍
| { value: firstName, title: 'User\'s first name', }, | ||
| { value: lastName, title: 'User\'s last name', } |
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.
do we need first and last name? up to you
if we just do username does that hurt anything, would be simpler
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.
names = easy to tell who's who on frontend
like who gonna know who "doggymuncher" is if there's no names you get what i mean
| const doesUserExist = await checkIfUserExists(leetcodeUsername, token); | ||
| if (doesUserExist.responseData) { | ||
| setIsError(true); | ||
| setMessage('This username is already registered, please try again'); | ||
| return; | ||
| } | ||
| const newUser = { | ||
| username: leetcodeUsername, | ||
| firstName, | ||
| lastName | ||
| }; | ||
| if (!await addUser(newUser, token)) { |
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.
isntead of a second endpoint you could have the create api check if they exist
the create api can return 409, and if the frontend gets 409 then we just say they already are registered
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.
👍
No description provided.