Add archive training day feature with combined ranking #91
Conversation
This commit implements the archive training day feature which allows: - Archiving a training day by extracting attendance and ranking data - Storing attendance data (status, location, delay time, delay reasons) - Storing ranking data (tag, task scores, score history) - Deleting the contest and participations after archiving - Displaying archived training days separately from active ones - New Attendance tab to view attendance data for all archived training days New database models: - ArchivedAttendance: stores attendance data for archived training days - ArchivedStudentRanking: stores ranking data for archived training days Changes to TrainingDay model: - Added name and description fields (synced with contest while it exists) - Made contest_id nullable (NULL after archiving) New handlers: - ArchiveTrainingDayHandler: handles archiving with IP selection for location - TrainingProgramAttendanceHandler: displays attendance data with filtering UI updates: - Training days page now shows active and archived training days separately - Archive button added to active training days - New Attendance tab in training program sidebar - Archive confirmation page with class IP selection 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:
|
📝 WalkthroughWalkthroughAdds archival support: two new ORM models (ArchivedAttendance, ArchivedStudentRanking), TrainingDay fields/relationships to hold archived metadata, admin handlers and templates to perform and view archives/attendance, and a DB migration creating archival tables and adjusting training_days.contest FK. Changes
Sequence DiagramsequenceDiagram
participant Admin as Admin UI
participant Handler as ArchiveTrainingDayHandler
participant DB as Database
participant Notify as Notification System
Admin->>Handler: GET archive page (show shared IPs)
Admin->>Handler: POST archive request
Handler->>DB: Load TrainingDay & Contest
Handler->>DB: For each participation -> create ArchivedAttendance
Handler->>DB: For each student -> create ArchivedStudentRanking
Handler->>DB: Save TrainingDay archival metadata (name/description/start_time/archived_tasks_data)
Handler->>DB: Delete Contest (cascades participations/submissions)
alt success
Handler->>Notify: emit success
Notify->>Admin: show confirmation
else failure
Handler->>Notify: emit error
Notify->>Admin: show error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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. ✨ Finishing touches
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 |
- Changed ArchivedStudentRanking.student_tags from JSONB to ARRAY(Unicode) - Added GIN index for efficient filtering by tag - Updated archiving logic to store all tags as a list Co-Authored-By: Ron Ryvchin <[email protected]>
The SQLAlchemy model uses Python-side default=list, not a database-level DEFAULT, so the SQL migration should not have a DEFAULT clause. Co-Authored-By: Ron Ryvchin <[email protected]>
…n.ip The starting_ip_addresses field stores the actual IPs from which students participated, while participation.ip is for allowed IP restrictions. Co-Authored-By: Ron Ryvchin <[email protected]>
- Fix 'None' has no attribute 'id' error on training_program_tasks.html by checking if training_day.contest is not none before accessing .id - Fix attendance archiving bug: lookup student by user_id instead of participation_id since Student.participation_id points to managing contest participation, not training day participation - Add start_time field to TrainingDay model to store contest start time after archiving - Display start_time in archived training days table with sorting Co-Authored-By: Ron Ryvchin <[email protected]>
- Fix ScoreHistory.time -> ScoreHistory.timestamp in archiving code (the model uses 'timestamp' not 'time') - Fix sidebar to filter out archived training days (where contest is None) to prevent 'None' has no attribute errors when accessing td.contest Co-Authored-By: Ron Ryvchin <[email protected]>
- Add colored badges with icons for student statuses (On Time, Delayed, Missed) - Add colored badges for location (Class, Home, Both) - Color columns within each training day (status, location, reasons) - Change training day titles to show description with start date in brackets - Fix date filtering to actually filter archived training days by start_time - Order results by start_time instead of position - Add Clear button to reset date filters - Fix location to default to 'home' when no class IP is chosen during archiving - Fix location to default to 'home' when participant has no IP recorded 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: 1
🤖 Fix all issues with AI agents
In `@cms/server/admin/templates/training_program_attendance.html`:
- Around line 233-239: The template builds the student edit link with the wrong
ID: it uses student.participation.user.id but the route expects the student ID;
update the link generation to pass student.id (i.e. change the url call in the
table header from student.participation.user.id to student.id so the path
url('training_program', training_program.id, 'student', student.id, 'edit')
points to the correct handler).
🧹 Nitpick comments (6)
cms/server/admin/templates/archive_training_day.html (1)
58-66: Consider adding explicit confirmation for this destructive action.Since archiving permanently deletes the contest and participations, consider adding an additional confirmation mechanism such as requiring the admin to type the contest name or check a confirmation checkbox. This provides an extra safeguard against accidental archiving.
💡 Optional: Add confirmation checkbox
<h2>Confirm Archive</h2> <p> + <label> + <input type="checkbox" name="confirm_archive" required /> + I understand this action is irreversible and will permanently delete the contest + </label> + </p> + <p> <input type="submit" value="Archive Training Day" {% if not admin.permission_all %} disabled {% endif %} /> </p>cms/server/admin/templates/training_program_attendance.html (1)
259-269: Consider adding a fallback for missing location.When
att.locationisNone, no location is displayed. While the archiving handler should always set a value, adding a fallback would improve robustness.Optional: Add fallback for None location
<div class="location-row"> {% if att.location == "class" %} <span class="location-icon">🏫</span> Class {% elif att.location == "home" %} <span class="location-icon">🏠</span> Home {% elif att.location == "both" %} <span class="location-icon">⇄</span> Both {% elif att.location %} <span>{{ att.location }}</span> + {% else %} + <span class="location-icon">❓</span> Unknown {% endif %} </div>cms/db/training_day.py (1)
123-126: Type annotation inconsistent with nullable contest_id.The
contestrelationship type hint is"Contest"but sincecontest_idis now nullable, the relationship can beNoneafter archiving. Consider updating the type annotation for consistency.Suggested fix
- contest: "Contest" = relationship( + contest: "Contest | None" = relationship( "Contest", back_populates="training_day", )cms/server/admin/handlers/trainingprogram.py (3)
2175-2175: Consider moving import to top of file.The
DelayRequestimport is inside the method. While this works, it's more idiomatic to import at the module level for consistency with other imports.Suggested change at top of file (around line 30-33)
from cms.db import Contest, TrainingProgram, Participation, Submission, \ User, Task, Question, Announcement, Student, StudentTask, Team, \ TrainingDay, TrainingDayGroup, ArchivedAttendance, ArchivedStudentRanking, \ - ScoreHistory + ScoreHistory, DelayRequestThen remove the import from line 2175.
2228-2228: Consider moving import to top of file.Similar to
DelayRequest,ParticipationTaskScoreis imported inside the method. Consider adding it to the module-level imports for consistency.Suggested change at top of file (around line 30-33)
from cms.db import Contest, TrainingProgram, Participation, Submission, \ User, Task, Question, Announcement, Student, StudentTask, Team, \ TrainingDay, TrainingDayGroup, ArchivedAttendance, ArchivedStudentRanking, \ - ScoreHistory + ScoreHistory, DelayRequest, ParticipationTaskScoreThen remove the import from line 2228.
2305-2306: Use existing datetime import for timedelta.The
timedeltais imported inside the method, butdatetimeis already imported asdtat the top. You can usedtmodule's timedelta or import timedelta alongside at module level.Suggested fix
Change line 24 to include timedelta:
-from datetime import datetime as dt +from datetime import datetime as dt, timedeltaThen change lines 2305-2306 to:
- from datetime import timedelta query = query.filter(TrainingDay.start_time < end_date + timedelta(days=1))
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
cms/db/__init__.pycms/db/archived_attendance.pycms/db/archived_student_ranking.pycms/db/training_day.pycms/server/admin/handlers/__init__.pycms/server/admin/handlers/trainingprogram.pycms/server/admin/templates/archive_training_day.htmlcms/server/admin/templates/base.htmlcms/server/admin/templates/training_program_attendance.htmlcms/server/admin/templates/training_program_tasks.htmlcms/server/admin/templates/training_program_training_days.htmlcmscontrib/updaters/update_from_1.5.sql
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-09T19:33:59.218Z
Learnt from: ronryv
Repo: ioi-isr/cms PR: 68
File: cms/server/admin/handlers/dataset.py:1323-1387
Timestamp: 2026-01-09T19:33:59.218Z
Learning: In cms/server/admin/handlers/**/*.py, follow the existing pattern: do not emit explicit error notifications when try_commit() fails. Rely on centralized error handling and logging instead. Apply this consistently to all new and updated handlers to maintain uniform behavior and maintainability.
Applied to files:
cms/server/admin/handlers/trainingprogram.pycms/server/admin/handlers/__init__.py
🧬 Code graph analysis (4)
cms/db/archived_attendance.py (3)
cms/db/base.py (1)
Base(67-323)cms/db/training_day.py (1)
TrainingDay(65-156)cms/db/student.py (1)
Student(38-89)
cms/db/__init__.py (2)
cms/db/archived_attendance.py (1)
ArchivedAttendance(38-98)cms/db/archived_student_ranking.py (1)
ArchivedStudentRanking(38-97)
cms/db/training_day.py (2)
cms/db/archived_attendance.py (1)
ArchivedAttendance(38-98)cms/db/archived_student_ranking.py (1)
ArchivedStudentRanking(38-97)
cms/server/admin/handlers/__init__.py (1)
cms/server/admin/handlers/trainingprogram.py (2)
ArchiveTrainingDayHandler(2015-2267)TrainingProgramAttendanceHandler(2270-2341)
🪛 Ruff (0.14.11)
cms/server/admin/handlers/trainingprogram.py
2098-2098: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (25)
cms/server/admin/templates/base.html (2)
313-317: LGTM! Correctly filters out archived training days from sidebar.The filter
if td.contestappropriately excludes archived training days (wherecontest_idis null) from the sidebar navigation, since they no longer have active contests to link to.
482-482: LGTM! New Attendance navigation entry.The new menu entry follows the existing pattern and provides navigation to the attendance feature for archived training days.
cms/db/__init__.py (2)
58-58: LGTM! New archived models added to exports.The new
ArchivedAttendanceandArchivedStudentRankingmodels are correctly added to__all__alongside related training program models.
114-115: LGTM! Import statements follow existing pattern.The imports for the new archived data models are consistent with the module's import organization.
cms/server/admin/templates/training_program_tasks.html (1)
76-80: LGTM! Properly handles archived training days without contests.The updated conditional logic correctly handles the case where a training day exists but its contest has been archived (deleted). The fallback to
td.nameor"(archived)"provides clear indication to users.cms/server/admin/templates/training_program_training_days.html (4)
12-13: LGTM! Clear separation of active and archived training days.The template correctly uses
selectattr("contest")to filter active training days (those with associated contests) and presents them in a dedicated section with the ability to archive.Also applies to: 48-49
63-65: LGTM! Archive action link added.The archive link correctly routes to the archive confirmation page for each active training day.
82-112: LGTM! Well-structured archived training days section.The archived section correctly uses
rejectattr("contest")to show only archived days, and provides appropriate fallbacks for missing name/description/start_time fields.
114-118: LGTM! Table sorting enabled for archived days.The sort initialization for the archived table uses correct column index (2 for Start column in the 3-column table).
cms/server/admin/templates/archive_training_day.html (2)
12-21: LGTM! Clear warning about destructive action.The warning notification clearly communicates the consequences of archiving, including the permanent deletion of the contest and participations. This is important for preventing accidental data loss.
33-56: LGTM! Well-designed class IP selection interface.The IP selection table is sorted by student count in descending order (most common IPs first), making it easier for admins to identify classroom IPs. The fallback message when no shared IPs exist is appropriate.
cms/db/archived_attendance.py (1)
38-98: LGTM! Well-structured archival model.The model correctly implements the archival pattern with:
- Appropriate FK constraints with CASCADE delete behavior
- Composite unique constraint preventing duplicate records
- Proper indexing on foreign key columns
- Clear separation between required (
status) and optional (location,delay_time,delay_reasons) fieldscms/server/admin/handlers/__init__.py (1)
187-189: LGTM! Routes and imports follow established patterns.The new handlers are properly imported and routed, consistent with the existing training program URL structure.
Also applies to: 361-362
cmscontrib/updaters/update_from_1.5.sql (1)
568-664: LGTM! Migration is well-structured and complete.The SQL migration correctly:
- Adds archival columns to
training_days(name, description, start_time)- Changes
contest_idfrom CASCADE to SET NULL for proper archival behavior- Creates both archival tables with appropriate constraints, indexes (including GIN for tags), and foreign keys matching the ORM models
cms/db/archived_student_ranking.py (1)
38-97: LGTM! Well-designed archival ranking model.The model correctly implements:
- JSONB columns for flexible storage of task scores and score history
- PostgreSQL ARRAY with GIN index for efficient tag-based filtering
- Consistent structure with
ArchivedAttendancemodel- Proper type hints reflecting the expected data structures
cms/db/training_day.py (3)
86-92: LGTM on the nullable contest_id change.The change to make
contest_idnullable withondelete="SET NULL"correctly supports the archival workflow where the contest is deleted but the training day record persists.
99-116: LGTM on the new archival fields.The
name,description, andstart_timefields appropriately preserve contest metadata after archiving. The comments clearly document the sync-while-active / preserve-after-archive behavior.
146-156: LGTM on the archived relationships.The
cascade="all, delete-orphan"configuration ensures archived attendance and ranking data are properly cleaned up when a training day is deleted.cms/server/admin/handlers/trainingprogram.py (7)
32-33: LGTM on the new imports.The imports for
ArchivedAttendance,ArchivedStudentRanking, andScoreHistoryare necessary for the archiving functionality.
449-451: LGTM on skipping archived training days.Correctly guards against attempting to create participations for archived training days that no longer have an associated contest.
2018-2056: LGTM on the archive confirmation page handler.The validation checks and IP counting logic are well-implemented. The shared IP detection (filtering to IPs with >1 student) provides useful context for the admin to identify class IPs.
2098-2103: Broad exception handling is consistent with existing patterns.While static analysis flags
except Exception, this follows the established handler pattern in this codebase for gracefully handling various failure modes during complex operations. The notification and redirect behavior is appropriate. Based on learnings, the centralized error handling aftertry_commit()is relied upon for commit failures.
2114-2196: LGTM on attendance archival logic.The attendance extraction correctly:
- Looks up students via
Participation.user_idjoin (since Student references managing contest participation)- Handles all location edge cases (no class IPs, no participant IPs, mixed IPs)
- Defaults to "home" when IP data is unavailable
- Aggregates delay reasons from DelayRequest records
2198-2267: LGTM on ranking archival logic.The ranking extraction correctly:
- Preserves all student tags as an array for filtering
- Stores task scores keyed by task ID (as string for JSON compatibility)
- Converts score history timestamps to offsets from contest start in RWS format
2270-2341: LGTM on attendance display handler.The handler correctly:
- Filters archived training days by date range
- Builds a student-to-training-day attendance matrix
- Orders training days by start_time for consistent display
- Handles missing participation gracefully in the sort key
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
ronryv
force-pushed
the
devin/1768469828-archive-training-day
branch
from
a09184a to
7b6ee72
Compare
- Add TrainingProgramCombinedRankingHandler for main page with date filtering - Add TrainingProgramCombinedRankingHistoryHandler for JSON history endpoint - Add TrainingProgramCombinedRankingDetailHandler for student detail view - Create templates for combined ranking and detail pages - Add navigation link in sidebar - Support multi-contest format in history view (each training day is a contest) Co-Authored-By: Ron Ryvchin <[email protected]>
devin-ai-integration
bot
changed the title
The archiving code was incorrectly using contest.tasks (which is empty for training day contests) instead of training_day.tasks. Also changed to use the task_score function with the managing contest participation to compute actual scores, matching how the ranking handler works. Changes: - Use training_day.tasks instead of contest.tasks - Get managing contest participation for each user - Use task_score() to compute scores filtered by training day - Filter score history to only include tasks in the training day Co-Authored-By: Ron Ryvchin <[email protected]>
Changed the archiving code to use get_cached_score_entry which: 1. Uses the score cache efficiently instead of recalculating 2. Handles training day participations correctly (queries submissions from managing contest filtered by training_day_id) 3. Uses training_day.tasks instead of contest.tasks Also fixed ScoreHistory query to use training day participation ID since history is stored with the training day participation. Co-Authored-By: Ron Ryvchin <[email protected]>
…nd task metadata - Store task_scores for ALL visible tasks (including 0 scores) to differentiate between inaccessible tasks and 0-score tasks - Add submissions field to ArchivedStudentRanking for rendering submission tables - Add archived_tasks_data to TrainingDay to preserve scoring scheme at archive time - Update combined ranking table to show N/A for inaccessible tasks - Update combined ranking detail page to use archived task metadata and submissions - Simplify data model: use task_scores keys for visibility instead of separate field Co-Authored-By: Ron Ryvchin <[email protected]>
Co-Authored-By: Ron Ryvchin <[email protected]>
…tTask records When archiving a training day, determine which tasks should be visible to each student based on their tags (using can_access_task), rather than only including tasks where the student already has a StudentTask record. This ensures students who missed the start button still get the training day's tasks in their archive with 0 scores. Co-Authored-By: Ron Ryvchin <[email protected]>
When archiving a training day, create StudentTask records for visible tasks that the student doesn't already have. This allows students who missed the training to still submit those tasks from home via the training program management contest, mirroring the behavior of the start button. Co-Authored-By: Ron Ryvchin <[email protected]>
The contest begin/end times were set to absolute timestamps (td.start_time), but the history and submission times are stored as offsets from the original contest start. This caused: 1. Graph scale to be incorrect (data points outside the visible range) 2. Submission times to display incorrectly (negative values) Fix: Set contest begin to 0 and calculate end from the max history time, since all times are stored as offsets from contest start. Co-Authored-By: Ron Ryvchin <[email protected]>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@cms/server/admin/static/aws_style.css`:
- Around line 1223-1292: The global selectors .page-header and .btn-primary in
aws_style.css are overriding Bootstrap site-wide; scope them by nesting or
renaming—e.g., change selectors to .training-program-view .page-header and
.training-program-view .btn-primary (or rename to .tp-page-header and
.tp-btn-primary) and update any templates that render training program pages to
wrap content in the .training-program-view container so only
training-program-specific headers and primary buttons get the new flex and teal
styles while leaving Bootstrap's defaults elsewhere; keep other classes like
.filter-card/.filter-group/.btn-text unchanged.
In `@cms/server/admin/templates/training_program_combined_ranking.html`:
- Around line 86-91: The empty-state colspan currently uses "{{ 1 +
archived_training_days|length }}" which only covers the date column and archived
training day columns; update it to compute the full number of table columns by
adding the task columns as well (e.g. replace with "{{ 1 +
archived_training_days|length + tasks|length }}" or the correct task-list
variable used in this template) so the "No students found" row spans all task
columns and keeps the table aligned.
🧹 Nitpick comments (2)
cmscontrib/updaters/update_from_1.5.sql (1)
568-572: Consider populatingnameanddescriptionfor existing rows.After adding these nullable columns, existing
training_daysrows will haveNULLvalues fornameanddescription. If the application code expects these to be populated (even for active training days with contests), you may want to add anUPDATEstatement to initialize them from the associated contest data.Proposed migration addition
-- After adding the columns, populate from existing contest data UPDATE public.training_days td SET name = c.name, description = c.description FROM public.contests c WHERE td.contest_id = c.id AND td.name IS NULL;cms/server/admin/handlers/trainingprogram.py (1)
2385-2413: Consider extracting date filtering logic.The date parsing and archived training day query pattern is duplicated across
TrainingProgramAttendanceHandler,TrainingProgramCombinedRankingHandler,TrainingProgramCombinedRankingHistoryHandler, andTrainingProgramCombinedRankingDetailHandler.A helper method could reduce duplication:
♻️ Example helper extraction
def _parse_date_range(self) -> tuple[dt | None, dt | None]: """Parse start_date and end_date query arguments.""" start_date = end_date = None start_str = self.get_argument("start_date", None) end_str = self.get_argument("end_date", None) if start_str: try: start_date = dt.fromisoformat(start_str) except ValueError: pass if end_str: try: end_date = dt.fromisoformat(end_str) except ValueError: pass return start_date, end_date def _get_archived_training_days( self, training_program_id: int, start_date: dt | None, end_date: dt | None ) -> list[TrainingDay]: """Query archived training days with optional date filtering.""" query = ( self.sql_session.query(TrainingDay) .filter(TrainingDay.training_program_id == training_program_id) .filter(TrainingDay.contest_id.is_(None)) ) if start_date: query = query.filter(TrainingDay.start_time >= start_date) if end_date: query = query.filter(TrainingDay.start_time < end_date + timedelta(days=1)) return query.order_by(TrainingDay.start_time).all()
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
cms/db/archived_student_ranking.pycms/db/training_day.pycms/server/admin/handlers/__init__.pycms/server/admin/handlers/trainingprogram.pycms/server/admin/static/aws_style.csscms/server/admin/templates/archive_training_day.htmlcms/server/admin/templates/base.htmlcms/server/admin/templates/student_tasks.htmlcms/server/admin/templates/training_program_attendance.htmlcms/server/admin/templates/training_program_combined_ranking.htmlcms/server/admin/templates/training_program_combined_ranking_detail.htmlcms/server/admin/templates/training_program_task_remove.htmlcms/server/contest/templates/task_submissions.htmlcmscontrib/updaters/update_from_1.5.sql
🚧 Files skipped from review as they are similar to previous changes (2)
- cms/server/admin/templates/training_program_attendance.html
- cms/server/admin/templates/archive_training_day.html
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-09T19:33:59.218Z
Learnt from: ronryv
Repo: ioi-isr/cms PR: 68
File: cms/server/admin/handlers/dataset.py:1323-1387
Timestamp: 2026-01-09T19:33:59.218Z
Learning: In cms/server/admin/handlers/**/*.py, follow the existing pattern: do not emit explicit error notifications when try_commit() fails. Rely on centralized error handling and logging instead. Apply this consistently to all new and updated handlers to maintain uniform behavior and maintainability.
Applied to files:
cms/server/admin/handlers/trainingprogram.pycms/server/admin/handlers/__init__.py
🧬 Code graph analysis (4)
cms/db/training_day.py (5)
cms/db/base.py (1)
Base(67-323)cms/db/training_program.py (1)
TrainingProgram(36-81)cms/db/training_day_group.py (1)
TrainingDayGroup(37-82)cms/db/archived_attendance.py (1)
ArchivedAttendance(38-98)cms/db/archived_student_ranking.py (1)
ArchivedStudentRanking(38-106)
cms/server/admin/handlers/trainingprogram.py (8)
cms/db/contest.py (1)
Contest(47-384)cms/db/training_program.py (1)
TrainingProgram(36-81)cms/db/training_day.py (1)
TrainingDay(66-165)cms/db/archived_attendance.py (1)
ArchivedAttendance(38-98)cms/db/archived_student_ranking.py (1)
ArchivedStudentRanking(38-106)cms/db/scorecache.py (1)
ScoreHistory(120-168)cms/server/util.py (3)
get_all_student_tags(64-79)deduplicate_preserving_order(436-451)can_access_task(157-193)cms/server/contest/handlers/trainingprogram.py (1)
get(44-167)
cms/db/archived_student_ranking.py (3)
cms/db/base.py (1)
Base(67-323)cms/db/training_day.py (1)
TrainingDay(66-165)cms/db/student.py (1)
Student(38-89)
cms/server/admin/handlers/__init__.py (1)
cms/server/admin/handlers/trainingprogram.py (6)
RemoveTrainingDayGroupHandler(1731-1749)ArchiveTrainingDayHandler(2030-2374)TrainingProgramAttendanceHandler(2377-2447)TrainingProgramCombinedRankingHandler(2450-2543)TrainingProgramCombinedRankingHistoryHandler(2546-2595)TrainingProgramCombinedRankingDetailHandler(2598-2776)
🪛 Ruff (0.14.11)
cms/server/admin/handlers/trainingprogram.py
2113-2113: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test (debian:bookworm)
- GitHub Check: test (ubuntu:noble)
🔇 Additional comments (26)
cms/server/admin/static/aws_style.css (2)
1294-1382: Sticky header + first-column implementation looks solid.Good use of sticky positioning, z-index layering, and overflow handling to keep headers and student names visible.
1384-1498: Attendance & combined ranking visual styles look good.The badge system, date/task headers, and score cell differentiation are clear and consistent with the rest of the admin theme.
cms/server/admin/templates/training_program_task_remove.html (1)
11-14: LGTM!The additional
task.training_day.contestcheck correctly guards against accessing.contest.descriptionwhen the training day has been archived (contest_id is NULL). This aligns well with the nullable contest_id schema change.cms/server/admin/templates/base.html (2)
313-317: LGTM!The filter
if td.contestcorrectly excludes archived training days (wherecontest_idis NULL) from the sidebar navigation. This prevents displaying broken links and aligns with the schema change makingcontest_idnullable.
482-483: LGTM!New navigation entries for Attendance and Combined Ranking tabs are correctly placed within the training program menu section.
cmscontrib/updaters/update_from_1.5.sql (3)
582-620: LGTM!The
archived_attendancestable schema is well-designed:
- Unique constraint on
(training_day_id, student_id)prevents duplicate entries- Proper foreign keys with CASCADE delete for referential integrity
- Appropriate indexes on both foreign key columns
621-661: LGTM!The
archived_student_rankingstable is properly structured:
- GIN index on
student_tagsarray enables efficient tag-based queries- JSONB columns for flexible storage of scores, submissions, and history
- Consistent constraint and index patterns with the attendance table
663-668: LGTM!Adding
start_timeandarchived_tasks_datacolumns completes the archive metadata storage requirements for training days.cms/server/contest/templates/task_submissions.html (1)
411-411: LGTM!The fallback chain
training_day.contest.description if training_day.contest else (training_day.description or training_day.name)correctly handles archived training days where the contest has been deleted. This ensures a meaningful description is always displayed.cms/server/admin/templates/student_tasks.html (1)
57-61: LGTM!The conditional logic correctly handles archived training days:
- When
contestexists: renders a clickable link to the contest- When
contestis NULL: displays the training day's description/name with a sensible "Archived Training Day" fallbackThis prevents broken links and provides clear context for archived sources.
cms/server/admin/templates/training_program_combined_ranking.html (1)
8-22: Filter controls look good.
Clear date inputs + reset link make the flow straightforward for admins.cms/server/admin/templates/training_program_combined_ranking_detail.html (1)
317-412: JS wiring for the archived detail view looks solid.
The override/init flow cleanly adapts the existing renderer to archived data.cms/db/training_day.py (1)
87-125: Archival metadata fields align with the new workflow.
Persisting name/description/start_time/tasks data after contest deletion is consistent with the archive semantics.cms/db/archived_student_ranking.py (1)
38-105: Model definition looks consistent with the archival requirements.
Constraints and column structure are coherent for per‑student historical data.cms/server/admin/handlers/__init__.py (1)
364-368: Routes for archive/attendance/combined ranking views are wired cleanly.cms/server/admin/handlers/trainingprogram.py (11)
24-49: LGTM!Import additions are well-organized and align with the new archiving functionality. The utilities (
can_access_task,get_all_student_tags, etc.) and models (ArchivedAttendance,ArchivedStudentRanking, etc.) are appropriately imported for the new handlers.
464-466: LGTM!Good defensive check to skip archived training days (where
contestisNone) when adding a new student. This prevents attempting to create participations for contests that no longer exist.
2030-2071: LGTM!The GET handler correctly validates ownership, checks archive status, and computes shared IPs for class IP selection. The logic to identify IPs with more than one student (
count > 1) effectively filters for likely classroom IPs.
2098-2127: Archive operation is well-structured.The archiving flow correctly:
- Preserves contest metadata on the TrainingDay before deletion
- Archives attendance and ranking data
- Deletes the contest (cascading to participations)
The broad
Exceptioncatch at line 2113 (flagged by static analysis) follows the existing codebase pattern for form submission handlers. Based on learnings, this is consistent with how other handlers in this file manage errors.
2129-2210: LGTM!The attendance archival logic correctly handles:
- Student lookup via
user_id(sinceStudent.participation_idreferences the managing contest, not training day)- Location determination with proper handling of edge cases (no class IPs, no IP recorded, mixed IPs)
- Delay reason aggregation from
DelayRequestrecords
2212-2298: Well-designed ranking archival logic.Key aspects are correctly implemented:
can_access_taskdetermines per-student task visibility based on tagsStudentTaskrecords are created for visible tasks, enabling students who missed training to submit later- Task metadata is preserved in
archived_tasks_datafor historical accuracyThe inline import of
get_cached_score_entryat line 2228 is acceptable for avoiding potential circular imports.
2299-2374: LGTM!The score and submission data collection is well-implemented:
- Time offsets are calculated consistently from
contest.start- String keys for
task_scoresalign with JSONB storage patterns- Nullable handling (
if task_scores else None) avoids storing empty dicts
2377-2447: LGTM!Attendance display handler correctly:
- Filters for archived training days (
contest_id.is_(None))- Applies optional date range filtering
- Builds a nested dict structure for easy template access
- Gracefully handles students without participation records in sort key
2546-2595: LGTM!History endpoint correctly aggregates score history entries across archived training days and returns them in the expected JSON format for graph rendering.
2598-2776: Well-implemented detail view handler.The handler correctly:
- Calculates graph scale with
begin=0andendfrom max history time (fixing the submission time display per PR description)- Preserves original scoring scheme via
archived_tasks_data- Builds comprehensive data structures for the detail template
Note: The
Query.get()usage at line 2683 has the same SQLAlchemy 2.0 deprecation concern flagged above.
2512-2520: : The codebase uses SQLAlchemy 1.3.24 (constrained to<1.4), whereQuery.get()is the standard and correct pattern. Deprecation warnings forQuery.get()only apply to SQLAlchemy 2.0+. This pattern is used consistently throughout the codebase, including in utility functions indb/base.py, and requires no changes.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| /* --- Training Program Views (Attendance & Ranking) --- */ | ||
|
|
||
| /* Layout & Container */ | ||
| .page-header { | ||
| display: flex; | ||
| justify-content: space-between; | ||
| align-items: center; | ||
| margin-bottom: 20px; | ||
| flex-wrap: wrap; | ||
| gap: 15px; | ||
| } | ||
|
|
||
| .core_title h1 { | ||
| margin: 0; | ||
| font-size: 1.75rem; | ||
| color: #111827; | ||
| } | ||
|
|
||
| /* Filter Bar */ | ||
| .filter-card { | ||
| background-color: #f9fafb; | ||
| border: 1px solid #e5e7eb; | ||
| border-radius: 8px; | ||
| padding: 12px 16px; | ||
| margin-bottom: 24px; | ||
| display: flex; | ||
| align-items: center; | ||
| gap: 10px; | ||
| flex-wrap: wrap; | ||
| } | ||
|
|
||
| .filter-group { | ||
| display: flex; | ||
| align-items: center; | ||
| gap: 8px; | ||
| } | ||
|
|
||
| .filter-label { | ||
| font-weight: 600; | ||
| font-size: 0.85rem; | ||
| color: #4b5563; | ||
| } | ||
|
|
||
| .filter-input { | ||
| padding: 6px 10px; | ||
| border: 1px solid #d1d5db; | ||
| border-radius: 4px; | ||
| font-size: 0.9rem; | ||
| } | ||
|
|
||
| .btn-primary { | ||
| background-color: #0F766E; | ||
| color: white; | ||
| border: none; | ||
| padding: 7px 16px; | ||
| border-radius: 4px; | ||
| cursor: pointer; | ||
| font-weight: 500; | ||
| transition: background 0.2s; | ||
| } | ||
|
|
||
| .btn-primary:hover { background-color: #0d655e; } | ||
|
|
||
| .btn-text { | ||
| color: #6b7280; | ||
| text-decoration: underline; | ||
| font-size: 0.85rem; | ||
| margin-left: auto; | ||
| } | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for usage of the generic class names in HTML templates and views
find . -type f \( -name "*.html" -o -name "*.jinja" -o -name "*.jinja2" \) | head -20Repository: ioi-isr/cms
Length of output: 166
🏁 Script executed:
# Check usage of the identified generic class names across the entire repo
rg -n '\b(page-header|filter-card|filter-group|btn-primary|btn-text|table-container)\b' --type html --type cssRepository: ioi-isr/cms
Length of output: 19125
🏁 Script executed:
# Check the surrounding CSS context to see if there's already a scoping container
sed -n '1200,1500p' cms/server/admin/static/aws_style.css | head -100Repository: ioi-isr/cms
Length of output: 1940
Scope these generic selectors to training program views—they override existing Bootstrap styles used site-wide.
The classes .page-header and .btn-primary are already defined in Bootstrap CSS and used across contest templates. Adding unscoped versions in the global admin stylesheet will override those styles throughout the system:
.page-header: Used in 12+ contest templates (task submissions, communication, printing, etc.). The new flexbox definition differs from Bootstrap's border-bottom style..btn-primary: Used 40+ times across admin and contest. The new teal color (#0F766E) overrides Bootstrap's blue (#0088cc).
This will unintentionally change appearance of buttons and headers across the entire contest interface. Scope these selectors under a training-program-specific container (e.g., .training-program-view) or rename with a prefix. The other classes (.filter-card, .filter-group, .btn-text, .table-container) appear only in training program templates and are safe.
Example scoping pattern
- .page-header {
+ .training-program-view .page-header {
display: flex;
justify-content: space-between;
align-items: center;
margin-bottom: 20px;
flex-wrap: wrap;
gap: 15px;
}
- .btn-primary {
+ .training-program-view .btn-primary {
background-color: `#0F766E`;
color: white;
border: none;
padding: 7px 16px;
border-radius: 4px;
cursor: pointer;
font-weight: 500;
transition: background 0.2s;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /* --- Training Program Views (Attendance & Ranking) --- */ | |
| /* Layout & Container */ | |
| .page-header { | |
| display: flex; | |
| justify-content: space-between; | |
| align-items: center; | |
| margin-bottom: 20px; | |
| flex-wrap: wrap; | |
| gap: 15px; | |
| } | |
| .core_title h1 { | |
| margin: 0; | |
| font-size: 1.75rem; | |
| color: #111827; | |
| } | |
| /* Filter Bar */ | |
| .filter-card { | |
| background-color: #f9fafb; | |
| border: 1px solid #e5e7eb; | |
| border-radius: 8px; | |
| padding: 12px 16px; | |
| margin-bottom: 24px; | |
| display: flex; | |
| align-items: center; | |
| gap: 10px; | |
| flex-wrap: wrap; | |
| } | |
| .filter-group { | |
| display: flex; | |
| align-items: center; | |
| gap: 8px; | |
| } | |
| .filter-label { | |
| font-weight: 600; | |
| font-size: 0.85rem; | |
| color: #4b5563; | |
| } | |
| .filter-input { | |
| padding: 6px 10px; | |
| border: 1px solid #d1d5db; | |
| border-radius: 4px; | |
| font-size: 0.9rem; | |
| } | |
| .btn-primary { | |
| background-color: #0F766E; | |
| color: white; | |
| border: none; | |
| padding: 7px 16px; | |
| border-radius: 4px; | |
| cursor: pointer; | |
| font-weight: 500; | |
| transition: background 0.2s; | |
| } | |
| .btn-primary:hover { background-color: #0d655e; } | |
| .btn-text { | |
| color: #6b7280; | |
| text-decoration: underline; | |
| font-size: 0.85rem; | |
| margin-left: auto; | |
| } | |
| /* --- Training Program Views (Attendance & Ranking) --- */ | |
| /* Layout & Container */ | |
| .training-program-view .page-header { | |
| display: flex; | |
| justify-content: space-between; | |
| align-items: center; | |
| margin-bottom: 20px; | |
| flex-wrap: wrap; | |
| gap: 15px; | |
| } | |
| .core_title h1 { | |
| margin: 0; | |
| font-size: 1.75rem; | |
| color: `#111827`; | |
| } | |
| /* Filter Bar */ | |
| .filter-card { | |
| background-color: `#f9fafb`; | |
| border: 1px solid `#e5e7eb`; | |
| border-radius: 8px; | |
| padding: 12px 16px; | |
| margin-bottom: 24px; | |
| display: flex; | |
| align-items: center; | |
| gap: 10px; | |
| flex-wrap: wrap; | |
| } | |
| .filter-group { | |
| display: flex; | |
| align-items: center; | |
| gap: 8px; | |
| } | |
| .filter-label { | |
| font-weight: 600; | |
| font-size: 0.85rem; | |
| color: `#4b5563`; | |
| } | |
| .filter-input { | |
| padding: 6px 10px; | |
| border: 1px solid `#d1d5db`; | |
| border-radius: 4px; | |
| font-size: 0.9rem; | |
| } | |
| .training-program-view .btn-primary { | |
| background-color: `#0F766E`; | |
| color: white; | |
| border: none; | |
| padding: 7px 16px; | |
| border-radius: 4px; | |
| cursor: pointer; | |
| font-weight: 500; | |
| transition: background 0.2s; | |
| } | |
| .training-program-view .btn-primary:hover { background-color: `#0d655e`; } | |
| .btn-text { | |
| color: `#6b7280`; | |
| text-decoration: underline; | |
| font-size: 0.85rem; | |
| margin-left: auto; | |
| } |
🤖 Prompt for AI Agents
In `@cms/server/admin/static/aws_style.css` around lines 1223 - 1292, The global
selectors .page-header and .btn-primary in aws_style.css are overriding
Bootstrap site-wide; scope them by nesting or renaming—e.g., change selectors to
.training-program-view .page-header and .training-program-view .btn-primary (or
rename to .tp-page-header and .tp-btn-primary) and update any templates that
render training program pages to wrap content in the .training-program-view
container so only training-program-specific headers and primary buttons get the
new flex and teal styles while leaving Bootstrap's defaults elsewhere; keep
other classes like .filter-card/.filter-group/.btn-text unchanged.
| {% else %} | ||
| <tr> | ||
| <td colspan="{{ 1 + archived_training_days|length }}" style="text-align:center; padding: 20px; color: #6b7280;"> | ||
| No students found in this training program. | ||
| </td> | ||
| </tr> |
There was a problem hiding this comment.
Fix empty-state colspan to span all task columns.
The colspan only accounts for training days, so the row won’t cover per‑task columns and the table looks misaligned when tasks exist. Consider computing the full column count.
🐛 Proposed fix
{% else %}
+ {% set total_cols = namespace(value=1) %}
+ {% for td in archived_training_days %}
+ {% set total_cols.value = total_cols.value + (training_day_tasks.get(td.id, [])|length) + 1 %}
+ {% endfor %}
<tr>
- <td colspan="{{ 1 + archived_training_days|length }}" style="text-align:center; padding: 20px; color: `#6b7280`;">
+ <td colspan="{{ total_cols.value }}" style="text-align:center; padding: 20px; color: `#6b7280`;">
No students found in this training program.
</td>
</tr>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {% else %} | |
| <tr> | |
| <td colspan="{{ 1 + archived_training_days|length }}" style="text-align:center; padding: 20px; color: #6b7280;"> | |
| No students found in this training program. | |
| </td> | |
| </tr> | |
| {% else %} | |
| {% set total_cols = namespace(value=1) %} | |
| {% for td in archived_training_days %} | |
| {% set total_cols.value = total_cols.value + (training_day_tasks.get(td.id, [])|length) + 1 %} | |
| {% endfor %} | |
| <tr> | |
| <td colspan="{{ total_cols.value }}" style="text-align:center; padding: 20px; color: `#6b7280`;"> | |
| No students found in this training program. | |
| </td> | |
| </tr> |
🤖 Prompt for AI Agents
In `@cms/server/admin/templates/training_program_combined_ranking.html` around
lines 86 - 91, The empty-state colspan currently uses "{{ 1 +
archived_training_days|length }}" which only covers the date column and archived
training day columns; update it to compute the full number of table columns by
adding the task columns as well (e.g. replace with "{{ 1 +
archived_training_days|length + tasks|length }}" or the correct task-list
variable used in this template) so the "No students found" row spans all task
columns and keeps the table aligned.
Add archive training day feature with combined ranking
Summary
This PR implements the archive training day feature for training programs. When a training day is archived:
New database models
ArchivedAttendanceandArchivedStudentRankingstore the extracted data. TheTrainingDaymodel now has nullablecontest_id(SET NULL on delete) and newname/description/start_time/archived_tasks_datafields.UI changes include:
Updates since last revision
Previous updates (still relevant)
StudentTaskrecords are now created for visible tasks that the student doesn't already have. This allows students who missed the training to still submit those tasks from home via the training program management contest, mirroring the behavior of the start button.can_access_task), rather than only including tasks where the student already has aStudentTaskrecord. This ensures students who missed the start button still get the training day's tasks in their archive with 0 scores.visible_task_idsfield fromArchivedStudentRanking. The presence of a task_id key intask_scoresnow indicates the task was visible to that student (including 0 scores).archived_tasks_data) is now stored once onTrainingDayinstead of duplicated per-student inArchivedStudentRanking. This preserves the original scoring scheme at archive time.ArchivedStudentRankingnow stores submission data for each task in RWS format for rendering submission tables in the history view.task_scores.keys()for visibility andtd.archived_tasks_datafor task metadata.task_scoreskeys.get_cached_score_entry()for archiving: Per feedback, changed from usingtask_score()toget_cached_score_entry()which efficiently uses the score cache.ARRAY(Unicode)with a GIN index.Review & Testing Checklist for Human
Test history view graphs and submission times - Click "[history]" for a student with submissions and verify: (1) the score/rank graphs display with correct scale (x-axis should show time from 0 to contest duration), (2) submission times in the table display correctly (not negative or extremely large numbers).
Verify StudentTask creation during archiving - Archive a training day where some students missed the start button. Check that
StudentTaskrecords are created for those students' visible tasks (based on tags). Verify they can then submit those tasks from the training program management contest.Test archiving with students who missed start button - Create a training day, have some students NOT press start, then archive. Verify those students still get the training day's tasks in their archive with 0 scores based on their tags.
Verify tag-based task visibility - Create tasks with
visible_to_tagsrestrictions. Archive and verify students only get tasks matching their tags (same behavior as the start button).Test combined ranking table - Check that tasks visible to at least one student appear as columns, scores display correctly, and "N/A" shows for inaccessible tasks.
Recommended test plan:
visible_to_tagsrestrictions)StudentTaskrecords for their visible tasksNotes
Summary by CodeRabbit
Release Notes
New Features
Enhancements
✏️ Tip: You can customize this high-level summary in your review settings.