Reduce code duplication in training program handlers #115
Conversation
- Add build_user_to_student_map() utility for O(1) user->student lookups - Add get_student_context() utility to consolidate student lookup pattern - Add render_params_for_students_page() helper method to BaseHandler - Update archive handlers to use build_user_to_student_map for efficiency - Update student handlers to use get_student_context utility - Update TrainingProgramStudentsHandler and BulkAddTrainingProgramStudentsHandler to use render_params_for_students_page helper This reduces ~160 lines of duplicated code across multiple handlers. 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:
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the
Comment |
- Create StudentBaseHandler class that provides setup_student_context() - Update StudentHandler, StudentTagsHandler, StudentTasksHandler, StudentTaskSubmissionsHandler, AddStudentTaskHandler, and RemoveStudentTaskHandler to inherit from StudentBaseHandler - Remove 'create student if None' logic from StudentHandler.post and StudentTagsHandler.post - now returns 404 consistently - Remove get_student_context utility function from util.py This provides a cleaner OOP approach that's more consistent with the existing handler inheritance patterns in the codebase. Co-Authored-By: Ron Ryvchin <[email protected]>
![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/handlers/base.py`:
- Around line 974-1017: The setup_student_context method should validate and
cast the user_id string to an integer before querying Participation to avoid DB
errors; update setup_student_context to attempt int(user_id) (or use a helper)
and if casting raises ValueError raise tornado.web.HTTPError(404) before
performing the Participation/Student queries (references: setup_student_context,
user_id, Participation, Student, safe_get_item, sql_session,
training_program_id).
🧹 Nitpick comments (1)
cms/server/admin/handlers/base.py (1)
559-604: Guard against unintended commits inrender_params_for_students_page.
This helper commits the session; if a caller ever invokes it with pending writes, those changes will be persisted unexpectedly. Consider either moving the commit to callers or explicitly asserting a clean session before committing.🛡️ Suggested guard to prevent accidental commits
# Calculate task archive progress for each student using shared utility student_progress = {} for student in training_program.students: student_progress[student.id] = calculate_task_archive_progress( student, student.participation, managing_contest, self.sql_session ) # Commit to release any advisory locks taken by get_cached_score_entry + if self.sql_session.new or self.sql_session.dirty or self.sql_session.deleted: + raise RuntimeError( + "render_params_for_students_page must be called with a clean session" + ) self.sql_session.commit()
ronryv
force-pushed
the
devin/1769850361-reduce-code-duplication
branch
from
43bef92 to
449661f
Compare
- Create training_analytics.py for ranking/attendance analytics handlers - Create excel.py for Excel export logic and handlers - Create studenttask.py for student task-related handlers - Create trainingprogramtask.py for training program task handlers - Reduce archive.py to only ArchiveTrainingDayHandler with re-exports - Update student.py and trainingprogram.py with re-exports for backward compatibility - Extract RankingCommonMixin to base.py for shared ranking logic - Add GIN index optimizations for tag filtering queries Co-Authored-By: Ron Ryvchin <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@cms/server/admin/handlers/student.py`:
- Around line 29-36: The file is missing the Submission import from cms.db which
causes NameError when the code in student detail and removal confirmation
handlers references Submission (used in functions rendering the student detail
page and removal confirmation). Fix by adding Submission to the existing import
list from cms.db (alongside TrainingProgram, Participation, User, Question,
Student, Team) so the Submission symbol is available where it’s referenced.
In `@cms/server/admin/handlers/studenttask.py`:
- Around line 223-233: The query is using task_id (a URL string) against the
integer StudentTask.task_id column; convert/validate it to an integer before
querying (like the GET method does with safe_get_item). Update the handler to
obtain a validated int task id (e.g., call safe_get_item(Task, task_id) or
int(task_id) with try/except) prior to calling setup_student_context and the
StudentTask query, and raise a 404 on invalid conversion or if the Task doesn’t
exist so the subsequent .filter(StudentTask.task_id == task_id) uses an integer
value.
In `@cms/server/admin/handlers/trainingprogramtask.py`:
- Around line 250-257: The code uses assert for validating task_id in the try
block (self.get_argument("task_id") / task_id != "null"); replace the assert
with an explicit check such as: after retrieving task_id, if task_id is None or
task_id == "null" or task_id.strip() == "": raise ValueError("Please select a
valid task") so the existing except block (which calls
self.service.add_notification(make_datetime(), "Invalid field(s)", repr(error))
and self.redirect(fallback_page)) still catches and logs the error reliably even
when Python is run with -O.
- Around line 280-308: The code calls _shift_task_nums with task_num which may
be None; before the final _shift_task_nums invocation (the one using
Task.contest and Task.num and managing_contest), add a guard to only call
_shift_task_nums when task_num is not None (mirroring the training_day_num
check), so that if task.num was already None no shifting is attempted; update
the block around Task, task_num, Task.contest, Task.num and managing_contest to
perform the nil check and skip the shift when task_num is None.
In `@cms/server/util.py`:
- Line 1: The file begins with a UTF-8 BOM before the shebang "#!/usr/bin/env
python3", which causes Ruff EXE005 and prevents the shebang from being
recognized; remove the BOM so the shebang is the very first byte of the file
(ensure the file is saved without BOM/with UTF-8 no-BOM encoding) and verify the
leading line remains exactly "#!/usr/bin/env python3".
🧹 Nitpick comments (8)
cms/server/admin/handlers/archive.py (1)
74-92: Ruff RUF022: sort__all__for lint compliance.♻️ Optional: sorted `__all__`
-__all__ = [ - "ArchiveTrainingDayHandler", - "build_attendance_data", - "build_ranking_data", - "TrainingProgramFilterMixin", - "TrainingProgramAttendanceHandler", - "TrainingProgramCombinedRankingHandler", - "TrainingProgramCombinedRankingHistoryHandler", - "TrainingProgramCombinedRankingDetailHandler", - "UpdateAttendanceHandler", - "ExportAttendanceHandler", - "ExportCombinedRankingHandler", - "excel_build_filename", - "excel_setup_student_tags_headers", - "excel_build_training_day_title", - "excel_get_zebra_fills", - "excel_write_student_row", - "excel_write_training_day_header", -] +__all__ = [ + "ArchiveTrainingDayHandler", + "ExportAttendanceHandler", + "ExportCombinedRankingHandler", + "TrainingProgramAttendanceHandler", + "TrainingProgramCombinedRankingDetailHandler", + "TrainingProgramCombinedRankingHandler", + "TrainingProgramCombinedRankingHistoryHandler", + "TrainingProgramFilterMixin", + "UpdateAttendanceHandler", + "build_attendance_data", + "build_ranking_data", + "excel_build_filename", + "excel_build_training_day_title", + "excel_get_zebra_fills", + "excel_setup_student_tags_headers", + "excel_write_student_row", + "excel_write_training_day_header", +]cms/server/admin/handlers/trainingprogram.py (1)
63-79: Ruff RUF022: sort__all__for lint compliance.♻️ Optional: sorted `__all__`
-__all__ = [ - "TrainingProgramListHandler", - "TrainingProgramHandler", - "AddTrainingProgramHandler", - "RemoveTrainingProgramHandler", - "TrainingProgramTasksHandler", - "AddTrainingProgramTaskHandler", - "RemoveTrainingProgramTaskHandler", - "TrainingProgramRankingHandler", - "TrainingProgramSubmissionsHandler", - "TrainingProgramAnnouncementsHandler", - "TrainingProgramAnnouncementHandler", - "TrainingProgramQuestionsHandler", - "TrainingProgramOverviewRedirectHandler", - "TrainingProgramResourcesListRedirectHandler", - "_shift_task_nums", -] +__all__ = [ + "AddTrainingProgramHandler", + "AddTrainingProgramTaskHandler", + "RemoveTrainingProgramHandler", + "RemoveTrainingProgramTaskHandler", + "TrainingProgramAnnouncementHandler", + "TrainingProgramAnnouncementsHandler", + "TrainingProgramHandler", + "TrainingProgramListHandler", + "TrainingProgramOverviewRedirectHandler", + "TrainingProgramQuestionsHandler", + "TrainingProgramRankingHandler", + "TrainingProgramResourcesListRedirectHandler", + "TrainingProgramSubmissionsHandler", + "TrainingProgramTasksHandler", + "_shift_task_nums", +]cms/server/admin/handlers/base.py (1)
958-1024: Useraise ... from Noneto suppress unrelated exception context.
Line 991 raises a new, unrelated exception inside anexceptblock; suppressing the original exception context keeps logs clean and aligns with Ruff B904.♻️ Suggested refactor
- except ValueError: - raise tornado.web.HTTPError(404) + except ValueError: + raise tornado.web.HTTPError(404) from Nonecms/server/admin/handlers/trainingprogramtask.py (4)
78-84: Consider batching flush operations for better performance.The current implementation flushes after each individual task update. For training programs with many tasks, this could be slow. Consider collecting updates and flushing once at the end, or using a bulk update query.
However, the current approach is safer for avoiding unique constraint violations during reordering, so this is a minor optimization opportunity.
148-156: Chain exception for better traceability.When re-raising a
ValueErrorfrom a caughtJSONDecodeError, use exception chaining to preserve the original traceback for debugging.Proposed fix
try: order_list = json.loads(reorder_data) except json.JSONDecodeError as e: logging.warning( "Failed to parse reorder data: %s. Payload: %s", e.msg, reorder_data[:500], ) - raise ValueError(f"Invalid JSON in reorder data: {e.msg}") + raise ValueError(f"Invalid JSON in reorder data: {e.msg}") from e
161-164: Validatetask_idbefore conversion to avoid unhelpful error messages.Line 162 assumes all items have a valid
task_id. Iftask_idis missing or not an integer, the error will bubble up with a generic message. Consider validatingtask_idpresence explicitly, similar to thenew_numvalidation below.Proposed fix
expected_ids = {t.id for t in contest.tasks} - received_ids = {int(item.get("task_id")) for item in order_list} + received_ids = set() + for item in order_list: + if "task_id" not in item: + raise ValueError("Missing 'task_id' in reorder data entry") + try: + received_ids.add(int(item["task_id"])) + except (TypeError, ValueError): + raise ValueError( + f"Invalid 'task_id' value: {item.get('task_id')!r} is not an integer" + ) from None if received_ids != expected_ids: raise ValueError("Reorder data must include each task exactly once")
382-386: Remove redundantlocals()check.The
participation_datavariable is always defined at line 350, so theif "participation_data" in locals():check at line 383 is always true and can be removed.Proposed fix
# Re-assign task_statuses after commit (SQLAlchemy expired them) - if "participation_data" in locals(): - for p in self.contest.participations: - if p.id in participation_data: - p.task_statuses, p.total_score = participation_data[p.id] + for p in self.contest.participations: + if p.id in participation_data: + p.task_statuses, p.total_score = participation_data[p.id]cms/server/admin/handlers/excel.py (1)
222-234: Consider refactoring mixin composition pattern.The current approach manually assigns
sql_sessionandget_argumentto aTrainingProgramFilterMixininstance. While functional, this pattern is fragile - if the mixin later accesses other handler attributes, this will silently fail.An alternative would be to have this handler inherit from
TrainingProgramFilterMixin, BaseHandler(as done intraining_analytics.py), or extract the filtering logic into standalone functions.
ronryv
force-pushed
the
devin/1769850361-reduce-code-duplication
branch
from
9ed1b91 to
ffdec0d
Compare
…ts query - ExportAttendanceHandler and ExportCombinedRankingHandler now inherit from TrainingProgramFilterMixin instead of monkey-patching it - Replace len(tp.managing_contest.participations) with a count query to avoid loading all participations into memory Co-Authored-By: Ron Ryvchin <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@cms/server/admin/handlers/excel.py`:
- Around line 179-207: The training-day title written by
excel_write_training_day_header must be sanitized to prevent formula injection:
call the existing excel_safe on the result of excel_build_training_day_title and
use that sanitized string when setting the cell value (replace title with
safe_title) before applying font/fill/border/alignment and merging; reference
excel_write_training_day_header, excel_build_training_day_title, and excel_safe
so the change is localized and consistent.
- Around line 342-505: The task header cells are written directly from
user-controlled task["name"], which risks Excel formula injection; in
ExportCombinedRankingHandler inside the loop that writes subheaders (the
ws.cell(row=2, column=col + i, value=... ) call), wrap the task name with
excel_safe (i.e., use excel_safe(task["name"])) before assigning to the cell
value so all task header text is sanitized; update the corresponding place that
writes task header values (referencing the loop that creates cell for each task)
to call excel_safe.
In `@cms/server/admin/handlers/training_analytics.py`:
- Around line 448-492: The history endpoint currently includes archived rankings
for students that may be marked hidden; update
TrainingProgramCombinedRankingHistoryHandler.get to skip hidden participations
the same way the main ranking view does by adding a guard when iterating
td.archived_student_rankings: if the ranking itself or its related participation
is marked hidden (e.g., check getattr(ranking, "hidden", False) or
getattr(ranking, "participation", None) and getattr(ranking.participation,
"hidden", False)), continue before applying tags or appending history entries so
hidden accounts are excluded from the result payload.
- Around line 738-799: Remove the broad "except Exception" handler around the
validation/assignment block so unexpected errors (including AttributeError and
DB errors not handled by try_commit()) bubble up as 500s; specifically, keep the
existing JSONDecodeError handler, leave try_commit() as-is, and either delete
the final except block entirely or replace it with a narrow "except
AttributeError as e" only if you have a clear, documented way to map that to a
client error—reference the try_commit() call and the attendance attribute
assignments (attendance.justified, attendance.comment, attendance.recorded) when
locating the code to change.
In `@cms/server/admin/handlers/trainingprogramtask.py`:
- Around line 250-265: Before reassigning task.num and task.contest, verify that
the retrieved Task (from safe_get_item(Task, task_id)) is either unassigned
(task.contest is None) or already belongs to managing_contest; if task.contest
is non-null and not equal to managing_contest, abort: call
self.service.add_notification(make_datetime(), "Invalid field(s)", "Task already
assigned to another contest") then self.redirect(fallback_page) and return so
you don't reassign it, otherwise proceed to set task.num and task.contest and
commit with try_commit().
🧹 Nitpick comments (1)
cms/server/admin/handlers/student.py (1)
54-66: Optional: sort__all__to satisfy RUF022.🧹 Suggested ordering
__all__ = [ - "TrainingProgramStudentsHandler", - "AddTrainingProgramStudentHandler", - "BulkAddTrainingProgramStudentsHandler", - "RemoveTrainingProgramStudentHandler", - "StudentHandler", - "StudentTagsHandler", - "StudentTasksHandler", - "StudentTaskSubmissionsHandler", "AddStudentTaskHandler", + "AddTrainingProgramStudentHandler", + "BulkAddTrainingProgramStudentsHandler", "BulkAssignTaskHandler", "RemoveStudentTaskHandler", + "RemoveTrainingProgramStudentHandler", + "StudentHandler", + "StudentTagsHandler", + "StudentTaskSubmissionsHandler", + "StudentTasksHandler", + "TrainingProgramStudentsHandler", ]
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cms/server/admin/handlers/student.py (1)
121-163:⚠️ Potential issue | 🟡 MinorReplace
assertwith explicit validation.Assertions are removed when running with Python's
-Ooptimization flag, which would bypass theuser_idvalidation. Use explicit exception handling instead, consistent with the codebase pattern (seeuser.pyandtrainingprogramtask.py).🛠️ Suggested fix
user_id: str = self.get_argument("user_id") - assert user_id != "", "Please select a valid user" + if not user_id or user_id.strip() == "": + raise ValueError("Please select a valid user")
🤖 Fix all issues with AI agents
In `@cms/server/admin/handlers/base.py`:
- Around line 991-995: The except block that catches ValueError when converting
user_id to int should suppress the original exception context; update the
handler where user_id_int is set so that instead of simply raising
tornado.web.HTTPError(404) you raise it with no context (i.e., use "from None")
to prevent exception chaining from the ValueError to the HTTPError; locate the
try/except around converting user_id to int and change the raise accordingly.
In `@cms/server/admin/handlers/student.py`:
- Around line 470-494: The post handler currently writes an explicit error JSON
when try_commit() fails; change it to follow the project pattern by removing the
custom error payload and relying on centralized failure handling: in the post
method (student.post) after setting self.student.student_tags and calling
self.try_commit(), if try_commit() returns False simply call
self.set_status(500) and return without calling self.write(...) (leave the
setup_student_context and exception handling unchanged).
In `@cms/server/admin/handlers/training_analytics.py`:
- Around line 503-531: The code is excluding historically-visible students based
on their current student.participation.hidden flag; change the logic in the loop
that builds active_students_per_td (and the similar block around the later
guard) to base inclusion on the archived visibility stored on the
ranking/archived snapshot instead of the live participation.hidden flag: when
iterating td.archived_student_rankings, consult the ranking's archived
visibility/snapshot field (or simply treat presence in
td.archived_student_rankings as proof of historical visibility) to populate
active_students_per_td[td.id]. Also update the detail-view guard so a student is
denied only if they never appear in any archived results under the current
filters (use _get_filtered_context output), and keep the live
student.participation.hidden value only for display purposes. Ensure these
changes are applied to the same conditional checks referenced by
active_students_per_td and the later block (lines ~578-584).
- Around line 145-146: The loop filtering out students by their current hidden
flag (the check on student.participation.hidden) must be removed/changed so
archived rankings keep students who were visible at training time; update the
combined-ranking construction to consult the visibility as of the training
timestamp (e.g., use a participation archived visibility field such as
participation.hidden_at_training or a visibility flag stored on the archived
ranking record) instead of the current student.participation.hidden check;
locate and replace the conditional "if student.participation and
student.participation.hidden: continue" in the ranking-building code so
inclusion is based on the participation/archived visibility at training time.
In `@cms/server/admin/handlers/trainingprogramtask.py`:
- Around line 294-307: When removing a task from a training day, guard the call
to _shift_task_nums by verifying task.training_day_num is not None: unset
task.training_day and task.training_day_num as you already do, but only call
_shift_task_nums(self.sql_session, Task.training_day, training_day,
Task.training_day_num, training_day_num, -1) when training_day_num is an int
(not None); if training_day_num is None, skip the shift to avoid passing None as
the threshold. Ensure the check references task.training_day_num and uses the
local training_day_num variable captured before nulling the fields.
a554dbb to
bc85992
Compare
ronryv
force-pushed
the
devin/1769850361-reduce-code-duplication
branch
from
bc85992 to
dda2630
Compare
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| self.set_status(500) | ||
| self.write({"error": "Failed to save"}) | ||
| return |
There was a problem hiding this comment.
🟡 StudentTagsHandler returns empty body on commit failure
When try_commit() fails in StudentTagsHandler.post(), the handler sets HTTP status 500 but returns without writing any response body. This is an AJAX endpoint that expects JSON responses.
Click to expand
Root Cause
The old code at cms/server/admin/handlers/student.py wrote {"error": "Failed to save"} when commit failed:
if self.try_commit():
self.write({"success": True, "tags": student.student_tags})
else:
self.set_status(500)
self.write({"error": "Failed to save"})The new code at lines 487-491 removed the error message:
if self.try_commit():
self.write({"success": True, "tags": self.student.student_tags})
else:
self.set_status(500)
return # Missing response body!Impact
Clients calling this AJAX endpoint will receive an HTTP 500 with an empty body instead of a JSON error object. This may cause JavaScript errors when the client tries to parse the response as JSON.
Recommendation: Add self.write({"error": "Failed to save"}) before the return statement on line 491.
Was this helpful? React with 👍 or 👎 to provide feedback.
| for ranking in td.archived_student_rankings: | ||
| if self._tags_match(ranking.student_tags, student_tags): | ||
| active_students_per_td[td.id].add(ranking.student_id) |
There was a problem hiding this comment.
🟡 Missing hidden student filter in active_students_per_td loop
In TrainingProgramCombinedRankingDetailHandler, when building active_students_per_td for historical mode, hidden students are incorrectly included because the hidden check was removed during refactoring.
Click to expand
Root Cause
The old code in archive.py had a hidden student check:
for ranking in td.archived_student_rankings:
student_obj = ranking.student
if (
student_obj
and student_obj.participation
and student_obj.participation.hidden
):
continue
if self._tags_match(ranking.student_tags, student_tags):
active_students_per_td[td.id].add(ranking.student_id)The new code at training_analytics.py:524-526 removed this check:
for ranking in td.archived_student_rankings:
if self._tags_match(ranking.student_tags, student_tags):
active_students_per_td[td.id].add(ranking.student_id)Impact
Hidden students will be counted in active_students_per_td, which affects the user_count calculation used for relative rank computations. This could inflate user counts and skew rank percentages.
Recommendation: Add hidden student check before line 525: student_obj = ranking.student; if student_obj and student_obj.participation and student_obj.participation.hidden: continue
Was this helpful? React with 👍 or 👎 to provide feedback.
| for ranking in td.archived_student_rankings: | ||
| # Apply student tag filter | ||
| if student_tags: | ||
| if student_tags_mode == "current": | ||
| if ranking.student_id not in current_tag_student_ids: | ||
| continue | ||
| else: # historical mode | ||
| if not self._tags_match(ranking.student_tags, student_tags): | ||
| continue | ||
| if ranking.task_scores: | ||
| task_ids_in_contest.update(int(k) for k in ranking.task_scores.keys()) |
There was a problem hiding this comment.
🟡 Missing hidden student filter in task_ids_in_contest loop
In TrainingProgramCombinedRankingDetailHandler, when collecting visible task IDs from rankings, hidden students' tasks are incorrectly included because the hidden check was removed during refactoring.
Click to expand
Root Cause
The old code in archive.py had a hidden student check:
for ranking in td.archived_student_rankings:
student_obj = ranking.student
if (
student_obj
and student_obj.participation
and student_obj.participation.hidden
):
continue
# Apply student tag filter
if student_tags:
...
if ranking.task_scores:
task_ids_in_contest.update(int(k) for k in ranking.task_scores.keys())The new code at training_analytics.py:570-580 removed this check:
for ranking in td.archived_student_rankings:
# Apply student tag filter
if student_tags:
if student_tags_mode == "current":
if ranking.student_id not in current_tag_student_ids:
continue
else: # historical mode
if not self._tags_match(ranking.student_tags, student_tags):
continue
if ranking.task_scores:
task_ids_in_contest.update(int(k) for k in ranking.task_scores.keys())Impact
Tasks from hidden students' rankings will be included in task_ids_in_contest, potentially exposing task data that should be filtered out. When student_tags is empty, all rankings including hidden students will contribute their tasks.
Recommendation: Add hidden student check after line 570: student_obj = ranking.student; if student_obj and student_obj.participation and student_obj.participation.hidden: continue
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
This PR reduces code duplication across training program handlers by extracting common patterns into shared utilities and a base handler class:
StudentBaseHandler- New base handler class for student-related pages that providessetup_student_context()to look up training_program, managing_contest, participation, and student. Raises 404 if participation or student is not found. Used byStudentHandler,StudentTagsHandler,StudentTasksHandler,StudentTaskSubmissionsHandler,AddStudentTaskHandler, andRemoveStudentTaskHandler.build_user_to_student_map()- Builds auser_id -> Studentdict for O(1) lookups instead of repeated O(n) database queries. Used in archive handlers that iterate over participations.render_params_for_students_page()- Helper method on BaseHandler that sets up all render params needed for the students page (unassigned users, student progress, task/tag lists).Behavior Change
StudentHandler.postandStudentTagsHandler.postpreviously had logic to create a Student record if one didn't exist. This has been removed - they now return 404 consistently like other student handlers. This was identified as defensive coding for an edge case that shouldn't occur in normal operation (Participation and Student are always created together).Review & Testing Checklist for Human
StudentHandler.postandStudentTagsHandler.postno longer auto-create Student records if missingStudent.participationis never None -build_user_to_student_maprelies on this assumption (based onparticipation_idhavingnullable=Falsein the Student model)/training_program/{id}/students) to verify it still displays correctlyStudentBaseHandler)Recommended test plan: Navigate to a training program's students page, verify the student list and progress bars display correctly. Click into a student's edit page and save changes. Then go to the student's tasks page and try adding/removing a task.
Notes
TrainingProgramOverviewHandler,TrainingDaysHandler,ScoreboardDataHandler) were not refactored as they have slightly different behavior patterns and the duplication is shorter (~10 lines)Link to Devin run: https://app.devin.ai/sessions/e12bc3d632394291a0ed70fbb2ece81b
Requested by: Ron Ryvchin (@ronryv)
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.