Add Training Days page to contest server with scores and sorting #94
Conversation
- Add 'Training Days' menu item to contest server training programs - Create new TrainingDaysHandler and training_days.html template - Show ongoing/upcoming trainings section with countdown timers - Show past trainings with training score, home score, and total score - Add training score and home score columns to admin archive table - Add assigned date and training source columns to task archive table - Add sorting capability for assigned date and source columns Co-Authored-By: Ron Ryvchin <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR enhances the training program feature by introducing a dedicated training days page with score tracking and improved user interface. It adds home and training score calculations to admin handlers and templates, implements a new TrainingDaysHandler for displaying ongoing/upcoming and past trainings with eligibility checks and timing computations, refactors the training overview page with sortable columns and improved progress bar styling, and includes client-side functionality for countdowns and sorting. Changes
Sequence DiagramsequenceDiagram
participant Client
participant TrainingDaysHandler
participant Participation
participant ArchivedStudentRanking
participant Template
Client->>TrainingDaysHandler: GET /training_days
activate TrainingDaysHandler
alt Ongoing/Upcoming Trainings
TrainingDaysHandler->>TrainingDaysHandler: check_training_day_eligibility
TrainingDaysHandler->>TrainingDaysHandler: compute_effective_times
TrainingDaysHandler->>Template: render ongoing_upcoming_trainings
end
alt Past Trainings (Archived)
TrainingDaysHandler->>Participation: get cached task_scores
TrainingDaysHandler->>ArchivedStudentRanking: fetch archived task_scores
TrainingDaysHandler->>TrainingDaysHandler: _build_past_training_info
TrainingDaysHandler->>Template: render past_trainings with scores
end
deactivate TrainingDaysHandler
Template->>Client: Return training_days.html
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Remove Total column from past trainings table (same as home score) - Add has_ended flag to handler for trainings with actual_phase >= 1 - Show red stripe and 'ENDED' label for ended trainings - Change LIVE pill to green color for active trainings Co-Authored-By: Ron Ryvchin <[email protected]>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@cms/server/contest/templates/training_days.html`:
- Around line 131-140: The current template uses contest_url("tasks", task.name,
"description") for every task but archived training days (pt where contest_id is
None) only have data in archived_tasks_data and will 404 when
TaskDescriptionHandler calls get_task; update the template logic around pt.tasks
(or where pt is iterated) to detect archived trainings (pt.contest_id is None)
and render task.name as plain text (no anchor) for those entries, preserving the
existing score span, so links are only generated for non-archived trainings that
have live contest tasks and continue to use contest_url for those.
In `@cms/server/contest/templates/training_program_overview.html`:
- Around line 111-119: The aria-valuenow on the .progress-track should be
clamped to 0–100 like the fill width; change the template to compute a clamped
value from the existing percentage (e.g. clamped = min(max(percentage, 0), 100))
and use that clamped value for aria-valuenow and for the progress-fill width
instead of the raw {{ percentage }} so both the accessibility attribute and
visual width stay consistent; update the progress-track div and progress-fill
usage of {{ percentage }} to reference the clamped variable.
🧹 Nitpick comments (5)
cms/server/contest/handlers/trainingprogram.py (2)
174-281: Consider extracting shared logic withTrainingProgramOverviewHandler.The
get()method duplicates significant logic fromTrainingProgramOverviewHandler(student lookup, eligibility checks, timing computations, sorting). While the PR notes mention this for future refactor, extracting common helpers (e.g.,_get_student_for_program,_compute_training_day_timing) would reduce maintenance burden.
298-303: Potential N+1 query forArchivedStudentRanking.This query runs once per past training day inside the loop (line 208). For students with many past trainings, this could cause performance issues. Consider fetching all
ArchivedStudentRankingrecords for the student in a single query before the loop and indexing bytraining_day_id.♻️ Suggested optimization
# Before the loop in get(), fetch all rankings at once: if student is not None: all_rankings = { r.training_day_id: r for r in self.sql_session.query(ArchivedStudentRanking) .filter(ArchivedStudentRanking.student_id == student.id) .all() } else: all_rankings = {} # Then pass to _build_past_training_info and use: archived_ranking = all_rankings.get(training_day.id)cms/server/contest/templates/training_days.html (2)
121-121: Unuseddata-total-scoreattribute.The
data-total-scoreattribute is included but the "Total" column was removed per PR objectives. This is harmless but could be cleaned up for clarity.♻️ Suggested cleanup
- <tr data-training-score="{{ pt.training_score }}" data-home-score="{{ pt.home_score }}" data-total-score="{{ pt.max_score }}" data-date="{{ pt.start_time.timestamp() if pt.start_time else 0 }}"> + <tr data-training-score="{{ pt.training_score }}" data-home-score="{{ pt.home_score }}" data-date="{{ pt.start_time.timestamp() if pt.start_time else 0 }}">
160-262: Consider moving styles tocws_style.css.This is a substantial block of CSS (~100 lines) that would be better maintained in the shared stylesheet. This also enables caching and reduces template size.
cms/server/admin/handlers/trainingprogram.py (1)
1792-1811: Batch archived ranking lookups to avoid N+1 queries.The per-task
ArchivedStudentRankingquery inside the loop can trigger many DB hits (and repeat work for the same training day). Prefetch by distinctsource_training_day_idand map the results once.♻️ Proposed refactor (batch ranking lookup)
# Build training scores from archived student rankings training_scores = {} - for st in student.student_tasks: - if st.source_training_day_id is not None: - archived_ranking = ( - self.sql_session.query(ArchivedStudentRanking) - .filter(ArchivedStudentRanking.training_day_id == st.source_training_day_id) - .filter(ArchivedStudentRanking.student_id == student.id) - .first() - ) - if archived_ranking and archived_ranking.task_scores: - task_id_str = str(st.task_id) - if task_id_str in archived_ranking.task_scores: - training_scores[st.task_id] = archived_ranking.task_scores[task_id_str] + source_training_day_ids = { + st.source_training_day_id + for st in student.student_tasks + if st.source_training_day_id is not None + } + archived_rankings = {} + if source_training_day_ids: + archived_rankings = { + r.training_day_id: r + for r in ( + self.sql_session.query(ArchivedStudentRanking) + .filter(ArchivedStudentRanking.training_day_id.in_(source_training_day_ids)) + .filter(ArchivedStudentRanking.student_id == student.id) + .all() + ) + } + + for st in student.student_tasks: + if st.source_training_day_id is None: + continue + archived_ranking = archived_rankings.get(st.source_training_day_id) + if archived_ranking and archived_ranking.task_scores: + task_id_str = str(st.task_id) + if task_id_str in archived_ranking.task_scores: + training_scores[st.task_id] = archived_ranking.task_scores[task_id_str]
Changed task links from contest_url (managing contest) to use the training program's URL pattern: /<training_program>/tasks/<task>/description Co-Authored-By: Ron Ryvchin <[email protected]>
…1 queries - Revert task link change (contest_url was correct) - Fix aria-valuenow clamping in progress bar (0-100 range) - Remove unused data-total-score attribute from training_days.html - Optimize N+1 query in TrainingDaysHandler (batch ArchivedStudentRanking) - Optimize N+1 query in StudentTasksHandler (batch ArchivedStudentRanking) Co-Authored-By: Ron Ryvchin <[email protected]>
… pages When inside a training day contest, show a 'Back to Training Program' button instead of the logout button, since login is now via the training program. Co-Authored-By: Ron Ryvchin <[email protected]>
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
* Add Training Days page to contest server with scores and sorting - Add 'Training Days' menu item to contest server training programs - Create new TrainingDaysHandler and training_days.html template - Show ongoing/upcoming trainings section with countdown timers - Show past trainings with training score, home score, and total score - Add training score and home score columns to admin archive table - Add assigned date and training source columns to task archive table - Add sorting capability for assigned date and source columns Co-Authored-By: Ron Ryvchin <[email protected]> * styling * Remove Total column and fix ended trainings display - Remove Total column from past trainings table (same as home score) - Add has_ended flag to handler for trainings with actual_phase >= 1 - Show red stripe and 'ENDED' label for ended trainings - Change LIVE pill to green color for active trainings Co-Authored-By: Ron Ryvchin <[email protected]> * Fix task links in past trainings to use training program URL Changed task links from contest_url (managing contest) to use the training program's URL pattern: /<training_program>/tasks/<task>/description Co-Authored-By: Ron Ryvchin <[email protected]> * Address PR feedback: revert task link, fix aria-valuenow, optimize N+1 queries - Revert task link change (contest_url was correct) - Fix aria-valuenow clamping in progress bar (0-100 range) - Remove unused data-total-score attribute from training_days.html - Optimize N+1 query in TrainingDaysHandler (batch ArchivedStudentRanking) - Optimize N+1 query in StudentTasksHandler (batch ArchivedStudentRanking) Co-Authored-By: Ron Ryvchin <[email protected]> * Replace logout button with 'Back to Training Program' in training day pages When inside a training day contest, show a 'Back to Training Program' button instead of the logout button, since login is now via the training program. Co-Authored-By: Ron Ryvchin <[email protected]> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Ron Ryvchin <[email protected]>
Summary
This PR adds a new "Training Days" page to the contest server for training programs, along with enhancements to display score information in both the contest server and admin interface.
New Training Days page (
/training_days):Task Archive table enhancements (training program overview):
Admin interface enhancements (student tasks page):
Navigation enhancement:
Updates since last revision
actual_phase >= 1data-total-scoreattribute from past trainings table rowsTrainingDaysHandlerandStudentTasksHandlerby batch-fetchingArchivedStudentRankingrecordsReview & Testing Checklist for Human
actual_phase >= 1. Confirm this threshold correctly identifies trainings that have finished.ArchivedStudentRanking.task_scores, home score fromparticipation.task_scorescache. Confirm this matches expected behavior.contest is None. Verify this correctly identifies archived trainings.Recommended test plan:
Notes
Link to Devin run: https://app.devin.ai/sessions/331e76ca9c504acca396b205c653afda
Requested by: @ronryv