Skip to content

Leaderboard #1238

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 42 commits into
base: master
Choose a base branch
from
Open

Leaderboard #1238

wants to merge 42 commits into from
+1,150 −296

Conversation

tohyzhong
Copy link

@tohyzhong tohyzhong commented Mar 17, 2025 •
edited
Loading

Description

  • Added Overall Leaderboard and Contest Leaderboards
  • XP and contest score fetching from backend
  • Top X Contest Entries to display, Top X Overall leaderboard entries to display
  • contest scores update button in ground control (under voting features)
  • Update assessment workspace Leaderboard logic (based on customised X desired contest entries to display) + csv export fix
  • Automatic XP assignment for contest winners based on XML file (fallback on default value iif XML file does not specify any, put 0 in for rank 1 to not give any XP)
  • Leaderboards are ranked such that the same rank can be given for same score/XP, and we will skip however many ranks that are duplicated. (e.g. if we have two rank 1s, the next will be rank 3)
  • Updated contest winner fetching for assessment workspace leaderboard to align with new Leaderboard page

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Code quality improvements

How to test

Checklist

  • I have tested this code
  • I have updated the documentation

Blerargh and others added 14 commits February 14, 2025 02:02
@Blerargh
added 'enable_leaderboard' columns in courses table
e80c289
@tohyzhong
Leaderboard create course config, leaderboard page routing, leaderboa…
2464ad4 
…rd sql query for all users in course
@Blerargh
added 'top_leaderboard_display' columns in courses table
e840385
@Blerargh
added 'all_user_total_xp' function for leaderboard display
0a11acd
@tohyzhong
add top leaderboard display options to course settings (select how ma…
3093544 
…ny to display in leaderboard)
@tohyzhong
added contest scores fetching and contest score calculation
fe9b9bf
@tohyzhong
Refactor query execution in assessments module for improved readability
595bdea
@Blerargh
Merge remote-tracking branch 'origin/master' into leaderboard
e760976
@Blerargh
added functions to fetch contest scoring and voting
c806f63
@Blerargh
changes to default values
f99a898
@Blerargh
updated tests
f42c4cd
@tohyzhong
Merge branch 'leaderboard' of https://github.com/source-academy/backend…
05f8e80 
… into leaderboard
@tohyzhong
Fixed xp fetching for all users
82e633f
@tohyzhong
Add top contest leaderboard display configuration and update related …
64e812b 
…tests.

Updated leaderboard fetching and exporting for assessment workspace leaderboard.
Added Leaderboard Dropdown contests fetching.
@tohyzhong tohyzhong self-assigned this Mar 17, 2025
@coveralls
Copy link

Coverage Status

coverage: 91.307% (-2.3%) from 93.605%
when pulling 64e812b on leaderboard
into 41e3ce1 on master.

Blerargh and others added 14 commits March 18, 2025 16:06
@Blerargh
Added automatic XP assignment for winning contest entries
731bf09
@tohyzhong
Implement XP assignment for winning contest entries based on contest …
0e89b53 
…voting XML and added dispatch endpoint for XP customisation
@tohyzhong
Add default value for XP values and improve XP assignment logic for c…
31d821d 
…ontest entries
@Blerargh
No tiebreak for contest scoring
9410c4f
@tohyzhong
Refactor contest scoring endpoints for authentication errors
4e78a24
@tohyzhong
Enhance leaderboard update logic and improve error handling for votin…
04f9d20 
…g questions
@tohyzhong
Refactor XP assignment logic for voting questions and set default XP …
9500bcb 
…values.

Refactor Score calculation logic to reset to 0 before calculating.
@tohyzhong
Temporary Assessment Workspace leaderboard fix for testing
ab255e0
@tohyzhong
Fixed tests for assessments (default XP to award for contests)
1166189
@tohyzhong
Refactor contest fetching logic to filter by voting question contest …
1096a05 
…numbers
@tohyzhong
Refactor leaderboard query logic to use RANK() and improve code reada…
61a4561 
…bility.

Uncommented leaderboard portions after finalising testing
@tohyzhong
temporary fix for STePS
0e1ed61
@tohyzhong
Add ranking to assessment workspace leaderboard queries and update vi…
80fcb05 
…ew helpers to include rank
@tohyzhong
Merge branch 'master' into leaderboard
97345b0
@tohyzhong tohyzhong marked this pull request as ready for review April 17, 2025 08:15
GabrielCWT
GabrielCWT requested changes Apr 28, 2025
View reviewed changes
Copy link
Contributor

@GabrielCWT GabrielCWT 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 this! Made quite a few comments, do look through them. One issue I have is the lack of pagination, sending over 1000 students in a json for each request seems expensive to me. Also would it be possible to implement some tests? Some errors I caught would've been caught if there were tests.

@tohyzhong tohyzhong requested a review from GabrielCWT April 30, 2025 10:31
GabrielCWT
GabrielCWT requested changes May 1, 2025
View reviewed changes
Copy link
Contributor

@GabrielCWT GabrielCWT left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for reviewing my comments so quickly. Just a few more minor nits. Also do update the tests to make sure they pass.

@tohyzhong tohyzhong requested a review from GabrielCWT May 4, 2025 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Blerargh Blerargh Blerargh left review comments

@GabrielCWT GabrielCWT Awaiting requested review from GabrielCWT

Requested changes must be addressed to merge this pull request.

Assignees

@Blerargh Blerargh

@tohyzhong tohyzhong

Labels
None yet
Projects
SA 2526 CP3108 P09: Leaderboard
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tohyzhong @coveralls @Blerargh @GabrielCWT