Add StudentTask model for task archive feature #87
Conversation
This commit implements the task archive feature for training programs: Database changes: - Add StudentTask model to track which tasks each student has access to - Add student_tasks relationship to Student and Task models - Add SQL migration for student_tasks table (version 50) - Add update_50.py for dump imports Core functionality: - When a student starts a training day, visible tasks are automatically added to their StudentTask records - Task archive now filters tasks based on StudentTask records only - Score calculations only include tasks in the student's archive Admin UI: - Add StudentTasksHandler to view/manage student's task archive - Add AddStudentTaskHandler to manually assign tasks to students - Add RemoveStudentTaskHandler to remove tasks from student's archive - Add BulkAssignTaskHandler to assign tasks to all students with a tag - Add student_tasks.html template for viewing/managing student tasks - Add bulk_assign_task.html template for bulk task assignment - Add link to task archive from student details page - Add link to bulk assign from students list page 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 a StudentTask ORM and DB migration; links Student/Task to StudentTask; admin UI, handlers, and routes to add/remove/bulk-assign student tasks; automatic assignment of training-day tasks on contest start; task-archive progress calculation and training-program access checks requiring StudentTask records. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant StartHandler
participant ParticipationDB as Participation
participant StudentDB as Student
participant TrainingDay
participant TaskDB as Task
participant StudentTaskDB as StudentTask
User->>StartHandler: POST /contest/start
StartHandler->>ParticipationDB: update starting_time and metadata
StartHandler->>StudentDB: query Student by user + training_program
alt Student exists
StartHandler->>TrainingDay: get visible tasks
TrainingDay-->>StartHandler: tasks[]
loop for each task
StartHandler->>StudentTaskDB: exists(student,task)?
alt not exists
StartHandler->>StudentTaskDB: insert StudentTask(student, task, source_training_day, assigned_at)
end
end
else Student missing
StartHandler-->>StartHandler: log warning and skip
end
sequenceDiagram
participant Admin
participant AdminHandler
participant StudentDB as Student
participant StudentTaskDB as StudentTask
participant Notifier
Admin->>AdminHandler: GET/POST manage tasks / bulk-assign
AdminHandler->>StudentDB: validate student(s) / tags
AdminHandler->>StudentTaskDB: query/create/delete assignments (dedupe)
alt assignments changed
AdminHandler->>Notifier: emit notification
AdminHandler-->>Admin: success response
else error / duplicate
AdminHandler-->>Admin: error response
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
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (17)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2026-01-09T19:33:59.218ZApplied to files:
🧬 Code graph analysis (11)cms/server/admin/handlers/__init__.py (1)
cms/server/util.py (2)
cmscontrib/importing.py (1)
cms/db/__init__.py (1)
cms/server/admin/handlers/trainingprogram.py (3)
cms/server/contest/handlers/main.py (3)
cms/db/task.py (1)
cms/db/student.py (2)
cms/server/contest/handlers/contest.py (3)
cms/server/contest/handlers/trainingprogram.py (3)
cms/db/student_task.py (4)
🪛 Ruff (0.14.11)cms/server/admin/handlers/trainingprogram.py1819-1819: Abstract (TRY301) 1819-1819: Avoid specifying long messages outside the exception class (TRY003) 1831-1831: Abstract (TRY301) 1831-1831: Avoid specifying long messages outside the exception class (TRY003) 1842-1842: Do not catch blind exception: (BLE001) 1951-1951: Abstract (TRY301) 1951-1951: Avoid specifying long messages outside the exception class (TRY003) 1955-1955: Abstract (TRY301) 1955-1955: Avoid specifying long messages outside the exception class (TRY003) 1968-1968: Abstract (TRY301) 1968-1968: Avoid specifying long messages outside the exception class (TRY003) 1994-1994: Do not catch blind exception: (BLE001) 🔇 Additional comments (31)
✏️ Tip: You can disable this entire section by setting 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 |
Fix CI failure by adding Task.student_tasks to the update specification in update_task(). Student task assignments are managed separately via the admin UI and should not be updated during task import. Co-Authored-By: Ron Ryvchin <[email protected]>
…timization, UI improvements - Hide tasks from sidebar in training programs (contest.html) - Add access control for direct URL access to tasks in training programs (contest.py) - Remove version number increase (keep at 49) and delete update_50.py - Optimize score calculation to use ParticipationTaskScore cache (trainingprogram.py) - Change 'bulk assign' terminology to 'Assign Task to a Group' and use tagify (bulk_assign_task.html) Co-Authored-By: Ron Ryvchin <[email protected]>
CMS Base.__init__ skips foreign key columns, so student_id, task_id, and source_training_day_id cannot be passed as constructor arguments. Fixed all three places where StudentTask is created to set these attributes after instantiation. Co-Authored-By: Ron Ryvchin <[email protected]>
Shows percentage and score (e.g., '75.0% (150.0/200.0)') for each student, plus a 'Task Archive' button linking to their task archive page. Uses ParticipationTaskScore cache for efficient score lookup. Co-Authored-By: Ron Ryvchin <[email protected]>
- Add calculate_task_archive_progress() utility function in cms/server/util.py - Update admin handler to use shared utility - Combine progress and details link: '50.0% (100.0/200.0) [details]' - Fix column names: 'First Name' and 'Last Name' (proper capitalization) Co-Authored-By: Ron Ryvchin <[email protected]>
…utility Extended the utility to optionally return per-task breakdown (include_task_details=True). Both admin and contest handlers now use the same shared logic for score calculation. 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: 0
🧹 Nitpick comments (6)
cms/server/contest/handlers/contest.py (1)
429-444: Access control implementation looks correct.The logic properly enforces that:
- A
Studentrecord must exist linking the user to the training program- The task must be in the student's task archive (
student_tasks)One minor consideration: the query retrieves the full
Studentobject and then accessesstudent.student_tasks, which triggers a second query if not eagerly loaded. For a single student this is acceptable, but if this becomes a hot path, consider using a subquery orexists()check directly.🔧 Optional: More efficient task access check using exists()
If performance becomes a concern, you could check task access directly without loading all student tasks:
from sqlalchemy import exists from cms.db import StudentTask # For training programs, check if student has a StudentTask record if self.training_program is not None: has_access = self.sql_session.query( exists() .where(StudentTask.task_id == task.id) .where(StudentTask.student_id == Student.id) .where(Student.participation_id == Participation.id) .where(Participation.contest_id == self.contest.id) .where(Participation.user_id == self.current_user.user_id) .where(Student.training_program_id == self.training_program.id) ).scalar() return has_accesscms/server/util.py (2)
44-44: Unused import:ParticipationTaskScoreis not directly referenced.The import is added but
ParticipationTaskScoreis not explicitly used in the code—it's accessed indirectly throughparticipation.task_scores. If this import is for documentation purposes or future type annotations, consider adding a type hint to make the usage explicit; otherwise, it can be removed.
225-237: Potential inconsistency betweentask_countand score accumulation.
task_countis set tolen(student_task_ids)(all tasks assigned to the student), butmax_scoreandtotal_scoreonly accumulate for tasks that are also incontest.get_tasks(). If a student has tasks assigned that don't belong to the contest, the count will be inflated relative to the score totals.If this is intentional (e.g., counting all assigned tasks regardless of contest membership), consider adding a clarifying comment. Otherwise, consider counting tasks inside the loop:
Suggestion to count tasks in-loop
total_score = 0.0 max_score = 0.0 - task_count = len(student_task_ids) + task_count = 0 task_scores = [] if include_task_details else None for task in contest.get_tasks(): if task.id not in student_task_ids: continue + task_count += 1 max_task_score = task.active_dataset.score_type_object.max_score \ if task.active_dataset else 100.0cms/server/admin/handlers/trainingprogram.py (3)
376-383: Consider eager loading to avoid N+1 queries.This loop accesses
student.student_tasksandstudent.participation.task_scoresfor each student, which may trigger lazy loading queries. For training programs with many students, consider eager loading these relationships:from sqlalchemy.orm import joinedload students = ( self.sql_session.query(Student) .filter(Student.training_program == training_program) .options( joinedload(Student.student_tasks), joinedload(Student.participation).joinedload(Participation.task_scores) ) .all() ) for student in students: student_progress[student.id] = calculate_task_archive_progress(...)
1960-1969: Filter students at database level for better performance.Currently loads all students and filters in Python, bypassing the GIN index on
student_tags. Use SQLAlchemy's array containment to filter at the database level:Use database-level filtering with the GIN index
- # Find all students with the given tag - students_with_tag = ( - self.sql_session.query(Student) - .filter(Student.training_program == training_program) - .all() - ) - - # Filter to students that have the tag - matching_students = [ - s for s in students_with_tag if tag in s.student_tags - ] + # Find all students with the given tag (uses GIN index) + matching_students = ( + self.sql_session.query(Student) + .filter(Student.training_program == training_program) + .filter(Student.student_tags.any(tag)) + .all() + )
1974-1991: Consider batch optimization for existence checks.The loop performs a query per student to check for existing assignments. For bulk operations with many students, this could be optimized by pre-fetching all existing assignments:
Pre-fetch existing assignments to reduce queries
# Pre-fetch existing assignments for this task existing_student_ids = set( row[0] for row in self.sql_session.query(StudentTask.student_id) .filter(StudentTask.task_id == task.id) .filter(StudentTask.student_id.in_([s.id for s in matching_students])) .all() ) assigned_count = 0 for student in matching_students: if student.id not in existing_student_ids: student_task = StudentTask(assigned_at=make_datetime()) student_task.student_id = student.id student_task.task_id = task.id student_task.source_training_day_id = None self.sql_session.add(student_task) assigned_count += 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
cms/db/__init__.pycms/db/student.pycms/db/student_task.pycms/db/task.pycms/server/admin/handlers/__init__.pycms/server/admin/handlers/trainingprogram.pycms/server/admin/templates/bulk_assign_task.htmlcms/server/admin/templates/student.htmlcms/server/admin/templates/student_tasks.htmlcms/server/admin/templates/training_program_students.htmlcms/server/contest/handlers/contest.pycms/server/contest/handlers/main.pycms/server/contest/handlers/trainingprogram.pycms/server/contest/templates/contest.htmlcms/server/util.pycmscontrib/importing.pycmscontrib/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/__init__.pycms/server/admin/handlers/trainingprogram.py
🧬 Code graph analysis (10)
cmscontrib/importing.py (1)
cms/db/task.py (1)
Task(56-365)
cms/db/__init__.py (1)
cms/db/student_task.py (1)
StudentTask(39-96)
cms/db/student.py (2)
cms/db/training_program.py (1)
TrainingProgram(36-81)cms/db/student_task.py (1)
StudentTask(39-96)
cms/server/admin/handlers/__init__.py (1)
cms/server/admin/handlers/trainingprogram.py (4)
StudentTasksHandler(1733-1781)AddStudentTaskHandler(1784-1856)RemoveStudentTaskHandler(1859-1911)BulkAssignTaskHandler(1914-2007)
cms/server/contest/handlers/main.py (3)
cms/db/student.py (1)
Student(38-89)cms/db/student_task.py (1)
StudentTask(39-96)cms/server/contest/handlers/contest.py (1)
get_visible_tasks(449-468)
cms/server/util.py (4)
cms/db/student.py (1)
Student(38-89)cms/db/task.py (2)
Task(56-365)score_type_object(574-594)cms/db/user.py (1)
Participation(179-345)cms/db/scorecache.py (1)
ParticipationTaskScore(36-117)
cms/db/task.py (1)
cms/db/student_task.py (1)
StudentTask(39-96)
cms/server/contest/handlers/contest.py (2)
cms/db/student.py (1)
Student(38-89)cms/db/user.py (1)
Participation(179-345)
cms/server/admin/handlers/trainingprogram.py (6)
cms/db/student.py (1)
Student(38-89)cms/db/student_task.py (1)
StudentTask(39-96)cms/db/training_day.py (1)
TrainingDay(63-123)cms/db/training_day_group.py (1)
TrainingDayGroup(37-82)cms/server/util.py (2)
get_all_student_tags(64-79)calculate_task_archive_progress(196-261)cms/db/training_program.py (1)
TrainingProgram(36-81)
cms/db/student_task.py (4)
cms/db/base.py (1)
Base(67-323)cms/db/student.py (1)
Student(38-89)cms/db/task.py (1)
Task(56-365)cms/db/training_day.py (1)
TrainingDay(63-123)
🪛 Ruff (0.14.10)
cms/server/admin/handlers/trainingprogram.py
1819-1819: Abstract raise to an inner function
(TRY301)
1819-1819: Avoid specifying long messages outside the exception class
(TRY003)
1831-1831: Abstract raise to an inner function
(TRY301)
1831-1831: Avoid specifying long messages outside the exception class
(TRY003)
1842-1842: Do not catch blind exception: Exception
(BLE001)
1951-1951: Abstract raise to an inner function
(TRY301)
1951-1951: Avoid specifying long messages outside the exception class
(TRY003)
1955-1955: Abstract raise to an inner function
(TRY301)
1955-1955: Avoid specifying long messages outside the exception class
(TRY003)
1972-1972: Abstract raise to an inner function
(TRY301)
1972-1972: Avoid specifying long messages outside the exception class
(TRY003)
1993-1993: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (30)
cmscontrib/importing.py (1)
316-317: LGTM!The addition of
Task.student_tasks: Falseto the update spec is consistent with the existing pattern for relationships managed separately (e.g.,Task.training_day,Task.participation_scores). This correctly prevents the loader from overwriting admin-managed student task assignments during imports.cms/db/student_task.py (1)
39-96: Well-structured ORM model.The
StudentTaskmodel is well-designed with appropriate constraints:
- Composite unique constraint on
(student_id, task_id)prevents duplicate assignments- Cascade delete behaviors are correct: deleting a student or task removes the assignment, while deleting a training day preserves the assignment (sets
source_training_day_idto NULL)- All foreign key columns are properly indexed
Note: The
assigned_atcolumn has no default value, so callers must provide it explicitly (e.g., usingdatetime.utcnow()). This appears intentional based on the PR's mention of fixing a TypeError by setting foreign keys after instantiation.cms/db/student.py (1)
83-89: LGTM!The
student_tasksrelationship is correctly configured:
cascade="all, delete-orphan"ensures task assignments are deleted when the student is deletedpassive_deletes=Trueworks correctly with theON DELETE CASCADEforeign key constraint inStudentTaskback_populates="student"correctly references the inverse relationship inStudentTaskcms/db/task.py (1)
328-333: LGTM!The
student_tasksrelationship follows the same pattern as other one-to-many relationships inTask(e.g.,participation_scores,submissions). The cascade and passive_deletes configuration ensures proper cleanup when a task is deleted.cms/db/__init__.py (2)
57-57: LGTM!
StudentTaskis correctly added to__all__in the contest section alongside related entities (TrainingDay,TrainingDayGroup).
112-112: LGTM!The import is properly placed and follows the existing module organization pattern.
cms/server/admin/templates/student.html (1)
28-31: LGTM!The Task Archive link is correctly placed and uses the
student.student_tasksrelationship to display the count. The URL construction follows the existing patterns in this template.cmscontrib/updaters/update_from_1.5.sql (1)
524-567: LGTM!The migration correctly defines the
student_taskstable with appropriate:
- Unique constraint on
(student_id, task_id)to prevent duplicate assignments- Cascading deletes for
student_idandtask_idFKs to maintain referential integrityON DELETE SET NULLforsource_training_day_idwhich preserves manually assigned tasks when training days are deleted- Indexes on all foreign key columns for query performance
cms/server/contest/templates/contest.html (1)
183-195: LGTM!The condition correctly hides the task list from the sidebar for training programs, where task visibility is now controlled through the Task Archive feature. Students will access their assigned tasks through the dedicated Task Archive interface instead.
cms/server/admin/templates/training_program_students.html (4)
14-17: LGTM!The bulk assign task link is correctly placed and follows the existing URL pattern conventions.
51-54: LGTM!Header capitalization is now consistent ("First Name", "Last Name"), and the new "Task Archive Progress" column header is appropriately placed.
75-81: LGTM!The progress display logic correctly handles both cases:
- When tasks exist: shows percentage, scores, and details link
- When no tasks: shows "No tasks" with details link for manual assignment
The format strings
"%.1f"provide consistent one-decimal precision.
60-60: Verified:student_progressis correctly passed from the handler.The handler in
trainingprogram.pyproperly initializes and populatesstudent_progresswith the return value ofcalculate_task_archive_progress(), which returns a dict containing all expected keys:task_count,percentage,total_score, andmax_score. The template's safe access with.get(student.id, {})handles missing student entries correctly.cms/server/contest/handlers/contest.py (1)
415-417: LGTM!The docstring update accurately describes the new access control behavior for training programs.
cms/server/admin/templates/bulk_assign_task.html (2)
53-58: The placeholder value"null"is a string literal.The
value="null"on line 54 is a string. Ensure the backend handler (BulkAssignTaskHandler.post) validates this as an invalid selection. Based on the relevant code snippets, the handler appears to check for"null"string, so this is consistent.
15-36: LGTM! Tagify initialization is well-configured.The JavaScript properly checks for Tagify availability before initialization, uses
enforceWhitelist: trueto restrict input to valid tags, and configures the dropdown appropriately.cms/server/admin/handlers/__init__.py (2)
178-181: LGTM! Handler exports follow existing patterns.The new handler imports are properly added and maintain consistency with the existing export style.
341-344: LGTM! URL routes are well-structured.The new routes follow the established URL pattern conventions for the training program section. The route hierarchy (
/training_program/{id}/student/{id}/tasks,/training_program/{id}/bulk_assign_task) is consistent with existing patterns.cms/server/admin/templates/student_tasks.html (1)
1-79: LGTM! Template is well-structured with proper security controls.The template correctly implements:
- CSRF protection on all forms
- Permission checks on submit buttons
- Confirmation dialog for destructive remove action
- Clean table layout with relevant information (task, source, date, actions)
cms/server/contest/handlers/trainingprogram.py (2)
57-65: LGTM! Student lookup query is correct.The query properly joins
StudenttoParticipationand applies the necessary filters to find the student record for the current user within the specific training program.
67-81: Good refactor to use shared utility.Using
calculate_task_archive_progresscentralizes the score calculation logic, improving maintainability. The fallback to zeros when no student record exists is appropriate.cms/server/contest/handlers/main.py (3)
348-351: LGTM! Training day task assignment is correctly integrated.The helper method is called before the commit, ensuring all StudentTask records are persisted atomically with the participation start time update.
391-405: LGTM! Correct pattern for creating StudentTask with FK constraints.The code correctly:
- Uses a set for O(1) lookup of existing task IDs
- Creates
StudentTaskwithassigned_atfirst, then sets FK attributes (per CMS ORM pattern noted in comments)- Adds informative logging for debugging
380-385: This warning and graceful degradation pattern is intentional and appropriate.The codebase consistently handles missing Student records by degrading gracefully. Student records are created on-demand by admin handlers, not automatically when users participate. This design is reflected across multiple handlers:
contest.py(returns False),trainingprogram.py(shows zero scores), andmain.py(returns early). The warning appropriately alerts about the missing Student record while the early return prevents null-pointer errors when accessingstudent.student_tasks.cms/server/util.py (1)
196-261: LGTM on the overall implementation.The function is well-documented and handles edge cases appropriately (null
active_dataset, emptymax_score). The use of dictionaries for O(1) lookups is efficient.cms/server/admin/handlers/trainingprogram.py (5)
31-33: LGTM on imports.The new imports correctly bring in
StudentTaskandcalculate_task_archive_progressneeded for the new handlers.
1733-1781: LGTM onStudentTasksHandler.The handler correctly validates the participation and student, computes available tasks for the dropdown, and sorts assigned tasks by assignment date.
1833-1840: LGTM on StudentTask creation pattern.The comment correctly documents the CMS Base.init behavior that requires FK columns to be set after instantiation. This matches the PR fix for the TypeError issue.
1859-1911: LGTM onRemoveStudentTaskHandler.The handler correctly validates the chain of participation → student → student_task before deletion. The success notification provides good feedback to the admin.
1842-1847: Exception handling follows established codebase pattern.The broad
except Exceptionclauses align with the existing pattern in this file (e.g., lines 133, 399, 673). Based on learnings, this is the standard approach incms/server/admin/handlers/**/*.py. The static analysis hints (BLE001, TRY301, TRY003) can be safely disregarded for consistency.Also applies to: 1993-1998
✅ Actions performedReview triggered.
|
ronryv
force-pushed
the
devin/1768218330-student-task-archive
branch
from
c656f98 to
44fc53c
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Add StudentTask model for task archive feature
Summary
This PR implements a task archive feature for training programs where students only see tasks they've encountered in training days or that were manually assigned by an admin.
Database changes:
StudentTaskmodel to track which tasks each student has access tostudent_id,task_id,source_training_day_id(null = manual assignment),assigned_atupdate_from_1.5.sql(no version bump - will be merged with training_program branch)Core functionality:
Admin UI:
Updates since last revision
StudentTask.__init__()was receiving foreign key columns as constructor arguments. CMS's customBase.__init__skips foreign key columns, sostudent_id,task_id, andsource_training_day_idmust be set as attributes after instantiation. Fixed in all 3 places where StudentTask is created.contest.html)can_access_task()to block direct URL access to unassigned tasksupdate_50.pysince training_program branch will be merged as a wholeParticipationTaskScorecache lookup for efficient score calculationcalculate_task_archive_progress()intocms/server/util.py- now used by both admin students page AND contest training program overview pageReview & Testing Checklist for Human
/tasks/taskname/description) returns 404 for tasks not in student's archiveRecommended test plan:
Notes
source_training_day_idfield is nullable: null means manual assignment, set means the task was assigned when the student started that training dayBase.__init__skips foreign key columns - when creating StudentTask objects, foreign keys must be set as attributes after instantiation, not passed to constructorParticipationTaskScorecache - ensure scoring service is running to keep cache updatedLink to Devin run: https://app.devin.ai/sessions/08206c8f40124bdba9d0302ea6a6792d
Requested by: Ron Ryvchin (@ronryv)
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.