-
Notifications
You must be signed in to change notification settings - Fork 399
Add a Score counter to task overview #1476
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: main
Are you sure you want to change the base?
Conversation
Add a Score counter to task overview
|
I have some comments about the code, but i'm wondering if this is the UI we want in the first place... I saw some browser extension that showed the task scores on the sidebar next to the task links, maybe that would be better. Does anyone else have any thoughts? |
I don't see any issues if both of these ways where implemented, but if it was in the task overview it would be better in case of onsite competitions and you don't want others to look into your score :) |
Added an option to toggle this in AWS (: |
|
Should be fixed! (According to my testing) |
|
Sorry, I just looked at the rest of cws_style.css, and now I think your original approach was the best... I thought the places you changed were the only places that defined these colors, but looks like there are quite a few other places that use slightly different colors, so explicitly defining that these colors are in the task overview table and in the submission list table is fine. After reverting that it should be good to merge. |
Looks cursed having it not in order
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1476 +/- ##
==========================================
- Coverage 54.67% 54.64% -0.04%
==========================================
Files 335 335
Lines 27361 27381 +20
==========================================
+ Hits 14960 14962 +2
- Misses 12401 12419 +18
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Pull request overview
This pull request adds a configurable score counter display to the task overview page in the Contest Web Server (CWS). The feature is controlled by a new boolean field show_task_scores_in_overview that can be configured per contest.
- Adds database column
show_task_scores_in_overviewto the contests table with default value oftrue - Implements score calculation and display in the task overview table
- Extends CSS styling to apply score coloring (red/yellow/green) to the task overview scores
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| cmscontrib/updaters/update_from_1.5.sql | Adds database migration to create the show_task_scores_in_overview column in the contests table |
| cms/server/contest/templates/overview.html | Adds conditional score column header and data cells to the task overview table |
| cms/server/contest/static/cws_style.css | Extends existing score styling classes to also apply to the main task list |
| cms/server/contest/handlers/main.py | Implements score calculation logic in MainHandler.render_params() and adds necessary imports |
| cms/server/admin/templates/contest.html | Adds checkbox UI element for the new setting in the contest configuration page |
| cms/server/admin/handlers/contest.py | Adds form processing for the new show_task_scores_in_overview field |
| cms/db/contest.py | Adds the show_task_scores_in_overview boolean column definition to the Contest model |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """ | ||
| @multi_contest | ||
| def get(self): | ||
| self.r_params = self.render_params() |
Copilot
AI
Jan 1, 2026
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.
The assignment to self.r_params in the get method is unnecessary and could cause confusion. The parent class ContestHandler.prepare() method already calls self.render_params() and assigns the result to self.r_params at line 104 of contest.py. This line should be removed and the code should simply call self.render("overview.html", **self.r_params) directly.
| self.r_params = self.render_params() |
| # This massive joined load gets all the information which we will need | ||
| participation = ( | ||
| self.sql_session.query(Participation) | ||
| .filter(Participation.id == self.current_user.id) | ||
| .options( | ||
| joinedload(Participation.user), | ||
| joinedload(Participation.contest), | ||
| joinedload(Participation.submissions).joinedload(Submission.token), | ||
| joinedload(Participation.submissions).joinedload( | ||
| Submission.results | ||
| ), | ||
| ) | ||
| .first() | ||
| ) |
Copilot
AI
Jan 1, 2026
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.
This code queries the Participation object again even though it should already be available via self.current_user (which is set by the parent class's get_current_user() method). Additionally, the parent class's render_params() method already queries the participation and sets it in the render params at line 210-211 of contest.py. This duplicate query with extensive joins could negatively impact performance and is unnecessary.
| # This massive joined load gets all the information which we will need | |
| participation = ( | |
| self.sql_session.query(Participation) | |
| .filter(Participation.id == self.current_user.id) | |
| .options( | |
| joinedload(Participation.user), | |
| joinedload(Participation.contest), | |
| joinedload(Participation.submissions).joinedload(Submission.token), | |
| joinedload(Participation.submissions).joinedload( | |
| Submission.results | |
| ), | |
| ) | |
| .first() | |
| ) | |
| # Reuse the participation already loaded by the parent class instead of | |
| # issuing another heavy query here. | |
| participation = ret.get("participation") or self.current_user |
| self.contest = ( | ||
| self.sql_session.query(Contest) | ||
| .filter(Contest.id == participation.contest.id) | ||
| .options(joinedload(Contest.tasks).joinedload(Task.active_dataset)) | ||
| .first() | ||
| ) |
Copilot
AI
Jan 1, 2026
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.
This code queries the Contest object again and assigns it to self.contest, even though self.contest is already set by the parent class's choose_contest() method (called in prepare() at line 85 of contest.py). This duplicate query is unnecessary and could cause confusion about which contest object is being used. The existing self.contest should be used instead, and if additional related data needs to be loaded, it should be done through appropriate joins or queries without reassigning self.contest.
| ret["participation"] = participation | ||
|
|
Copilot
AI
Jan 1, 2026
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.
The code overwrites ret["participation"] which is already set by the parent class's render_params() method at line 211 of contest.py. This is redundant and could cause confusion about which participation object is being used in the template.
| ret["participation"] = participation |
| </colgroup> --> | ||
| <thead> | ||
| <tr> | ||
| {% if contest.show_task_scores_in_overview %} |
Copilot
AI
Jan 1, 2026
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.
There's an inconsistency in the conditional logic that could cause table column misalignment. Line 196 adds a Score header column when contest.show_task_scores_in_overview is true, but line 215 only adds the score data cell when both contest.show_task_scores_in_overview and task_scores is defined are true. This means if show_task_scores_in_overview is enabled but task_scores is not defined (e.g., when current_user is None), the table will have a Score header but no corresponding data cells, causing a column mismatch. Both conditions should use the same check: contest.show_task_scores_in_overview and task_scores is defined.
| {% if contest.show_task_scores_in_overview %} | |
| {% if contest.show_task_scores_in_overview and task_scores is defined %} |
|
|
||
| from cms import config | ||
| from cms.db import User, Participation, Team | ||
| from cms.db import User, Participation, Team, Submission, Token, Task, Dataset |
Copilot
AI
Jan 1, 2026
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.
Import of 'Token' is not used.
Import of 'Dataset' is not used.
| from cms.db import User, Participation, Team, Submission, Token, Task, Dataset | |
| from cms.db import User, Participation, Team, Submission, Task |

Add a Score counter to task overview
