Add justified absences, comments, recorded status, and Excel export to attendance #109
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
📝 WalkthroughWalkthroughAdds three fields (justified, comment, recorded) to ArchivedAttendance with migration, POST handler to update them, Excel export handlers for attendance and combined rankings, client-side modal UI/JS to edit/save attendance, template/badge updates, CSS for modal/badges, and openpyxl dependency. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser as Client(JS)
participant Server as API
participant DB as Database
User->>Browser: Click attendance cell
Browser->>Browser: openAttendanceModal(fill fields)
User->>Browser: Edit fields and Save
Browser->>Server: POST /training_program/{tp}/attendance/{id} (JSON + CSRF)
Server->>DB: SELECT archived_attendance BY id
Server->>Server: validate ownership and input
Server->>DB: UPDATE archived_attendance (justified, comment, recorded)
DB-->>Server: commit
Server-->>Browser: {"success": true}
Browser->>Browser: update cell UI / close modal
sequenceDiagram
participant User
participant Browser as Client
participant Server as API
participant DB as Database
participant XLSX as ExcelGenerator
User->>Browser: Click "Export archived attendance"
Browser->>Server: GET /training_program/{tp}/attendance/export?filters
Server->>DB: Query program, training days, archived_attendances, students
Server->>XLSX: Build workbook (rows per student, columns per session, styles)
XLSX-->>Server: .xlsx binary
Server-->>Browser: Response with downloadable .xlsx
Browser->>User: Prompt download
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ronryv
force-pushed
the
devin/1769650405-attendance-features
branch
from
ec41fd7 to
bfb8eb6
Compare
- Add justified, comment, and proctored fields to ArchivedAttendance model - Add UpdateAttendanceHandler for updating attendance records via AJAX - Update attendance page with modal for editing justified/proctored/comment - Show justified status with green checkmark, proctored with camera icon - Add comment indicator icon when comments exist - Update combined ranking page to show justified and proctored badges - Add CSS styles for new badges Co-Authored-By: Ron Ryvchin <[email protected]>
ronryv
force-pushed
the
devin/1769650405-attendance-features
branch
from
bfb8eb6 to
215155e
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
@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: 3
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/templates/training_program_attendance.html (1)
148-199: Justified status should show a green checkmark.Right now it uses a memo emoji; the requirement calls for a green checkmark to distinguish justified absences from missed.
🔧 Proposed update
- <span>{% if att.justified %}📝 Justified{% else %}⛔ Missed{% endif %}</span> + <span>{% if att.justified %}✅ Justified{% else %}⛔ Missed{% endif %}</span>
🤖 Fix all issues with AI agents
In `@cms/server/admin/handlers/archive.py`:
- Around line 1142-1157: The handler currently converts inputs with bool(...)
which treats strings like "false" as True and also allows justifying any
attendance; change this to (1) validate and coerce incoming boolean-like values
from data["justified"] and data["recorded"] explicitly (accept True/False,
integers 1/0, and case-insensitive "true"/"false", "yes"/"no") instead of using
bool(...), and (2) enforce a server-side guard before setting
attendance.justified so True may only be applied when the attendance represents
a missed record (e.g. check attendance.status == "missed" or attendance.missed
is True) — otherwise reject or set justified to False; update the same explicit
boolean parsing for attendance.recorded; operate on the existing variables
attendance and data in this handler.
In `@cms/server/admin/templates/training_program_attendance.html`:
- Around line 334-347: The success branch currently does a full page reload
(window.location.reload()) after the fetch in the save handler for attendance
(the block using currentAttendanceId); instead, locate the attendance
cell/element that was edited (e.g., via a data-attendance-id attribute or the
element that opened the modal), update its badge text/classes and any dataset
attributes to reflect the new attendance state from result (and update any
counts/totals shown), then programmatically close the modal (call the modal
close routine used in this file) and optionally trigger any small UI refresh
events — remove the window.location.reload() and perform these in-place DOM
updates in the same .then(result.success) path.
In `@cms/server/admin/templates/training_program_combined_ranking.html`:
- Around line 367-387: The justified badge currently shows the red cross icon in
both template branches; update both occurrences where the condition "attendance
and attendance.status == 'missed' and attendance.justified" renders the
justified-badge to use a green checkmark (e.g. ✅ or HTML entity ✅) instead
of the red cross (❌📝), keeping the class "justified-badge" and
the title "Justified absence" intact so both instances render the correct green
check icon.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cms/db/archived_attendance.py`:
- Around line 91-107: Add explicit ORM defaults for the non-nullable Boolean
columns to match DB defaults: import text from sqlalchemy and update the Column
definitions for justified and recorded (the attributes named "justified" and
"recorded" on ArchivedAttendance) to include server_default=text('false') and a
Python-side default (default=False) so SQLAlchemy will not try to insert NULL
when instances are created without those fields; ensure the import statement for
text is added at the top of the file.
🧹 Nitpick comments (4)
cms/server/admin/static/aws_tp_styles.css (1)
1234-1240: Scope the duplicate.location-rowstyles to avoid global overrides.
The later block overrides the earlier.location-rowdefinition for all contexts. Consider scoping it to the attendance modal.♻️ Suggested adjustment
-.location-row { +.attendance-modal .location-row { display: flex; align-items: center; gap: 6px; margin-top: 4px; flex-wrap: wrap; }cms/server/admin/handlers/__init__.py (1)
210-216: Align UpdateAttendanceHandler with the try_commit error-handling convention.
Before exposing this handler, please remove the explicit error response on try_commit failure and rely on centralized handling.Based on learnings: "do not emit explicit error notifications when try_commit() fails. Rely on centralized error handling and logging instead."
cms/server/admin/handlers/archive.py (1)
1124-1124: Remove redundant import.
jsonis already imported at the module level (line 24). This local import is unnecessary.Suggested fix
def post(self, training_program_id: str, attendance_id: str): """Update an attendance record's justified status, comment, and/or recorded.""" - import json - training_program = self.safe_get_item(TrainingProgram, training_program_id)cms/server/admin/templates/training_program_attendance.html (1)
148-149: Consider usingtojsonfilter for the comment data attribute.While Jinja2 auto-escapes HTML entities, comments containing quotes, newlines, or other special characters could potentially break the HTML attribute parsing. Using
|tojsonensures proper encoding for embedding in attributes.Suggested fix
- <td data-value="{{ data_value }}" class="attendance-cell" data-attendance-id="{{ att.id }}" data-status="{{ att.status }}" data-justified="{{ 'true' if att.justified else 'false' }}" data-recorded="{{ 'true' if att.recorded else 'false' }}" data-comment="{{ att.comment or '' }}" onclick="openAttendanceModal(this)"> + <td data-value="{{ data_value }}" class="attendance-cell" data-attendance-id="{{ att.id }}" data-status="{{ att.status }}" data-justified="{{ 'true' if att.justified else 'false' }}" data-recorded="{{ 'true' if att.recorded else 'false' }}" data-comment="{{ (att.comment or '') | tojson }}" onclick="openAttendanceModal(this)">Then in JavaScript, parse it:
comment: JSON.parse(ds.comment)
devin-ai-integration
bot
changed the title
ronryv
force-pushed
the
devin/1769650405-attendance-features
branch
from
41320fc to
215155e
Compare
Without default=False, archiving training days fails with IntegrityError because the ORM doesn't have default values for these NOT NULL columns. Co-Authored-By: Ron Ryvchin <[email protected]>
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
ronryv
force-pushed
the
devin/1769650405-attendance-features
branch
from
7beb6ab to
5aad17f
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
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 303-365: The updateAttendanceCell function updates visual badges
but never updates the numeric sorting key in the data-value attribute, so change
updateAttendanceCell to recompute and set cell.dataset.value whenever
status/justified/recorded/comment change; specifically use the same scoring
logic as the table sorter (e.g., missed => "3", justified missed => "2.5",
present/recorded => appropriate lower value) and assign to cell.dataset.value
after you set cell.dataset.status/justified/recorded/comment so sorting uses the
new value; update the logic around cell.dataset.status, cell.dataset.justified,
cell.dataset.recorded and cell.dataset.comment inside updateAttendanceCell to
compute and write the correct data-value string.
🧹 Nitpick comments (4)
cms/db/archived_attendance.py (1)
92-110: LGTM!The new columns are correctly defined with
server_default=text("false")for the boolean fields, which ensures the database provides default values during INSERT operations. This addresses the previously identified issue where ArchivedAttendance instantiation wasn't passing these fields.Optional enhancement: Consider adding
default=Falsealongsideserver_defaultfor Python-side convenience, allowing immediate access to these fields after instantiation (before flush):justified: bool = Column( Boolean, nullable=False, default=False, server_default=text("false"), )This is optional since the current implementation works correctly at the database level.
cms/server/admin/static/aws_tp_styles.css (1)
1095-1256: Minor: Consolidate duplicate CSS selectors.Several selectors in this block conflict with or duplicate earlier definitions:
.recorded-badge(lines 1241-1243) overrides the earlier definition (lines 1091-1093) with!important.location-row(lines 1245-1251) duplicates the earlier definition (lines 693-699) with differentgapand addedmargin-top,flex-wrapThis fragmentation makes the CSS harder to maintain. Consider consolidating these into single definitions.
📝 Suggested consolidation for .location-row
Remove the duplicate at lines 1245-1251 and update the original definition at lines 693-699:
.location-row { display: flex; align-items: center; - gap: 4px; + gap: 6px; font-size: 0.75rem; color: var(--tp-text-light); + margin-top: 4px; + flex-wrap: wrap; }cms/server/admin/handlers/archive.py (1)
1124-1124: Remove redundant import.
jsonis already imported at the module level (line 24), so this local import is unnecessary.♻️ Proposed fix
def post(self, training_program_id: str, attendance_id: str): """Update an attendance record's justified status, comment, and/or recorded.""" - import json - training_program = self.safe_get_item(TrainingProgram, training_program_id)cms/server/admin/templates/training_program_attendance.html (1)
429-435: Consider adding Escape key support for modal closure.The modal closes when clicking outside, but keyboard users expect Escape to close modals as well.
⌨️ Add Escape key handler
// Close modal when clicking outside document.addEventListener('click', function(e) { var modal = document.getElementById('attendance-modal'); if (e.target === modal) { closeAttendanceModal(); } }); + +// Close modal with Escape key +document.addEventListener('keydown', function(e) { + if (e.key === 'Escape') { + var modal = document.getElementById('attendance-modal'); + if (modal && modal.style.display === 'flex') { + closeAttendanceModal(); + } + } +});
ronryv
force-pushed
the
devin/1769650405-attendance-features
branch
from
5aad17f to
f6faf23
Compare
Co-Authored-By: Ron Ryvchin <[email protected]>
devin-ai-integration
bot
changed the title
… and filter-based filename Co-Authored-By: Ron Ryvchin <[email protected]>
Co-Authored-By: Ron Ryvchin <[email protected]>
Co-Authored-By: Ron Ryvchin <[email protected]>
ronryv
force-pushed
the
devin/1769650405-attendance-features
branch
from
f30ffc0 to
ef5f503
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
…ures' into devin/1769650405-attendance-features
Co-Authored-By: Ron Ryvchin <[email protected]>
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@cms/server/admin/handlers/archive.py`:
- Around line 136-155: User-controlled cell values can begin with =, +, -, or @
and be interpreted as Excel formulas; add a small sanitizer and apply it
everywhere inputs are written to sheets. Create a helper (e.g.,
sanitize_excel_cell(value) that returns value prefixed with a single quote or
otherwise escapes values starting with one of '=+-@' and use it when writing
student_name and tags_str in excel_write_student_row, when writing delay_reasons
and comment in ExportAttendanceHandler, and when writing task header strings in
ExportCombinedRankingHandler so all exported cells are sanitized before
ws.cell(..., value=...).
In `@cms/server/admin/static/aws_tp_styles.css`:
- Around line 1089-1091: The .justified-badge CSS rule uses var(--tp-success)
(green) but should use the danger color to match the red cross icon; update the
.justified-badge declaration to use var(--tp-danger) (or the project’s danger
token) so justified absences render with the intended red styling.
In `@cms/server/admin/templates/training_program_combined_ranking.html`:
- Around line 238-242: The URL for the Export link is building
training_day_types with join(', ') which inserts spaces and isn't URL-encoded;
update the template expression that emits training_day_types (the href in the
anchor in training_program_combined_ranking.html) to use join(',') and apply
urlencode (e.g., replace training_day_types | join(', ') with training_day_types
| join(',') | urlencode) so the parameter is properly encoded and matches the
approach used in the attendance template.
🧹 Nitpick comments (1)
cms/server/admin/templates/training_program_attendance.html (1)
405-431: Consider handling non-JSON error responses.If the server returns a non-2xx status with an HTML error page (e.g., 500),
response.json()will throw a parsing error with a confusing message. Consider checkingresponse.okfirst.🔧 Proposed fix
fetch('{{ url("training_program", training_program.id, "attendance") }}/' + currentAttendanceId, { method: 'POST', headers: { 'Content-Type': 'application/json', 'X-XSRFToken': get_cookie("_xsrf") }, body: JSON.stringify(data) }) - .then(function(response) { return response.json(); }) + .then(function(response) { + if (!response.ok) { + throw new Error('Server error: ' + response.status); + } + return response.json(); + }) .then(function(result) {
…o attendance (#109) * Add justified absences, comments, and proctored status to attendance - Add justified, comment, and proctored fields to ArchivedAttendance model - Add UpdateAttendanceHandler for updating attendance records via AJAX - Update attendance page with modal for editing justified/proctored/comment - Show justified status with green checkmark, proctored with camera icon - Add comment indicator icon when comments exist - Update combined ranking page to show justified and proctored badges - Add CSS styles for new badges Co-Authored-By: Ron Ryvchin <[email protected]> * Add default=False to justified and recorded columns Without default=False, archiving training days fails with IntegrityError because the ORM doesn't have default values for these NOT NULL columns. Co-Authored-By: Ron Ryvchin <[email protected]> * Fix client side updates * Add Excel export feature for attendance page Co-Authored-By: Ron Ryvchin <[email protected]> * Improve Excel export: add training types, zebra colors, student tags, and filter-based filename Co-Authored-By: Ron Ryvchin <[email protected]> * Change student tags to separate column instead of split row Co-Authored-By: Ron Ryvchin <[email protected]> * Change student tags to separate column instead of split row Co-Authored-By: Ron Ryvchin <[email protected]> * Small comment display change * Add Excel export for combined ranking page with shared utilities Co-Authored-By: Ron Ryvchin <[email protected]> * review comment * review comments --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Add justified absences, comments, recorded status, and Excel export to attendance
Summary
This PR expands the training program attendance page with four features:
Justified absences - Admins can mark missed training days as "justified" (e.g., for sick leave). Justified absences display with a clipboard icon instead of the red missed icon.
Comments - Admins can add comments to any attendance record. A speech bubble icon indicates when a comment exists.
Recorded status - Admins can mark individual student attendance as "recorded" (room/screen recording). Displays with a video camera icon.
Excel export - Admins can export the filtered attendance data to an Excel file. The export includes subcolumns for each training day: Status, Location, Recorded, Delay Reasons, and Comments. Status shows "Missed", "Justified Absent", "On Time", or "Delayed (Xm)".
UI Implementation:
Updates since last revision
openpyxl>=3.1,<4.0as a new dependency in pyproject.tomlReview & Testing Checklist for Human
update_from_1.5.sql) has both CREATE TABLE with the new columns AND ALTER TABLE statements that add them again (lines 666-668 and 701-705). This will cause errors if run on a fresh database. One set should be removed.justified,comment,recorded) were added toArchivedAttendance. Verify the migration works correctly on your environment.Recommended test plan:
Notes
Link to Devin run: https://app.devin.ai/sessions/f79a67e65ba544ad929cf98ca980fbf7
Requested by: Ron Ryvchin (@ronryv)
Summary by CodeRabbit
New Features
Style
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.