[Admin] 2 Hacker reviewers #510

IanWearsHat · 2024-12-20T18:07:18Z

Affected /applicants, /hackerApplicants, /release, /releaseHackerDecisions, /review, /hackerReview, /setThresholds, route
Still needs way to show an applicant's decision on frontend HackerApplicants.tsx
Still needs way to filter accepted, waitlisted, rejected and other filter

Testing

Create a sample app in the db
Navigate to that app, score it in the top right
- Ensure the score updates in the reviews log on that page, updates in the db, updates the number of reviewers in the applicants overview, and status is still set to pending_review
Review that app again with a different account
- Ensure avg_score updates in db, status is set to reviewed
Manually run /releaseHackerDecisions to ensure db status updates

github-actions · 2024-12-20T18:09:42Z

Deploy preview for irvinehacks-site-2025 ready!

Name	IrvineHacks Site
Preview	Visit Preview
Commit	`ceb7330`

taesungh

Have some high-level comments for now, but overall, this PR is looking quite hefty. Do you think you could break this down into smaller pieces? Suggested division:

Setting up placeholder Admin page structure for different applicant types and showing the hacker applicants
Adding the new hacker review logic and indicator
Adding the ability to adjust the thresholds

taesungh · 2024-12-21T04:30:35Z

apps/api/src/routers/admin.py

@@ -43,6 +43,16 @@ class ApplicantSummary(BaseRecord):
    application_data: ApplicationDataSummary


+class HackerApplicantSummary(BaseRecord):
+    uid: str = Field(alias="_id")


Note: I think BaseRecord already includes uid, so it doesn't need to be specified again here (perhaps doesn't hurt for clarity? but seems redundant).

taesungh · 2024-12-21T04:34:09Z

apps/api/src/routers/admin.py

@@ -73,6 +83,83 @@ async def applicants(
        raise RuntimeError("Could not parse applicant data.")


+@router.get("/hackerApplicants")


Thought: I'm imagining long term you'll also have a similar endpoint for mentor and volunteer applicants? Might make more sense to group the paths, e.g. /applicants/hackers.
Note: otherwise, let's name the API paths with kebab-case

taesungh · 2024-12-21T04:37:21Z

apps/api/src/routers/admin.py

+    applicant: str = Body(),
+    score: float = Body(),


Chore [can handle later]: I think we can update this to use a model for the Request Body.

taesungh · 2024-12-21T04:37:37Z

apps/api/src/routers/admin.py

@@ -97,6 +184,45 @@ async def applicant_summary() -> dict[ApplicantStatus, int]:
    return await summary_handler.applicant_summary()


+@router.post("/hackerReview")


Similar note on grouping routes, e.g. /review/hacker

taesungh · 2024-12-21T04:41:20Z

apps/api/src/routers/admin.py

+    records = await mongodb_handler.retrieve(
+        Collection.USERS,
+        {"status": Status.REVIEWED},
+        ["_id", "avg_score", "first_name"],


Question: I think it would make more sense for average score to be computed in the API than stored in the database?

taesungh · 2024-12-21T04:48:56Z

apps/api/src/routers/admin.py

+        _include_num_reviewers(record)
+        _include_avg_score(record)
+        _delete_reviews(record)


Suggestion: given the increasing amount of code in this router file, it might help to move these functions to a separate module, e.g. somewhere like src/admin/applicant_review_processor.py.

taesungh · 2024-12-21T04:51:55Z

apps/api/src/routers/admin.py

+@router.post("/releaseHackerDecisions", dependencies=[Depends(require_director)])
+async def release_hacker_decisions() -> None:
+    """Update applicant status based on decision and send decision emails."""


Note: I appreciate the eagerness in putting everything together, but it might help to break down the work into smaller pieces so new features can be reviewed incrementally (reduces reviewer fatigue and the possibility of overlooking something in the large amount of changes). Same applies to implementing setting the thresholds.

taesungh · 2024-12-21T04:57:32Z

apps/site/src/app/admin/applicants/page.tsx

-export { default as default } from "./Applicants";
+export { default as default } from "./HackerApplicants";


Question: do you have a long-term site structure in mind for the different types of applicants? Currently, this places the hacker applicants page at /admin/applicants. Similar to the suggestion on grouping the API endpoints, the actual page paths could also be grouped in a similar way, and the base page can be used to show more summary charts (#399). Might help to set up some placeholder pages for how we think things will eventually be laid out before fleshing things out more.

I believe @waalbert is working on making a tab each for Hackers, Mentors, and Volunteers. I will coordinate with him once that has been implemented, but in the meantime I will split up this PR.

- Added admin tests for /hackerApplicants and /submitHacker routes

waalbert · 2025-01-04T02:59:30Z

Closing as completed in #514, #526, and #527

IanWearsHat added 10 commits December 18, 2024 02:33

Created new schema for reviews

bab77ec

Updating backend from frontend works

abb3122

Created query for finding average score

47b3f0e

Set avg_score in applicant once avg is calculated

f8f8db6

Reviews show up in the frontend

72121b7

Created reviewed indicators for applicants

45e4f06

Moved to same backend structure

15244f7

Added ability to set thresholds and change decisions based on them

68aed90

Removed reject and added response message

21389c7

Fixed applicants route

0c3ae97

IanWearsHat linked an issue Dec 20, 2024 that may be closed by this pull request

[Admin] Allow 2 reviewers to review Hackers #504

Closed

IanWearsHat requested review from taesungh and waalbert December 20, 2024 18:07

taesungh reviewed Dec 21, 2024

View reviewed changes

- Added decision status to applicant cards

ceb7330

- Added admin tests for /hackerApplicants and /submitHacker routes

IanWearsHat mentioned this pull request Dec 21, 2024

[Admin] Hacker Applicants Overview #514

Merged

IanWearsHat marked this pull request as draft December 21, 2024 07:33

This was referenced Dec 23, 2024

[Admin] New hacker review logic and indicator #526

Merged

[Admin] Set hacker score thresholds #527

Merged

waalbert closed this Jan 4, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Admin] 2 Hacker reviewers #510

[Admin] 2 Hacker reviewers #510

IanWearsHat commented Dec 20, 2024 •

edited

Loading

github-actions bot commented Dec 20, 2024 •

edited

Loading

taesungh left a comment

taesungh Dec 21, 2024

taesungh Dec 21, 2024

taesungh Dec 21, 2024

taesungh Dec 21, 2024

taesungh Dec 21, 2024

taesungh Dec 21, 2024

taesungh Dec 21, 2024

taesungh Dec 21, 2024

IanWearsHat Dec 21, 2024

waalbert commented Jan 4, 2025

		@@ -73,6 +83,83 @@ async def applicants(
		raise RuntimeError("Could not parse applicant data.")


		@router.get("/hackerApplicants")

		@@ -97,6 +184,45 @@ async def applicant_summary() -> dict[ApplicantStatus, int]:
		return await summary_handler.applicant_summary()


		@router.post("/hackerReview")

		export { default as default } from "./Applicants";
		export { default as default } from "./HackerApplicants";

[Admin] 2 Hacker reviewers #510

[Admin] 2 Hacker reviewers #510

Conversation

IanWearsHat commented Dec 20, 2024 • edited Loading

Testing

github-actions bot commented Dec 20, 2024 • edited Loading

taesungh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

waalbert commented Jan 4, 2025

IanWearsHat commented Dec 20, 2024 •

edited

Loading

github-actions bot commented Dec 20, 2024 •

edited

Loading