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

Threshold and Invalid Input Fix #550

Merged
merged 12 commits into from
Jan 14, 2025
Merged

Conversation

Bl20052005
Copy link
Contributor

Closes #536 and #531

Copy link
Contributor

github-actions bot commented Jan 13, 2025

Deploy preview for irvinehacks-site-2025 ready!

Name IrvineHacks Site
Preview Visit Preview
Commit 30eca0f

@Bl20052005 Bl20052005 requested a review from waalbert January 13, 2025 02:34
Copy link
Contributor

@waalbert waalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome start Bo!

Can you also add frontend form validation for submitting a review score and also frontend form and backend validation for setting the thresholds?

Note for your reference: we will be moving the setting thresholds form to the director's dashboard, so since this PR is small, can you also remove HackerThresholdInputs from HackerApplicants? We will add HackerThresholdInputs to the director's dashboard in another PR.

apps/api/src/routers/admin.py Outdated Show resolved Hide resolved
@Bl20052005 Bl20052005 requested a review from waalbert January 13, 2025 06:44
Copy link
Contributor

@waalbert waalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, make sure to add form validation in HackerThresholdsInput for valid threshold values: 0 to 10 and make sure that the accept threshold value must be greater than or equal to the waitlist threshold value .

Note: when the input field is empty, the value of -1 is sent to /api/admin/set-thresholds for when the accept or waitlist threshold value should stay the same.

Actually forget what I said about moving HackerThresholdsInput to the director's page in another PR. I've decided to keep it here on the same page, but make it only visible for directors. There is an authorization roles check called isDirector that can check to see if the user is a director.

apps/api/src/routers/admin.py Show resolved Hide resolved
@Bl20052005 Bl20052005 requested a review from waalbert January 13, 2025 08:08
Copy link
Contributor

@waalbert waalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost done! A few more nitpicks

Copy link
Contributor

@waalbert waalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the work Bo!

@waalbert waalbert merged commit c09f855 into main Jan 14, 2025
4 checks passed
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.

Restrict threshold modification to directors
2 participants